On Oct 9 2013, at 10:33 , Brent Christian wrote: > On 10/9/13 3:12 AM, Alan Bateman wrote: >> On 08/10/2013 20:55, Brent Christian wrote: > >> >>> I've beefed up the test case > > >> Thanks for the update and expanding the test. I skimmed over the new >> test cases and they looks good. > > Thanks. > > > There are a few commented out cases and >> it's not clear why this also. > > Ooops - I was confirming the new tests work using 5- and 3- element > collections. I'll take the commented lines back out. > >> Also a minor observation is that the >> method names are a bit inconsistent with the other method names in the >> test, they might be something to look at before pushing. > > Anything in particular? > > Other than clear(), I don't see other methods that take a collection and make > it ready for testing. Most collections are populated inline, but my tests > need a Map setup the exact same way many times, so I added > prep[Map|Set]ForDescItrTests(). > > A lot of "helper" test methods are named "checkXYZ", and that's how I named > mine. > > The new tests check pretty specific behaviors, and I chose pretty descriptive > names. I guess most existing method names are fully written out rather than > abbreviated (e.g. checkNavigableMapKeys()), which I generally like to do, but > mine got *really* long. I abbreviated the names to keep things under 80 > characters without having to break everything onto multiple lines. > > > I think this is ready for someone to push (I can send a changeset), unless > more should be done with the method names in the test case.
I haven't looked at the latest webrev but if you send me a changeset I will review it and push it. Mike
