diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorService.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorService.java index 7818c0fb48f7..365f72d67d80 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorService.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorService.java @@ -276,6 +276,7 @@ void invokeAll(List testTasks) { private List forkConcurrentChildren(List children, Consumer isolatedTaskCollector, List sameThreadTasks) { + int index = 0; List queueEntries = new ArrayList<>(children.size()); for (TestTask child : children) { if (requiresGlobalReadWriteLock(child)) { @@ -285,7 +286,7 @@ else if (child.getExecutionMode() == SAME_THREAD) { sameThreadTasks.add(child); } else { - queueEntries.add(WorkQueue.Entry.create(child)); + queueEntries.add(WorkQueue.Entry.createWithIndex(child, index++)); } } @@ -570,12 +571,16 @@ boolean isEmpty() { return queue.isEmpty(); } - private record Entry(TestTask task, CompletableFuture<@Nullable Void> future, int level) + private record Entry(TestTask task, CompletableFuture<@Nullable Void> future, int level, int index) implements Comparable { static Entry create(TestTask task) { + return createWithIndex(task, 0); + } + + static Entry createWithIndex(TestTask task, int index) { int level = task.getTestDescriptor().getUniqueId().getSegments().size(); - return new Entry(task, new CompletableFuture<>(), level); + return new Entry(task, new CompletableFuture<>(), level, index); } @SuppressWarnings("FutureReturnValueIgnored") @@ -593,10 +598,14 @@ static Entry create(TestTask task) { @Override public int compareTo(Entry that) { var result = Integer.compare(that.level, this.level); - if (result == 0) { - result = Boolean.compare(this.isContainer(), that.isContainer()); + if (result != 0) { + return result; } - return result; + result = Boolean.compare(this.isContainer(), that.isContainer()); + if (result != 0) { + return result; + } + return Integer.compare(that.index, this.index); } private boolean isContainer() { diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorServiceTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorServiceTests.java index 306c7c9dbb36..b996eb887352 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorServiceTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorServiceTests.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Stream; @@ -230,13 +231,14 @@ void prioritizesChildrenOfStartedContainers() throws Exception { service = new ConcurrentHierarchicalTestExecutorService(configuration(2)); var leavesStarted = new CountDownLatch(2); - var leaf = new TestTaskStub(ExecutionMode.CONCURRENT, leavesStarted::countDown) // - .withName("leaf").withLevel(3); - var child1 = new TestTaskStub(ExecutionMode.CONCURRENT, () -> requiredService().submit(leaf).get()) // + + var child1 = new TestTaskStub(ExecutionMode.CONCURRENT, leavesStarted::await) // .withName("child1").withLevel(2); var child2 = new TestTaskStub(ExecutionMode.CONCURRENT, leavesStarted::countDown) // .withName("child2").withLevel(2); - var child3 = new TestTaskStub(ExecutionMode.SAME_THREAD, leavesStarted::await) // + var leaf = new TestTaskStub(ExecutionMode.CONCURRENT, leavesStarted::countDown) // + .withName("leaf").withLevel(3); + var child3 = new TestTaskStub(ExecutionMode.CONCURRENT, () -> requiredService().submit(leaf).get()) // .withName("child3").withLevel(2); var root = new TestTaskStub(ExecutionMode.SAME_THREAD, @@ -246,11 +248,11 @@ void prioritizesChildrenOfStartedContainers() throws Exception { service.submit(root).get(); root.assertExecutedSuccessfully(); - assertThat(List.of(child1, child2, child3)).allSatisfy(TestTaskStub::assertExecutedSuccessfully); + assertThat(List.of(child1, child2, leaf, child3)).allSatisfy(TestTaskStub::assertExecutedSuccessfully); leaf.assertExecutedSuccessfully(); assertThat(leaf.startTime).isBeforeOrEqualTo(child2.startTime); - assertThat(leaf.executionThread).isSameAs(child1.executionThread); + assertThat(leaf.executionThread).isSameAs(child3.executionThread); } @Test @@ -371,6 +373,84 @@ public void release() { .containsOnly(child2.executionThread); } + @RepeatedTest(value = 100, failureThreshold = 1) + void executesChildrenInOrder() throws Exception { + service = new ConcurrentHierarchicalTestExecutorService(configuration(1, 1)); + + var leaf1a = new TestTaskStub(ExecutionMode.CONCURRENT) // + .withName("leaf1a").withLevel(2); + var leaf1b = new TestTaskStub(ExecutionMode.CONCURRENT) // + .withName("leaf1b").withLevel(2); + var leaf1c = new TestTaskStub(ExecutionMode.CONCURRENT) // + .withName("leaf1c").withLevel(2); + var leaf1d = new TestTaskStub(ExecutionMode.CONCURRENT) // + .withName("leaf1d").withLevel(2); + + var root = new TestTaskStub(ExecutionMode.SAME_THREAD, + () -> requiredService().invokeAll(List.of(leaf1a, leaf1b, leaf1c, leaf1d))) // + .withName("root").withLevel(1); + + service.submit(root).get(); + + assertThat(List.of(root, leaf1a, leaf1b, leaf1c, leaf1d)) // + .allSatisfy(TestTaskStub::assertExecutedSuccessfully); + + assertThat(Stream.of(leaf1a, leaf1b, leaf1c, leaf1d)) // + .extracting(TestTaskStub::startTime) // + .isSorted(); + } + + @RepeatedTest(value = 100, failureThreshold = 1) + void workIsStolenInReverseOrder() throws Exception { + service = new ConcurrentHierarchicalTestExecutorService(configuration(2, 2)); + + // Execute tasks pairwise + CyclicBarrier cyclicBarrier = new CyclicBarrier(2); + Executable behavior = cyclicBarrier::await; + + // With half of the leaves to be executed normally + var leaf1a = new TestTaskStub(ExecutionMode.CONCURRENT, behavior) // + .withName("leaf1a").withLevel(2); + var leaf1b = new TestTaskStub(ExecutionMode.CONCURRENT, behavior) // + .withName("leaf1b").withLevel(2); + var leaf1c = new TestTaskStub(ExecutionMode.CONCURRENT, behavior) // + .withName("leaf1c").withLevel(2); + + // And half of the leaves to be stolen + var leaf2a = new TestTaskStub(ExecutionMode.CONCURRENT, behavior) // + .withName("leaf2a").withLevel(2); + var leaf2b = new TestTaskStub(ExecutionMode.CONCURRENT, behavior) // + .withName("leaf2b").withLevel(2); + var leaf2c = new TestTaskStub(ExecutionMode.CONCURRENT, behavior) // + .withName("leaf2c").withLevel(2); + + var root = new TestTaskStub(ExecutionMode.SAME_THREAD, + () -> requiredService().invokeAll(List.of(leaf1a, leaf1b, leaf1c, leaf2a, leaf2b, leaf2c))) // + .withName("root").withLevel(1); + + service.submit(root).get(); + + assertThat(List.of(root, leaf1a, leaf1b, leaf1c, leaf2a, leaf2b, leaf2c)) // + .allSatisfy(TestTaskStub::assertExecutedSuccessfully); + + // If the last node was stolen. + assertThat(leaf1a.executionThread).isNotEqualTo(leaf2c.executionThread); + // Then it must follow that the last half of the nodes were stolen + assertThat(Stream.of(leaf1a, leaf1b, leaf1c)) // + .extracting(TestTaskStub::executionThread) // + .containsOnly(leaf1a.executionThread); + assertThat(Stream.of(leaf2a, leaf2b, leaf2c)) // + .extracting(TestTaskStub::executionThread) // + .containsOnly(leaf2c.executionThread); + + assertThat(Stream.of(leaf1a, leaf1b, leaf1c)) // + .extracting(TestTaskStub::startTime) // + .isSorted(); + assertThat(Stream.of(leaf2c, leaf2b, leaf2a)) // + .extracting(TestTaskStub::startTime) // + .isSorted(); + } + private static ExclusiveResource exclusiveResource(LockMode lockMode) { return new ExclusiveResource("key", lockMode); } @@ -486,6 +566,11 @@ Thread executionThread() { return executionThread; } + @Nullable + Instant startTime() { + return startTime; + } + @Override public String toString() { return "%s @ %s".formatted(new ToStringBuilder(this).append("name", name), Integer.toHexString(hashCode()));