Skip to content

Conversation

@HeikoKlare
Copy link
Contributor

This contributes to fixing the randomly failing HoverTests:

  • Removes unnecessary focus enforcing: The code checks for the text editor having focus and afterwards uses different methods to force focus to the widget's shell again. This is unnecessary and is only prone to introduce further problems.
  • Introduces further assertions: in order to better locate the cause of the failing tests, assertions are added for the conditions, on which the execution waits, to be actually fulfilled.

Contributes to #926

Contributes to #1808

@HeikoKlare HeikoKlare marked this pull request as ready for review July 16, 2024 11:18
@github-actions
Copy link
Contributor

github-actions bot commented Jul 16, 2024

Test Results

 1 821 files  +  307   1 821 suites  +307   1h 55m 49s ⏱️ + 20m 50s
 7 725 tests ±    0   7 497 ✅ +    2  228 💤  -   1  0 ❌  - 1 
24 336 runs  +1 846  23 589 ✅ +1 648  747 💤 +199  0 ❌  - 1 

Results for commit ba6a578. ± Comparison against base commit 10d96b5.

♻️ This comment has been updated with latest results.

@HeikoKlare
Copy link
Contributor Author

With these changes, the test randomly fails as follows:

hover data not found
java.lang.AssertionError: hover data not found
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.eclipse.ui.genericeditor.tests.HoverTest.triggerCompletionAndRetrieveInformationControlManager(HoverTest.java:259)
	at org.eclipse.ui.genericeditor.tests.HoverTest.testEnabledWhenHover(HoverTest.java:97)

Copy link

@Wittmaxi Wittmaxi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I have seen SWT tests fail because some control wasn't in focus (happens in the FInd/Replace tests if you click into the window while something is being tested, for example). I think these assertions can at least notify us if something like that is happenign and help us further diagnose what is happening.

…orm#926 eclipse-platform#1808

This contributes to fixing the randomly failing HoverTests:
- Removes unnecessary focus enforcing: The code checks for the text
editor having focus and afterwards uses different methods to force focus
to the widget's shell again. This is unnecessary and is only prone to
introduce further problems.
- Introduces further assertions: in order to better locate the cause of
the failing tests, assertions are added for the conditions, on which the
execution waits, to be actually fulfilled.
- Adds a retry functionality for simulating the hover event.

Contributes to
eclipse-platform#926
Contributes to
eclipse-platform#1808
@HeikoKlare HeikoKlare merged commit beb9918 into eclipse-platform:master Nov 8, 2024
17 checks passed
@HeikoKlare HeikoKlare deleted the issue-1808 branch November 8, 2024 16:19
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.

2 participants