http://gwt-code-reviews.appspot.com/91806/diff/1/2 File user/src/com/google/gwt/junit/JUnitMessageQueue.java (right):
http://gwt-code-reviews.appspot.com/91806/diff/1/2#newcode413 Line 413: assert (result != null); This can be null. The result is set to null to indicate that the test has started, but no results have been received from the client. With respect to retry, the results will be null if we retry after a remote browser dies in the middle of a test. http://gwt-code-reviews.appspot.com/91806/diff/1/3 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/91806/diff/1/3#newcode908 Line 908: if (batchingStrategy instanceof NoBatchingStrategy) { You should put a comment explaining why this is true. "If a BatchingStrategy is present, the client will already have moved passed the failed test case." http://gwt-code-reviews.appspot.com/91806/diff/1/3#newcode912 Line 912: "batching does not work with retries, so not being retried"); I think we should check this when we process the arguments. In fact, we should probably add a sanity check after the args processor to verify that we don't have conflicting args in general. http://gwt-code-reviews.appspot.com/91806/diff/1/3#newcode1005 Line 1005: prevTestInfo = currentTestInfo; Since runTestImpl is called recursively, can you just pass numTries as an arg? Then you can do away with the numTries and prevTestInfo instance variables. The static runTest methods can pass 0. http://gwt-code-reviews.appspot.com/91806/diff/1/4 File user/src/com/google/gwt/junit/RunStyle.java (right): http://gwt-code-reviews.appspot.com/91806/diff/1/4#newcode46 Line 46: public int getTries() { You should add a command line -retryCount arg to JUnitShell so the user can specify this? I think the number of retries would have more to do with the users test setup than with the run style. This method should be getDefaultRetryCount, because it is the default if the user does not specify a count. http://gwt-code-reviews.appspot.com/91806/diff/1/6 File user/test/com/google/gwt/junit/JUnitMessageQueueTest.java (right): http://gwt-code-reviews.appspot.com/91806/diff/1/6#newcode379 Line 379: int testsPerBlock = 1; You can reuse your static values here. http://gwt-code-reviews.appspot.com/91806/diff/1/7 File user/test/com/google/gwt/junit/client/GWTTestCaseTest.java (right): http://gwt-code-reviews.appspot.com/91806/diff/1/7#newcode377 Line 377: public void testRetry() { You should add a note that this method MUST appear after testSetRetry(). http://gwt-code-reviews.appspot.com/91806 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
