Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Execute browser tests for Edge #671 #672

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Execute browser tests for Edge #671 #672

wants to merge 2 commits into from

Conversation

HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented May 15, 2023 •
edited
Loading

Browser tests were only executed for the default configuration of a system's browser using the SWT.NONE flag. Other configurations, such as using the Edge browser in Windows, were not tested.

This change parameterizes the browser tests to also execute them for the Edge browser on Windows. It also deactivates those tests for the Edge browser for which the implementation does (currently) not work. This allows to detect regressions when performing future changes to the Edge browser.

Fixes #671

@github-actions
Copy link
Contributor

github-actions bot commented May 15, 2023 •
edited
Loading

Test Results

   382 files  ±  0     382 suites  ±0   7m 33s ⏱️ + 1m 37s
 4 251 tests  -  35   4 241 ✅  -  30   7 💤  - 7  3 ❌ +2 
12 306 runs  +156  12 214 ✅ +150  88 💤 +3  4 ❌ +3 

For more details on these failures, see this check.

Results for commit 916faf9. ± Comparison against base commit 43b6820.

This pull request removes 35 tests. 
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testTextTransfer
…

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare force-pushed the issue-671 branch 8 times, most recently from abe200b to f4ac8dc Compare May 22, 2023 15:49
HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this pull request May 22, 2023
@HeikoKlare
Parameterize browser tests to execute them for Edge browser eclipse-p…
4064741 
…latform#671

Browser tests were only executed for the default configuration of a
system's browser using the SWT.NONE flag. Other configurations, such as
using the Edge browser in Windows, were not tested.

This change parameterizes the browser tests to also execute them for the
Edge browser on Windows. It also deactivates those tests for the Edge
browser for which the implementation does (currently) not work. This
allows to detect regressions when performing future changes to the Edge
browser.

Fixes eclipse-platform#672
@HeikoKlare HeikoKlare force-pushed the issue-671 branch 3 times, most recently from 31c2a39 to 549a829 Compare May 23, 2023 13:57
@HeikoKlare HeikoKlare marked this pull request as ready for review May 23, 2023 15:25
HeikoKlare
HeikoKlare commented May 23, 2023
View reviewed changes
....swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java
Comment on lines +232 to +254
if (isEdge) {
// wait for and process pending events to properly cleanup Edge browser resources
do {
processUiEvents();
try {
Thread.sleep(100);
} catch (InterruptedException e) {
}
} while (Display.getCurrent().readAndDispatch());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary because otherwise some OS events that are somehow produced by one test occur and are processed by the next test and may interfere with the Edge browser instantation. I have invested quite some time to identify where the events come from and how to properly handle them, but I did not find a proper way to do so. If someone is able to identify the reasons for the events and how to properly process them, I would be glad to have a better solution than this,

@vogella
Copy link
Contributor

vogella commented Jun 14, 2023

@HeikoKlare can this be merged?

@jukzi
Copy link
Contributor

jukzi commented Jun 15, 2023 

creating browser took too long: 23469ms
java.lang.AssertionError: creating browser took too long: 23469ms
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser.createBrowser(Test_org_eclipse_swt_browser_Browser.java:290)
	at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser.setUp(Test_org_eclipse_swt_browser_Browser.java:173)

@HeikoKlare HeikoKlare marked this pull request as draft June 16, 2023 12:26
@HeikoKlare
Copy link
Contributor Author

I converted this back to draft due to the recent test failure and I will have another look at it.
Further increasing the waiting time between test cases would probably "solve"(/hide) the problem, but I'd prefer a solution that makes waiting times between test cases obsolete.

@HeikoKlare HeikoKlare mentioned this pull request Oct 20, 2023
Closed
@HeikoKlare HeikoKlare mentioned this pull request Jan 30, 2024
Closed
@HeikoKlare HeikoKlare force-pushed the issue-671 branch 4 times, most recently from 9dd213d to b002ced Compare January 31, 2024 10:37
@HeikoKlare HeikoKlare mentioned this pull request Jan 31, 2024
Merged
@HeikoKlare HeikoKlare force-pushed the issue-671 branch 2 times, most recently from 4d2a5a1 to fde88d9 Compare February 5, 2024 15:19
@HeikoKlare HeikoKlare mentioned this pull request Feb 20, 2024
Closed
@HeikoKlare HeikoKlare mentioned this pull request Aug 6, 2024
Merged
@HeikoKlare HeikoKlare force-pushed the issue-671 branch 2 times, most recently from ffb43cd to b26001f Compare September 25, 2024 14:39
@sratz sratz added the edge Edge Browser label Sep 25, 2024
@HeikoKlare HeikoKlare mentioned this pull request Nov 5, 2024
Merged
@HeikoKlare HeikoKlare changed the title Parameterize browser tests to execute them for Edge browser #671 Execute browser tests for Edge #671 Dec 10, 2024
@HeikoKlare
Execute browser tests for Edge eclipse-platform#671
a92c14b 
Browser tests were only executed for the default configuration of a
system's browser using the SWT.NONE flag. Other configurations, such as
using the Edge browser in Windows, were not tested. A parameterization
has been added to allow other configurations to be tested.

This change adds Edge to the test configurations executed for the
browser. This allows to detect regressions when performing future
changes to the Edge browser.

Fixes
eclipse-platform#671
@HeikoKlare HeikoKlare force-pushed the issue-671 branch 4 times, most recently from 648c9a5 to 4c829a9 Compare December 10, 2024 16:10
This was referenced Dec 10, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
edge Edge Browser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Browser tests are not executed for Edge browser
4 participants
@HeikoKlare @vogella @jukzi @sratz