MarcusSorealheis commented on PR #12208: URL: https://github.com/apache/lucene/pull/12208#issuecomment-1498148310
> Sorry this is not what I meant. So first, the intend of the added unit test should test the explain code you added, but not rewriting nor explain implementation of TermQuery or PhraseQuery. Then, look at how IndexSearcher implement explain ([src](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java#L831)), it will first rewrite the query, and then call the explain for the _rewritten_ query's weight. So, if we want to test the code you just added, we need to make sure the rewritten query is actually still TermAutomatonQuery. Last, let's look at the rewrite logic [here](https://github.com/apache/lucene/blob/main/lucene/sandbox/src/java/org/apache/lucene/sandbox/search/TermAutomatonQuery.java#L529), I haven't dive too deep there but basically there's a comment > > ``` > // Try for either PhraseQuery or MultiPhraseQuery, which only works when the automaton is a > // sausage: > ``` > > So I guess we should be able to figure out a way to let it not rewrite to other query type. > > So to summary, I think in the test we should: > > 1. create a query that won't rewrite to other type > 2. call searcher.rewrite and assert the query type to double check > 3. check what you claimed (like NoMatchingDocument or 2 documents matched) > 4. check the explain implementation, > 4.1. if there's no document matched, then make sure the explain returns `NO_MATCH` for any documents > 4.2. if there's document matched, make sure the document matched returns proper explanation. > > I hope it makes sense It does make sense. To be fair, it probably made sense from the first time you said it. I think it needed to percolate in my brain for a bit. I think I've got it now. Or I hope I do. I don't want to get into modifying rewrite because that would be a big change, out of scope for this effort, do the pref testing required a regression risks. -- 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
