zacharymorn commented on PR #972: URL: https://github.com/apache/lucene/pull/972#issuecomment-1170684358
> With this change, I suspect that some scorers created in `TestWANDScorer` would now use your new `BlockMaxMaxScoreScorer`, which is going to decrease the coverage of WANDScorer. Can we somehow make sure that `TestWANDScorer` always gets a `WANDScorer`? E.g. I spotted this query under `TestWANDScorer#testBasics` which likely uses your now scorer: > > ```java > // test a filtered disjunction > query = > new BooleanQuery.Builder() > .add( > new BooleanQuery.Builder() > .add( > new BoostQuery( > new ConstantScoreQuery(new TermQuery(new Term("foo", "A"))), 2), > Occur.SHOULD) > .add(new ConstantScoreQuery(new TermQuery(new Term("foo", "B"))), Occur.SHOULD) > .build(), > Occur.MUST) > .add(new TermQuery(new Term("foo", "C")), Occur.FILTER) > .build(); > ``` Yeah this is a good question. In my newly added tests I have used something like this to confirm it's testing the right scorer, but I'm not totally happy about this approach myself : ``` if (scorer instanceof AssertingScorer) { assertTrue(((AssertingScorer) scorer).getIn() instanceof BlockMaxMaxscoreScorer); } else { assertTrue(scorer instanceof BlockMaxMaxscoreScorer); } ``` One alternative approach could be instantiating `WANDScorer` directly inside the test for lower level tests, and moving the higher level tests into another test class that doesn't care about the specific scorer implementation for disjunction? This may require duplicating some code from `BooleanWeight`, `AssertingWeight` etc though but should be do-able. On the other hand, if we don't plan on initiating `WANDScorer` directly in the test, varying the query clauses and asserting like above might be the best we could do I feel? This has the potential test coverage decrease issue as you suggested so may not be ideal either. -- 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