Bug 509411 - [Webkit 1] Crash during Test_*_Browser evaluate() tests (2)
1)
Skip newly added Browser.evaluate() tests (#505591) for webkit1 because
webkit1 is not stable with these and hudson consistently crashes.
Note: These tests do not fail on Cocoa/Win32 and Webkit2.
2)
Added a few notes to few other tests that cause sporadic crashes on
webkit1 due to use of context_iteration (that also don't occur on
webkit2). See bug for details.
3)
Also changing setup/teardown of legacy tests so that they don't run
for tests that are skipped on hudson. #509658
4) Tidy/refactor
Change-Id: I35a398ad56f36e7a618537cb6eca492c8ef7f1ac
Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
index 9db0510..7270442 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
@@ -68,6 +68,10 @@
public final static boolean isSolaris = System.getProperty("os.name").equals("Solaris") || System.getProperty("os.name").equals("SunOS");
public final static boolean isHPUX = System.getProperty("os.name").equals("HP-UX");
+ public final static boolean isRunningOnEclipseOrgHudsonGTK = isGTK
+ && ("hudsonbuild".equalsIgnoreCase(System.getProperty("user.name"))
+ || "genie.platform".equalsIgnoreCase(System.getProperty("user.name")));
+
static {
testFontName = "Helvetica";
}
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 eb05a66..f49364e 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
@@ -14,8 +14,8 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
-import java.util.Properties;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicIntegerArray;
@@ -60,6 +60,8 @@
boolean browser_debug = false;
+ boolean isWebkit1 = false;
+
/**
* Normally, sleep in 1 ms intervals 1000 times. During browser_debug, sleep 1000 ms for 1 interval.
* This allows one to see the browser shell for a few seconds, which would normally not be visible during
@@ -73,15 +75,39 @@
public void setUp() {
super.setUp();
+ // Print test name if running on hudson. This makes it easier to tell in the logs which test case caused crash.
+ if (SwtTestUtil.isRunningOnEclipseOrgHudsonGTK) {
+ System.out.println("Running Test_org_eclipse_swt_browser_Browser#" + name.getMethodName());
+ }
+
secondsToWaitTillFail = 3;
waitMS = browser_debug ? 1000 : 1;
loopMultipier = browser_debug ? 1 : 1000;
shell.setLayout(new FillLayout());
- setTitle(shell);
-
browser = new Browser(shell, SWT.NONE);
+ String shellTitle = name.getMethodName();
+ if (SwtTestUtil.isGTK && browser.getBrowserType().equals("webkit")) {
+
+ // Note, webkitGtk version is only available once Browser is instantiated.
+ String webkitGtkVersionStr = System.getProperty("org.eclipse.swt.internal.webkitgtk.version"); //$NON-NLS-1$
+
+ shellTitle = shellTitle + " Webkit version: " + webkitGtkVersionStr;
+
+ String[] webkitGtkVersionStrParts = webkitGtkVersionStr.split("\\.");
+ int[] webkitGtkVersionInts = new int[3];
+ for (int i = 0; i < 3; i++) {
+ webkitGtkVersionInts[i] = Integer.parseInt(webkitGtkVersionStrParts[i]);
+ }
+
+ // webkitgtk 2.5 and onwards uses webkit2.
+ if (webkitGtkVersionInts[0] == 1 || (webkitGtkVersionInts[0] == 2 && webkitGtkVersionInts[1] <= 4)) {
+ isWebkit1 = true;
+ }
+ }
+ shell.setText(shellTitle);
+
/*
* setWidget() is needed for Browser to occupy the whole shell.
* (Without setWidget(), composite and browser receive half the shell each).
@@ -94,27 +120,10 @@
if (! SwtTestUtil.isWindows) {
setWidget(browser);
}
-}
-/**
- * Append relevant information to the shell title.
- * On Gtk, we support multiple versions of Webkit. It's useful to know which webkit version the test runs on.
- * @param title
- */
-void setTitle(Shell shell) {
- String title = name.getMethodName();
- if (SwtTestUtil.isGTK) {
- String SWT_WEBKITGTK_VERSION = "org.eclipse.swt.internal.webkitgtk.version"; //$NON-NLS-1$
- Properties sp = System.getProperties();
- String webkitGtkVer = sp.getProperty(SWT_WEBKITGTK_VERSION);
- if (webkitGtkVer != null)
- title = title + " Webkit version: " + webkitGtkVer;
- }
- shell.setText(title);
}
-
/**
* Test that if Browser is constructed with the parent being "null", Browser throws an exception.
*/
@@ -124,7 +133,7 @@
Browser browser = new Browser(shell, SWT.NONE);
browser.dispose();
browser = new Browser(shell, SWT.BORDER);
- System.out.println("Browser#getBrowserType(): " + browser.getBrowserType());
+ // System.out.println("Test_org_eclipse_swt_browser_Browser#test_Constructor*#getBrowserType(): " + browser.getBrowserType());
browser.dispose();
try {
browser = new Browser(null, SWT.NONE);
@@ -216,6 +225,13 @@
for (int i = 0; i < 100; i++) browser.removeProgressListener(listener);
}
+@Override
+@Test
+public void test_isVisible() {
+ // Note. This test sometimes crashes with webkit1 because shell.setVisible() calls g_main_context_iteration(). See Bug 509411
+ // To reproduce, try running test suite 20 times in a loop.
+ super.test_isVisible();
+}
/**
* Test that if addStatusTextListener() is called without a listener, IllegalArgumentException is thrown.
@@ -650,6 +666,8 @@
*/
@Test
public void test_setUrl() {
+ // Note, this test sometimes crashes on webkit1. See Bug 509411
+
/* THIS TEST REQUIRES WEB ACCESS! How else can we really test the http:// part of a browser widget? */
assert(browser.setUrl("http://www.eclipse.org/swt"));
runLoopTimer(2000);
@@ -674,6 +692,8 @@
*/
@Test
public void test_evaluate_string() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
+
final AtomicReference<String> returnValue = new AtomicReference<>();
browser.addProgressListener(new ProgressListener() {
@Override
@@ -705,6 +725,7 @@
*/
@Test
public void test_evaluate_number_normal() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
Double testNum = 123.0;
boolean passed = evaluate_number_helper(testNum);
assertTrue(passed);
@@ -716,6 +737,8 @@
*/
@Test
public void test_evaluate_number_negative() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
+
Double testNum = -123.0;
boolean passed = evaluate_number_helper(testNum);
assertTrue(passed);
@@ -727,6 +750,8 @@
*/
@Test
public void test_evaluate_number_big() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
+
Double testNum = 10000000000.0;
boolean passed = evaluate_number_helper(testNum);
assertTrue(passed);
@@ -766,6 +791,7 @@
*/
@Test
public void test_evaluate_boolean() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
final AtomicBoolean atomicBoolean = new AtomicBoolean(false);
browser.addProgressListener(new ProgressListener() {
@Override
@@ -797,9 +823,9 @@
*/
@Test
public void test_evaluate_null() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
// Boolen only used as dummy placeholder so the object is not null.
final AtomicReference<Object> returnValue = new AtomicReference<>(new Boolean(true));
- System.out.println(returnValue);
browser.addProgressListener(new ProgressListener() {
@Override
public void changed(ProgressEvent event) {
@@ -830,6 +856,8 @@
*/
@Test
public void test_evaluate_invalid_return_value() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
+
if (SwtTestUtil.isWindows) {
/* Bug 508210 . Inconsistent beahiour on windows at the moment.
* Fixing requires deeper investigation. Disabling newly added test for now.
@@ -878,6 +906,7 @@
*/
@Test
public void test_evaluate_evaluation_failed_exception() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
final AtomicInteger exception = new AtomicInteger(-1);
browser.addProgressListener(new ProgressListener() {
@Override
@@ -914,6 +943,8 @@
*/
@Test
public void test_evaluate_array_numbers() {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
+
// Small note:
// evaluate() returns 'Double' type. Java doesn't have AtomicDouble
// for convienience we simply convert double to int as we're dealing with integers anyway.
@@ -955,6 +986,8 @@
*/
@Test
public void test_evaluate_array_strings () {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
+
final AtomicReferenceArray<String> atomicStringArray = new AtomicReferenceArray<>(3);
atomicStringArray.set(0, "executing");
browser.addProgressListener(new ProgressListener() {
@@ -995,6 +1028,7 @@
*/
@Test
public void test_evaluate_array_mixedTypes () {
+ assumeFalse(webkit1SkipMsg(), isWebkit1); // Bug 509411
final AtomicReferenceArray<Object> atomicArray = new AtomicReferenceArray<>(3);
atomicArray.set(0, "executing");
browser.addProgressListener(new ProgressListener() {
@@ -1051,4 +1085,9 @@
Display display = Display.getCurrent();
while (!exit[0] && !shell.isDisposed()) if (!display.readAndDispatch()) display.sleep();
}
+
+private String webkit1SkipMsg() {
+ return "Test_org_eclipse_swt_browser. Bug 509411. Skipping test on Webkit1 due to sporadic crash: "+ name.getMethodName();
+}
+
}
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/browser/Test_BrowserSuite.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/browser/Test_BrowserSuite.java
index 2031e11..5bbd5dc 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/browser/Test_BrowserSuite.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/browser/Test_BrowserSuite.java
@@ -11,11 +11,10 @@
package org.eclipse.swt.tests.junit.browser;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
import org.eclipse.swt.tests.junit.SwtTestUtil;
import org.eclipse.swt.widgets.Display;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestName;
@@ -27,79 +26,93 @@
*/
public class Test_BrowserSuite {
-public static boolean isRunningOnEclipseOrgHudsonGTK = SwtTestUtil.isGTK
- && ("hudsonbuild".equalsIgnoreCase(System.getProperty("user.name"))
- || "genie.platform".equalsIgnoreCase(System.getProperty("user.name")));
-
@Rule
public TestName name = new TestName();
+String skipMsg() {
+ return "Test_BrowserSuite." + name.getMethodName() + "() skipped, see bug 499159, 509411";
+}
+
@Test
public void testBrowser1() {
- if (isRunningOnEclipseOrgHudsonGTK) {
- System.out.println("Test_BrowserSuite." + name.getMethodName() + "() skipped, see bug 499159");
- return;
- }
+ assumeFalse(skipMsg(), SwtTestUtil.isRunningOnEclipseOrgHudsonGTK);
+ manualSetUp();
assertTrue(Browser1_location_progress_fromURL.test());
+ manualTearDown();
}
@Test
public void testBrowser2() {
+ manualSetUp();
assertTrue(Browser2_location_and_progress_advanced.test());
+ manualTearDown();
}
@Test
public void testBrowser3() {
- if (isRunningOnEclipseOrgHudsonGTK) {
- System.out.println("Test_BrowserSuite." + name.getMethodName() + "() skipped, see bug 499159");
- return;
- }
+ assumeFalse(skipMsg(), SwtTestUtil.isRunningOnEclipseOrgHudsonGTK);
+ System.out.println("testbrowse3 starting " + SwtTestUtil.isRunningOnEclipseOrgHudsonGTK);
+ manualSetUp();
assertTrue(Browser3_window_open_visibility_listeners.test());
+ manualTearDown();
}
@Test
public void testBrowser4() {
+ manualSetUp();
assertTrue(Browser4_window_close_listener.test());
+ manualTearDown();
}
@Test
public void testBrowser5() {
+ manualSetUp();
assertTrue(Browser5_sizing_and_bounds.test());
+ manualTearDown();
}
@Test
public void testBrowser6() {
- if (isRunningOnEclipseOrgHudsonGTK) {
- System.out.println("Test_BrowserSuite." + name.getMethodName() + "() skipped, see bug 499159");
- return;
- }
+ assumeFalse(skipMsg(), SwtTestUtil.isRunningOnEclipseOrgHudsonGTK);
+ manualSetUp();
assertTrue(Browser6_title_change_listener.test());
+ manualTearDown();
}
@Test
public void testBrowser7() {
+ manualSetUp();
assertTrue(Browser7_child_browsers.test());
+ manualTearDown();
}
@Test
public void testBrowser8() {
+ manualSetUp();
assertTrue(Browser8_execute_fromMemory.test());
+ manualTearDown();
}
@Test
public void testBrowser9() {
+ manualSetUp();
assertTrue(Browser9_execute_fromFile.test());
+ manualTearDown();
}
-@Before
-public void setUp() {
- System.out.println("Browser#setUp(): " + name.getMethodName());
+//Bug 509658. Display.dispose can cause webkit1 to crash in some cases.
+// Thus we have a situation where the setup causes a crash even in test cases
+// that are marked not to be run on hudson. This is very confusing.
+//
+// For now, to avoid calling display.dispose for problematic test webkit1 test cases
+// we call setup/teardown manually.
+public void manualSetUp() {
+ System.out.println("Test_BrowserSuite#setUp(): " + name.getMethodName());
Display display = Display.getCurrent();
if (display != null) display.dispose();
}
-@After
-public void tearDown() {
+public void manualTearDown() {
Display display = Display.getCurrent();
if (display != null) display.dispose();
}
