Since Lex and I are push/pull'ing you on the flag name, I'd leave it the
way you have it unless a consensus otherwise emerges.  But the
when-to-enable flag is wrong, and I think you've moved from a
too-expansive to a too-restricted spec!


http://gwt-code-reviews.appspot.com/47802/diff/2003/2009
File user/src/com/google/gwt/junit/JUnitShell.java (right):

http://gwt-code-reviews.appspot.com/47802/diff/2003/2009#newcode779
Line 779: if (!argList.contains("-XdisableAssertions") &&
argList.contains("-web")) {
I think my reaction on this one got lost in an off-line thread... I
actually think "assertions are on for tests, unles s you turn them off"
is simpler/better than "assertions are on for tests, but only in web
mode,...", which I think makes the JUnitShell constructor change
superior.

There are also ways to run "web mode" that do not use -web (namely,
-remoteweb, -selenium, and -manual), so this contains test is flawed
anyway.  My suggestion for doing it here was actually to always assert
-ea (in position 0, and regardless of anything in argList), and let that
be overridden by a later -XdisableAssertions flag if need be.

So, regardless of intended spec, the test here is wrong.  I claim that
"correct" is to be mode-insensitive (i.e. JUnitShell defaults to
assertions enabled, but hosted mode and compiler default to disabled).
Copy GWTC for dissent, speak now or hold your peace.

http://gwt-code-reviews.appspot.com/47802

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to