Revision: 6265 Author: [email protected] Date: Wed Sep 30 12:07:05 2009 Log: 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 http://code.google.com/p/google-web-toolkit/source/detail?r=6265 Modified: /trunk/user/src/com/google/gwt/junit/BatchingStrategy.java /trunk/user/src/com/google/gwt/junit/JUnitShell.java /trunk/user/src/com/google/gwt/junit/RunStyle.java /trunk/user/src/com/google/gwt/junit/RunStyleLocalHosted.java /trunk/user/src/com/google/gwt/junit/client/GWTTestCase.java /trunk/user/test/com/google/gwt/junit/BatchingStrategyTest.java /trunk/user/test/com/google/gwt/junit/JUnitSuite.java ======================================= --- /trunk/user/src/com/google/gwt/junit/BatchingStrategy.java Tue Sep 15 11:52:45 2009 +++ /trunk/user/src/com/google/gwt/junit/BatchingStrategy.java Wed Sep 30 12:07:05 2009 @@ -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; } } ======================================= --- /trunk/user/src/com/google/gwt/junit/JUnitShell.java Mon Sep 21 10:33:24 2009 +++ /trunk/user/src/com/google/gwt/junit/JUnitShell.java Wed Sep 30 12:07:05 2009 @@ -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; } @@ -726,11 +748,17 @@ */ 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; } ======================================= --- /trunk/user/src/com/google/gwt/junit/RunStyle.java Mon Sep 21 10:33:24 2009 +++ /trunk/user/src/com/google/gwt/junit/RunStyle.java Wed Sep 30 12:07:05 2009 @@ -34,15 +34,6 @@ public RunStyle(JUnitShell shell) { this.shell = shell; } - - /** - * 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. ======================================= --- /trunk/user/src/com/google/gwt/junit/RunStyleLocalHosted.java Tue Sep 15 11:52:45 2009 +++ /trunk/user/src/com/google/gwt/junit/RunStyleLocalHosted.java Wed Sep 30 12:07:05 2009 @@ -31,15 +31,6 @@ RunStyleLocalHosted(JUnitShell shell) { 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() { ======================================= --- /trunk/user/src/com/google/gwt/junit/client/GWTTestCase.java Mon Sep 21 10:33:24 2009 +++ /trunk/user/src/com/google/gwt/junit/client/GWTTestCase.java Wed Sep 30 12:07:05 2009 @@ -272,11 +272,6 @@ @Override 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. ======================================= --- /trunk/user/test/com/google/gwt/junit/BatchingStrategyTest.java Tue Sep 15 11:52:45 2009 +++ /trunk/user/test/com/google/gwt/junit/BatchingStrategyTest.java Wed Sep 30 12:07:05 2009 @@ -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[]>(); ======================================= --- /trunk/user/test/com/google/gwt/junit/JUnitSuite.java Tue Sep 15 11:52:45 2009 +++ /trunk/user/test/com/google/gwt/junit/JUnitSuite.java Wed Sep 30 12:07:05 2009 @@ -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); --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
