From 603eecdebc52c09897d4916fbef5e566de216c66 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Mon, 15 May 2023 17:10:22 +0200 Subject: [PATCH] Execute browser tests for Edge #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. 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 https://github.com/eclipse-platform/eclipse.platform.swt/issues/671 Co-authored-by: Federico Jeanne --- .../win32/org/eclipse/swt/browser/Edge.java | 1 + .../Test_org_eclipse_swt_browser_Browser.java | 46 +++++++++++++++---- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java index e3b33262f58..2e4d7055b71 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java @@ -596,6 +596,7 @@ void setupBrowser(int hr, long pv) { error(SWT.ERROR_THREAD_INVALID_ACCESS, hr); break; default: + System.err.println("WebView instantiation failed with result: " + hr); containingEnvironment.instances().remove(this); error(SWT.ERROR_NO_HANDLES, hr); } diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java index 1662896d78b..34555f1e5ed 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java @@ -81,6 +81,7 @@ import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.Text; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.FixMethodOrder; import org.junit.Rule; import org.junit.Test; @@ -144,13 +145,10 @@ private void testLogAppend(String msg) { @Parameters(name = "browser flags: {0}") public static Collection browserFlagsToTest() { List browserFlags = new ArrayList<>(); + browserFlags.add(new Object[] {SWT.NONE}); if (SwtTestUtil.isWindows) { - // NOTE: This is currently disabled due to test issues in the CI - // Execute Edge tests first, because IE starts some OS timer that conflicts with Edge event handling - // browserFlags.add(0, new Object[] {SWT.EDGE}); + // Execute IE tests after Edge, because IE starts some OS timer that conflicts with Edge event handling browserFlags.add(new Object[] {SWT.IE}); - } else { - browserFlags.add(new Object[] {SWT.NONE}); } return browserFlags; } @@ -159,6 +157,16 @@ public Test_org_eclipse_swt_browser_Browser(int swtBrowserSettings) { this.swtBrowserSettings = swtBrowserSettings; } +@BeforeClass +public static void setupEdgeEnvironment() { + // initialize Edge environment before any test runs to isolate environment setup + if (SwtTestUtil.isWindows) { + Shell shell = new Shell(); + new Browser(shell, SWT.EDGE); + shell.dispose(); + } +} + @Override @Before public void setUp() { @@ -215,6 +223,7 @@ protected void afterDispose(Display display) { if(!shell.isDisposed()) { System.out.println("Not disposed shell: " + shell); + shell.dispose(); disposedShells ++; } } @@ -239,6 +248,16 @@ protected void afterDispose(Display display) { printThreadsInfo(); } } + 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()); + } if (SwtTestUtil.isGTK) { int descriptorDiff = reportOpenedDescriptors(); if(descriptorDiff > 0) { @@ -285,6 +304,8 @@ private Browser createBrowser(Shell s, int flags) { long maximumBrowserCreationMilliseconds = 10_000; long createStartTime = System.currentTimeMillis(); Browser b = new Browser(s, flags); + // Wait for asynchronous initialization via getting URL + b.getUrl(); createdBroswers.add(b); long createDuration = System.currentTimeMillis() - createStartTime; assertTrue("creating browser took too long: " + createDuration + "ms", createDuration < maximumBrowserCreationMilliseconds); @@ -722,14 +743,17 @@ public void changed(LocationEvent event) { @Test public void test_LocationListener_LocationListener_ordered_changing () { - List locations = new ArrayList<>(); - browser.addLocationListener(changingAdapter(event -> locations.add(event.location))); + assumeFalse("Currently broken for Edge", isEdge); + List locations = Collections.synchronizedList(new ArrayList<>()); + browser.addLocationListener(changingAdapter(event -> { + locations.add(event.location); + })); shell.open(); browser.setText("You should not see this message."); String url = getValidUrl(); browser.setUrl(url); - waitForPassCondition(() -> locations.size() == 2); - assertTrue("Change of locations do not fire in order.", locations.get(0).equals("about:blank") && locations.get(1).contains("testWebsiteWithTitle.html")); + assertTrue("Change of locations do not fire in order: " + locations.toString(), waitForPassCondition(() -> locations.size() == 2)); + assertTrue("Change of locations do not fire in order", locations.get(0).equals("about:blank") && locations.get(1).contains("testWebsiteWithTitle.html")); } private String getValidUrl() { @@ -1942,6 +1966,7 @@ public void test_evaluate_null() { // Boolen only used as dummy placeholder so the object is not null. final AtomicReference returnValue = new AtomicReference<>(true); browser.addProgressListener(completedAdapter(event -> { + returnValue.set(false); Object evalResult = browser.evaluate("return null"); returnValue.set(evalResult); if (debug_verbose_output) @@ -1951,7 +1976,7 @@ public void test_evaluate_null() { browser.setText("HelloWorld"); shell.open(); boolean passed = waitForPassCondition(() -> returnValue.get() == null); - assertTrue("Evaluate did not return a null. Timed out.", passed); + assertTrue("Evaluate did not return a null (current value: " + returnValue.get() + "). Timed out.", passed); } /** @@ -2156,6 +2181,7 @@ public void test_evaluate_array_mixedTypes () { */ @Test public void test_BrowserFunction_callback () { + assumeFalse("Currently broken for Edge", isEdge); AtomicBoolean javaCallbackExecuted = new AtomicBoolean(false); class JavascriptCallback extends BrowserFunction { // Note: Local class defined inside method.