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

Execute browser tests for Edge #671 #672

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+37 −10
Open

Execute browser tests for Edge #671 #672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
46 changes: 36 additions & 10 deletions ...t.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_browser_Browser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -144,13 +145,10 @@ private void testLogAppend(String msg) {
@Parameters(name = "browser flags: {0}")
public static Collection<Object[]> browserFlagsToTest() {
List<Object[]> 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;
}
Expand All @@ -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() {
Expand Down Expand Up @@ -215,6 +223,7 @@ protected void afterDispose(Display display) {

if(!shell.isDisposed()) {
System.out.println("Not disposed shell: " + shell);
shell.dispose();
disposedShells ++;
}
}
Expand All @@ -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());
}
Comment on lines +251 to +260
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,

if (SwtTestUtil.isGTK) {
int descriptorDiff = reportOpenedDescriptors();
if(descriptorDiff > 0) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -722,14 +743,17 @@ public void changed(LocationEvent event) {

@Test
public void test_LocationListener_LocationListener_ordered_changing () {
List<String> locations = new ArrayList<>();
browser.addLocationListener(changingAdapter(event -> locations.add(event.location)));
assumeFalse("Currently broken for Edge", isEdge);
List<String> 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() {
Expand Down Expand Up @@ -1942,6 +1966,7 @@ public void test_evaluate_null() {
// Boolen only used as dummy placeholder so the object is not null.
final AtomicReference<Object> returnValue = new AtomicReference<>(true);
browser.addProgressListener(completedAdapter(event -> {
returnValue.set(false);
Object evalResult = browser.evaluate("return null");
returnValue.set(evalResult);
if (debug_verbose_output)
Expand All @@ -1951,7 +1976,7 @@ public void test_evaluate_null() {
browser.setText("<html><body>HelloWorld</body></html>");
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);
}

/**
Expand Down Expand Up @@ -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.
Expand Down
Loading