Reduce complexity by duplication

Author: mpkorstanje

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

@@ -14,7 +14,6 @@
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.CompletableFuture.completedFuture;
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static java.util.function.Predicate.isEqual;
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 import static org.junit.platform.commons.util.ExceptionUtils.throwAsUncheckedException;
 import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE;
@@ -46,7 +45,6 @@
 import java.util.function.BiFunction;
 import java.util.function.BooleanSupplier;
 import java.util.function.Consumer;
-import java.util.function.Predicate;
 
 import org.apiguardian.api.API;
 import org.jspecify.annotations.Nullable;
@@ -294,15 +292,29 @@ void processQueueEntries(WorkerLease workerLease, BooleanSupplier doneCondition)
 		}
 
 		private void processQueueEntries() {
-			var queueEntriesByResult = tryToStealWorkWithoutBlocking(workQueue,
-				isEqual(WorkStealResult.EXECUTED_BY_THIS_WORKER));
-			var queueModified = queueEntriesByResult.containsKey(WorkStealResult.EXECUTED_BY_THIS_WORKER) //
-					|| queueEntriesByResult.containsKey(WorkStealResult.EXECUTED_BY_DIFFERENT_WORKER);
+			var entriesRequiringResourceLocks = new ArrayList<WorkQueue.Entry>();
+			var queueModified = false;
+
+			for (var entry : workQueue) {
+				var result = tryToStealWork(entry, BlockingMode.NON_BLOCKING);
+				// After executing a test a significant amount of time has passed.
+				// Process the queue from the beginning
+				if (result == WorkStealResult.EXECUTED_BY_THIS_WORKER) {
+					return;
+				}
+				if (result == WorkStealResult.EXECUTED_BY_DIFFERENT_WORKER) {
+					queueModified = true;
+				}
+				if (result == WorkStealResult.RESOURCE_LOCK_UNAVAILABLE) {
+					entriesRequiringResourceLocks.add(entry);
+				}
+			}
+			// The queue changed while we looked at it.
+			// Check from the start before processing any blocked items.
 			if (queueModified) {
 				return;
 			}
-			var entriesRequiringResourceLocks = queueEntriesByResult.get(WorkStealResult.RESOURCE_LOCK_UNAVAILABLE);
-			if (entriesRequiringResourceLocks != null) {
+			if (!entriesRequiringResourceLocks.isEmpty()) {
 				// One entry at a time to avoid blocking too much
 				tryToStealWork(entriesRequiringResourceLocks.get(0), BlockingMode.BLOCKING);
 			}
@@ -339,7 +351,7 @@ void invokeAll(List<? extends TestTask> testTasks) {
 			List<TestTask> sameThreadTasks = new ArrayList<>(testTasks.size());
 			var queueEntries = forkConcurrentChildren(testTasks, isolatedTasks::add, sameThreadTasks);
 			executeAll(sameThreadTasks);
-			var queueEntriesByResult = tryToStealWorkWithoutBlocking(queueEntries, __ -> false);
+			var queueEntriesByResult = tryToStealWorkWithoutBlocking(queueEntries);
 			tryToStealWorkWithBlocking(queueEntriesByResult);
 			waitFor(queueEntriesByResult);
 			executeAll(isolatedTasks);
@@ -374,10 +386,10 @@ else if (child.getExecutionMode() == SAME_THREAD) {
 		}
 
 		private Map<WorkStealResult, List<WorkQueue.Entry>> tryToStealWorkWithoutBlocking(
-				Iterable<WorkQueue.Entry> queueEntries, Predicate<? super WorkStealResult> stopCondition) {
+				Iterable<WorkQueue.Entry> queueEntries) {
 
 			Map<WorkStealResult, List<WorkQueue.Entry>> queueEntriesByResult = new EnumMap<>(WorkStealResult.class);
-			tryToStealWork(queueEntries, BlockingMode.NON_BLOCKING, queueEntriesByResult, stopCondition);
+			tryToStealWork(queueEntries, BlockingMode.NON_BLOCKING, queueEntriesByResult);
 			return queueEntriesByResult;
 		}
 
@@ -386,18 +398,14 @@ private void tryToStealWorkWithBlocking(Map<WorkStealResult, List<WorkQueue.Entr
 			if (entriesRequiringResourceLocks == null) {
 				return;
 			}
-			tryToStealWork(entriesRequiringResourceLocks, BlockingMode.BLOCKING, queueEntriesByResult, __ -> false);
+			tryToStealWork(entriesRequiringResourceLocks, BlockingMode.BLOCKING, queueEntriesByResult);
 		}
 
 		private void tryToStealWork(Iterable<WorkQueue.Entry> entries, BlockingMode blocking,
-				Map<WorkStealResult, List<WorkQueue.Entry>> queueEntriesByResult,
-				Predicate<? super WorkStealResult> stopCondition) {
+				Map<WorkStealResult, List<WorkQueue.Entry>> queueEntriesByResult) {
 			for (var entry : entries) {
 				var result = tryToStealWork(entry, blocking);
 				queueEntriesByResult.computeIfAbsent(result, __ -> new ArrayList<>()).add(entry);
-				if (stopCondition.test(result)) {
-					break;
-				}
 			}
 		}