mrkm4ntr commented on code in PR #16133:
URL: https://github.com/apache/lucene/pull/16133#discussion_r3339407328
##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestProfilerCollector.java:
##########
@@ -86,6 +94,35 @@ protected String deriveCollectorName(Collector c) {
assertEquals("TestCollector(TotalHitCountCollector)", collector.getName());
}
+ public void testCompetitiveIteratorDelegation() throws IOException {
Review Comment:
Testing FilterLeafCollector directly for competitiveIterator() delegation
would be tautological — it would just assert that the method calls through to
in. The same applies to setScorer, collect, and finish, none of which have
delegation tests either. There's no reason to treat competitiveIterator
differently. The sandbox test (testCompetitiveIteratorDelegation) verifies the
delegation indirectly through a real use case with ProfilerCollectorWrapper.
##########
lucene/sandbox/src/test/org/apache/lucene/sandbox/search/TestProfilerCollector.java:
##########
@@ -86,6 +94,35 @@ protected String deriveCollectorName(Collector c) {
assertEquals("TestCollector(TotalHitCountCollector)", collector.getName());
}
+ public void testCompetitiveIteratorDelegation() throws IOException {
+ Directory dir = newDirectory();
+ RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+ final int numDocs = TestUtil.nextInt(random(), 10, 100);
+ for (int i = 0; i < numDocs; i++) {
+ Document doc = new Document();
+ doc.add(new LongPoint("score", i));
+ doc.add(new NumericDocValuesField("score", i));
+ w.addDocument(doc);
+ }
+ w.forceMerge(1);
+ DirectoryReader reader = w.getReader();
+ w.close();
+
+ SortField sortField = new SortField("score", SortField.Type.LONG);
+ TopFieldCollectorManager topFieldCollectorManager =
+ new TopFieldCollectorManager(new Sort(sortField), 1,
Integer.MAX_VALUE);
+
+ ProfilerCollectorWrapper wrapper =
+ new ProfilerCollectorWrapper(topFieldCollectorManager.newCollector());
+ LeafCollector leafCollector =
wrapper.getLeafCollector(reader.leaves().get(0));
+
+ DocIdSetIterator competitiveIterator = leafCollector.competitiveIterator();
+ assertNotNull(competitiveIterator);
Review Comment:
The correctness of the competitive iterator itself (e.g., that it properly
skips non-competitive documents) is already tested in TestNumericComparator and
related tests. The responsibility of this fix is ensuring that
FilterLeafCollector delegates competitiveIterator() rather than silently
returning null. Verifying non-null is sufficient to confirm delegation is
working, and going further would be testing the comparator's behavior rather
than the fix itself.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
