BAEL-9123: Added unit tests for elasticsearch wildcard service #18879
Conversation
Signed-off-by: Diego Torres <[email protected]>
| <!--These two dependencies are inherited from spring-data-elasticsearch. Overriding to the newer version | ||
| will break tests. --> | ||
| <!-- <dependency>--> | ||
| <!-- <groupId>co.elastic.clients</groupId>--> | ||
| <!-- <artifactId>elasticsearch-java</artifactId>--> | ||
| <!-- <version>${elasticsearch.version}</version>--> | ||
| <!-- </dependency>--> | ||
| <!-- <dependency>--> | ||
| <!-- <groupId>com.fasterxml.jackson.core</groupId>--> | ||
| <!-- <artifactId>jackson-databind</artifactId>--> | ||
| <!-- <version>${jackson.version}</version>--> | ||
| <!-- </dependency>--> |
There was a problem hiding this comment.
What breaks here exactly?
| @@ -0,0 +1,6 @@ | |||
| appender.console.type = Console | |||
There was a problem hiding this comment.
If I remember correctly I think we have a standard logback resource you can include - can you check other modules to see how they set up their logging
|
|
||
| @Container | ||
| static ElasticsearchContainer elasticsearchContainer = new ElasticsearchContainer( | ||
| "docker.elastic.co/elasticsearch/elasticsearch:8.11.1") |
There was a problem hiding this comment.
Indentation doesn't match the Baeldung formatter (same in some other places) e.g. we should be seeing 4 space indent
| } | ||
|
|
||
| @Test | ||
| void testWildcardSearch_IntegrationTest() throws IOException { |
There was a problem hiding this comment.
We use BDD style method names like givenXXX_whenXXX_thenXXX or whenXXX_thenXXX (if no givens)
| // Verify that all results contain names starting with "john" (case-insensitive) | ||
| results.forEach(result -> { | ||
| Object nameObj = result.get("name"); | ||
| assertNotNull(nameObj, "Name field should not be null"); | ||
|
|
||
| String name = nameObj.toString(); | ||
| logger.info("Checking name: '{}'", name); | ||
|
|
||
| assertTrue(name.toLowerCase().startsWith("john"), | ||
| "Expected name to start with 'john', but got: " + name); | ||
| }); |
There was a problem hiding this comment.
When assertions get complex we have a guideline to use AssertJ which has fluent assertion helper methods - you could do something similar to this, depending on how you want to write it:
assertThat(results)
.extracting(result -> result.get("name"))
.doesNotContainNull()
.extracting(Object::toString)
.allSatisfy(name -> assertThat(name.toLowerCase()).startsWith("john"));
Similar elsewhere
| <version>${spring-boot.version}</version> | ||
| </dependency> | ||
|
|
||
| <!-- https://mvnrepository.com/artifact/org.testcontainers/elasticsearch --> |
There was a problem hiding this comment.
| <!-- https://mvnrepository.com/artifact/org.testcontainers/elasticsearch --> |
| <spring-data-elasticsearch.version>5.1.2</spring-data-elasticsearch.version> | ||
| <elasticsearch.version>8.9.0</elasticsearch.version> | ||
| <commons-csv.version>1.12.0</commons-csv.version> | ||
| <java.version>11</java.version> |
There was a problem hiding this comment.
Intentional to have 11 here but 17 in compiler plugin?
Also check if the parent already has maven-compiler-plugin and spring-boot-maven-plugin defined
| public List<Map<String, Object>> combinedWildcardSearch(String indexName, String field1, String wildcard1, String field2, String wildcard2) throws IOException { | ||
| logger.info("Performing combined wildcard search on index: {}", indexName); | ||
|
|
||
| SearchResponse<ObjectNode> response = elasticsearchClient.search(s -> s.index(indexName).query(q -> q.bool(b -> b.must(m -> m.wildcard(w -> w.field(field1).value(wildcard1))).must(m -> m.wildcard(w -> w.field(field2).value(wildcard2))))).size(maxResults), ObjectNode.class); |
There was a problem hiding this comment.
We have a convention to put fluent calls on separate lines (same elsewhere)
| // Wait for documents to be indexed | ||
| try { | ||
| Thread.sleep(1500); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| } |
There was a problem hiding this comment.
We can't use Thread.sleep - there should be a better way to specifically wait until the documents are indexed - as written this will either be a flaky test or waste CI build time
|
|
||
| /** | ||
| * Unit tests for ElasticsearchWildcardService | ||
| * These tests use Mockito to mock the ElasticsearchClient - no Docker required! |
There was a problem hiding this comment.
I only just realised this test is using mocks - while good practice to have coverage, I'm not sure how useful showing the unit testing is for the article itself - I think we should focus on the "real" integration test usage in the artcile
2 participants
No description provided.