Brian Slesinsky has posted comments on this change.

Change subject: Refactors c.g.gwt.junit to use common SerializableThrowable & StacktraceDeobfuscator.
......................................................................


Patch Set 2: Code-Review+1

(8 comments)

Seems okay, after nits.

....................................................
Commit Message
Line 9: - Gets rid of dublicate code in junit for deobfuscation of stack traces
"duplicate"


....................................................
File user/src/com/google/gwt/junit/server/JUnitHostImpl.java
Line 171:       System.err.println("Cannot deobfuscate stack trace:");
At first I misread this as saying that the obfuscated stack trace follows, but actually it's the exception that we got while attempting to deobfuscate a stack trace.

To clear this up, how about:
"Unable to deobfuscate a stack trace due to an error:"


....................................................
File user/test/com/google/gwt/junit/TestResultWithExpectedFailures.java
Line 83: String msg = e + "\n(Asserted exception is reported below via 'cause by')";
"Actual exception" seems clearer than "Asserted exception"

s/cause by/caused by/


....................................................
File user/test/com/google/gwt/junit/client/DefaultExceptionAsserter.java
Line 30: public void assertException(Throwable throwable, ExpectedFailure annotation) {
s/throwable/actual/


Line 32: assertTrue(getExceptionMessage(throwable).contains(annotation.withMessage()));
add a message:
"The test threw an exception without the expected message"


....................................................
File user/test/com/google/gwt/junit/client/ExceptionAsserter.java
Line 24: public interface ExceptionAsserter {
"ExceptionChecker" seems like a better name?


Line 26: void assertException(Throwable throwable, ExpectedFailure annotation);
This could be called "check" and the first argument could be "actual":

void check(Throwable actual, ExpectedFailure annotation);

But by analogy with assertEquals(), perhaps the expected part should come first and the actual part second:

void check(ExpectedFailure annotation, Throwable actual);


....................................................
File user/test/com/google/gwt/junit/client/GWTTestCaseTest.java
Line 98: // We loose some type information if class meta data is not available, setting expected failure
s/loose/lose/


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e1021bc99ac88ea6d9d47c3d23c83e79a896213
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <[email protected]>
Gerrit-Reviewer: Brian Slesinsky <[email protected]>
Gerrit-Reviewer: Goktug Gokdogan <[email protected]>
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 [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to