Skip to content

Commit 7787b70

Browse files
authored
Extract per-doc assertion helpers in QueryUtils (#16258)
Move the 70-line assertion+diagnostic block from `checkSkipTo`'s collect() into `assertDocAndScore()`, and the advance-check for-loop body from `checkFirstSkipTo`'s collect() into `assertAdvanceTo()`. No behaviour change.
1 parent 0be06f9 commit 7787b70

2 files changed

Lines changed: 117 additions & 93 deletions

File tree

lucene/CHANGES.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,8 @@ Other
654654

655655
* GITHUB#16215: Extract segment-boundary check into helper in QueryUtils (Luca Cavanna)
656656

657+
* GITHUB#16258: Extract per-doc assertion helpers in QueryUtils (Luca Cavanna)
658+
657659
======================= Lucene 10.4.0 =======================
658660

659661
API Changes

lucene/test-framework/src/java/org/apache/lucene/tests/search/QueryUtils.java

Lines changed: 115 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -380,74 +380,7 @@ public void collect(int doc) throws IOException {
380380
op == skip_op
381381
? iterator.advance(scorer.docID() + 1) != DocIdSetIterator.NO_MORE_DOCS
382382
: iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
383-
int scorerDoc = scorer.docID();
384-
float scorerScore = scorer.score();
385-
float scorerScore2 = scorer.score();
386-
float scoreDiff = Math.abs(score - scorerScore);
387-
float scorerDiff = Math.abs(scorerScore2 - scorerScore);
388-
389-
boolean success = false;
390-
try {
391-
assertTrue(more);
392-
assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
393-
assertTrue(
394-
"score=" + score + ", scorerScore=" + scorerScore, scoreDiff <= maxDiff);
395-
assertTrue(
396-
"scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
397-
scorerDiff <= maxDiff);
398-
success = true;
399-
} finally {
400-
if (!success) {
401-
if (LuceneTestCase.VERBOSE) {
402-
StringBuilder sbord = new StringBuilder();
403-
for (int i = 0; i < order.length; i++) {
404-
sbord.append(order[i] == skip_op ? " skip()" : " next()");
405-
}
406-
System.out.println(
407-
"ERROR matching docs:"
408-
+ "\n\t"
409-
+ (doc != scorerDoc ? "--> " : "")
410-
+ "doc="
411-
+ doc
412-
+ ", scorerDoc="
413-
+ scorerDoc
414-
+ "\n\t"
415-
+ (!more ? "--> " : "")
416-
+ "tscorer.more="
417-
+ more
418-
+ "\n\t"
419-
+ (scoreDiff > maxDiff ? "--> " : "")
420-
+ "scorerScore="
421-
+ scorerScore
422-
+ " scoreDiff="
423-
+ scoreDiff
424-
+ " maxDiff="
425-
+ maxDiff
426-
+ "\n\t"
427-
+ (scorerDiff > maxDiff ? "--> " : "")
428-
+ "scorerScore2="
429-
+ scorerScore2
430-
+ " scorerDiff="
431-
+ scorerDiff
432-
+ "\n\thitCollector.doc="
433-
+ doc
434-
+ " score="
435-
+ score
436-
+ "\n\t Scorer="
437-
+ scorer
438-
+ "\n\t Query="
439-
+ q
440-
+ " "
441-
+ q.getClass().getName()
442-
+ "\n\t Searcher="
443-
+ s
444-
+ "\n\t Order="
445-
+ sbord
446-
+ "\n\t Op="
447-
+ (op == skip_op ? " skip()" : " next()"));
448-
}
449-
}
450-
}
383+
assertDocAndScore(doc, score, more, scorer, maxDiff, order, op, skip_op, q, s);
451384
} catch (IOException e) {
452385
throw new RuntimeException(e);
453386
}
@@ -508,31 +441,7 @@ public void collect(int doc) throws IOException {
508441
// The intervalTimes32 trick helps contain the runtime of this check: first we check
509442
// every single doc in the interval, then after 32 docs we check every 2 docs, etc.
510443
for (int i = lastDoc[0] + 1; i <= doc; i += intervalTimes32++ / 1024) {
511-
ScorerSupplier supplier = w.scorerSupplier(context.get(leafPtr));
512-
Scorer scorer = supplier.get(1L); // only checking one doc, so leadCost = 1
513-
assertTrue(
514-
"query collected " + doc + " but advance(" + i + ") says no more docs!",
515-
scorer.iterator().advance(i) != DocIdSetIterator.NO_MORE_DOCS);
516-
assertEquals(
517-
"query collected " + doc + " but advance(" + i + ") got to " + scorer.docID(),
518-
doc,
519-
scorer.docID());
520-
float advanceScore = scorer.score();
521-
assertEquals(
522-
"unstable advance(" + i + ") score!", advanceScore, scorer.score(), maxDiff);
523-
assertEquals(
524-
"query assigned doc "
525-
+ doc
526-
+ " a score of <"
527-
+ score
528-
+ "> but advance("
529-
+ i
530-
+ ") has <"
531-
+ advanceScore
532-
+ ">!",
533-
score,
534-
advanceScore,
535-
maxDiff);
444+
assertAdvanceTo(doc, i, score, w, context.get(leafPtr), maxDiff);
536445
}
537446
lastDoc[0] = doc;
538447
} catch (IOException e) {
@@ -600,6 +509,119 @@ private static void assertNoPastSegmentEnd(
600509
}
601510
}
602511

512+
/**
513+
* Verifies that the shadow scorer's position and score match the outer scorer's for {@code doc}.
514+
* Prints a detailed diagnostic on failure when {@link LuceneTestCase#VERBOSE} is set.
515+
*/
516+
private static void assertDocAndScore(
517+
int doc,
518+
float score,
519+
boolean more,
520+
Scorer scorer,
521+
float maxDiff,
522+
int[] order,
523+
int op,
524+
int skipOp,
525+
Query q,
526+
IndexSearcher s)
527+
throws IOException {
528+
int scorerDoc = scorer.docID();
529+
float scorerScore = scorer.score();
530+
float scorerScore2 = scorer.score();
531+
float scoreDiff = Math.abs(score - scorerScore);
532+
float scorerDiff = Math.abs(scorerScore2 - scorerScore);
533+
boolean success = false;
534+
try {
535+
assertTrue(more);
536+
assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
537+
assertTrue("score=" + score + ", scorerScore=" + scorerScore, scoreDiff <= maxDiff);
538+
assertTrue(
539+
"scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2, scorerDiff <= maxDiff);
540+
success = true;
541+
} finally {
542+
if (!success) {
543+
if (LuceneTestCase.VERBOSE) {
544+
StringBuilder sbord = new StringBuilder();
545+
for (int i = 0; i < order.length; i++) {
546+
sbord.append(order[i] == skipOp ? " skip()" : " next()");
547+
}
548+
System.out.println(
549+
"ERROR matching docs:"
550+
+ "\n\t"
551+
+ (doc != scorerDoc ? "--> " : "")
552+
+ "doc="
553+
+ doc
554+
+ ", scorerDoc="
555+
+ scorerDoc
556+
+ "\n\t"
557+
+ (!more ? "--> " : "")
558+
+ "tscorer.more="
559+
+ more
560+
+ "\n\t"
561+
+ (scoreDiff > maxDiff ? "--> " : "")
562+
+ "scorerScore="
563+
+ scorerScore
564+
+ " scoreDiff="
565+
+ scoreDiff
566+
+ " maxDiff="
567+
+ maxDiff
568+
+ "\n\t"
569+
+ (scorerDiff > maxDiff ? "--> " : "")
570+
+ "scorerScore2="
571+
+ scorerScore2
572+
+ " scorerDiff="
573+
+ scorerDiff
574+
+ "\n\thitCollector.doc="
575+
+ doc
576+
+ " score="
577+
+ score
578+
+ "\n\t Scorer="
579+
+ scorer
580+
+ "\n\t Query="
581+
+ q
582+
+ " "
583+
+ q.getClass().getName()
584+
+ "\n\t Searcher="
585+
+ s
586+
+ "\n\t Order="
587+
+ sbord
588+
+ "\n\t Op="
589+
+ (op == skipOp ? " skip()" : " next()"));
590+
}
591+
}
592+
}
593+
}
594+
595+
/** Verifies that a freshly created scorer can advance to {@code doc} from position {@code i}. */
596+
private static void assertAdvanceTo(
597+
int doc, int i, float score, Weight w, LeafReaderContext leafContext, float maxDiff)
598+
throws IOException {
599+
ScorerSupplier supplier = w.scorerSupplier(leafContext);
600+
Scorer scorer = supplier.get(1L); // only checking one doc, so leadCost = 1
601+
assertTrue(
602+
"query collected " + doc + " but advance(" + i + ") says no more docs!",
603+
scorer.iterator().advance(i) != DocIdSetIterator.NO_MORE_DOCS);
604+
assertEquals(
605+
"query collected " + doc + " but advance(" + i + ") got to " + scorer.docID(),
606+
doc,
607+
scorer.docID());
608+
float advanceScore = scorer.score();
609+
assertEquals("unstable advance(" + i + ") score!", advanceScore, scorer.score(), maxDiff);
610+
assertEquals(
611+
"query assigned doc "
612+
+ doc
613+
+ " a score of <"
614+
+ score
615+
+ "> but advance("
616+
+ i
617+
+ ") has <"
618+
+ advanceScore
619+
+ ">!",
620+
score,
621+
advanceScore,
622+
maxDiff);
623+
}
624+
603625
/** Check that the scorer and bulk scorer advance consistently. */
604626
public static void checkBulkScorerSkipTo(Random r, Query query, IndexSearcher searcher)
605627
throws IOException {

0 commit comments

Comments
 (0)