Address TestTaskExecutor test failure

Turns out that testCancelTasksOnException require a single threaded
executor. Given that most tests in the class rely make more sense with a
single thread, I went back to 1 thread for the shared executor and used
a multi-threaded executor in the only test that relies on multiple
threads.
This commit is contained in:
Luca Cavanna 2023-10-24 20:39:27 +02:00
parent f2bf5339e5
commit f6ca54523c
1 changed files with 39 additions and 50 deletions

View File

@ -33,6 +33,7 @@ import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.store.Directory;
import org.apache.lucene.tests.index.RandomIndexWriter;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.lucene.tests.util.TestUtil;
import org.apache.lucene.util.NamedThreadFactory;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
@ -47,7 +48,7 @@ public class TestTaskExecutor extends LuceneTestCase {
public static void createExecutor() {
executorService =
Executors.newFixedThreadPool(
random().nextBoolean() ? 1 : 2,
1,
new NamedThreadFactory(TestTaskExecutor.class.getSimpleName()));
}
@ -259,12 +260,7 @@ public class TestTaskExecutor extends LuceneTestCase {
});
}
expectThrows(RuntimeException.class, () -> taskExecutor.invokeAll(callables));
int maximumPoolSize = ((ThreadPoolExecutor) executorService).getMaximumPoolSize();
if (maximumPoolSize == 1) {
assertEquals(1, tasksExecuted.get());
} else {
MatcherAssert.assertThat(tasksExecuted.get(), Matchers.greaterThanOrEqualTo(1));
}
assertEquals(1, tasksExecuted.get());
// the callables are technically all run, but the cancelled ones will be no-op
assertEquals(100, tasksStarted.get());
}
@ -274,50 +270,46 @@ public class TestTaskExecutor extends LuceneTestCase {
* as suppressed exceptions to the first one caught.
*/
public void testInvokeAllCatchesMultipleExceptions() {
TaskExecutor taskExecutor = new TaskExecutor(executorService);
List<Callable<Void>> callables = new ArrayList<>();
int maximumPoolSize = ((ThreadPoolExecutor) executorService).getMaximumPoolSize();
// if we have multiple threads, make sure both are started before an exception is thrown,
// otherwise there may or may not be a suppressed exception
CountDownLatch latchA = new CountDownLatch(1);
CountDownLatch latchB = new CountDownLatch(1);
callables.add(
() -> {
if (maximumPoolSize > 1) {
latchA.countDown();
latchB.await();
}
throw new RuntimeException("exception A");
});
callables.add(
() -> {
if (maximumPoolSize > 1) {
latchB.countDown();
latchA.await();
}
throw new IllegalStateException("exception B");
});
//this test requires multiple threads, while all the other tests in this class rely on a single threaded executor
ExecutorService multiThreadedExecutor = Executors.newFixedThreadPool(2);
try {
TaskExecutor taskExecutor = new TaskExecutor(multiThreadedExecutor);
List<Callable<Void>> callables = new ArrayList<>();
// if we have multiple threads, make sure both are started before an exception is thrown,
// otherwise there may or may not be a suppressed exception
CountDownLatch latchA = new CountDownLatch(1);
CountDownLatch latchB = new CountDownLatch(1);
callables.add(
() -> {
latchA.countDown();
latchB.await();
throw new RuntimeException("exception A");
});
callables.add(
() -> {
latchB.countDown();
latchA.await();
throw new IllegalStateException("exception B");
});
RuntimeException exc =
expectThrows(RuntimeException.class, () -> taskExecutor.invokeAll(callables));
Throwable[] suppressed = exc.getSuppressed();
RuntimeException exc =
expectThrows(RuntimeException.class, () -> taskExecutor.invokeAll(callables));
Throwable[] suppressed = exc.getSuppressed();
if (maximumPoolSize == 1) {
assertEquals(0, suppressed.length);
} else {
assertEquals(1, suppressed.length);
if (exc.getMessage().equals("exception A")) {
assertEquals("exception B", suppressed[0].getMessage());
} else {
assertEquals("exception A", suppressed[0].getMessage());
assertEquals("exception B", exc.getMessage());
}
assertEquals(1, suppressed.length);
if (exc.getMessage().equals("exception A")) {
assertEquals("exception B", suppressed[0].getMessage());
} else {
assertEquals("exception A", suppressed[0].getMessage());
assertEquals("exception B", exc.getMessage());
}
} finally {
TestUtil.shutdownExecutorService(multiThreadedExecutor);
}
}
public void testCancelTasksOnException() {
TaskExecutor taskExecutor = new TaskExecutor(executorService);
int maximumPoolSize = ((ThreadPoolExecutor) executorService).getMaximumPoolSize();
final int numTasks = random().nextInt(10, 50);
final int throwingTask = random().nextInt(numTasks);
boolean error = random().nextBoolean();
@ -334,7 +326,8 @@ public class TestTaskExecutor extends LuceneTestCase {
throw new RuntimeException();
}
}
if (index > throwingTask && maximumPoolSize == 1) {
if (index > throwingTask) {
//with a single thread we are sure that the last task to run is the one that throws, following ones must not run
throw new AssertionError("task should not have started");
}
executedTasks.incrementAndGet();
@ -348,10 +341,6 @@ public class TestTaskExecutor extends LuceneTestCase {
throwable = expectThrows(RuntimeException.class, () -> taskExecutor.invokeAll(tasks));
}
assertEquals(0, throwable.getSuppressed().length);
if (maximumPoolSize == 1) {
assertEquals(throwingTask, executedTasks.get());
} else {
MatcherAssert.assertThat(executedTasks.get(), Matchers.greaterThanOrEqualTo(throwingTask));
}
assertEquals(throwingTask, executedTasks.get());
}
}