I did not review the tests that were blocked, but they don't seem that bad in general. I am a bit surprised at a few of them, but we can improve them later.
http://gwt-code-reviews.appspot.com/54809/diff/1/32 File user/src/com/google/gwt/junit/DoNotRunWith.java (right): http://gwt-code-reviews.appspot.com/54809/diff/1/32#newcode27 Line 27: * want each exception to be listed separately here. It seems like in some cases, such as testing a mobile platform feature, you might want to only test a specific platform. So, while I think exceptions for tests that don't work should be done this way, I also see the utility of having a RunWith annotation. This obviously can be done later. http://gwt-code-reviews.appspot.com/54809/diff/1/32#newcode29 Line 29: * TODO(amitmanjhi): Make this work with batching of test cases. What happens now regarding batching? http://gwt-code-reviews.appspot.com/54809/diff/1/34 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/54809/diff/1/34#newcode398 Line 398: private static final int TEST_BEGIN_TIMEOUT_MILLIS = 6000000; I assume this was for testing and should be reverted. http://gwt-code-reviews.appspot.com/54809/diff/1/34#newcode736 Line 736: && bannedPlatforms.contains(Platform.Htmlunit); This can be done later, but in the next-gen build discussions we were talking about labelling a test as flaky on a platform and it would still be run but a failure wouldn't block the build. http://gwt-code-reviews.appspot.com/54809/diff/1/35 File user/src/com/google/gwt/junit/Platform.java (right): http://gwt-code-reviews.appspot.com/54809/diff/1/35#newcode22 Line 22: * between Htmlunit and non-Htmlunit platforms. How can you represent non-HTMLUnit platforms when there is only one value? Also, you might want to use different browser emulations in HTMLUnit and specify those as well (ie, HtmlunitFF3). http://gwt-code-reviews.appspot.com/54809/diff/1/33 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right): http://gwt-code-reviews.appspot.com/54809/diff/1/33#newcode93 Line 93: // TODO(jat): is this necessary? Did you investigate this TODO? http://gwt-code-reviews.appspot.com/54809/diff/38/1001 File eclipse/user/.classpath (right): http://gwt-code-reviews.appspot.com/54809/diff/38/1001#newcode6 Line 6: <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/> Apparently the diff didn't match what was in trunk -- see the error: old chunk mismatch http://gwt-code-reviews.appspot.com/54809 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
