quux00 commented on code in PR #12523:
URL: https://github.com/apache/lucene/pull/12523#discussion_r1344356526
##########
lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java:
##########
@@ -267,7 +265,132 @@ protected LeafSlice[] slices(List<LeafReaderContext>
leaves) {
return slices.toArray(new LeafSlice[0]);
}
};
- searcher.search(new MatchAllDocsQuery(), 10);
- assertEquals(leaves.size(), numExecutions.get());
+ TopDocs topDocs = searcher.search(new MatchAllDocsQuery(), 10);
+ assertTrue(topDocs.totalHits.value > 0);
+ if (leaves.size() <= 1) {
+ assertEquals(0, numExecutions.get());
+ } else {
+ assertEquals(leaves.size(), numExecutions.get());
+ }
+ }
+
+ /**
+ * The goal of this test is to ensure that TaskExecutor.invokeAll waits for
all tasks/callables to
+ * finish even if one or more of them throw an Exception.
+ *
+ * <p>To make the test deterministic, a custom single threaded executor is
used. And to ensure
+ * that TaskExecutor.invokeAll does not return early upon getting an
Exception, two Exceptions are
+ * thrown in the underlying Query class (in the Weight#scorer method). The
first Exception is
+ * thrown by the first call to Weight#scorer and the last Exception is
thrown by the last call to
+ * Weight#scorer. Since TaskExecutor.invokeAll adds subsequent Exceptions to
the first one caught
+ * as a suppressed Exception, we can check that both exceptions were thrown,
ensuring that all
+ * TaskExecutor#invokeAll check all tasks (using future.get()) before it
returned.
+ */
+ public void testMultipleSegmentsOnTheExecutorWithException() {
+ List<LeafReaderContext> leaves = reader.leaves();
+ IndexSearcher searcher =
+ new IndexSearcher(
+ reader,
+ task -> {
+ task.run();
Review Comment:
Good idea. That is cleaner to do in the `TestTaskExcecutor`. I have added
two tests there, one testing leaving no tasks behind (similar to yours) and
another that throws two exceptions and ensures that the second is added as a
suppressed exception.
##########
lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java:
##########
@@ -267,7 +265,132 @@ protected LeafSlice[] slices(List<LeafReaderContext>
leaves) {
return slices.toArray(new LeafSlice[0]);
}
};
- searcher.search(new MatchAllDocsQuery(), 10);
- assertEquals(leaves.size(), numExecutions.get());
+ TopDocs topDocs = searcher.search(new MatchAllDocsQuery(), 10);
+ assertTrue(topDocs.totalHits.value > 0);
+ if (leaves.size() <= 1) {
+ assertEquals(0, numExecutions.get());
+ } else {
+ assertEquals(leaves.size(), numExecutions.get());
+ }
+ }
+
+ /**
+ * The goal of this test is to ensure that TaskExecutor.invokeAll waits for
all tasks/callables to
+ * finish even if one or more of them throw an Exception.
+ *
+ * <p>To make the test deterministic, a custom single threaded executor is
used. And to ensure
+ * that TaskExecutor.invokeAll does not return early upon getting an
Exception, two Exceptions are
+ * thrown in the underlying Query class (in the Weight#scorer method). The
first Exception is
+ * thrown by the first call to Weight#scorer and the last Exception is
thrown by the last call to
+ * Weight#scorer. Since TaskExecutor.invokeAll adds subsequent Exceptions to
the first one caught
+ * as a suppressed Exception, we can check that both exceptions were thrown,
ensuring that all
+ * TaskExecutor#invokeAll check all tasks (using future.get()) before it
returned.
+ */
+ public void testMultipleSegmentsOnTheExecutorWithException() {
+ List<LeafReaderContext> leaves = reader.leaves();
+ IndexSearcher searcher =
+ new IndexSearcher(
+ reader,
+ task -> {
+ task.run();
Review Comment:
Good idea. That is cleaner to do it in the `TestTaskExcecutor`. I have added
two tests there, one testing leaving no tasks behind (similar to yours) and
another that throws two exceptions and ensures that the second is added as a
suppressed exception.
--
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
