bruno-roustant commented on a change in pull request #1998:
URL: https://github.com/apache/lucene-solr/pull/1998#discussion_r509919492



##########
File path: 
lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java
##########
@@ -163,6 +163,17 @@ public void testExitableFilterTermsIndexReader() throws 
Exception {
     searcher.search(query, 10);
     reader.close();
 
+    // Set a fairly high timeout value (infinite) and expect the query to 
complete in that time frame.
+    // Not checking the validity of the result, but checking the sampling 
kicks in to reduce the number of timeout check
+    CountingQueryTimeout queryTimeout = new CountingQueryTimeout();
+    directoryReader = DirectoryReader.open(directory);
+    exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, 
queryTimeout);
+    reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader));
+    searcher = new IndexSearcher(reader);
+    searcher.search(query, 10);
+    reader.close();
+    assertEquals(3, queryTimeout.getShouldExitCallCount());

Review comment:
       This test is good. Can you actually add it to the PR? It should replace 
the lines added to testExitableFilterTermsIndexReader().
   
   Your investigation reveals something I overlooked - the MultiTerm query is 
not rewritten to TermQueries above 16 matching terms.
   - Just replace assertEquals(54, ...) by assertEquals(5, ...).
   - Please add a comment above the assertEquals(5, ...) to explain it depends 
on ExitableDirectoryReader.ExitableTermsEnum.NUM_CALLS_PER_TIMEOUT_CHECK and 
MultiTermQueryConstantScoreWrapper.BOOLEAN_REWRITE_TERM_COUNT_THRESHOLD (so if 
someone changes one of these constants - unlikely -, she will know why this 
test fail and that it needs to be adjusted).
   - I remove my remark about sampling ExitableTermsEnum constructor because we 
don't need it.




----------------------------------------------------------------
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.

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

Reply via email to