Skip to content

Conversation

@vogella
Copy link
Contributor

@vogella vogella commented Nov 12, 2025

Summary

Fixes the flaky PartRenderingEngineTests.ensureCleanUpAddonCleansUp test that was failing intermittently due to a race condition with async cleanup logic.

Root Cause

  • CleanupAddon performs cleanup asynchronously via Display.asyncExec() (CleanupAddon.java:352)
  • The test was calling waitForCondition() immediately after hiding parts, before the async cleanup tasks were even posted to the event queue
  • This created a race where the wait could timeout before cleanup had a chance to start

Changes

  • Add contextRule.spinEventLoop() after hiding partB and partC to ensure async cleanup tasks are processed before waiting
  • Wrap waitForCondition() in assertTrue() for clearer error messages on timeout
  • Remove redundant assertFalse() statements

This matches the pattern used in the adjacent testBug332463 test.

Testing

  • Verified with 5 consecutive test runs - all passed consistently
  • Test now completes in ~0.028-0.036s

Fixes #751

🤖 Generated with Claude Code

…ndition

The test was failing intermittently with "CleanupAddon should ensure that
partStack is not rendered anymore, as all childs have been removed" because
it was checking the cleanup result before the async cleanup logic had a
chance to execute.

Root cause:
- CleanupAddon performs cleanup asynchronously via Display.asyncExec()
  (CleanupAddon.java:352)
- When hidePart() is called, it triggers events that schedule async cleanup
  tasks on the event queue
- The test was calling waitForCondition() immediately after hiding parts,
  before the async cleanup tasks were even posted to the event queue
- This created a race where the wait could timeout before cleanup started

Fix:
- Add contextRule.spinEventLoop() after hiding partB and partC to ensure
  async cleanup tasks are processed before waiting for the condition
- Wrap waitForCondition() in assertTrue() to fail immediately with a clear
  message if cleanup doesn't complete within timeout
- Remove redundant assertFalse() statements

This matches the pattern used in the adjacent testBug332463 test which calls
spinEventLoop() after each hidePart() call.

Verified with 5 consecutive test runs - all passed (consistently completing
in ~0.028-0.036s).

Fixes eclipse-platform#751

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

Co-Authored-By: Claude <[email protected]>
@vogella vogella force-pushed the fix-flaky-cleanup-addon-test branch from 9dc35d9 to 8ad3ee0 Compare November 12, 2025 09:20
@github-actions
Copy link
Contributor

Test Results

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

Results for commit 8ad3ee0. ± Comparison against base commit 601b0d3.

@vogella vogella marked this pull request as ready for review November 12, 2025 10:08
@vogella
Copy link
Contributor Author

vogella commented Nov 12, 2025

@ptziegler WDYT?

@vogella vogella merged commit 7a24db1 into eclipse-platform:master Nov 12, 2025
18 checks passed
@vogella vogella deleted the fix-flaky-cleanup-addon-test branch November 12, 2025 17:22
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.

PartRenderingEngineTests test fails randomly

1 participant