Skip to content

Commit

Permalink
Fix Flaky Test In TestBlockJoinBulkScorer (#13785)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mikep86 authored Sep 16, 2024
1 parent 73173a4 commit e7a6382
Showing 1 changed file with 43 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
Expand Down Expand Up @@ -260,6 +261,15 @@ private static void assertScores(
Float minScore,
Map<Integer, Float> expectedScores)
throws IOException {
assertScores(bulkScorer, scoreMode, minScore, List.of(expectedScores));
}

private static void assertScores(
BulkScorer bulkScorer,
org.apache.lucene.search.ScoreMode scoreMode,
Float minScore,
List<Map<Integer, Float>> expectedScoresList)
throws IOException {
Map<Integer, Float> actualScores = new HashMap<>();
bulkScorer.score(
new LeafCollector() {
Expand All @@ -283,7 +293,26 @@ public void collect(int doc) throws IOException {
null,
0,
NO_MORE_DOCS);
assertEquals(expectedScores, actualScores);

if (expectedScoresList.size() == 1) {
assertEquals(expectedScoresList.getFirst(), actualScores);
} else {
assertEqualsToOneOf(expectedScoresList, actualScores);
}
}

private static void assertEqualsToOneOf(List<?> expectedList, Object actual) {
boolean foundMatch = false;
for (Object expected : expectedList) {
if (Objects.equals(expected, actual)) {
foundMatch = true;
break;
}
}

if (!foundMatch) {
throw new AssertionError("expected one of: " + expectedList + " but was: " + actual);
}
}

public void testScoreRandomIndices() throws IOException {
Expand Down Expand Up @@ -370,18 +399,26 @@ public void testSetMinCompetitiveScoreWithScoreModeMax() throws IOException {
}

{
// Doc 16 is returned because MaxScoreBulkScorer scores assuming A will match in doc 13,
// leading to a potential max score of 6. By the time it determines that A doesn't match,
// scoring is complete and thus there is no advantage to not collecting the doc.
Map<Integer, Float> expectedScores =
// This test case has two potential results.
// If all docs are scored in the same batch, then doc 16 is collected because
// MaxScoreBulkScorer scores assuming A will match in doc 13, leading to a potential max
// score of 6.
// If the scoring is split across two or more batches, then doc 16 is not collected
// because MaxScoreBulkScorer does not assume A will match in doc 13, leading to a
// potential max score of 5.
Map<Integer, Float> expectedScores1 =
Map.of(
2, 6.0f,
10, 10.0f);
Map<Integer, Float> expectedScores2 =
Map.of(
2, 6.0f,
10, 10.0f,
16, 5.0f);

ScorerSupplier ss = weight.scorerSupplier(searcher.getIndexReader().leaves().get(0));
ss.setTopLevelScoringClause();
assertScores(ss.bulkScorer(), scoreMode, 6.0f, expectedScores);
assertScores(ss.bulkScorer(), scoreMode, 6.0f, List.of(expectedScores1, expectedScores2));
}

{
Expand Down

0 comments on commit e7a6382

Please sign in to comment.