Prioritize tests over containers

Author: marcphilipp

junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorService.java

@@ -473,10 +473,18 @@ Entry incrementAttempts() {
 			public int compareTo(Entry that) {
 				var result = Integer.compare(that.level, this.level);
 				if (result == 0) {
-					return Integer.compare(that.attempts, this.attempts);
+					result = Boolean.compare(this.isContainer(), that.isContainer());
+					if (result == 0) {
+						result = Integer.compare(that.attempts, this.attempts);
+					}
 				}
 				return result;
 			}
+
+			private boolean isContainer() {
+				return task.getTestDescriptor().isContainer();
+			}
+
 		}
 
 	}

platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ConcurrentHierarchicalTestExecutorServiceTests.java

@@ -16,6 +16,8 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.platform.commons.test.PreconditionAssertions.assertPreconditionViolationFor;
 import static org.junit.platform.commons.util.ExceptionUtils.throwAsUncheckedException;
+import static org.junit.platform.engine.TestDescriptor.Type.CONTAINER;
+import static org.junit.platform.engine.TestDescriptor.Type.TEST;
 import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
@@ -245,6 +247,33 @@ void prioritizesChildrenOfStartedContainers() throws Exception {
 		assertThat(leaf.startTime).isBeforeOrEqualTo(child2.startTime);
 	}
 
+	@Test
+	void prioritizesTestsOverContainers() throws Exception {
+		service = new ConcurrentHierarchicalTestExecutorService(configuration(2));
+
+		var leavesStarted = new CountDownLatch(2);
+		var leaf = new TestTaskStub(ExecutionMode.CONCURRENT, leavesStarted::countDown) //
+				.withName("leaf").withLevel(3).withType(TEST);
+		var child1 = new TestTaskStub(ExecutionMode.CONCURRENT, () -> requiredService().submit(leaf).get()) //
+				.withName("child1").withLevel(2).withType(CONTAINER);
+		var child2 = new TestTaskStub(ExecutionMode.CONCURRENT, leavesStarted::countDown) //
+				.withName("child2").withLevel(2).withType(TEST);
+		var child3 = new TestTaskStub(ExecutionMode.SAME_THREAD, leavesStarted::await) //
+				.withName("child3").withLevel(2).withType(TEST);
+
+		var root = new TestTaskStub(ExecutionMode.SAME_THREAD,
+			() -> requiredService().invokeAll(List.of(child1, child2, child3))) //
+					.withName("root").withLevel(1);
+
+		service.submit(root).get();
+
+		root.assertExecutedSuccessfully();
+		assertThat(List.of(child1, child2, child3)).allSatisfy(TestTaskStub::assertExecutedSuccessfully);
+		leaf.assertExecutedSuccessfully();
+
+		assertThat(child2.startTime).isBeforeOrEqualTo(child1.startTime);
+	}
+
 	private static ExclusiveResource exclusiveResource() {
 		return new ExclusiveResource("key", ExclusiveResource.LockMode.READ_WRITE);
 	}
@@ -267,6 +296,7 @@ private static final class TestTaskStub implements TestTask {
 		private ResourceLock resourceLock = NopLock.INSTANCE;
 		private @Nullable String name;
 		private int level = 1;
+		private TestDescriptor.Type type = TEST;
 
 		private final CompletableFuture<@Nullable Void> result = new CompletableFuture<>();
 		private volatile @Nullable Instant startTime;
@@ -292,6 +322,11 @@ TestTaskStub withLevel(int level) {
 			return this;
 		}
 
+		TestTaskStub withType(TestDescriptor.Type type) {
+			this.type = type;
+			return this;
+		}
+
 		TestTaskStub withResourceLock(ResourceLock resourceLock) {
 			this.resourceLock = resourceLock;
 			return this;
@@ -314,7 +349,12 @@ public TestDescriptor getTestDescriptor() {
 			for (var i = 1; i < level; i++) {
 				uniqueId = uniqueId.append("child", name);
 			}
-			return new TestDescriptorStub(uniqueId, name);
+			return new TestDescriptorStub(uniqueId, name) {
+				@Override
+				public Type getType() {
+					return type;
+				}
+			};
 		}
 
 		@Override