Skip to content

Conversation

@vogella
Copy link
Contributor

@vogella vogella commented Nov 9, 2025

Summary

Fixes #370 - The testKeepOneProperty test was failing randomly due to timing issues when multiple jobs with KEEPONE_PROPERTY finish simultaneously.

Problem

The test exhibited flaky behavior where it would:

  • Sometimes see 20 kept jobs instead of the expected 1
  • Sometimes see 0 kept jobs
  • Fail with "expected:<1> but was:<20>" or similar errors

The root cause was a race condition in the test's timing logic when 20 jobs with KEEPONE_PROPERTY finish at the same time.

Solution

Made three key improvements to eliminate the race condition:

  1. Condition-based waiting for job startup: Changed from a fixed 500ms wait to waiting until all jobs are actually running (job.inProgress is true) with a 3000ms timeout
  2. Event processing after job completion: Added explicit processEvents() call after jobs complete to ensure job completion events are handled
  3. Event processing after marker job: Added another processEvents() call after the marker error job appears to ensure all KEEPONE cleanup logic has completed

Changes

File: tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressContantsTest.java

  • Line 245: Replace processEventsUntil(null, 500) with condition-based waiting
  • Line 251: Add processEvents() after joinJobs()
  • Line 258: Add processEvents() after marker job appears

Testing

  • Ran the specific test 10 consecutive times - all passed
  • Ran all tests in ProgressContantsTest class - all passed
  • No test regressions introduced

Test Plan

  • CI tests pass (especially on Windows and MacOS where the issue was reported)
  • No regressions in other progress view tests

🤖 Generated with Claude Code

@vogella vogella mentioned this pull request Nov 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2025

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 18m 47s ⏱️ + 1m 9s
 8 234 tests ±0   7 985 ✅ ±0  249 💤 ±0  0 ❌ ±0 
23 622 runs  ±0  22 828 ✅ ±0  794 💤 ±0  0 ❌ ±0 

Results for commit 53671b4. ± Comparison against base commit 37ad17d.

♻️ This comment has been updated with latest results.

The testKeepOneProperty test was failing randomly due to timing issues
when multiple jobs with KEEPONE_PROPERTY finish simultaneously.

Changes:
- Replace fixed 500ms wait with condition-based waiting that ensures
  all jobs have actually started (job.inProgress is true)
- Add explicit processEvents() calls after jobs complete and after
  marker job appears to ensure all UI event processing is complete
- Increase timeout from 500ms to 3000ms for job startup check

This eliminates the race condition by:
1. Ensuring all jobs are running before signaling them to finish
2. Ensuring job completion events are processed before checking state
3. Ensuring KEEPONE cleanup logic completes before final assertion

Tested with 10 consecutive runs - all passed.

Fixes eclipse-platform#370

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@vogella vogella force-pushed the fix-issue-370-flaky-progress-test branch from 617b60e to 53671b4 Compare November 10, 2025 08:34
@vogella
Copy link
Contributor Author

vogella commented Nov 10, 2025

LGTM

@vogella vogella merged commit 39d028e into eclipse-platform:master Nov 10, 2025
18 checks passed
@vogella vogella deleted the fix-issue-370-flaky-progress-test branch November 10, 2025 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tests] Random failing ProgressContantsTest.testKeepOneProperty

1 participant