I'm fine with the production code. This is mostly nitpicks, except that we need to make sure that the code in RequestPayloadTest is actually run.
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; I'm puzzled why all these fields are public when the State class is "protected". But I suppose it's okay. http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java (right): http://gwt-code-reviews.appspot.com/1601806/diff/45014/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode310 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:310: public void testChangedEditCollectionsOfEntityProxies() { Nice, thanks for these tests. 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) { It's weird that this method isn't just inlined. 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( 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? 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(); 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. http://gwt-code-reviews.appspot.com/1601806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
