Fix flaky HoverTest.testEnabledWhenHover() race condition #3524
+7
−0
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
Contributor
The testEnabledWhenHover test was failing randomly when trying to retrieve the hover shell for the second part of the test. Root cause: - The test has two parts: first with EnabledPropertyTester.setEnabled(true), then with setEnabled(false) - After the first hover is shown and checked, cleanFileAndEditor() is called - However, the hover shell from the first part may not be fully disposed when the second editor is opened and the second hover is triggered - This causes getHoverShell() to timeout waiting for the new hover shell Fix: - Capture a reference to the first hover shell before calling cleanFileAndEditor() - After cleanFileAndEditor(), explicitly wait for the first shell to be disposed using DisplayHelper.waitForCondition() with a 3000ms timeout - This ensures the hover state is fully reset before the second part begins This approach follows the pattern used in other recent flaky test fixes in this repository (e.g., ProgressContantsTest, ProgressViewTests) which use condition-based waiting to ensure proper cleanup between test phases. Fixes eclipse-platform#926 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
vogella
force-pushed
the
fix-issue-926-flaky-hover-test-v2
branch
from
9a569ed to
c8e1aba
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes the random failure in
HoverTest.testEnabledWhenHover()by ensuring the hover shell from the first test part is fully disposed before the second part begins.Root Cause
The test has two parts that test hover behavior with different
EnabledPropertyTesterstates:setEnabled(true)- shows one hoversetEnabled(false)- shows a different hoverThe race condition occurred because:
cleanFileAndEditor()was calledgetHoverShell()would timeout waiting for the new hover shell (line 175 in the stack trace)Fix
Added explicit waiting for the first hover shell to be disposed:
cleanFileAndEditor()as beforeDisplayHelper.waitForCondition()to wait up to 3000ms for the shell to be disposedThis ensures complete state reset between the two test phases.
Pattern
This fix follows the same pattern used in other recent flaky test fixes in this repository:
Both use condition-based waiting with
DisplayHelper.waitForCondition()to ensure proper cleanup between test phases rather than relying on timing.Test Plan
Due to environment issues, I was unable to run the full test suite locally. However, the fix follows established patterns from recent successful flaky test fixes in this repository.
Fixes #926
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]