dweiss commented on code in PR #14518:
URL: https://github.com/apache/lucene/pull/14518#discussion_r2051512251
##########
lucene/core/src/test/org/apache/lucene/document/TestDocument.java:
##########
@@ -207,7 +219,7 @@ public void testGetValuesForIndexedDocument() throws
Exception {
// ensure that queries return expected results without DateFilter first
ScoreDoc[] hits = searcher.search(query, 1000).scoreDocs;
- assertEquals(1, hits.length);
+ assertThat(hits, arrayWithSize(1));
Review Comment:
Same here. It's more verbose and does the same thing.
##########
lucene/core/src/test/org/apache/lucene/document/TestDocument.java:
##########
@@ -98,32 +110,32 @@ public void testBinaryField() throws Exception {
*/
public void testRemoveForNewDocument() throws Exception {
Document doc = makeDocumentWithFields();
- assertEquals(10, doc.getFields().size());
+ assertThat(doc.getFields(), hasSize(10));
doc.removeFields("keyword");
- assertEquals(8, doc.getFields().size());
+ assertThat(doc.getFields(), hasSize(8));
Review Comment:
This is an example of changes that don't provide any extra value, I think.
##########
lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java:
##########
@@ -810,10 +810,7 @@ public MergeSpecification findMerges(CodecReader...
readers) throws IOException
}
for (MergePolicy.OneMerge merge : merges) {
if (merge.getMergeInfo() != null) {
- assertFalse(
- Arrays.stream(c.destDir.listAll())
- .collect(Collectors.toSet())
- .containsAll(merge.getMergeInfo().files()));
+
assertFalse(Set.of(c.destDir.listAll()).containsAll(merge.getMergeInfo().files()));
Review Comment:
I think the previous version was actually more intuitive?
##########
lucene/core/src/test/org/apache/lucene/util/TestArrayUtil.java:
##########
@@ -113,16 +119,11 @@ public void testParseInt() throws Exception {
parseInt("0.34");
});
- int test = parseInt("1");
- assertTrue(test + " does not equal: " + 1, test == 1);
- test = parseInt("-10000");
- assertTrue(test + " does not equal: " + -10000, test == -10000);
- test = parseInt("1923");
- assertTrue(test + " does not equal: " + 1923, test == 1923);
- test = parseInt("-1");
- assertTrue(test + " does not equal: " + -1, test == -1);
- test = ArrayUtil.parseInt("foo 1923 bar".toCharArray(), 4, 4);
- assertTrue(test + " does not equal: " + 1923, test == 1923);
+ assertThat(parseInt("1"), equalTo(1));
+ assertThat(parseInt("-10000"), equalTo(-10000));
+ assertThat(parseInt("1923"), equalTo(1923));
+ assertThat(parseInt("-1"), equalTo(-1));
+ assertThat(ArrayUtil.parseInt("foo 1923 bar".toCharArray(), 4, 4),
equalTo(1923));
Review Comment:
This is an example of a potentially better message but what's wrong with
assertEquals(a, b)?
##########
lucene/core/src/test/org/apache/lucene/document/TestDocument.java:
##########
@@ -312,21 +324,21 @@ public void testFieldSetValue() throws Exception {
// ensure that queries return expected results without DateFilter first
ScoreDoc[] hits = searcher.search(query, 1000).scoreDocs;
- assertEquals(3, hits.length);
- int result = 0;
+ assertThat(hits, arrayWithSize(3));
+ Set<String> seen = new HashSet<>();
StoredFields storedFields = searcher.storedFields();
for (int i = 0; i < 3; i++) {
Document doc2 = storedFields.document(hits[i].doc);
Field f = (Field) doc2.getField("id");
- if (f.stringValue().equals("id1")) result |= 1;
- else if (f.stringValue().equals("id2")) result |= 2;
- else if (f.stringValue().equals("id3")) result |= 4;
- else fail("unexpected id field");
+ switch (f.stringValue()) {
+ case "id1", "id2", "id3" -> seen.add(f.stringValue());
+ default -> fail("unexpected id field");
+ }
}
writer.close();
reader.close();
dir.close();
- assertEquals("did not see all IDs", 7, result);
+ assertThat("did not see all IDs", seen, containsInAnyOrder("id1", "id2",
"id3"));
Review Comment:
I think this could be just assertEquals(seen, Set.of(...)) - this would
detect anything extra and check for the occurrence of all ids?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
