[gwt-contrib] Re: Adds {moz,webkit}RequestAnimationFrame support to animations. (issue1355805)
http://gwt-code-reviews.appspot.com/1355805/diff/6003/user/src/com/google/gwt/user/client/ui/DeckPanel.java File user/src/com/google/gwt/user/client/ui/DeckPanel.java (right): http://gwt-code-reviews.appspot.com/1355805/diff/6003/user/src/com/google/gwt/user/client/ui/DeckPanel.java#newcode108 user/src/com/google/gwt/user/client/ui/DeckPanel.java:108: // Figure out if the deck panel has a fixed height I'm not sure I'm a good checker, but it smells funny to me that onStart is losing a check that it used to have without (that I saw) routing through the new code in newWidget, nor why newWidget, having decided to animate, doesn't touch onStart. I'm willing to believe there's a good reason for it, but I'd like to know what it is. ;-) http://gwt-code-reviews.appspot.com/1355805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Investigate test flakiness: (issue1401803)
LGTM http://gwt-code-reviews.appspot.com/1401803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Modify the version of xalan-2.7.1.jar used to stop including an old version of the Java Cup runt... (issue1404801)
LGTM http://gwt-code-reviews.appspot.com/1404801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a no-op emulation of TestSuite, to prevent error spam (or outright (issue1399803)
LGTM with nits about which comments are helpful.
http://gwt-code-reviews.appspot.com/1399803/diff/1/user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java
File
user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java
(right):
http://gwt-code-reviews.appspot.com/1399803/diff/1/user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java#newcode22
user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java:22:
* messages this emulation is meant to avoid.
I'd probably lose the comments as noise.
http://gwt-code-reviews.appspot.com/1399803/diff/1/user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java#newcode34
user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java:34:
* error messages, or out right compile failures in -strict mode.
Probably add a blurb here about the missing methods:
...
There are a few methods useful to TestRunners which we can't
emulate for GWT; fortunately, we don't have to. But we do
want to include methods that a TestSuite might sensibly call
on itself. They don't have to do more than compile, though.
http://gwt-code-reviews.appspot.com/1399803/diff/1/user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java#newcode42
user/super/com/google/gwt/junit/translatable/junit/framework/TestSuite.java:42:
// public static Constructor? getTestConstructor(Class? theClass)
throws NoSuchMethodException {
keep these, but lose the bodies, and add a header bar saying why they're
commented out:
// getTestConstructor uses reflective classes; we can't emulate it.
// public static Constructor? getTestConstructor(...)
http://gwt-code-reviews.appspot.com/1399803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a no-op emulation of TestSuite, to prevent error spam (or outright (issue1399803)
LGTM, now without nits. http://gwt-code-reviews.appspot.com/1399803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Moving UnitTestTreeLogger from dev/core/src to dev/core/tests, so gwt-dev (issue1383804)
http://gwt-code-reviews.appspot.com/1383804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Moving UnitTestTreeLogger from dev/core/src to dev/core/tests, so gwt-dev (issue1383804)
Reviewers: scottb, Description: Moving UnitTestTreeLogger from dev/core/src to dev/core/tests, so gwt-dev doesn't have an unnecessary dependency on junit. Please review this at http://gwt-code-reviews.appspot.com/1383804/ Affected files: D dev/core/src/com/google/gwt/dev/util/UnitTestTreeLogger.java A dev/core/test/com/google/gwt/dev/util/UnitTestTreeLogger.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Replacing the testng-5.14.1.jar with testng-5.14.1-nojunit.jar in the gwt-user .classpath file. ... (issue1377803)
LGTM, too. Shoulda caught that in my change, sorry. On 2011/03/09 15:26:32, pdr wrote: On 2011/03/09 15:23:11, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/1377803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Switching to the junit4 jars, although the @annotation stuff isn't (issue1374801)
http://gwt-code-reviews.appspot.com/1374801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Switching to the junit4 jars, although the @annotation stuff isn't (issue1374801)
http://gwt-code-reviews.appspot.com/1374801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Switching to the junit4 jars, although the @annotation stuff isn't (issue1374801)
Reviewers: jat,
Description:
Switching to the junit4 jars, although the @annotation stuff isn't
going to work.
Review by: j...@google.com
Please review this at http://gwt-code-reviews.appspot.com/1374801/
Affected files:
M build-tools/ant-gwt/build.xml
M common.ant.xml
M dev/build.xml
M doc/build.xml
M eclipse/build-tools/ant-gwt/.classpath
M eclipse/dev/.classpath
M eclipse/dev/compiler/.classpath
M eclipse/doc/.classpath
M eclipse/servlet/.classpath
M eclipse/user/.classpath
M tools/api-checker/build.xml
M tools/cldr-import/build.xml
M user/build.xml
M user/test/org/hibernate/jsr303/tck/util/TckTestSuiteWrapper.java
Index: build-tools/ant-gwt/build.xml
===
--- build-tools/ant-gwt/build.xml (revision 9793)
+++ build-tools/ant-gwt/build.xml (working copy)
@@ -20,7 +20,7 @@
gwt.javac srcdir=test destdir=${javac.junit.out}
classpath
pathelement location=${javac.out} /
-pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
/classpath
/gwt.javac
/target
Index: common.ant.xml
===
--- common.ant.xml (revision 9793)
+++ common.ant.xml (working copy)
@@ -190,7 +190,7 @@
sequential
taskdef name=junit
classname=org.apache.tools.ant.taskdefs.optional.junit.JUnitTask
classpath
- pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+ pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
pathelement location=${gwt.tools.antlib}/ant-junit-1.6.5.jar
/
pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
/classpath
@@ -233,7 +233,7 @@
path refid=project.classpath.class /
pathelement location=${emma.lib} /
pathelement location=${gwt.dev.staging.jar} /
- pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+ pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
pathelement location=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg1.jar /
Index: dev/build.xml
===
--- dev/build.xml (revision 9793)
+++ dev/build.xml (working copy)
@@ -27,7 +27,7 @@
classpath
pathelement location=${javac.out} /
pathelement location=${alldeps.jar} /
-pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
/classpath
/gwt.javac
gwt.javac srcdir=${gwt.root}/user/src destdir=${javac.junit.out}
@@ -35,7 +35,7 @@
classpath
pathelement location=${javac.out} /
pathelement
location=${gwt.tools.lib}/tomcat/servlet-api-2.5.jar /
-pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
pathelement
location=${gwt.tools.lib}/jfreechart/jfreechart-1.0.3.jar /
pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
pathelement location=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
@@ -213,7 +213,7 @@
pathelement location=${gwt.tools.lib}/apache/ant-1.6.5.jar /
pathelement location=${gwt.tools.lib}/eclipse/jdt-3.4.2.jar /
pathelement
location=${gwt.tools.lib}/tomcat/commons-collections-3.1.jar /
- pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+ pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
pathelement
location=${gwt.tools.lib}/guava/guava-r06/guava-r06-rebased-2.jar /
/classpath
/gwt.javac
@@ -224,7 +224,7 @@
src path=core/src /
classpath
pathelement location=${alldeps.jar} /
-pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
/classpath
/gwt.javac
copy todir=${javac.out}
Index: doc/build.xml
===
--- doc/build.xml (revision 9793)
+++ doc/build.xml (working copy)
@@ -26,7 +26,7 @@
pathelement location=${gwt.dev.jar} /
pathelement location=${gwt.user.jar} /
pathelement
location=${gwt.tools}/redist/json/r2_20080312/json-1.5.jar /
-pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
pathelement
location=${gwt.tools.lib}/javax/validation/validation-api-1.0.0.GA.jar /
pathelement
[gwt-contrib] Re: Switching to the junit4 jars, although the @annotation stuff isn't (issue1374801)
Yep, the tools update just happened at r9794.
http://gwt-code-reviews.appspot.com/1374801/diff/1/user/test/org/hibernate/jsr303/tck/util/TckTestSuiteWrapper.java
File user/test/org/hibernate/jsr303/tck/util/TckTestSuiteWrapper.java
(left):
http://gwt-code-reviews.appspot.com/1374801/diff/1/user/test/org/hibernate/jsr303/tck/util/TckTestSuiteWrapper.java#oldcode86
user/test/org/hibernate/jsr303/tck/util/TckTestSuiteWrapper.java:86: *
Returns a test which will fail and log a warning message.
On 2011/03/02 21:14:39, jat wrote:
Why is this removed?
It's included (verbatim) in the junit4 copy of TestSuite, but
with public visibility. So I started making this public, then realized
this was probably cribbed from that:
from junit.framework.TestSuite:
/**
* Returns a test which will fail and log a warning message.
*/
public static Test warning(final String message) {
return new TestCase(warning) {
@Override
protected void runTest() {
fail(message);
}
};
}
http://gwt-code-reviews.appspot.com/1374801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix 'ant dist-dev' and 'ant-doc' builds: (issue953801)
Lgtm. http://gwt-code-reviews.appspot.com/953801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fixing the javadoc build breaks. One problem is that the doc/build.xml (issue963801)
Reviewers: jlabanca, Description: Fixing the javadoc build breaks. One problem is that the doc/build.xml has drifted stale against user/build.xml's classpath (we should unify them, but the goal here is to fix the break); another is citing @examples that don't (yet) exist. Please review this at http://gwt-code-reviews.appspot.com/963801/show Affected files: M doc/build.xml M user/src/com/google/gwt/cell/client/AbstractCell.java M user/src/com/google/gwt/cell/client/AbstractEditableCell.java M user/src/com/google/gwt/cell/client/Cell.java M user/src/com/google/gwt/user/cellview/client/CellBrowser.java M user/src/com/google/gwt/user/cellview/client/CellList.java M user/src/com/google/gwt/user/cellview/client/CellTable.java M user/src/com/google/gwt/user/cellview/client/CellTree.java M user/src/com/google/gwt/user/cellview/client/SimplePager.java M user/src/com/google/gwt/view/client/AsyncDataProvider.java M user/src/com/google/gwt/view/client/ListDataProvider.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix 'ant dist-dev' and 'ant-doc' builds: (issue953801)
http://gwt-code-reviews.appspot.com/953801/diff/1/5 File user/src/com/google/gwt/requestfactory/client/impl/messages/JsonResults.java (right): http://gwt-code-reviews.appspot.com/953801/diff/1/5#newcode28 user/src/com/google/gwt/requestfactory/client/impl/messages/JsonResults.java:28: // javac complains when this is all on one line which javac, including revision? Comment should probably say... http://gwt-code-reviews.appspot.com/953801/diff/1/7 File user/src/com/google/gwt/safehtml/shared/SafeHtmlHostedModeUtils.java (right): http://gwt-code-reviews.appspot.com/953801/diff/1/7#newcode112 user/src/com/google/gwt/safehtml/shared/SafeHtmlHostedModeUtils.java:112: // The following annotation causes javadoc to crash: Do we have any idea why?? It seems like we should have version information, too. http://gwt-code-reviews.appspot.com/953801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove the output from the link step before attempting to (issue928801)
Assuming we like the concept, I have two issues noted here. At the larger level, people should be doing this today in e.g. build.xml as a wrapper, which also works... I'm not sure whether I'd be more annoyed to fail on existing dirt, or to have something I'd tried to preserve be overwritten. The first, however, is annoying; the second is potentially costly! http://gwt-code-reviews.appspot.com/928801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Extract a default timeout constant in JUnitShell. (issue917801)
LGTM http://gwt-code-reviews.appspot.com/917801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix IncompatibleRemoteServiceException in GWTTestCases under JUnit4. (issue916801)
LGTM http://gwt-code-reviews.appspot.com/916801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove -dumpSignatures in favor of a simpler command line invocation to get GWT's JRE support. (issue844801)
One checkstyle nit, but the larger question is whether we think anyone else is using dumpSignatures... I certainly hope not, and can't think of a good one (even when it was made!), but having released with it, I'm nervous about suddenly neutering it like this. http://gwt-code-reviews.appspot.com/844801/diff/1/6 File dev/core/src/com/google/gwt/dev/Precompile.java (right): http://gwt-code-reviews.appspot.com/844801/diff/1/6#newcode43 dev/core/src/com/google/gwt/dev/Precompile.java:43: import com.google.gwt.dev.shell.StandardRebindOracle; isn't this bad checkstyle alpha-sorting, 'C' 'S'? http://gwt-code-reviews.appspot.com/844801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix broken build. (issue805801)
LGTM. http://gwt-code-reviews.appspot.com/805801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Not compile-building expenses sample, just collecting its files (issue794802)
Reviewers: cramsdale, Description: Not compile-building expenses sample, just collecting its files into the distro. Review by: cramsd...@google.com Please review this at http://gwt-code-reviews.appspot.com/794802/show Affected files: M samples/build.xml M samples/common.ant.xml A samples/expenses/user-build.xml -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Not compile-building expenses sample, just collecting its files (issue794802)
http://gwt-code-reviews.appspot.com/794802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Not compile-building expenses sample, just collecting its files (issue794802)
http://gwt-code-reviews.appspot.com/794802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Allow merging of modules with the same prefix, for example if you have a (issue754802)
Reviewers: unnurg, Description: Allow merging of modules with the same prefix, for example if you have a directory of widgets that are separated into multiple modules (but in the same java package), or if you have src/test roots that each have modules for the same package. The existing exclude attribute is used to mark something that no modue should ever touch, i.e. known non-translatable source. A new skip attribute is used to mark something that *this* module doesn't want, but that *other* modules might need. (You could probably be clever with just include for this case, but sometimes it's nice to be able to express a negative.) Review by: unn...@google.com Please review this at http://gwt-code-reviews.appspot.com/754802/show Affected files: M dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java M dev/core/src/com/google/gwt/dev/cfg/ModuleDefSchema.java M dev/core/src/com/google/gwt/dev/resource/impl/DefaultFilters.java M dev/core/src/com/google/gwt/dev/resource/impl/PathPrefix.java M dev/core/src/com/google/gwt/dev/resource/impl/PathPrefixSet.java A dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/One.gwt.xml A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/Three.gwt.xml A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/Two.gwt.xml A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/client/InOne.java A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/client/InTwo.java A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/client/Shared.java A dev/core/test/com/google/gwt/dev/cfg/testdata/merging/client/Toxic.java M dev/core/test/com/google/gwt/dev/resource/impl/DefaultFiltersTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Refactoring to one top level class per .java file, since some (issue686801)
So, is that a LGTM as-is, a request to move the now-separate classes into static nested ones, or a hold for more discussion? On 2010/07/12 21:19:29, scottb wrote: Actually, I may have jumped the gun on that. Making it static nested will have an impact on command line callers who are specifying a strategy on the command line, I think. (Separate discussion from whether we should flog the responsible parties for polluting the junit package directly.) :) On Mon, Jul 12, 2010 at 5:18 PM, Scott Blum mailto:sco...@google.com wrote: +1. Static nested FTW. :) On Mon, Jul 12, 2010 at 5:14 PM, mailto:j...@google.com wrote: LGTM, though do all the *Strategy ones need to be top-level classes rather than static nested classes? http://gwt-code-reviews.appspot.com/686801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors http://gwt-code-reviews.appspot.com/686801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Refactoring to one top level class per .java file, since some (issue686801)
http://gwt-code-reviews.appspot.com/686801/diff/1/3 File user/src/com/google/gwt/i18n/tools/ArgHandlerValueChooser.java (right): http://gwt-code-reviews.appspot.com/686801/diff/1/3#newcode49 user/src/com/google/gwt/i18n/tools/ArgHandlerValueChooser.java:49: private Class? extends Localizable argValue = Constants.class; On 2010/07/12 21:14:14, jat wrote: Extra space. Done. http://gwt-code-reviews.appspot.com/686801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Refactoring to one top level class per .java file, since some (issue686801)
Reviewers: jat, Description: Refactoring to one top level class per .java file, since some tools don't understand more than that. Review by: jat Please review this at http://gwt-code-reviews.appspot.com/686801/show Affected files: M dev/core/super/com/google/gwt/lang/LongLibBase.java A user/src/com/google/gwt/i18n/tools/ArgHandlerValueChooser.java M user/src/com/google/gwt/i18n/tools/I18NSync.java M user/src/com/google/gwt/junit/BatchingStrategy.java A user/src/com/google/gwt/junit/ClassBatchingStrategy.java M user/src/com/google/gwt/junit/CompileStrategy.java A user/src/com/google/gwt/junit/ModuleBatchingStrategy.java A user/src/com/google/gwt/junit/NoBatchingStrategy.java A user/src/com/google/gwt/junit/ParallelCompileStrategy.java A user/src/com/google/gwt/junit/PreCompileStrategy.java A user/src/com/google/gwt/junit/SimpleCompileStrategy.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)
I have mixed feelings... if bikeshed is null now, do we think we will/won't need such a workspace in the future? I thought something like that was being considered as the incubator replacement model, for example. I suppose we can turn it back on at will, but it seems to me that having build.xml always build the perhaps-empty bikeshed. I'm less sure about distro-source, though... it's explicitly for in-progress work, so I'm not sure it's part of a release even if not empty! On 2010/06/25 14:23:11, amitmanjhi wrote: Yes, it is no longer needed. We can drop it from the maven report for the next milestone. On Jun 25, 2010 6:37 AM, mailto:jasonpar...@google.com wrote: Does this mean gwt-bikeshed.jar is no longer needed? Do we need it at all in the Maven repo for the next milestone? http://gwt-code-reviews.appspot.com/665801/show http://gwt-code-reviews.appspot.com/665801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Wholesale move of the app, requestfactory, and valuestore packages from the bikeshed dir to user... (issue660802)
Sorry to get to this a bit late... I've got a couple of concerns: 1. regarding especially the user.compile use of javax validation, does that suggest we should embed that library in gwt-dev.jar with all of our other runtime dependencies? (I don't love that practice, but it has been our practice...) 2. bikeshed/scripts/maven_script.sh seems kind of orphaned if the rest goes away. There's nothing really bikeshed-y about it, is there? Should it move up to a top-level scripts directory? On 2010/06/24 20:29:26, amitmanjhi wrote: http://gwt-code-reviews.appspot.com/660802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Removed the dependency of SampleDataPopulator on gwt-dev by inlining the dependency. (issue652802)
LGTM, for a slightly loose definition of good. ;) Thanks. http://gwt-code-reviews.appspot.com/652802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Correcting the output names for SoycTest.java (issue442801)
Reviewers: Lex, Description: Correcting the output names for SoycTest.java Review by: sp...@google.com Please review this at http://gwt-code-reviews.appspot.com/442801/show Affected files: M dev/core/test/com/google/gwt/dev/SoycTest.java Index: dev/core/test/com/google/gwt/dev/SoycTest.java === --- dev/core/test/com/google/gwt/dev/SoycTest.java (revision 8015) +++ dev/core/test/com/google/gwt/dev/SoycTest.java (working copy) @@ -43,12 +43,17 @@ new Compiler(options).run(logger); // make sure the files have been produced -assertTrue(new File(options.getExtraDir() + /hello/soycReport/compile-report/index.html).exists()); -assertTrue(new File(options.getExtraDir() + /hello/soycReport/compile-report/SoycDashboard-1-index.html).exists()); -assertTrue(new File(options.getExtraDir() + /hello/soycReport/compile-report/total-1-overallBreakdown.html).exists()); -assertTrue(new File(options.getExtraDir() + /hello/soycReport/compile-report/soyc.css).exists()); +assertTrue(missing compile-report/index.html, + new File(options.getExtraDir() + /hello/soycReport/compile-report/index.html).exists()); +assertTrue(missing compile-report/SoycDashboard-0-index.html, + new File(options.getExtraDir() + /hello/soycReport/compile-report/SoycDashboard-0-index.html).exists()); +assertTrue(missing compile-report/total-0-overallBreakdown.html, + new File(options.getExtraDir() + /hello/soycReport/compile-report/total-0-overallBreakdown.html).exists()); +assertTrue(missing compile-report/soyc.css, + new File(options.getExtraDir() + /hello/soycReport/compile-report/soyc.css).exists()); -assertFalse(new File(options.getExtraDir() + /hello/soycReport/compile-report/index2.html).exists()); +assertFalse(improperly has compile-report/index2.html, +new File(options.getExtraDir() + /hello/soycReport/compile-report/index2.html).exists()); Util.recursiveDelete(options.getExtraDir(), false); } } -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Test patch. (issue420801)
Reviewers: fabbott, Description: Test patch. Please review this at http://gwt-code-reviews.appspot.com/420801/show Affected files: M /bikeshed/README Index: /bikeshed/README === --- /bikeshed/README(revision 7979) +++ /bikeshed/README(working copy) @@ -5,3 +5,4 @@ If you're wondering why it's called the bikeshed: http://en.wikipedia.org/wiki/Parkinson's_Law_of_Triviality +Testing the mirroring tools; this comment should not submit. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Added method to change the size of a MutableArray. Added factory method to construct a nonempty ... (issue319801)
LGTM, too http://gwt-code-reviews.appspot.com/319801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using remove me as the subject.
[gwt-contrib] Re: Lightweight collections. ImmutableArray and MutableArray.freeze() implementation. (issue291801)
The biggie is MutableArray.java:60 http://gwt-code-reviews.appspot.com/291801/diff/4001/5003 File bikeshed/src/com/google/gwt/collections/ImmutableArrayEmptyImpl.java (right): http://gwt-code-reviews.appspot.com/291801/diff/4001/5003#newcode27 bikeshed/src/com/google/gwt/collections/ImmutableArrayEmptyImpl.java:27: Assertions.assertIndexInRange(index, 0, 0); this will always fail... is it worth some more specific assertion? Or at least a syntax sugar that reduces to the same, but is clear at this call site that 0 is actually excluded? http://gwt-code-reviews.appspot.com/291801/diff/4001/5004 File bikeshed/src/com/google/gwt/collections/ImmutableArrayImpl.java (right): http://gwt-code-reviews.appspot.com/291801/diff/4001/5004#newcode25 bikeshed/src/com/google/gwt/collections/ImmutableArrayImpl.java:25: private final E[] elems; you might consider making this default visibility, to test that we don't copy, since that's the point to some of the hoops we're doing (see comment in ImmutableArrayTest). http://gwt-code-reviews.appspot.com/291801/diff/4001/5004#newcode28 bikeshed/src/com/google/gwt/collections/ImmutableArrayImpl.java:28: Assertions.assertNotNull(elems); do you also want to assert that elem.length 0? http://gwt-code-reviews.appspot.com/291801/diff/4001/5005 File bikeshed/src/com/google/gwt/collections/MutableArray.java (right): http://gwt-code-reviews.appspot.com/291801/diff/4001/5005#newcode60 bikeshed/src/com/google/gwt/collections/MutableArray.java:60: elems = null; Huh?! Can't I keep using my existing MutableArray, post-freeze, so long as I don't change it?! Doesn't this line zero it out (but leave it frozen)? http://gwt-code-reviews.appspot.com/291801/diff/4001/5006 File bikeshed/test/com/google/gwt/collections/ImmutableArrayTest.java (right): http://gwt-code-reviews.appspot.com/291801/diff/4001/5006#newcode58 bikeshed/test/com/google/gwt/collections/ImmutableArrayTest.java:58: I think that if you test ma.size() here, you get an unexpected 0 rather than 2... http://gwt-code-reviews.appspot.com/291801/diff/4001/5006#newcode60 bikeshed/test/com/google/gwt/collections/ImmutableArrayTest.java:60: ma.add(1); I like 0, 1, many testing in general, but I'm not sure the 1 case helps here. http://gwt-code-reviews.appspot.com/291801/diff/4001/5006#newcode158 bikeshed/test/com/google/gwt/collections/ImmutableArrayTest.java:158: Something I'd like to test, but we don't have visibility to here, is that we didn't copy... i.e. that ia1.elems === ia2.elems === ma.elems. http://gwt-code-reviews.appspot.com/291801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)
LGTM, through the checkstyle patch #5. Given we're in bikeshed anyway, I'd submit and do a new issue/review for your next increment http://gwt-code-reviews.appspot.com/232801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)
LGTM http://gwt-code-reviews.appspot.com/232801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)
Looks almost good to me; I think you're non-invariant (but maybe correct) if you remove the last element, so that needs a test, and you may have been better off with getModuleName() returning null, depending on what you're trying to test in this increment. Can you edit the issue, to cc br...@google.com vice unqualified bruce? http://gwt-code-reviews.appspot.com/232801/diff/25001/26007 File bikeshed/src/com/google/gwt/collections/MutableArray.java (right): http://gwt-code-reviews.appspot.com/232801/diff/25001/26007#newcode112 bikeshed/src/com/google/gwt/collections/MutableArray.java:112: elems = newElems; Does this set you up badly after the length 1 to length 0 transition, i.e. removing the last element? It seems _likely_ to work as-is, but is worth (a) a test case, and (b) maybe restoring elem==null as an invariant. http://gwt-code-reviews.appspot.com/232801/diff/25001/26008 File bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java (right): http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode35 bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:35: assertionsEnabled = true; this is okay, but I didn't have a problem with desiredAssertionStatus (which I think has the general advantage of being compile-time constant). http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode41 bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:41: return com.google.gwt.collections.Collections; These mean different things; which do you want? return null tests as a vanilla JUnit test, which I thought met your Java only incremental statement. Return string tests in GWT, i.e. as client code, either java (devmode) or JS. You're probably fine with this change, but they're not synonymous. I'd expected, from the first, that you'd derive a JsObjectArrayTest and override to return string, so you'd have two tests one as server Java and one as client Java/JS. Which may be overkill, server java and client java being not so much different, but the server-java test should run faster without our startup overhead. http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode115 bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:115: add a add/remove/add test for the 1-0 transition and recovery. http://gwt-code-reviews.appspot.com/232801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)
Comments as noted; also, what semantics do we want for equality?
(Identity is certainly cheapest, and probably good, just wanted to make
an explicit choice there).
I think you're buggy if your first element is null, and I'm not entirely
sure the singleElem special case is worth the time-cost of your decision
branches.
I think you will be buggy, once you add immutability, if you don't check
isfrozen.
http://gwt-code-reviews.appspot.com/232801/diff/3001/4002
File bikeshed/src/com/google/gwt/collections/Assertions.java (right):
http://gwt-code-reviews.appspot.com/232801/diff/3001/4002#newcode37
bikeshed/src/com/google/gwt/collections/Assertions.java:37: static
String msgShouldNotBeNull() {
I'm not entirely sure these are useful (vice just using the constants
in-line in the sole call sites), but okay...
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007
File bikeshed/src/com/google/gwt/collections/MutableArray.java (right):
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode23
bikeshed/src/com/google/gwt/collections/MutableArray.java:23: // Used
when there is exactly one element in progress
in progress doesn't mean much for me here.
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode38
bikeshed/src/com/google/gwt/collections/MutableArray.java:38:
@SuppressWarnings(unchecked)
what's the unchecked warning from?
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode56
bikeshed/src/com/google/gwt/collections/MutableArray.java:56: } else if
(singleElem != null) {
two questions: first, is the whole singleElem special case actually
gaining anything here? second, am I allowed to have an array of size 1
containing only null? (And if not, why not, and how do I know?)
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode75
bikeshed/src/com/google/gwt/collections/MutableArray.java:75:
I think you need a check here (and remove and set) against isfrozen?
(Or, if immutability is a copy op, you don't need isfrozen, one or the
other...)
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode79
bikeshed/src/com/google/gwt/collections/MutableArray.java:79: //
TODO(bruce): grow smartly
not likely to be bruce. (Not sure we attribute our todo's generally,
anyway.)
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode92
bikeshed/src/com/google/gwt/collections/MutableArray.java:92: singleElem
= elem;
you lose if elem==null
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode110
bikeshed/src/com/google/gwt/collections/MutableArray.java:110: // Bad!
But only happens if index is bad and assertions are off
I know bruce was imagining minimal safety overhead, but I'd like to see
an assertion here, just in case we're someday wrong/buggy about the
precondition assertions. I can live with assertions-only correctness
checking, but get queasy about deliberate holes.
http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode131
bikeshed/src/com/google/gwt/collections/MutableArray.java:131: // Bad!
But only happens if index is bad and assertions are off
as above, wrt assertions for checking.
http://gwt-code-reviews.appspot.com/232801/diff/3001/4008
File bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java
(right):
http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode29
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:29: throw
new IllegalStateException(This test case requires assertions to be
enabled);
um. I dunno that this should be true, or if true I think we might want
it to pass (but be a no-op) rather than fail when run without 'em.
http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode108
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:108:
b.add(null);
yeah, try doing this (add of null) as your first item. ;-)
http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode126
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:126:
b.set(3, eclair);
check result?
http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode177
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:177:
fail(That should have been okay);
can you check the assertion message, here and elsewhere, to be sure it's
the one we want? I remember seeing some issues with junit 4 where
assertions could cause problems (e.g. the junit assertions and your SUT
assertions co-mingled).
Also, again, I'd rather you checked assertion state and decided what to
expect (or short-circuited the test) then, so the test will work in
either mode.
http://gwt-code-reviews.appspot.com/232801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
To unsubscribe from this group, send email to
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with
the words REMOVE ME as the subject.
[gwt-contrib] Re: Remove global IncubatorSuite and modify incubator build.xml to include all Suites
LGTM. In build.xml, I don't think you need to add -ea explicitly to devmode tests. Not wrong, but they're already on. A bigger problem, but mine not yours, is that rietveld is (wrongly) basing this on http://google-web-toolkit, not on incubator, so the side-by-sides are all wrong. (I assume you used my wrapper around upload.py; I'll have to make it either auto-detect, or at least let you specify, which it is!) http://gwt-code-reviews.appspot.com/159805 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Added the ability to customize the timeout to begin the tests.
LGTM http://gwt-code-reviews.appspot.com/153819 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] AsyncProxy fix for top-level classes
Reviewers: Lex, Description: turns out that the insistence on concreteType.isStatic() prevents top-level concrete types, because it's false for top-level types (where explicit static is forbidden). Scott and Lex are nervous about making isStatic() true for top-level types, but we can just test isMemberType() also. Please review this at http://gwt-code-reviews.appspot.com/149801 Affected files: user/src/com/google/gwt/user/rebind/AsyncProxyGenerator.java user/test/com/google/gwt/user/client/AsyncProxyTest.java user/test/com/google/gwt/user/client/AsyncProxyTestTopLevelImpl.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Ant tests should fail with status 2
LGTM. http://gwt-code-reviews.appspot.com/128803 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Don't use to mean emma's not in play
Reviewers: jlabanca,
Description:
This fixes the ServletMappingSuite buildbreak.
The issue appears to be that a pathelement location=/ is taken, much
like a :: substring in -classpath, to mean this directory rather than
nothing: it puts gwt-root/user onto the classpath. And, when the
test.ServletMappingSuite module is in play, test/... is in the
source path, so gwt-root/user/test/... comes into play and wreaks
havoc.
Please review this at http://gwt-code-reviews.appspot.com/100807
Affected files:
common.ant.xml
Index: common.ant.xml
===
--- common.ant.xml (revision 6874)
+++ common.ant.xml (working copy)
@@ -179,7 +179,8 @@
antcall target=-create.emma.coverage.if.enabled
param name=test.emma.coverage value=@{test.emma.coverage}/
/antcall
- condition property=emma.lib
value=${emma.dir}/emma-2.0.5312-patched.jar else=
+ condition property=emma.lib
value=${emma.dir}/emma-2.0.5312-patched.jar
+else=${emma.dir}/no-emma-requested
isset property=emma.enabled /
/condition
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Increasing timeout of HttpSuite tests
LGTM. Sorry for the delay on it. http://gwt-code-reviews.appspot.com/93804 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add names to test targets so we know which ones failed
LGTM, with one whitespace nit. Pity ant doesn't have a built-in $ant.target property, though. http://gwt-code-reviews.appspot.com/91810/diff/1/3 File user/build.xml (right): http://gwt-code-reviews.appspot.com/91810/diff/1/3#newcode380 Line 380: test.cases=test.dev.htmlunit.tests I don't think you want the 3rd line to indent deeper, do you? Means the 4th and later lines run out of space. ;-) http://gwt-code-reviews.appspot.com/91810 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Getting more info from test failures
lgtm http://gwt-code-reviews.appspot.com/93805 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] scripts should be executable out of the box
Reviewers: scottb,
Description:
Two issues here, both arguably ant bugs. Although the scripts have
correct perms, and zip creates a non-executable copy unless you
explicitly set fileperm in the zipfileset. That gets you a correct
build/dist/gwt-X.X.X.zip, and unzip will do the right thing with it...
but ant unzip again gives you non-executable files (?!, ant 1.7.1), so
I added a chmod to make that right in build/staging also.
N.B. this is issue 4718.i
Please review this at http://gwt-code-reviews.appspot.com/89805
Affected files:
distro-source/build.xml
Index: distro-source/build.xml
===
--- distro-source/build.xml (revision 6512)
+++ distro-source/build.xml (working copy)
@@ -31,7 +31,7 @@
!-- raw files --
zipfileset dir=${dist.resources} prefix=${project.distname} /
- zipfileset dir=src prefix=${project.distname} /
+ zipfileset filemode=755 dir=src prefix=${project.distname} /
!-- doc --
zipfileset dir=${gwt.build.out} prefix=${project.distname}
@@ -58,5 +58,12 @@
--
mkdir dir=${gwt.build.staging} /
unzip src=${project.dist} dest=${gwt.build.staging} /
+!-- cute. zipinfo says the zip has good perms, unzip gets it right,
but
+ ant unzip leaves the perms as non-executable... go figure. --
+chmod perm=uga+rx
+ fileset dir=${gwt.build.staging}/gwt-${gwt.version}
+patternset refid=chmod.executables/
+ /fileset
+/chmod
/target
/project
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] scripts should be executable out of the box
Reviewers: ,
Description:
Two issues here, both arguably ant bugs. Although the scripts have
correct perms, and zip creates a non-executable copy unless you
explicitly set fileperm in the zipfileset. That gets you a correct
build/dist/gwt-X.X.X.zip, and unzip will do the right thing with it...
but ant unzip again gives you non-executable files (?!, ant 1.7.1), so
I added a chmod to make that right in build/staging also.
N.B. this is issue 4718.
Please review this at http://gwt-code-reviews.appspot.com/89806
Affected files:
distro-source/build.xml
Index: distro-source/build.xml
===
--- distro-source/build.xml (revision 6512)
+++ distro-source/build.xml (working copy)
@@ -31,7 +31,7 @@
!-- raw files --
zipfileset dir=${dist.resources} prefix=${project.distname} /
- zipfileset dir=src prefix=${project.distname} /
+ zipfileset filemode=755 dir=src prefix=${project.distname} /
!-- doc --
zipfileset dir=${gwt.build.out} prefix=${project.distname}
@@ -58,5 +58,12 @@
--
mkdir dir=${gwt.build.staging} /
unzip src=${project.dist} dest=${gwt.build.staging} /
+!-- cute. zipinfo says the zip has good perms, unzip gets it right,
but
+ ant unzip leaves the perms as non-executable... go figure. --
+chmod perm=uga+rx
+ fileset dir=${gwt.build.staging}/gwt-${gwt.version}
+patternset refid=chmod.executables/
+ /fileset
+/chmod
/target
/project
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] big banner warning about htmlunit suppression
Reviewers: scottb, Description: Now that we've admitted htmlunit isn't quite ready, but all other tests require odd property assignments, people should be warned that they maybe didn't run the testing they thought they did. I thought about getting fancy and testing the -D's and making a more specific message, but regardless we should warn that htmlunit *didn't* run, so the message works out to very minor variations anyway (between no user tests ran and only a subset user tests ran) Please review this at http://gwt-code-reviews.appspot.com/86809 Affected files: build.xml user/build.xml Index: build.xml === --- build.xml (revision 6483) +++ build.xml (working copy) @@ -112,6 +112,11 @@ call-subproject subproject=user subtarget=test / call-subproject subproject=servlet subtarget=test / call-subproject subproject=tools subtarget=test / +!--TODO: remove this echo (and from user/build.xml) once it's untrue -- +echo message=WARNING: htmlunit has been suppressed as unstable, and DID NOT RUN./ +echo message= Remote user tests ran per -Dgwt.hosts.*.remote or/ +echo message= -Dgwt.remote.browsers, and/or -Dgwt.hosts.*.selenium/ +echo message= or -Dgwt.selenium.hosts, but that's all./ /target path id=emma.classpath.src Index: user/build.xml === --- user/build.xml (revision 6483) +++ user/build.xml (working copy) @@ -464,6 +464,8 @@ !-- antcall target=test.web.htmlunit/ -- !-- antcall target=test.draft.htmlunit/ -- !-- antcall target=test.nometa.htmlunit/ -- + !-- TODO(jlabanca): remove this echo (and from toplevel) once it's untrue -- + echo message=WARNING: htmlunit has been suppressed as unstable, and DID NOT RUN./ /parallel /limit /target --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] incubator shouldn't need gwt-dev-${platform}.jar anymore
Reviewers: jlabanca,
Description:
let it compile!
Please review this at http://gwt-code-reviews.appspot.com/87803
Affected files:
common.ant.xml
Index: common.ant.xml
===
--- common.ant.xml (revision 1733)
+++ common.ant.xml (working copy)
@@ -67,7 +67,7 @@
property name=gwt.tools value=${project.root}/../tools /
property name=gwt.tools.lib location=${gwt.tools}/lib /
property name=gwt.tools.antlib location=${gwt.tools}/antlib /
- property name=gwt.dev.jar
location=${gwt.home}/gwt-dev-${gwt.platform}.jar /
+ property name=gwt.dev.jar location=${gwt.home}/gwt-dev.jar /
property name=gwt.user.jar location=${gwt.home}/gwt-user.jar /
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Fixes a couple of checkstyle errors introduced by r6432.
http://gwt-code-reviews.appspot.com/83806/diff/1/2 File dev/build.xml (right): http://gwt-code-reviews.appspot.com/83806/diff/1/2#newcode212 Line 212: filename name=com/google/gwt/dev/shell/remoteui/RemoteMessageProto.java negate=yes / perhaps too late, but should we generalize this to e.g. **/*Proto.java? http://gwt-code-reviews.appspot.com/83806 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] break up our too-large RPCSuite
Reviewers: Ray Ryan, Description: It's pretty ungainly, and hurts both pick-and-choose test execution and any attempts to parallelize test suites. (The divisions here are pretty arbitrary, though; I just forked of DeRPC and the couple biggest time contributors into their own suites.) Please review this at http://gwt-code-reviews.appspot.com/77814 Affected files: user/test/com/google/gwt/user/DeRPCSuite.java user/test/com/google/gwt/user/RPCSuite.java user/test/com/google/gwt/user/RPCUnicodeSuite.java user/test/com/google/gwt/user/RPCValueSuite.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] remote tests should depend on compile.tests
Reviewers: Ray Ryan,
Please review this at http://gwt-code-reviews.appspot.com/78807
Affected files:
build.xml
Index: build.xml
===
--- build.xml (revision 6346)
+++ build.xml (working copy)
@@ -108,11 +108,8 @@
/gwt.checkstyle
/target
- target name=remoteweb-test depends=test.remoteweb
-echo message=DEPRECATED: remoteweb-test has been renamed
test.remoteweb/
- /target
-
- target name=test.remoteweb description=Run a remoteweb test at the
given host and path if=gwt.remote.browsers
+ target name=test.remoteweb depends=compile, compile.tests
+ description=Run a remoteweb test at the given host and path
if=gwt.remote.browsers
echo message=Performing remote browser testing at
${gwt.remote.browsers} /
property name=test.remoteweb.args value=${test.args} /
fileset id=test.remoteweb.tests dir=${javac.junit.out}
includes=${gwt.junit.testcase.web.includes}
excludes=${gwt.junit.testcase.web.excludes} /
@@ -123,11 +120,8 @@
/gwt.junit
/target
- target name=selenium-test depends=test.selenium
-echo message=DEPRECATED: selenium-test has been renamed
test.selenium/
- /target
-
- target name=test.selenium description=Run a remote test using
Selenium RC test at the given host and path if=gwt.selenium.hosts
+ target name=test.selenium depends=compile, compile.tests
+description=Run a remote test using Selenium RC test at the given
host and path if=gwt.selenium.hosts
echo message=Performing remote browser testing using Selenium RC at
${gwt.selenium.hosts} /
property name=test.selenium.args value=${test.args} /
fileset id=test.selenium.tests dir=${javac.junit.out}
includes=${gwt.junit.testcase.web.includes}
excludes=${gwt.junit.testcase.web.excludes} /
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Restore UiBinder's JRE tests
LGTM. I'm a bit hesitant about die == UnableToCompleteException in TreeLogger in general, but have no issues with it here. (As an example, I'm not sure that's the right exception to throw for an issue early in starting hosted mode shell, although it's maybe not clearly the wrong exception either.) http://gwt-code-reviews.appspot.com/72805 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: JUnitShell does not handle test runners that shard test methods
Does there exist a CompileStrategyTest? (hint.)
http://gwt-code-reviews.appspot.com/74803/diff/1/3
File user/src/com/google/gwt/junit/BatchingStrategy.java (right):
http://gwt-code-reviews.appspot.com/74803/diff/1/3#newcode36
Line 36: private boolean isSingleTestOnly;
is this pattern better than having BatchingStrategy specify an abstract
boolean isSingleTestOnly(), and having constant-returning
implementations from the concrete classes? (Very minor storage-space
benefits to the second, as well as the ability to have deeper
hierarchies with varying values...)
http://gwt-code-reviews.appspot.com/74803/diff/1/4
File user/src/com/google/gwt/junit/CompileStrategy.java (right):
http://gwt-code-reviews.appspot.com/74803/diff/1/4#newcode50
Line 50: * Strategies that do not add test blocks during compilation
should override
do vice do not? (This impl is no-op, so why would I need to
override it if I don't add test blocks...)
http://gwt-code-reviews.appspot.com/74803/diff/1/4#newcode100
Line 100: boolean addTestBlocks) throws UnableToCompleteException {
would it be cleaner (more symmetric) to pass the whole BatchingStrategy,
not just the boolean, as you do in maybeAddTestBlockForCurrentTest(...)?
Although, I see your other strategies don't reference BatchingStrategy;
does it make sense to use different combinations with them? (If I'm
sharding, I can be that probably I'll get some methods from each module
in each shard, so maybe I'd want to precompile everything, for example,
but not batch due to sharding...)
http://gwt-code-reviews.appspot.com/74803
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] make htmlunit failures non-fatal
Reviewers: amitmanjhi, jat,
Description:
I see both sides of the fail/permit argument here, but we're seeing a
lot of flake to be penalizing everyone for it. Can we agree to run the
tests, but make them non-blocking?
The admitted danger is that we never fix the problem, and so never run
the tests in anger... but at the moment we have a problem we're so far
failing to fix, and a dashboard that looks much too angry.
Please review this at http://gwt-code-reviews.appspot.com/65801
Affected files:
user/build.xml
Index: user/build.xml
===
--- user/build.xml (revision 6077)
+++ user/build.xml (working copy)
@@ -175,11 +175,47 @@
target name=test.web.htmlunit depends=compile, compile.tests
description=Run htmlunit web-mode tests for this project.
!-- TODO: add more browsers later --
fileset id=test.web.htmlunit.tests dir=${javac.junit.out}
includes=${gwt.junit.testcase.web.includes}
excludes=${gwt.junit.testcase.web.excludes} /
-gwt.junit test.args=${test.args} -htmlunit FF3
test.out=${junit.out}/${build.host.platform}-htmlunit-web-mode
test.cases=test.web.htmlunit.tests
- extraclasspaths
+
+taskdef name=junit
classname=org.apache.tools.ant.taskdefs.optional.junit.JUnitTask
+ classpath
+pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement location=${gwt.tools.antlib}/ant-junit-1.6.5.jar /
+pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
+ /classpath
+/taskdef
+
+echo message=Writing test results to
${junit.out}/${build.host.platform}-htmlunit-web-mode/reports for
test.web.htmlunit.tests /
+mkdir
dir=${junit.out}/${build.host.platform}-htmlunit-web-mode/reports /
+
+echo message=${javac.out} ${javac.junit.out} /
+junit dir=${junit.out}/${build.host.platform}-htmlunit-web-mode
fork=yes printsummary=yes
+ failureproperty=htmlunit.failure
tempdir=${junit.out}/${build.host.platform}-htmlunit-web-mode
+ jvmarg line=${junit.platform.args} /
+ jvmarg line=-Xmx768m /
+ sysproperty key=gwt.args value=${junit.notheadless.arg}
${test.args} -htmlunit FF3 /
+ sysproperty key=java.awt.headless value=${junit.headless} /
+ sysproperty key=gwt.devjar value=${gwt.dev.staging.jar} /
+ classpath
+path refid=project.classpath.src /
+pathelement location=${gwt.root}/${project.tail}/super /
+pathelement location=${gwt.root}/${project.tail}/test /
+pathelement location=${javac.junit.out} /
+path refid=project.classpath.class /
+pathelement location=${gwt.dev.staging.jar} /
+pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
+pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
+pathelement location=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
+pathelement location=${gwt.tools.lib}/w3c/flute/flute-1.3.jar /
path refid=test.extraclasspath /
- /extraclasspaths
-/gwt.junit
+ /classpath
+
+ formatter type=plain /
+ formatter type=xml /
+
+ batchtest
todir=${junit.out}/${build.host.platform}-htmlunit-web-mode/reports
+fileset refid=test.web.htmlunit.tests /
+ /batchtest
+/junit
/target
target name=test.web.disableClassMetadata depends=compile,
compile.tests description=Run only web-mode tests for this project.
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Use a custom method to create temp files in LatestTimeJar
One race condition. In practice, it'd come up only if you had two or more builds at once (which I at least do fairly often), AND if they chose similar seed random numbers (which should be very unlikely)... but it's easy to close. http://gwt-code-reviews.appspot.com/61816/diff/1/2 File build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/LatestTimeJar.java (right): http://gwt-code-reviews.appspot.com/61816/diff/1/2#newcode163 Line 163: tmpFile.createNewFile(); you need to close the race condition in which somebody else makes the file between line 159 and here... test the return result of createNewFile(), and loop back up if it failed. Since any race is probably going to be with something else using this same algorithm, you might even re-seed if it fails (that is, pick a new random number). The actual File.createTempFile() promises never to reuse the same random file (that is, even if you delete it, you own the name)... but that assurance doesn't help against competing processes, either, so I think it's okay not to have the same guarantee here. http://gwt-code-reviews.appspot.com/61816 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: move SOYC source code underneath dev/core/src
I see removal of code from the old location here; shouldn't I also see
addition at the new? ('Cause I don't.)
http://gwt-code-reviews.appspot.com/56811/diff/1/10
File dev/common.ant.xml (right):
http://gwt-code-reviews.appspot.com/56811/diff/1/10#newcode33
Line 33: fileset file=${gwt.core.build}/alldeps.jar /
You probably want a fileset from ${gwt.tools.soyc} here, so that e.g. an
edit to the classLevel.css triggers a repackage.
http://gwt-code-reviews.appspot.com/56811/diff/1/10#newcode55
Line 55: fileset file=${gwt.tools.soyc}/roundedCorners.css/
These three want not to have any package location?
http://gwt-code-reviews.appspot.com/56811/diff/1/9
File tools/soyc-vis/build.xml (right):
http://gwt-code-reviews.appspot.com/56811/diff/1/9#newcode17
Line 17: mkdir dir=${javac.out}/
Given the mkdir that follows, this one is superfluous.
http://gwt-code-reviews.appspot.com/56811/diff/1/9#newcode30
Line 30: holding all the SOYC bits. --
The intent is that this jar should have only the images, right?
(a) if so, should it have a Main-Class attribute at all?
(b) it may be simpler/faster to just use jar rather than gwt.jar
here... the difference if small, but I'd rather be vanilla if there's no
reason we need jar.bydate.
http://gwt-code-reviews.appspot.com/56811
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: New lib jars to allow easyMock in tests
LGTM. On 2009/07/16 23:52:23, Ray Ryan wrote: http://gwt-code-reviews.appspot.com/47820 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: test fixes for issue 47802, assertions in all (esp. web mode) tests
On 2009/07/15 17:16:37, jat wrote: LGTM. I only reviewed the two requested files -- let me know if I should review anything else. http://gwt-code-reviews.appspot.com/47817/diff/1/9 File user/test/com/google/gwt/dev/jjs/test/CoverageTest.java (right): http://gwt-code-reviews.appspot.com/47817/diff/1/9#newcode199 Line 199: assertTrue(0.01 Math.abs(1.2f - val)); I find ordering it this way to be less clear -- I would prefer Math.abs() EPSILON (and defining EPSILON appropriately) as it seems to clearly indicate what is being done. Done. http://gwt-code-reviews.appspot.com/47817 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] noserver should be, well, -noserver
Reviewers: Lex, jat,
Description:
Minor (but significant!) typo from r5094... Passes on my XP box, at
least.
Please review this at http://gwt-code-reviews.appspot.com/47821
Affected files:
user/build.xml
Index: user/build.xml
===
--- user/build.xml (revision 5751)
+++ user/build.xml (working copy)
@@ -153,7 +153,7 @@
/target
target name=test.noserver depends=compile, compile.tests
description=Run noserver hosted-mode tests for this project.
-gwt.junit test.args=${test.args}
test.out=${junit.out}/${build.host.platform}-noserver-mode
test.cases=default.noserver.tests
+gwt.junit test.args=${test.args} -noserver
test.out=${junit.out}/${build.host.platform}-noserver-mode
test.cases=default.noserver.tests
extraclasspaths
path refid=test.extraclasspath /
/extraclasspaths
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] test fixes for issue 47802, assertions in all (esp. web mode) tests
Reviewers: Ray Ryan, jat, Description: CoverageTest.java and HandlerManagerTest.java need new review. The other files are welcome to be re-reviewed, but are by flin and reviewed by me already. The change in CoverageTest is to do a more correct epsilon-compare of floating point; apparently IE's JS engine is looser than Linux's for web mode tests. I have a harder time explaining how the HandlerManager one apparently passed linux web mode, while failing IE. Please review this at http://gwt-code-reviews.appspot.com/47817 Affected files: dev/core/src/com/google/gwt/dev/GWTShell.java dev/core/src/com/google/gwt/dev/HostedModeBase.java dev/core/src/com/google/gwt/dev/Precompile.java dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerDisableAssertions.java dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerEnableAssertions.java dev/core/test/com/google/gwt/dev/GWTShellTest.java user/src/com/google/gwt/junit/JUnitShell.java user/test/com/google/gwt/dev/jjs/test/CoverageTest.java user/test/com/google/gwt/event/shared/HandlerManagerTest.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] test in dev jni are overly aggressive
Reviewers: Ray Ryan,
Description:
As long as it's ant day... this is a performance tweak. Right now, when
we test in the native-code directories (dev and jni), we nominally test
all three operating systems, which can result in triplicate builds of
alldeps.jar. Since we, in fact, can only test for *this* OS that we're
on, there's no reason to pretend we can -do the full fanout.
Please review this at http://gwt-code-reviews.appspot.com/47812
Affected files:
dev/build.xml
jni/build.xml
Index: dev/build.xml
===
--- dev/build.xml (revision 5695)
+++ dev/build.xml (working copy)
@@ -3,6 +3,13 @@
property name=project.tail value=dev /
import file=${gwt.root}/platforms.ant.xml /
+
+ target name=test description=Tests this project, but only on this
platform
+gwt.ant dir=core target=test/
+gwt.ant dir=oophm target=test/
+gwt.ant dir=${build.host.platform} target=test/
+ /target
+
target name=clean description=Cleans this project's intermediate and
output files
delete dir=${project.build} failonerror=false /
delete failonerror=false
Index: jni/build.xml
===
--- jni/build.xml (revision 5695)
+++ jni/build.xml (working copy)
@@ -2,4 +2,9 @@
property name=gwt.root location=.. /
property name=project.tail value=jni /
import file=${gwt.root}/platforms.ant.xml /
+
+ target name=test description=Tests this project, but only on this
platform
+gwt.ant dir=core target=test/
+gwt.ant dir=${build.host.platform} target=test/
+ /target
/project
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] apicheck should run on Windows
Reviewers: amitmanjhi, Description: the old form got confused by a refJar path of 'C:\GWT\tools\api-checker\reference\gwt-dev-modified.jar:C:\GWT\tools\api-checker\reference\gwt-user-modified.jar' because splitting on colons was wrong. Worse, ant thought that it had succeeded because an arg-parsing error was caught, help printed, and ultimately 0 exited. The new form splits on the path.separator system propery instead, uses ant's arg path=.../ to input either a : or ; separated value depending on OS, and catches arg parse failures as non-zero return. Please review this at http://gwt-code-reviews.appspot.com/51801 Affected files: build.xml tools/api-checker/src/com/google/gwt/tools/apichecker/ApiCompatibilityChecker.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Assertion enabled by default when running GWT web mode tests and add the disabled flag -da
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
-~--~~~~--~~--~--~---
[gwt-contrib] top-level ant test runs user tests in triplicate
Reviewers: bobv, Description: This is build-timeout surgery... I have some suggestions for cleanup later. The real problem is that in r5557 I caused target test to call-subproject into to top-level servlet for subtarget test... and servlet depends on user, which uses gwt.ant (sensitive to $target) to descend into user and... test it. Ditto for tools. I find the use of $target mildly confusing, but the big issue is that while we want action rules like test to descend with some other target (e.g. test), we want subdirectory rules to build their dependencies with build and then descend with whatever $target is... but those dependencies can't figure out whether they were called from an action like test or from another subdirectory like servlet or tools. Since ant's if constructs are all nasty, I opted to make the action/subdirectory distinction explicit, and to ban depends from subdirectories. Please review this at http://gwt-code-reviews.appspot.com/43801 Affected files: build.xml --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] reworking ant targets to allow single-platform use
Reviewers: scottb, Description: Shaves 10min off a dist-one build vs dist (aka old build). Please review this at http://gwt-code-reviews.appspot.com/37803 Affected files: build.xml --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: reworking ant targets to allow single-platform use
http://gwt-code-reviews.appspot.com/37803/diff/1/2
File build.xml (right):
http://gwt-code-reviews.appspot.com/37803/diff/1/2#newcode21
Line 21: target name=dist-one depends=buildonly, tools, samples,
doc description=Make this platform's distributions
On 2009/06/11 17:41:56, zundel wrote:
Now would be a good opportunity to add comments to document what these
different
targets are intended to do. I like to have a summary of the 'user
oriented'
targets at the top of the ant file. I think that 'one' refers to
build a
distribution for just one platform, but someone else might think it
means
building one permutation of the samples or something like that.
I was actually planning to resort them (separately, and after this
approval, rather than introducing gratuitous diffs) so that ant -p
would have the sorting you describe, yes. (Currently it's at least
mostly alphabetical, which is readily computable but not particularly
useful...)
And the description is intended to suggest one - this platform,
though I can obviously rework that wording. I'd rather have a good
description than rely on comments (which, after all, might not be read,
aren't displayed by ant -p, etc.)
http://gwt-code-reviews.appspot.com/37803/diff/1/2#newcode72
Line 72: antcall target=buildtools
On 2009/06/11 17:41:56, zundel wrote:
Maybe a macrodef would help shrink the amount of code after getting
rid of the
-do construct?
macrodef name=call-checkstyle
attribute name=target /
sequential
antcall targ...@{target}
param name=target value=checkstyle /
/antcall
/sequential
/macrodef
call-checkstyle target=buildtools /
call-checkstyle target=dev /
...
(I wish I could find something like a for loop)
There's a foreach in ant-contrib, but it invokes a target as its body,
which I've always found really annoying and counter-clarifying. But
yes, we can make a macro (and parameterize both the subproject, what
you're calling target there, and the target within it, so the macro can
be reused below).
http://gwt-code-reviews.appspot.com/37803
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] configurable -localWorkers for samples
Reviewers: scottb,
Description:
This change saves me about 4 minutes on a work-to-do build. The idea is
that, if the default isn't good for people, they can set a local
override in GWT_ROOT/local.ant.properties.
Please review this at http://gwt-code-reviews.appspot.com/36802
Affected files:
samples/common.ant.xml
Index: samples/common.ant.xml
===
--- samples/common.ant.xml (revision 5537)
+++ samples/common.ant.xml (working copy)
@@ -3,6 +3,14 @@
property name=project.tail value=samples/${sample.root} /
import file=${gwt.root}/common.ant.xml /
+ !--
+Number of localworkers for sample compilation. This depends on your
+hardware, so it's a good candidate to specify in local.ant.properties
+if this default isn't good for you. Ideally, it should approximate
+the number of CPU cores in your machine.
+ --
+ property name=gwt.samples.localworkers value=2 /
+
property name=sample.lower value=${sample.root} /
property name=sample.upper value=${sample.module} /
@@ -47,9 +55,11 @@
targetfiles
path=${sample.build}/war/${sample.lower}/${sample.lower}.nocache.js /
sequential
mkdir dir=${sample.build}/war /
-gwt.timer name=${sample.upper}
+gwt.timer name=${sample.upper} with ${gwt.samples.localworkers}
localWorkers
java dir=${sample.build}
classname=com.google.gwt.dev.Compiler
classpath=src:${sample.build}/war/WEB-INF/classes:${gwt.user.jar}:${gwt.dev.jar}
fork=yes failonerror=true
jvmarg value=-Xmx256M/
+arg value=-localWorkers /
+arg value=${gwt.samples.localworkers} /
arg value=-war /
arg file=${sample.build}/war /
arg
value=com.google.gwt.sample.${sample.lower}.${sample.upper} /
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: i18n support for concrete default locale
On 2009/06/05 20:09:49, fabbott wrote: Ping for comment. Bruce had asked about additional possible use cases for the property provider template thing. The abstract answer is any case where a permuted property depends on a non-permuting one... these are hypothetical, and off-the-cuff so not particularly strong, but I can imagine: * analogous negotiation for content-type, perhaps to permute over efficient image or resource formats in bundles as supported by browser, with a configured default fallback. * also analogous charset selection * hoisting default specification of e.g. logging level out of the provider, so users can adjust it with a set operation without asking users to rewrite the property provider (if they want to change the packaged default, ours or third-party). http://gwt-code-reviews.appspot.com/34832 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] IE8 failing ElementTest.testHasAttribute() (hosted)
Reviewers: jgw,
Description:
Joel, I'm seeing my hosted mode tests fail as noted without this
one-liner. It seems that node node.specified returns undefined,
triggering a HostedModeException from JsValueGlue.get(), without
something like this == test.
I haven't checked any speed impact of the ==, or whether it
intelligently inlines away.
Please review this at http://gwt-code-reviews.appspot.com/34833
Affected files:
user/src/com/google/gwt/dom/client/DOMImplIE6.java
Index: user/src/com/google/gwt/dom/client/DOMImplIE6.java
===
--- user/src/com/google/gwt/dom/client/DOMImplIE6.java (revision 5524)
+++ user/src/com/google/gwt/dom/client/DOMImplIE6.java (working copy)
@@ -45,7 +45,7 @@
public native boolean hasAttribute(Element elem, String name) /*-{
var node = elem.getAttributeNode(name);
-return node node.specified;
+return (node node.specified) == true;
}-*/;
/*
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] bump delayTestFinish timeout of ImageResourceTest.testDedup
Reviewers: bobv,
Description:
I'm getting almost-consistent local faults in hosted and web mode (XP,
ie8) with a half-second timeout. Moving up to a full second, which
seems to reliably work
Please review this at http://gwt-code-reviews.appspot.com/33846
Affected files:
user/test/com/google/gwt/resources/client/ImageResourceTest.java
Index: user/test/com/google/gwt/resources/client/ImageResourceTest.java
===
--- user/test/com/google/gwt/resources/client/ImageResourceTest.java
(revision 5524)
+++ user/test/com/google/gwt/resources/client/ImageResourceTest.java
(working copy)
@@ -103,7 +103,7 @@
});
RootPanel.get().add(i);
-delayTestFinish(500);
+delayTestFinish(1000);
}
public void testPacking() {
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] i18n support for concrete default locale
Reviewers: jat, scottb, Description: This allows binding the default locale to a user-specified locale, so that requests for which locale negotiation fails end up being an actual locale with actual currency and number formatting specification. To achieve this, we also allow property providers to do template substitution of configuration properties. I should note that the hosted mode support checks out for Linux legacy hosted FF3 oophm. It was actually *failing* for me on Windows legacy (which means my IE, IE8, right?), although I couldn't figure out why. Since it's working on Linux, I now believe that's an unrelated issue, 'cept for the time it cost me. Please review this at http://gwt-code-reviews.appspot.com/34832 Affected files: dev/core/src/com/google/gwt/core/ext/SelectionProperty.java dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java dev/core/src/com/google/gwt/dev/cfg/StaticPropertyOracle.java dev/core/src/com/google/gwt/dev/shell/ModuleSpacePropertyOracle.java samples/i18n/src/com/google/gwt/sample/i18n/I18N.gwt.xml user/src/com/google/gwt/i18n/I18N.gwt.xml user/src/com/google/gwt/i18n/rebind/AbstractLocalizableImplCreator.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: ant de-duplication
Reviewers: scottb,
http://gwt-code-reviews.appspot.com/33837/diff/1/2
File common.ant.xml (right):
http://gwt-code-reviews.appspot.com/33837/diff/1/2#newcode145
Line 145: jar destfile=${project.lib} update=true duplicate=fail
index=true filesonly=true /
On 2009/06/01 17:56:26, scottb wrote:
I have to admit I'm a bit concerned about this. You've verified that
omitting
the directories has no impact? We used to require directories to be
in jars,
and I think there are still some cases where it's important. See
AbstractCompiler.INameEnvironmentImpl.isPackage() and
JDTCompiler.INameEnvironmentImpl.isPackage().
Is there any other way we could fix the timestamp issues?
Ah. Testing has been less than light, other than that the builds
complete... and, yes, I am seeing some failures now that I'm running the
tests.
In that case, there are two other possible fixes, of which I suspect
you'll prefer the second. We could:
1. revisit every instance of gwt.jar, and rearrange so
that the newest output directories will always be
first. That's brittle to future jars being wrong.
2. rewrite gwt.jar as a macro, taking e.g. sourcefiles
and generatedfiles explicitly, and explicitly naming
the generated ones first (i.e. new directories first).
This was, in fact, my plan before I noted filesonly.
I'll rework with the second approach.
http://gwt-code-reviews.appspot.com/33837/diff/1/5
File user/build.xml (right):
http://gwt-code-reviews.appspot.com/33837/diff/1/5#newcode5
Line 5:
On 2009/06/01 17:56:26, scottb wrote:
This reordering because..?
Because (1) ant properties are immutable, and (2) the
local.ant.properties file is read inside common.ant.xml. So by
relocating the default assignment of gwt.junit.emmatestcase.includes to
be after the import, users get a chance to set their own values.
Description:
We used to do a lot of repeated work on back-to-back build commands.
This patch makes us not do so.
Please review this at http://gwt-code-reviews.appspot.com/33837
Affected files:
common.ant.xml
dev/common.ant.xml
dev/core/build.xml
user/build.xml
Index: common.ant.xml
===
--- common.ant.xml (revision 5480)
+++ common.ant.xml (working copy)
@@ -1,4 +1,10 @@
project name=common
+ !-- it's okay for this not to exist, but it gives a place to store
+ your property settings, if any, persistently. For example, you
+ might use it to set gwt.junit.testcase.includes to a narrower subset
+ of test cases to exercise. --
+ property file=local.ant.properties /
+
!-- gwt.build.iscasesensitivefs is true if the filesystem of the
build machine is case-sensitive, false otherwise. Update with
new lines for any supported platforms with case-insensitive
@@ -136,7 +142,7 @@
/presetdef
presetdef name=gwt.jar
-jar destfile=${project.lib} update=true duplicate=preserve
index=true /
+jar destfile=${project.lib} update=true duplicate=fail
index=true filesonly=true /
/presetdef
macrodef name=gwt.junit
Index: dev/common.ant.xml
===
--- dev/common.ant.xml (revision 5480)
+++ dev/common.ant.xml (working copy)
@@ -18,11 +18,11 @@
target name=build depends=compile description=Build and package
this project
mkdir dir=${gwt.build.lib} /
-gwt.jar
+gwt.jar duplicate=preserve
fileset dir=src excludes=**/package.html/
fileset dir=${gwt.core.root}/src
exclude name=**/package.html/
-exclude name=com/google/gwt/dev/About.properties/
+exclude name=**/*.properties/ !-- copied and/or filtered into
bin --
/fileset
fileset dir=${gwt.core.root}/super excludes=**/package.html /
fileset dir=${javac.out} /
Index: dev/core/build.xml
===
--- dev/core/build.xml (revision 5480)
+++ dev/core/build.xml (working copy)
@@ -22,7 +22,13 @@
target name=build.alldeps.jar description=Merges all dependency jars
into a single jar
mkdir dir=${project.build} /
-gwt.jar destfile=${alldeps.jar}
+gwt.jar destfile=${alldeps.jar} duplicate=preserve
+
+ !-- We have many versions of javax.servlet.* floating around; take
care to
+ have the newest one first, or this rule will take several
iterations to
+ stop being out of date. --
+ zipfileset src=${gwt.tools.lib}/tomcat/servlet-api-2.5.jar /
+
zipfileset
src=${gwt.tools.lib}/apache/tapestry-util-text-4.0.2.jar /
zipfileset src=${gwt.tools.lib}/apache/ant-1.6.5.jar /
zipfileset src=${gwt.tools.lib}/eclipse/jdt-3.4.2.jar /
@@ -45,7 +51,6 @@
zipfileset src=${gwt.tools.lib}/tomcat/naming-factory-1.0.jar /
zipfileset src=${gwt.tools.lib}/tomcat/naming-java-1.0.jar /
zipfileset
