Fix race condition in NestedResourcesTests.testProjectHierarchy #3430
+14
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fedejeanne
reviewed
Oct 22, 2025
...se.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/NestedResourcesTests.java
Outdated
Show resolved
Hide resolved
...se.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/NestedResourcesTests.java
Outdated
Show resolved
Hide resolved
...se.ui.tests.navigator/src/org/eclipse/ui/tests/navigator/resources/NestedResourcesTests.java
Outdated
Show resolved
Hide resolved
This commit fixes the intermittent test failure reported in GitHub issue eclipse-platform#196 where testProjectHierarchy randomly fails with "expected:<2> but was:<1>" at line 96. Root cause: The NestedProjectManager uses an asynchronous IResourceChangeListener that responds to POST_CHANGE events to update its internal project hierarchy map (locationsToProjects). When projects are created and opened in rapid succession, the test may query getDirectChildrenProjects() before the manager has finished processing all resource change events, leading to incomplete results. The specific failure occurred at the assertion checking that projectAB has 2 children (projectABA and projectABB). The test would sometimes only see 1 child because the second project's creation event hadn't been processed yet. Solution: Added DisplayHelper.waitForCondition() calls at three critical synchronization points to wait for NestedProjectManager to process all resource changes before making assertions: 1. Wait for projectA to show 1 child (projectAB) 2. Wait for folderAA to show 1 child (projectAAA) 3. Wait for projectAB to show 2 children (projectABA and projectABB) This approach follows the same pattern already used successfully in the testProblemDecoration() method in the same test file, which also waits for asynchronous marker updates to propagate. The 2-second timeout (TIMEOUT constant) provides sufficient time for event processing while still failing quickly if assertions are genuinely incorrect. Since the waitForCondition() calls already verify the array lengths, the redundant Assert.assertEquals calls for array lengths have been removed per code review feedback. Fixes: eclipse-platform#196 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
vogella
force-pushed
the
flacky-test-nestled
branch
from
1f7b2c1 to
4e1f5df
Compare
|
@fedejeanne is this change now ok for you? |
fedejeanne
approved these changes
Oct 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes the intermittent test failure reported in GitHub issue #196 where testProjectHierarchy randomly fails with "expected:<2> but was:<1>" at line 96 (now line 111).
Root cause:
The NestedProjectManager uses an asynchronous IResourceChangeListener that responds to POST_CHANGE events to update its internal project hierarchy map (locationsToProjects). When projects are created and opened in rapid succession, the test may query getDirectChildrenProjects() before the manager has finished processing all resource change events, leading to incomplete results.
The specific failure occurred at the assertion checking that projectAB has 2 children (projectABA and projectABB). The test would sometimes only see 1 child because the second project's creation event hadn't been processed yet.
Solution:
Added DisplayHelper.waitForCondition() calls at three critical synchronization points to wait for NestedProjectManager to process all resource changes before making assertions:
This approach follows the same pattern already used successfully in the testProblemDecoration() method in the same test file (lines 161-188), which also waits for asynchronous marker updates to propagate.
The 2-second timeout (TIMEOUT constant) provides sufficient time for event processing while still failing quickly if assertions are genuinely incorrect.
Fixes: #196
🤖 Generated with Claude Code