http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java File user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java (right):
http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode117 user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:117: public boolean diffing; On 2012/05/23 18:05:24, skybrian wrote:
I'm puzzled why all these fields are public when the State class is
"protected".
But I suppose it's okay.
Yes, strange (and I could have made the new field 'private', given that the isDiffing() is package-protected :-/ ), but it's an "impl" class, so... http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java File user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java (right): http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java#newcode95 user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java:95: public static SimpleBar returnFirst(List<SimpleBar> list) { On 2012/05/23 18:05:24, skybrian wrote:
It's weird that this method isn't just inlined.
It can be called from the client-side, it's a "service method" (SimpleBarRequest). http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/shared/impl/RequestPayloadTest.java File user/test/com/google/web/bindery/requestfactory/shared/impl/RequestPayloadTest.java (right): http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/shared/impl/RequestPayloadTest.java#newcode78 user/test/com/google/web/bindery/requestfactory/shared/impl/RequestPayloadTest.java:78: context.findSimpleFooById(1L).with("barField", "oneToManyField").fire( On 2012/05/23 18:05:24, skybrian wrote:
I'm confused because this looks like an async test, but
delayTestFinish() and
finishTest() are never called. How do we know all this code is ever
reached? D'oh! Looks like you're right. It *will* be reached in RequestPayloadJreTest because the RequestTransports there are synchronous (so the 'fire()' methods are synchronous), but you're right it won't be reached when run as a GWTTestCase (in either dev mode or prod mode). http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/shared/impl/RequestPayloadTest.java#newcode117 user/test/com/google/web/bindery/requestfactory/shared/impl/RequestPayloadTest.java:117: SimpleProxyId<?> id = (SimpleProxyId<?>) foo.stableId(); On 2012/05/23 18:05:24, skybrian wrote:
There's no guarantee that this code is reached. (If too little data is
sent, the
test wouldn't fail.) It might be simpler to introduce a
"findOperation" helper
method and do something like this:
SimpleProxyId<?> fooId = findOperation(requestMessage, fooTypeToken); // ... do asserts ...
The findOperation() method should automatically assert that there's
exactly one
match.
IIRC, we're actually expecting several operationMessages for the same type (at least for valueTypeToken). However, this test is not about which messages are sent (these would be caught in all the other RF tests) but what they contain. For instance, we don't check that we don't have operations for other types than fooTypeToken and valueTypeToken, we only check what the messages of these types contain. When we'll revisit AbstractRequestContext#makeOperations to no longer send proxies that have not changed, then we'll revisit this test too, because then we'll care which proxy is sent to the server. Here, what we care is that "barField", "oneToManyField" and "oneToManySetField" don't appear as "modified properties". I could add a flag or counter in the 'if's and then check their values after the loops to assert this code has been reached? Or maybe simply add a comment making explicit what we're checking here? http://gwt-code-reviews.appspot.com/1601806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
