From 6dee57c6280701448568bbd9d7f5bbe6db9cff3e Mon Sep 17 00:00:00 2001
From: Heiko Klare <heiko.klare@vector.com>
Date: Mon, 4 Sep 2023 12:47:54 +0200
Subject: [PATCH] Fix faulty and random failing IJobManagerTest.testOrder #628

The test case IJobManagerTest.testOrder is randomly failing. It is
supposed to validate that Jobs are execute in the order in which they
are scheduled. To this end, it asserts that the run methods of the Jobs
are executed in the order in which the have been scheduled. There is,
however, no guarantee for an execution order of the run methods but only
for the order in which the Jobs are picked by a Worker to be executed.

This change rewrites the test case to only validate that whenever a
Job's run method is executed the previously scheduled jobs have started
running or have already finished as well, i.e., are either in the state
RUNNING or NONE.

Fixes https://github.com/eclipse-platform/eclipse.platform/issues/628
---
 .../tests/runtime/jobs/IJobManagerTest.java   | 46 ++++++++++---------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/runtime/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java b/runtime/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java
index 721af86f99a..ba1c62ed4c7 100644
--- a/runtime/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java
+++ b/runtime/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java
@@ -13,17 +13,18 @@
  *******************************************************************************/
 package org.eclipse.core.tests.runtime.jobs;
 
+import static java.util.Collections.synchronizedList;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.collection.IsEmptyCollection.empty;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
-import java.util.Queue;
 import java.util.Set;
-import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicIntegerArray;
@@ -1743,34 +1744,37 @@ public void testMutexRule() {
 		jobs[JOB_COUNT - 1].cancel();
 	}
 
-	public void testOrder() {
-		//ensure jobs are run in order from lowest to highest sleep time.
-		final Queue<Job> done = new ConcurrentLinkedQueue<>();
-		int[] sleepTimes = new int[] { 5, 200, 400, 600 };
-		Job[] jobs = new Job[sleepTimes.length];
-		for (int i = 0; i < sleepTimes.length; i++) {
-			jobs[i] = new Job("testOrder(" + i + ")") {
+	public void testOrder() throws Exception {
+		// ensure jobs are run in order from lowest to highest sleep time.
+		int[] sleepTimes = new int[] { 0, 1, 2, 5, 10, 15, 25, 50 };
+		final LinkedList<Job> allJobs = new LinkedList<>();
+		final List<Job> jobsRunningBeforePrevious = synchronizedList(new ArrayList<>());
 
+		for (int sleepTime : sleepTimes) {
+			final Job previouslyScheduledJob = allJobs.isEmpty() ? null : allJobs.getLast();
+			Job currentJob = new Job("testOrder job to be run with sleep time " + sleepTime) {
 				@Override
 				protected IStatus run(IProgressMonitor monitor) {
-					done.add(this);
+					if (!hasPreviousJobStartedRunning()) {
+						jobsRunningBeforePrevious.add(this);
+					}
 					return Status.OK_STATUS;
 				}
 
+				private boolean hasPreviousJobStartedRunning() {
+					return previouslyScheduledJob == null || previouslyScheduledJob.getState() == Job.RUNNING
+							|| previouslyScheduledJob.getState() == Job.NONE;
+				}
 			};
+			currentJob.schedule(sleepTime);
+			allJobs.add(currentJob);
 		}
-		for (int i = 0; i < sleepTimes.length; i++) {
-			jobs[i].schedule(sleepTimes[i]);
-		}
-		// make sure listener has had a chance to process the finished job
-		while (done.size() != jobs.length) {
-			Thread.yield();
-		}
-		Job[] doneOrder = done.toArray(new Job[done.size()]);
-		assertEquals("1.0", jobs.length, doneOrder.length);
-		for (int i = 0; i < doneOrder.length; i++) {
-			assertEquals("1.1." + i, jobs[i], doneOrder[i]);
+		for (Job job : allJobs) {
+			job.join();
 		}
+
+		assertThat("there have jobs started running before a previously scheduled one", jobsRunningBeforePrevious,
+				empty());
 	}
 
 	public void testReverseOrder() throws InterruptedException {