Patch adds the infrastructure to re-run a test. Useful for htmlUnit which can
be flaky sometimes.
Patch by: amitmanjhi
Review by: jlabanca
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@6687 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/junit/JUnitMessageQueue.java b/user/src/com/google/gwt/junit/JUnitMessageQueue.java
index 4662c5a..0137956 100644
--- a/user/src/com/google/gwt/junit/JUnitMessageQueue.java
+++ b/user/src/com/google/gwt/junit/JUnitMessageQueue.java
@@ -26,6 +26,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.Map.Entry;
/**
* A message queue to pass data between {@link JUnitShell} and
@@ -395,6 +396,38 @@
}
}
+ /*
+ * Returns true iff any there are no results, missing results, or any of the
+ * test results is an exception other than JUnitFatalLaunchException.
+ */
+ boolean needsRerunning(TestInfo testInfo) {
+ Map<String, JUnitResult> results = getResults(testInfo);
+ if (results == null) {
+ return true;
+ }
+ if (results.size() != numClients) {
+ return true;
+ }
+ for (Entry<String, JUnitResult> entry : results.entrySet()) {
+ JUnitResult result = entry.getValue();
+ if (result == null) {
+ return true;
+ }
+ Throwable exception = result.getException();
+ if (exception != null
+ && !(exception instanceof JUnitFatalLaunchException)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ void removeResults(TestInfo testInfo) {
+ synchronized (clientStatusesLock) {
+ testResults.remove(testInfo);
+ }
+ }
+
void waitForResults(int millis) {
synchronized (clientStatusesLock) {
try {
diff --git a/user/src/com/google/gwt/junit/JUnitShell.java b/user/src/com/google/gwt/junit/JUnitShell.java
index 93aa231..36b420d 100644
--- a/user/src/com/google/gwt/junit/JUnitShell.java
+++ b/user/src/com/google/gwt/junit/JUnitShell.java
@@ -241,7 +241,6 @@
}
});
- // TODO: currently, only two values but soon may have multiple values.
registerHandler(new ArgHandlerString() {
@Override
public String getPurpose() {
@@ -578,7 +577,7 @@
// TODO: install a shutdown hook? Not necessary with GWTShell.
unitTestShell.lastLaunchFailed = false;
}
-
+ unitTestShell.checkArgs();
return unitTestShell;
}
@@ -861,8 +860,8 @@
/**
* Accessor method to HostedModeBase.setHeadless -- without this, we get
- * IllegalAccessError from the -notHeadless arg handler. Compiler bug?
- *
+ * IllegalAccessError from the -notHeadless arg handler. Compiler bug?
+ *
* @param headlessMode
*/
void setHeadlessAccessor(boolean headlessMode) {
@@ -879,11 +878,17 @@
void setNumClients(int numClients) {
this.numClients = numClients;
}
-
+
void setStandardsMode(boolean standardsMode) {
this.standardsMode = standardsMode;
}
+ private void checkArgs() {
+ if (runStyle.getTries() > 1 && !(batchingStrategy instanceof NoBatchingStrategy)) {
+ throw new JUnitFatalLaunchException("Batching does not work with tries > 1");
+ }
+ }
+
/**
* Create the specified (or default) runStyle.
*
@@ -934,6 +939,20 @@
&& bannedPlatforms.contains(Platform.HtmlUnit);
}
+ private boolean mustRetry(int numTries) {
+ if (numTries >= runStyle.getTries()) {
+ return false;
+ }
+ assert (batchingStrategy instanceof NoBatchingStrategy);
+ // checked in {@code checkArgs()}
+ /*
+ * If a batching strategy is being used, the client will already have moved
+ * passed the failed test case. The whole block could be re-executed, but it
+ * would be more complicated.
+ */
+ return true;
+ }
+
private void processTestResult(TestCase testCase, TestResult testResult,
Strategy strategy) {
@@ -978,10 +997,15 @@
}
}
+ private void runTestImpl(GWTTestCase testCase, TestResult testResult)
+ throws UnableToCompleteException {
+ runTestImpl(testCase, testResult, 0);
+ }
+
/**
* Runs a particular test case.
*/
- private void runTestImpl(GWTTestCase testCase, TestResult testResult)
+ private void runTestImpl(GWTTestCase testCase, TestResult testResult, int numTries)
throws UnableToCompleteException {
testBatchingMethodTimeoutMillis = batchingStrategy.getTimeoutMultiplier()
@@ -1021,6 +1045,7 @@
currentTestInfo = new TestInfo(currentModule.getName(),
testCase.getClass().getName(), testCase.getName());
+ numTries++;
if (messageQueue.hasResults(currentTestInfo)) {
// Already have a result.
processTestResult(testCase, testResult, strategy);
@@ -1038,6 +1063,7 @@
return;
}
+ boolean mustRetry = mustRetry(numTries);
// Wait for test to complete
try {
// Set a timeout period to automatically fail if the servlet hasn't been
@@ -1048,17 +1074,29 @@
while (notDone()) {
messageQueue.waitForResults(1000);
}
- if (pendingException != null) {
+ if (!mustRetry && pendingException != null) {
UnableToCompleteException e = pendingException;
pendingException = null;
throw e;
}
} catch (TimeoutException e) {
- lastLaunchFailed = true;
- testResult.addError(testCase, e);
- return;
+ if (!mustRetry) {
+ lastLaunchFailed = true;
+ testResult.addError(testCase, e);
+ return;
+ }
}
+ if (mustRetry) {
+ if (messageQueue.needsRerunning(currentTestInfo)) {
+ // remove the result if it is present and rerun
+ messageQueue.removeResults(currentTestInfo);
+ getTopLogger().log(TreeLogger.WARN,
+ currentTestInfo + " is being retried, retry attempt = " + numTries);
+ runTestImpl(testCase, testResult, numTries);
+ return;
+ }
+ }
assert (messageQueue.hasResults(currentTestInfo));
processTestResult(testCase, testResult, testCase.getStrategy());
}
diff --git a/user/src/com/google/gwt/junit/RunStyle.java b/user/src/com/google/gwt/junit/RunStyle.java
index d054429..e6d7b94 100644
--- a/user/src/com/google/gwt/junit/RunStyle.java
+++ b/user/src/com/google/gwt/junit/RunStyle.java
@@ -40,6 +40,14 @@
}
/**
+ * Returns the number of times this test should be tried to run. A test
+ * succeeds if it succeeds even once.
+ */
+ public int getTries() {
+ return 1;
+ }
+
+ /**
* Initialize the runstyle with any supplied arguments.
*
* @param args arguments passed in -runStyle option, null if none supplied
diff --git a/user/src/com/google/gwt/junit/RunStyleHtmlUnit.java b/user/src/com/google/gwt/junit/RunStyleHtmlUnit.java
index 1923bf5..45e085f 100644
--- a/user/src/com/google/gwt/junit/RunStyleHtmlUnit.java
+++ b/user/src/com/google/gwt/junit/RunStyleHtmlUnit.java
@@ -186,6 +186,16 @@
}
@Override
+ public int getTries() {
+ // TODO: set this via the command-line
+ /*
+ * as long as this number is greater than 1, GWTTestCaseTest::testRetry will
+ * pass
+ */
+ return 3;
+ }
+
+ @Override
public boolean initialize(String args) {
if (args == null || args.length() == 0) {
// If no browsers specified, default to Firefox 3.
diff --git a/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java b/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java
index 0d34966..f8cd726 100644
--- a/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java
+++ b/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java
@@ -31,6 +31,10 @@
*/
public class JUnitMessageQueueTest extends TestCase {
+ private final static int TWO_CLIENTS = 2;
+ private final static int ONE_BLOCK = 1;
+ private final static int ONE_TEST_PER_BLOCK = 1;
+
public void testAddTestBlocks() {
JUnitMessageQueue queue = new JUnitMessageQueue(10);
assertEquals(0, queue.getTestBlocks().size());
@@ -171,7 +175,6 @@
}
public void testGetResults() {
- final long timeout = System.currentTimeMillis() + 15;
JUnitMessageQueue queue = createQueue(3, 2, 3);
TestInfo[] testBlock0 = queue.getTestBlocks().get(0);
TestInfo test0_0 = testBlock0[0];
@@ -273,7 +276,6 @@
}
public void testHasResults() {
- final long timeout = System.currentTimeMillis() + 15;
JUnitMessageQueue queue = createQueue(3, 2, 3);
TestInfo[] testBlock0 = queue.getTestBlocks().get(0);
TestInfo test0_0 = testBlock0[0];
@@ -343,6 +345,49 @@
}
}
+ public void testNeedRerunningExceptions() {
+ JUnitMessageQueue queue = createQueue(TWO_CLIENTS, ONE_BLOCK,
+ ONE_TEST_PER_BLOCK);
+ // an exception
+ TestInfo testInfo = queue.getTestBlocks().get(0)[0];
+ Map<TestInfo, JUnitResult> results = new HashMap<TestInfo, JUnitResult>();
+ JUnitResult junitResult = new JUnitResult();
+ junitResult.setException(new AssertionError());
+ results.put(testInfo, junitResult);
+ queue.reportResults("client0", "ie6", results);
+ results = new HashMap<TestInfo, JUnitResult>();
+ junitResult = new JUnitResult();
+ junitResult.setException(new JUnitFatalLaunchException());
+ results.put(testInfo, junitResult);
+ queue.reportResults("client1", "ff3", createTestResults(ONE_TEST_PER_BLOCK));
+ assertTrue(queue.needsRerunning(testInfo));
+
+ // an exception but exception in launch module
+ queue.removeResults(testInfo);
+ results = new HashMap<TestInfo, JUnitResult>();
+ junitResult = new JUnitResult();
+ junitResult.setException(new JUnitFatalLaunchException());
+ results.put(testInfo, junitResult);
+ queue.reportResults("client0", "ie6", results);
+ queue.reportResults("client1", "ff3", createTestResults(ONE_TEST_PER_BLOCK));
+ assertFalse(queue.needsRerunning(testInfo));
+ }
+
+ public void testNeedRerunningIncompleteResults() {
+ JUnitMessageQueue queue = createQueue(TWO_CLIENTS, ONE_BLOCK,
+ ONE_TEST_PER_BLOCK);
+ TestInfo testInfo = queue.getTestBlocks().get(0)[0];
+
+ // incomplete results
+ assertTrue(queue.needsRerunning(testInfo));
+ queue.reportResults("client0", "ff3", createTestResults(1));
+ assertTrue(queue.needsRerunning(testInfo));
+
+ // complete results
+ queue.reportResults("client1", "ie7", createTestResults(1));
+ assertFalse(queue.needsRerunning(testInfo));
+ }
+
public void testNewClients() {
final long timeout = System.currentTimeMillis() + 15;
JUnitMessageQueue queue = createQueue(15, 1, 1);
@@ -377,6 +422,42 @@
}
}
+ public void testRemove() {
+ JUnitMessageQueue queue = createQueue(TWO_CLIENTS, ONE_BLOCK,
+ ONE_TEST_PER_BLOCK);
+ TestInfo testInfo = queue.getTestBlocks().get(0)[0];
+ assertFalse(queue.hasResults(testInfo));
+
+ queue.reportResults("client0", "ie6", createTestResults(ONE_TEST_PER_BLOCK));
+ assertFalse(queue.hasResults(testInfo));
+ queue.reportResults("client1", "ff3", createTestResults(ONE_TEST_PER_BLOCK));
+ assertTrue(queue.hasResults(testInfo));
+
+ queue.removeResults(testInfo);
+ assertFalse(queue.hasResults(testInfo));
+ }
+
+ public void testRetries() {
+ JUnitMessageQueue queue = createQueue(TWO_CLIENTS, ONE_BLOCK,
+ ONE_TEST_PER_BLOCK);
+ TestInfo testInfo = queue.getTestBlocks().get(0)[0];
+ Map<TestInfo, JUnitResult> results = new HashMap<TestInfo, JUnitResult>();
+ JUnitResult junitResult = new JUnitResult();
+ junitResult.setException(new AssertionError());
+ results.put(testInfo, junitResult);
+ queue.reportResults("client0", "ff3", results);
+ assertTrue(queue.needsRerunning(testInfo));
+ assertTrue(queue.getResults(testInfo).get("client0").getException() != null);
+
+ queue.removeResults(testInfo);
+
+ queue.reportResults("client0", "ff3", createTestResults(ONE_TEST_PER_BLOCK));
+ queue.reportResults("client1", "ie6", createTestResults(ONE_TEST_PER_BLOCK));
+ assertFalse(queue.needsRerunning(testInfo));
+ // check that the updated result appears now.
+ assertTrue(queue.getResults(testInfo).get("client0").getException() == null);
+ }
+
/**
* Assert that two arrays are the same size and contain the same elements.
* Ordering does not matter.
@@ -442,7 +523,7 @@
private Map<TestInfo, JUnitResult> createTestResults(int numTests) {
Map<TestInfo, JUnitResult> results = new HashMap<TestInfo, JUnitResult>();
for (int i = 0; i < numTests; i++) {
- TestInfo testInfo = new TestInfo("testModule", "testClass", "testMethod"
+ TestInfo testInfo = new TestInfo("testModule0", "testClass", "testMethod"
+ i);
results.put(testInfo, new JUnitResult());
}
diff --git a/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java b/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java
index f3e32a1..3a7e091 100644
--- a/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java
+++ b/user/test/com/google/gwt/junit/client/GWTTestCaseTest.java
@@ -24,6 +24,8 @@
import static com.google.gwt.junit.client.GWTTestCaseTest.SetUpTearDownState.IS_SETUP;
import static com.google.gwt.junit.client.GWTTestCaseTest.SetUpTearDownState.IS_TORNDOWN;
+import com.google.gwt.junit.DoNotRunWith;
+import com.google.gwt.junit.Platform;
import com.google.gwt.user.client.Timer;
import junit.framework.AssertionFailedError;
@@ -47,6 +49,13 @@
*/
private static String outOfBandError = null;
+ /**
+ * These two variables test the retry functionality, currently used with
+ * HtmlUnit.
+ */
+ private static boolean attemptedOnce = false;
+ private static boolean htmlunitMode = true;
+
private static void assertNotEquals(double a, double b, double delta) {
boolean failed = false;
try {
@@ -357,6 +366,27 @@
fail("Unexpected exception during assertTrue(String, boolean) testing");
}
+ /*
+ * Just setting the htmlunit mode.
+ */
+ @DoNotRunWith(Platform.HtmlUnit)
+ public void testSetRetry() {
+ htmlunitMode = false;
+ }
+
+ /*
+ * This test MUST appear after testSetRetry().
+ */
+ public void testRetry() {
+ if (htmlunitMode && !attemptedOnce) {
+ // fail
+ attemptedOnce = true;
+ assertTrue(false);
+ } else {
+ assertTrue(true);
+ }
+ }
+
public void testSetUpTearDown() throws Exception {
assertSame(IS_SETUP, setupTeardownFlag);
tearDown();
