Fixed a bug in GWTTestCase where it would access JUnitShell from the wrong thread, creating a SWT error when later tests run.
Patch by: jlabanca
Review by: jat
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@6265 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/junit/BatchingStrategy.java b/user/src/com/google/gwt/junit/BatchingStrategy.java
index 1932854..14a5a11 100644
--- a/user/src/com/google/gwt/junit/BatchingStrategy.java
+++ b/user/src/com/google/gwt/junit/BatchingStrategy.java
@@ -19,6 +19,7 @@
import com.google.gwt.junit.client.impl.JUnitHost.TestInfo;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -37,6 +38,24 @@
* @return an ordered list of test blocks to run
*/
public abstract List<TestInfo[]> getTestBlocks(String syntheticModuleName);
+
+ /**
+ * Get the set of tests for this module, minus tests that should not be
+ * executed.
+ *
+ * @return the set of tests to execute
+ */
+ protected final Set<TestInfo> getTestsForModule(String syntheticModuleName) {
+ Set<TestInfo> toExecute = GWTTestCase.getTestsForModule(syntheticModuleName).getTests();
+ Set<TestInfo> toRemove = new HashSet<TestInfo>();
+ for (TestInfo info : toExecute) {
+ if (JUnitShell.mustNotExecuteTest(info)) {
+ toRemove.add(info);
+ }
+ }
+ toExecute.removeAll(toRemove);
+ return toExecute;
+ }
}
/**
@@ -46,8 +65,7 @@
class NoBatchingStrategy extends BatchingStrategy {
@Override
public List<TestInfo[]> getTestBlocks(String syntheticModuleName) {
- Set<TestInfo> allTestsInModule = GWTTestCase.getTestsForModule(
- syntheticModuleName).getTests();
+ Set<TestInfo> allTestsInModule = getTestsForModule(syntheticModuleName);
List<TestInfo[]> testBlocks = new ArrayList<TestInfo[]>();
for (TestInfo testInfo : allTestsInModule) {
testBlocks.add(new TestInfo[] {testInfo});
@@ -62,8 +80,7 @@
class ClassBatchingStrategy extends BatchingStrategy {
@Override
public List<TestInfo[]> getTestBlocks(String syntheticModuleName) {
- Set<TestInfo> allTestsInModule = GWTTestCase.getTestsForModule(
- syntheticModuleName).getTests();
+ Set<TestInfo> allTestsInModule = getTestsForModule(syntheticModuleName);
List<TestInfo[]> testBlocks = new ArrayList<TestInfo[]>();
String lastTestClass = null;
List<TestInfo> lastTestBlock = null;
@@ -96,11 +113,12 @@
class ModuleBatchingStrategy extends BatchingStrategy {
@Override
public List<TestInfo[]> getTestBlocks(String syntheticModuleName) {
- Set<TestInfo> allTestsInModule = GWTTestCase.getTestsForModule(
- syntheticModuleName).getTests();
- TestInfo[] testBlock = allTestsInModule.toArray(new TestInfo[allTestsInModule.size()]);
+ Set<TestInfo> allTestsInModule = getTestsForModule(syntheticModuleName);
List<TestInfo[]> testBlocks = new ArrayList<TestInfo[]>();
- testBlocks.add(testBlock);
+ if (allTestsInModule.size() > 0) {
+ TestInfo[] testBlock = allTestsInModule.toArray(new TestInfo[allTestsInModule.size()]);
+ testBlocks.add(testBlock);
+ }
return testBlocks;
}
}
diff --git a/user/src/com/google/gwt/junit/JUnitShell.java b/user/src/com/google/gwt/junit/JUnitShell.java
index 7dbf9de..e98603d 100644
--- a/user/src/com/google/gwt/junit/JUnitShell.java
+++ b/user/src/com/google/gwt/junit/JUnitShell.java
@@ -491,19 +491,34 @@
* @return the list of remote user agents
*/
public static String[] getRemoteUserAgents() {
- return getUnitTestShell().remoteUserAgents;
+ if (unitTestShell == null) {
+ return null;
+ }
+ return unitTestShell.remoteUserAgents;
}
/**
* Checks if a testCase should not be executed. Currently, a test is either
* executed on all clients (mentioned in this test) or on no clients.
*
- * @param testCase current testCase.
+ * @param testInfo the test info to check
* @return true iff the test should not be executed on any of the specified
* clients.
*/
- public static boolean mustNotExecuteTest(TestCase testCase) {
- return getUnitTestShell().mustNotExecuteTest(getBannedPlatforms(testCase));
+ public static boolean mustNotExecuteTest(TestInfo testInfo) {
+ if (unitTestShell == null) {
+ throw new IllegalStateException(
+ "mustNotExecuteTest cannot be called before runTest()");
+ }
+ try {
+ Class<?> testClass = TestCase.class.getClassLoader().loadClass(
+ testInfo.getTestClass());
+ return unitTestShell.mustNotExecuteTest(getBannedPlatforms(testClass,
+ testInfo.getTestMethod()));
+ } catch (ClassNotFoundException e) {
+ throw new IllegalArgumentException("Could not load test class: "
+ + testInfo.getTestClass());
+ }
}
/**
@@ -583,16 +598,19 @@
}
/**
- * returns the set of banned {@code Platform} for a test method.
+ * Returns the set of banned {@code Platform} for a test method.
+ *
+ * @param testClass the testClass
+ * @param methodName the name of the test method
*/
- private static Set<Platform> getBannedPlatforms(TestCase testCase) {
- Class<?> testClass = testCase.getClass();
+ private static Set<Platform> getBannedPlatforms(Class<?> testClass,
+ String methodName) {
Set<Platform> bannedSet = EnumSet.noneOf(Platform.class);
if (testClass.isAnnotationPresent(DoNotRunWith.class)) {
bannedSet.addAll(Arrays.asList(testClass.getAnnotation(DoNotRunWith.class).value()));
}
try {
- Method testMethod = testClass.getMethod(testCase.getName());
+ Method testMethod = testClass.getMethod(methodName);
if (testMethod.isAnnotationPresent(DoNotRunWith.class)) {
bannedSet.addAll(Arrays.asList(testMethod.getAnnotation(
DoNotRunWith.class).value()));
@@ -630,6 +648,10 @@
// TODO: install a shutdown hook? Not necessary with GWTShell.
unitTestShell.lastLaunchFailed = false;
}
+ if (unitTestShell.thread != Thread.currentThread()) {
+ throw new IllegalThreadStateException(
+ "JUnitShell can only be accessed from the thread that created it.");
+ }
return unitTestShell;
}
@@ -727,10 +749,16 @@
private long testMethodTimeout;
/**
+ * The thread that created the JUnitShell.
+ */
+ private Thread thread;
+
+ /**
* Enforce the singleton pattern. The call to {@link GWTShell}'s ctor forces
* server mode and disables processing extra arguments as URLs to be shown.
*/
private JUnitShell() {
+ thread = Thread.currentThread();
setRunTomcat(true);
setHeadless(true);
@@ -937,7 +965,8 @@
private void runTestImpl(GWTTestCase testCase, TestResult testResult)
throws UnableToCompleteException {
- if (mustNotExecuteTest(testCase)) {
+ if (mustNotExecuteTest(getBannedPlatforms(testCase.getClass(),
+ testCase.getName()))) {
return;
}
diff --git a/user/src/com/google/gwt/junit/RunStyle.java b/user/src/com/google/gwt/junit/RunStyle.java
index c09200d..9757ad4 100644
--- a/user/src/com/google/gwt/junit/RunStyle.java
+++ b/user/src/com/google/gwt/junit/RunStyle.java
@@ -36,15 +36,6 @@
}
/**
- * Initialize the RunStyle after the shell has finished loading.
- *
- * @throws UnableToCompleteException
- */
- public void init() throws UnableToCompleteException {
- // nothing to do
- }
-
- /**
* Returns whether or not the local UI event loop needs to be pumped.
*/
public abstract boolean isLocal();
diff --git a/user/src/com/google/gwt/junit/RunStyleLocalHosted.java b/user/src/com/google/gwt/junit/RunStyleLocalHosted.java
index 94ebe3f..5861c45 100644
--- a/user/src/com/google/gwt/junit/RunStyleLocalHosted.java
+++ b/user/src/com/google/gwt/junit/RunStyleLocalHosted.java
@@ -32,15 +32,6 @@
super(shell);
}
- /**
- * We need to open the browser window in the same thread as {@link JUnitShell}
- * or SWT will throw an error.
- */
- @Override
- public void init() throws UnableToCompleteException {
- browserWindow = shell.openNewBrowserWindow();
- }
-
@Override
public boolean isLocal() {
return true;
diff --git a/user/src/com/google/gwt/junit/client/GWTTestCase.java b/user/src/com/google/gwt/junit/client/GWTTestCase.java
index 4ad6866..176c8a0 100644
--- a/user/src/com/google/gwt/junit/client/GWTTestCase.java
+++ b/user/src/com/google/gwt/junit/client/GWTTestCase.java
@@ -273,11 +273,6 @@
public void setName(String name) {
super.setName(name);
- // If we can't run this test, don't add it to the map of all tests to batch.
- if (JUnitShell.mustNotExecuteTest(this)) {
- return;
- }
-
synchronized (ALL_GWT_TESTS_LOCK) {
// Once the name is set, we can add ourselves to the global set.
String syntheticModuleName = getSyntheticModuleName();
diff --git a/user/test/com/google/gwt/junit/BatchingStrategyTest.java b/user/test/com/google/gwt/junit/BatchingStrategyTest.java
index bd86c18..8a44b09 100644
--- a/user/test/com/google/gwt/junit/BatchingStrategyTest.java
+++ b/user/test/com/google/gwt/junit/BatchingStrategyTest.java
@@ -26,10 +26,35 @@
import java.util.Set;
/**
- * Tests of {@link BatchingStrategy}.
+ * Tests of {@link BatchingStrategy}. This test must run after a
+ * {@link GWTTestCase} to ensure that JUnitShell is already initialized.
*/
public class BatchingStrategyTest extends TestCase {
+ private static class TestClass0 {
+ public void testMethod0() {
+ }
+
+ public void testMethod1() {
+ }
+ }
+
+ private static class TestClass1 {
+ public void testMethod2() {
+ }
+
+ public void testMethod3() {
+ }
+
+ public void testMethod4() {
+ }
+ }
+
+ private static class TestClass2 {
+ public void testMethod5() {
+ }
+ }
+
/**
* The synthetic name of the module used for this test.
*/
@@ -41,17 +66,17 @@
private static TestModuleInfo fakeModuleInfo;
private static final TestInfo TEST_INFO_0_0 = new TestInfo(
- FAKE_MODULE_SYNTHETIC_NAME, "testClass0", "testMethod0");
+ FAKE_MODULE_SYNTHETIC_NAME, TestClass0.class.getName(), "testMethod0");
private static final TestInfo TEST_INFO_0_1 = new TestInfo(
- FAKE_MODULE_SYNTHETIC_NAME, "testClass0", "testMethod1");
+ FAKE_MODULE_SYNTHETIC_NAME, TestClass0.class.getName(), "testMethod1");
private static final TestInfo TEST_INFO_1_2 = new TestInfo(
- FAKE_MODULE_SYNTHETIC_NAME, "testClass1", "testMethod2");
+ FAKE_MODULE_SYNTHETIC_NAME, TestClass1.class.getName(), "testMethod2");
private static final TestInfo TEST_INFO_1_3 = new TestInfo(
- FAKE_MODULE_SYNTHETIC_NAME, "testClass1", "testMethod3");
+ FAKE_MODULE_SYNTHETIC_NAME, TestClass1.class.getName(), "testMethod3");
private static final TestInfo TEST_INFO_1_4 = new TestInfo(
- FAKE_MODULE_SYNTHETIC_NAME, "testClass1", "testMethod4");
+ FAKE_MODULE_SYNTHETIC_NAME, TestClass1.class.getName(), "testMethod4");
private static final TestInfo TEST_INFO_2_5 = new TestInfo(
- FAKE_MODULE_SYNTHETIC_NAME, "testClass2", "testMethod5");
+ FAKE_MODULE_SYNTHETIC_NAME, TestClass2.class.getName(), "testMethod5");
public void testClassBatchingStrategy() {
List<TestInfo[]> testBlocks = new ArrayList<TestInfo[]>();
diff --git a/user/test/com/google/gwt/junit/JUnitSuite.java b/user/test/com/google/gwt/junit/JUnitSuite.java
index ec1c25b..40f468e 100644
--- a/user/test/com/google/gwt/junit/JUnitSuite.java
+++ b/user/test/com/google/gwt/junit/JUnitSuite.java
@@ -27,17 +27,19 @@
public static Test suite() {
GWTTestSuite suite = new GWTTestSuite("Test for suite for com.google.gwt.junit");
- suite.addTestSuite(FakeMessagesMakerTest.class);
- suite.addTestSuite(BatchingStrategyTest.class);
- suite.addTestSuite(JUnitMessageQueueTest.class);
- suite.addTestSuite(GWTTestCaseNoClientTest.class);
- suite.addTestSuite(BenchmarkNoClientTest.class);
-
// client
// Suppressed due to flakiness on Linux
// suite.addTestSuite(BenchmarkTest.class);
suite.addTestSuite(GWTTestCaseTest.class);
+ // Must run after a GWTTestCase so JUnitShell is initialized.
+ suite.addTestSuite(BatchingStrategyTest.class);
+
+ suite.addTestSuite(FakeMessagesMakerTest.class);
+ suite.addTestSuite(JUnitMessageQueueTest.class);
+ suite.addTestSuite(GWTTestCaseNoClientTest.class);
+ suite.addTestSuite(BenchmarkNoClientTest.class);
+
// These two are intended only to be run manually. See class comments
// suite.addTestSuite(ParallelRemoteTest.class);
// suite.addTestSuite(TestManualAsync.class);
