[gwt-contrib] Re: Add assert for null provided fields, fixes #7024 (issue1604803)
LGTM too. I'll repost and submit this with a test. http://gwt-code-reviews.appspot.com/1604803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add assert for null provided fields, fixes #7024 (issue1604803)
LGTM too. I'll repost and submit this with a test. http://gwt-code-reviews.appspot.com/1604803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add assert for null provided fields, fixes #7024 (issue1604803)
LGTM too. I'll repost and submit this with a test. http://gwt-code-reviews.appspot.com/1604803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add assert for null provided fields, fixes #7024 (issue1602805)
Reviewers: rdayal, stephenh, scheglov, tbroyer, Description: Add assert for null provided fields, fixes #7024 Added a test to ensure the assertion happens. Repost from Rietveld issue 1604803 Thanks to stephenh for the original patch! Please review this at http://gwt-code-reviews.appspot.com/1602805/ Affected files: M user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java M user/test/com/google/gwt/uibinder/LazyWidgetBuilderSuite.java A user/test/com/google/gwt/uibinder/test/client/UiProvidedNullTest.java A user/test/com/google/gwt/uibinder/test/client/UiProvidedNullUi.java A user/test/com/google/gwt/uibinder/test/client/UiProvidedNullUi.ui.xml -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add assert for null provided fields, fixes #7024 (issue1604803)
See http://gwt-code-reviews.appspot.com/1602805 http://gwt-code-reviews.appspot.com/1604803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
LGTM http://gwt-code-reviews.appspot.com/1604804/diff/1/user/src/com/google/gwt/junit/FakeCssMaker.java File user/src/com/google/gwt/junit/FakeCssMaker.java (right): http://gwt-code-reviews.appspot.com/1604804/diff/1/user/src/com/google/gwt/junit/FakeCssMaker.java#newcode2 user/src/com/google/gwt/junit/FakeCssMaker.java:2: * Copyright 2009 Google Inc. 2011 http://gwt-code-reviews.appspot.com/1604804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add assert for null provided fields, fixes #7024 (issue1602805)
Awesome, nice test. Thanks! http://gwt-code-reviews.appspot.com/1602805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
http://gwt-code-reviews.appspot.com/1604804/diff/1/user/src/com/google/gwt/junit/FakeCssMaker.java
File user/src/com/google/gwt/junit/FakeCssMaker.java (right):
http://gwt-code-reviews.appspot.com/1604804/diff/1/user/src/com/google/gwt/junit/FakeCssMaker.java#newcode44
user/src/com/google/gwt/junit/FakeCssMaker.java:44:
FakeCssMaker.class.getClassLoader(), new Class[] {cssClass},
Is this the right classloader to use? FakeCssMaker is part of GWT,
while cssClass will typically be a user class.
I suggest using the thread context classloader instead.
http://gwt-code-reviews.appspot.com/1604804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
Right now, it won't make much difference as GWT gets user classes from the same classpath that GWT classes are loaded from. However, that leads to conflict for shared code where you want to use a different version of something than the version GWT itself needs to run. So, in the future we may provide a way to give separate classpaths. I think of the following guidelines for choosing which classloader to use: - if a class being loaded is bundled with some other class (ie, known to be from the same jar), use that class's classloader to load it. - if you want to load a system class and definitely don't want it to be overridden, use the system classloader - otherwise, use the thread context classloader. Since you have already submitted it and this will generally be used only in tests, this is just FYI and you don't need to update it. http://gwt-code-reviews.appspot.com/1604804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
Right now, it won't make much difference as GWT gets user classes from the same classpath that GWT classes are loaded from. However, that leads to conflict for shared code where you want to use a different version of something than the version GWT itself needs to run. So, in the future we may provide a way to give separate classpaths. I think of the following guidelines for choosing which classloader to use: - if a class being loaded is bundled with some other class (ie, known to be from the same jar), use that class's classloader to load it. - if you want to load a system class and definitely don't want it to be overridden, use the system classloader - otherwise, use the thread context classloader. Since you have already submitted it and this will generally be used only in tests, this is just FYI and you don't need to update it. http://gwt-code-reviews.appspot.com/1604804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
Reviewers: skybrian,
Description:
Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker.
Please review this at http://gwt-code-reviews.appspot.com/1604804/
Affected files:
A user/src/com/google/gwt/junit/FakeCssMaker.java
A user/test/com/google/gwt/junit/FakeCssMakerTest.java
Index: user/src/com/google/gwt/junit/FakeCssMaker.java
===
--- user/src/com/google/gwt/junit/FakeCssMaker.java (revision 0)
+++ user/src/com/google/gwt/junit/FakeCssMaker.java (revision 0)
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2009 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may
not
+ * use this file except in compliance with the License. You may obtain a
copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.junit;
+
+import com.google.gwt.resources.client.CssResource;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+
+/**
+ * Helper to make a fake implementation of any {@link CssResource}
interface via
+ * reflection, for use in JUnit tests. (This will not work in
GWTTestCase.) All
+ * calls to the returned object return the method name.
+ * p
+ * Sample use:
+ *
+ * preinterface MyCss extends CssResource {
+ * String myStyleName();
+ * }
+ *
+ * public void testSimple() {
+ * MyCss myCss = FakeCssMaker.create(MyCss.class);
+ * assertEquals(myStyleName, messages.myMessage());
+ * }
+ * /pre
+ */
+public class FakeCssMaker implements InvocationHandler {
+ public static T extends CssResource T create(ClassT cssClass) {
+return cssClass.cast(Proxy.newProxyInstance(
+FakeCssMaker.class.getClassLoader(), new Class[] {cssClass},
+new FakeCssMaker()));
+ }
+
+ public Object invoke(Object proxy, Method method, Object[] args)
+ throws Throwable {
+return method.getName();
+ }
+}
Index: user/test/com/google/gwt/junit/FakeCssMakerTest.java
===
--- user/test/com/google/gwt/junit/FakeCssMakerTest.java(revision 0)
+++ user/test/com/google/gwt/junit/FakeCssMakerTest.java(revision 0)
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may
not
+ * use this file except in compliance with the License. You may obtain a
copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.junit;
+
+import com.google.gwt.resources.client.CssResource;
+
+import junit.framework.TestCase;
+
+/**
+ * Tests of FakeCssMaker.
+ */
+public class FakeCssMakerTest extends TestCase {
+ interface MyCss extends CssResource {
+String myFirstStyleName();
+String mySecondStyleName();
+ }
+
+ public void testCreate() {
+MyCss css = FakeCssMaker.create(MyCss.class);
+assertEquals(myFirstStyleName, css.myFirstStyleName());
+assertEquals(mySecondStyleName, css.mySecondStyleName());
+ }
+}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
On 2011/11/30 20:26:19, jat wrote:
http://gwt-code-reviews.appspot.com/1604804/diff/1/user/src/com/google/gwt/junit/FakeCssMaker.java
File user/src/com/google/gwt/junit/FakeCssMaker.java (right):
http://gwt-code-reviews.appspot.com/1604804/diff/1/user/src/com/google/gwt/junit/FakeCssMaker.java#newcode44
user/src/com/google/gwt/junit/FakeCssMaker.java:44:
FakeCssMaker.class.getClassLoader(), new Class[] {cssClass},
Is this the right classloader to use? FakeCssMaker is part of GWT,
while
cssClass will typically be a user class.
I suggest using the thread context classloader instead.
Interesting comment. Under what circumstances would they be in different
classloaders? The JUnit testing environment for which this class is
intended shouldn't have multiple classloaders. Then again, if that
somehow occurred then I would think that the right classloader would be
that of 'cssClass', so maybe it's worth revisiting.
For what it's worth, this is the same technique that FakeMessagesMaker
uses without issue. I submitted the change prior to seeing your comment,
but I'm happy to make the change to both this and FakeMessagesMaker if
you think it has value.
http://gwt-code-reviews.appspot.com/1604804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add a FakeCssMaker for CSS resources similar to FakeMessagesMaker. (issue1604804)
On 2011/11/30 21:05:17, jat wrote: Right now, it won't make much difference as GWT gets user classes from the same classpath that GWT classes are loaded from. However, that leads to conflict for shared code where you want to use a different version of something than the version GWT itself needs to run. So, in the future we may provide a way to give separate classpaths. I think of the following guidelines for choosing which classloader to use: - if a class being loaded is bundled with some other class (ie, known to be from the same jar), use that class's classloader to load it. - if you want to load a system class and definitely don't want it to be overridden, use the system classloader - otherwise, use the thread context classloader. Since you have already submitted it and this will generally be used only in tests, this is just FYI and you don't need to update it. Thanks for the extra info, I agree with your assessment. http://gwt-code-reviews.appspot.com/1604804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add javadocs for ExternalTextResource.getText, fixes #7035 (issue1605803)
http://gwt-code-reviews.appspot.com/1605803/diff/1/user/src/com/google/gwt/resources/client/ExternalTextResource.java File user/src/com/google/gwt/resources/client/ExternalTextResource.java (right): http://gwt-code-reviews.appspot.com/1605803/diff/1/user/src/com/google/gwt/resources/client/ExternalTextResource.java#newcode23 user/src/com/google/gwt/resources/client/ExternalTextResource.java:23: * initialization. I didn't change the license/javadocs, but just formatted the file with the Eclipse formatter settings. I can undo that if needed. http://gwt-code-reviews.appspot.com/1605803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
