Skip to content

Commit 6dd04b2

Browse files
vogellaclaude
andcommitted
Fix flaky testItemOrder in ProgressViewTests
The test was failing randomly due to a race condition with the ProgressViewerComparator's stable sorting mechanism (lastIndexes). Root cause: - The comparator maintains a HashMap of previous positions to provide visual stability (preventing jobs from jumping around in the UI) - During test execution, throttled updates would occur while jobs were being scheduled, causing some jobs to be added to lastIndexes before others - This resulted in an inconsistent baseline where jobs were sorted by schedule order rather than priority order Fix: - Reopen the progress view after all jobs are scheduled and running to reset the comparator's lastIndexes state - This ensures all jobs are sorted by priority from a clean state, eliminating the race condition - Removed the unreliable timing-based approach that tried to manipulate throttled updates The test now passes reliably by working with the stable sorting behavior rather than against it. Fixes eclipse-platform#195 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 9c171a0 commit 6dd04b2

File tree

1 file changed

+28
-16
lines changed

1 file changed

+28
-16
lines changed

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressViewTests.java

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import java.util.Collections;
2626
import java.util.concurrent.TimeUnit;
2727

28-
import org.eclipse.core.runtime.IProgressMonitor;
2928
import org.eclipse.core.runtime.IStatus;
3029
import org.eclipse.core.runtime.Status;
3130
import org.eclipse.core.runtime.jobs.Job;
@@ -152,36 +151,49 @@ public void testItemOrder() throws Exception {
152151
// allJobs.add(keptJob);
153152

154153
try {
154+
// Set all jobs to not finish immediately so they stay running for comparison
155+
for (DummyJob job : jobsToSchedule) {
156+
job.shouldFinish = false;
157+
}
158+
159+
// Close and reopen the progress view to get a fresh comparator with empty lastIndexes
160+
hideProgressView();
161+
openProgressView();
162+
163+
// Schedule all jobs in random order rapidly to minimize throttled updates between schedules
155164
ArrayList<DummyJob> shuffledJobs = new ArrayList<>(jobsToSchedule);
156165
Collections.shuffle(shuffledJobs);
157166
StringBuilder scheduleOrder = new StringBuilder("Jobs schedule order: ");
158-
progressView.getViewer().refresh(); // order will only hold on the first time.
159-
Thread.sleep(200); // wait till throttled update ran.
160-
Job dummyJob = new Job("dummy throttled caller") {
161-
@Override
162-
protected IStatus run(IProgressMonitor monitor) {
163-
return Status.OK_STATUS;
164-
}
165-
};
166-
dummyJob.schedule(); // trigger throttled update to clear ProgressViewerComparator.lastIndexes
167-
// now hope the loop is executed before next throttled update (could fail if VM
168-
// is busy otherwise):
169167
for (DummyJob job : shuffledJobs) {
170-
job.shouldFinish = false;
171-
job.schedule(); // if the schedule updates the progress View (throttled) the sort order is
172-
// affected
168+
job.schedule();
173169
scheduleOrder.append(job.getName()).append(", ");
174170
}
175171
TestPlugin.getDefault().getLog()
176172
.log(new Status(IStatus.OK, TestPlugin.PLUGIN_ID, scheduleOrder.toString()));
177173

174+
// Wait for all jobs to be running
178175
for (DummyJob job : allJobs) {
179176
processEventsUntil(() -> job.inProgress, TimeUnit.SECONDS.toMillis(3));
180177
}
181-
progressView.getViewer().refresh();
178+
179+
// Wait for the progress view to be updated with all jobs
180+
processEventsUntil(() -> progressView.getViewer().getProgressInfoItems().length == allJobs.size(),
181+
TimeUnit.SECONDS.toMillis(5));
182+
183+
// The ProgressViewerComparator uses lastIndexes for stable sorting. After throttled updates
184+
// during job scheduling, lastIndexes may contain jobs in schedule order rather than priority order.
185+
// Close and reopen the view again to reset the comparator's state, then do a single refresh
186+
// to sort all jobs by priority from a clean state.
187+
hideProgressView();
188+
openProgressView();
189+
190+
// Wait for the view to be populated again
182191
processEventsUntil(() -> progressView.getViewer().getProgressInfoItems().length == allJobs.size(),
183192
TimeUnit.SECONDS.toMillis(5));
184193

194+
// Give time for any pending updates
195+
processEventsUntil(() -> false, 500);
196+
185197
ProgressInfoItem[] progressInfoItems = progressView.getViewer().getProgressInfoItems();
186198
assertEquals("Not all jobs visible in progress view", allJobs.size(), progressInfoItems.length);
187199
Object[] expected = allJobs.toArray();

0 commit comments

Comments
 (0)