Fixes a bug where JUnitShell does not respect @DoNotRunWith annotations. Patch by: jlabanca Review by: amitmanjhi git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@6173 8db76d5a-ed1c-0410-87a9-c151d255dfc7

diff --git a/user/build.xml b/user/build.xml
index 4d57f82..1fc0af0 100755
--- a/user/build.xml
+++ b/user/build.xml

@@ -177,7 +177,7 @@
   <target name="test.web.htmlunit" depends="compile, compile.tests" description="Run htmlunit web-mode tests for this project.">
     <!-- TODO: add more browsers later -->
     <fileset id="test.web.htmlunit.tests" dir="${javac.junit.out}" includes="${gwt.junit.testcase.web.includes}" excludes="${gwt.junit.testcase.web.excludes}" />
-    <gwt.junit test.args="${test.args} -htmlunit FF3" test.out="${junit.out}/${build.host.platform}-htmlunit-web-mode" test.cases="test.web.htmlunit.tests" >
+    <gwt.junit test.args="${test.args} -htmlunit FF3 -batch module" test.out="${junit.out}/${build.host.platform}-htmlunit-web-mode" test.cases="test.web.htmlunit.tests" >
       <extraclasspaths>
         <path refid="test.extraclasspath" />
       </extraclasspaths>

diff --git a/user/src/com/google/gwt/junit/DoNotRunWith.java b/user/src/com/google/gwt/junit/DoNotRunWith.java
index b0e6811..eda8012 100644
--- a/user/src/com/google/gwt/junit/DoNotRunWith.java
+++ b/user/src/com/google/gwt/junit/DoNotRunWith.java

@@ -26,8 +26,6 @@
  * the specified platforms. We chose DoNotRunWith instead of RunWith because we
  * want each exception to be listed separately here.
  * 
- * TODO(amitmanjhi): Make this work with batching of test cases. 
- * 
  * <pre>
  * &#064;DoNotRunWith({HtmlUnit})
  * public class EmulSuite {

diff --git a/user/src/com/google/gwt/junit/JUnitShell.java b/user/src/com/google/gwt/junit/JUnitShell.java
index 925d652..7dbf9de 100644
--- a/user/src/com/google/gwt/junit/JUnitShell.java
+++ b/user/src/com/google/gwt/junit/JUnitShell.java

@@ -495,6 +495,18 @@
   }
 
   /**
+   * 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.
+   * @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));
+  }
+
+  /**
    * Entry point for {@link com.google.gwt.junit.client.GWTTestCase}. Gets or
    * creates the singleton {@link JUnitShell} and invokes its
    * {@link #runTestImpl(GWTTestCase, TestResult)}.
@@ -571,6 +583,31 @@
   }
 
   /**
+   * returns the set of banned {@code Platform} for a test method.
+   */
+  private static Set<Platform> getBannedPlatforms(TestCase testCase) {
+    Class<?> testClass = testCase.getClass();
+    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());
+      if (testMethod.isAnnotationPresent(DoNotRunWith.class)) {
+        bannedSet.addAll(Arrays.asList(testMethod.getAnnotation(
+            DoNotRunWith.class).value()));
+      }
+    } catch (SecurityException e) {
+      // should not happen
+      e.printStackTrace();
+    } catch (NoSuchMethodException e) {
+      // should not happen
+      e.printStackTrace();
+    }
+    return bannedSet;
+  }
+
+  /**
    * Retrieves the JUnitShell. This should only be invoked during TestRunner
    * execution of JUnit tests.
    */
@@ -843,51 +880,13 @@
     super.compile(getTopLogger(), module);
   }
 
-  /**
-   * returns the set of banned {@code Platform} for a test method.
-   */
-  private Set<Platform> getBannedPlatforms(TestCase testCase) {
-    Class<?> testClass = testCase.getClass();
-    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());
-      if (testMethod.isAnnotationPresent(DoNotRunWith.class)) {
-        bannedSet.addAll(Arrays.asList(testMethod.getAnnotation(
-            DoNotRunWith.class).value()));
-      }
-    } catch (SecurityException e) {
-      // should not happen
-      e.printStackTrace();
-    } catch (NoSuchMethodException e) {
-      // should not happen
-      e.printStackTrace();
-    }
-    return bannedSet;
-  }
-
-  private boolean mostNotExecuteTest(Set<Platform> bannedPlatforms) {
+  private boolean mustNotExecuteTest(Set<Platform> bannedPlatforms) {
     // TODO (amitmanjhi): Remove this hard-coding. A RunStyle somehow needs to
     // specify how it interacts with the platforms.
     return runStyle instanceof RunStyleHtmlUnit
         && bannedPlatforms.contains(Platform.Htmlunit);
   }
 
-  /**
-   * 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.
-   * @return true iff the test should not be executed on any of the specified
-   *         clients.
-   */
-  private boolean mustNotExecuteTest(TestCase testCase) {
-    // TODO: collect stats on tests that were not run
-    return mostNotExecuteTest(getBannedPlatforms(testCase));
-  }
-
   private void processTestResult(TestCase testCase, TestResult testResult,
       Strategy strategy) {
 

diff --git a/user/src/com/google/gwt/junit/RunStyle.java b/user/src/com/google/gwt/junit/RunStyle.java
index b086e6a..c09200d 100644
--- a/user/src/com/google/gwt/junit/RunStyle.java
+++ b/user/src/com/google/gwt/junit/RunStyle.java

@@ -45,14 +45,6 @@
   }
 
   /**
-   * Returns true if clients are not known before the tests start and can
-   * connect after a delay. 
-   */
-  public boolean isClientConnectionDelayed() {
-    return false;
-  }
-
-  /**
    * 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/RunStyleManual.java b/user/src/com/google/gwt/junit/RunStyleManual.java
index 13e56ff..948e066 100644
--- a/user/src/com/google/gwt/junit/RunStyleManual.java
+++ b/user/src/com/google/gwt/junit/RunStyleManual.java

@@ -31,11 +31,6 @@
   }
 
   @Override
-  public boolean isClientConnectionDelayed() {
-    return true;
-  }
-
-  @Override
   public boolean isLocal() {
     return false;
   }

diff --git a/user/src/com/google/gwt/junit/client/GWTTestCase.java b/user/src/com/google/gwt/junit/client/GWTTestCase.java
index 176c8a0..4ad6866 100644
--- a/user/src/com/google/gwt/junit/client/GWTTestCase.java
+++ b/user/src/com/google/gwt/junit/client/GWTTestCase.java

@@ -273,6 +273,11 @@
   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();