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 [email protected] 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.

Reply via email to