I think I'll revert the changes to the existing tests (keeping only the
changes introducing the new ones exercizing SkipInterfaceValidation),
and make a second patch about fixing them.


http://gwt-code-reviews.appspot.com/1338807/diff/1/4
File
user/src/com/google/gwt/requestfactory/shared/SkipInterfaceValidation.java
(right):

http://gwt-code-reviews.appspot.com/1338807/diff/1/4#newcode33
user/src/com/google/gwt/requestfactory/shared/SkipInterfaceValidation.java:33:
* When applied to an interface, the validator doesn't mandate the
presence of a
On 2011/02/07 14:42:55, bobv wrote:
Is the interface-skipping implemented?

Er, oops! I initially implemented it before deciding that it probably
wasn't a good idea (as I said before: "it's a very advanced feature, so
you have to "pay a bit" for using it").
I then removed the ElementType.TYPE from the @Target but forgot to
update the javadoc.

http://gwt-code-reviews.appspot.com/1338807/diff/1/5
File
user/test/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
(left):

http://gwt-code-reviews.appspot.com/1338807/diff/1/5#oldcode98
user/test/com/google/gwt/requestfactory/server/RequestFactoryInterfaceValidatorTest.java:98:
interface DomainProxy extends EntityProxy {
On 2011/02/07 14:42:55, bobv wrote:
These are supposed to be EntityProxy.  This test class was rushed and
doesn't
actually check for particular error messages.  This type is used by
testMissingIdAndVersion(), which is now not failing for the wrong
reason.

Or actually, it is *passing* for the wrong reason ("not an EntityProxy"
instead of "missing Id and Version").

But testDateSubclass, for instance, could pass for the wrong reason if
you make DomainWithSqlDateProxy an EntityProxy ("missing id and version"
in addition to the expected "not a transferable type")
(but in the current state of the patch, it's still passing for the wrong
reason: "not an EntityProxy").

I think the proper fix is to add a new EntityProxy for
testMissingIdAndVersion; or to check that the errors.logs.contains() the
expected error (like the testBadLocatorName and testBadServiceName
tests).

Note that testUnexpectedIdAndVersion is probably passing for wrong
reasons: it's missing a findUnexpectedIdAndVersionDomain method, which I
don't think was what the method tried to test.
I manually checked every assertTrue(v.isPoisoned()) to make sure the
errors matched the test name, and it's the only one I found
(DomainWithOverloads is also missing the findDomainWithOverloads static
method, but once fixed or changed to a ValueProxy, the test passes for
the good reason; testUnexpectedIdAndVersion on the other hand fails once
you fix the missing "finder" method)

Also note that RFIV.getEntityProxyTypeName works equally well for
ValueProxy, so maybe it should be renamed?

http://gwt-code-reviews.appspot.com/1338807/show

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

Reply via email to