Goktug Gokdogan has posted comments on this change.

Change subject: Moves GWTTestCase to use Impl#setUncaughtExceptionHandlerForTest to prevent conflicts with production handlers.
......................................................................


Patch Set 1:

(8 comments)

....................................................
File user/src/com/google/gwt/junit/client/GWTTestCase.java
Line 364:   public void reportException(Throwable ex) {
I think fail and failAsync adds confusion as the semantics of this method is a lot different than other Assert.fail() method and also this may fail synchronously or asynchronously.


....................................................
File user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/GWTTestCase.java Line 222: public void setUncaughtExceptionHandlerForTest(UncaughtExceptionHandler handler) {
Done.


Line 223: if (GWT.getUncaughtExceptionHandler() instanceof TestCaseUncaughtExceptionHandler) {
Putting assert and adding a todo for reporting to GWTRunner.
(I am not sure if it should be reported as fatal as if we do that in a lot of scenarios when a test fails unexpectedly, it may potentially cause all other tests to fail.)


Line 224: GWT.setUncaughtExceptionHandler(handler); // set it only if its our handler
You are right.
Setting to null was my original implementation. However when uncaught exception handler is null and exception escapes in htmlunit and htmlunit dies.
https://code.google.com/p/google-web-toolkit/issues/detail?id=8007.
On the other hand, even that wasn't the case, perhaps it is also OK to not let the exception to escape to browser if the test adds its own handler.

Anyway, I'll set it to an empty handler to prevent the potential leak when the method called twice. Also adding a comment to point the issue.


Line 229:   public void reportException(Throwable ex) {
Done


Line 232:       assert (!testIsFinished);
Done


....................................................
File user/test/com/google/gwt/dev/jjs/test/RunAsyncTest.java
Line 111:   public void testUnhandledExceptions() {
Done


....................................................
File user/test/com/google/gwt/junit/client/GWTTestCaseUncaughtExceptionHandlerTest.java Line 85: // Suppressed due to http://code.google.com/p/google-web-toolkit/issues/detail?id=5016
Done


--
To view, visit https://gwt-review.googlesource.com/2190
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8d32c6f4ff4cbcd55f238116b8bcc4c8685d6f35
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Brian Slesinsky <skybr...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-HasComments: Yes

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- You received this message because you are subscribed to the Google Groups "Google Web Toolkit Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to