PointQuadTreeTest failed

[thuytrinh]

I'm not certain whether this test was obsolete. When I run all the tests on my Genymotion (Google Nexus S - 4.1.1 - API 16), PointQuadTreeTest failed.

junit.framework.AssertionFailedError: expected:<399600> but was:<399800>
at com.google.maps.android.quadtree.PointQuadTreeTest.testManyPoints(PointQuadTreeTest.java:139)
at java.lang.reflect.Method.invokeNative(Native Method)
at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:190)
at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:175)
at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1584)

[barbeau]

Confirmed, I'm seeing this test failure too on an LG G4 on the master branch.

[broady]

@markmcd @jfschmakeit didn't one of you see this, too?

very weird. must be some differences in floating point handling? or a change in the gms maps library?

[barbeau]

I just checked out tag 0.3.4 (https://github.com/googlemaps/android-maps-utils/tree/0.3.4), and this test passes on 0.3.4 (after some massaging of build.gradle, etc. to get it to sync/build).

EDIT
Above text should have read 0.3.1 - see below.

[markmcd]

What massaging did you do? I'm mainly curious what versions in the dependencies you used.

[barbeau]

Sorry, I actually got the version number wrong - it passed on a patched version of 0.3.1 tag:
https://github.com/barbeau/android-maps-utils/tree/0.3.1-patched

Here's the patch that updates the version numbers so it syncs/builds under the current version of Android Studio:
barbeau@20e0473

[barbeau]

So to clarify I haven't tested on 0.3.4.

[broady]

So, is this a regression on play-services?

[barbeau]

Doesn't look like this is a regression in play-services.

I just checked out the v0.3.4 tag, and patched it to run in the current version of Android Studio. I also reverted it to compile 'com.google.android.gms:play-services:4.3+' to match the v0.3.1 tag:
https://github.com/barbeau/android-maps-utils/tree/0.3.4-patched

Commit:
barbeau@4097e90

The test fails on 0.3.4, but passes on 0.3.1, both using the 4.3+ version of play-services.

[barbeau]

It's also not just that one test execution - the rest of the tests in the same group also fail:

// searching bounds that do not line up with main quadTree
assertEquals(399600, mTree.search(new Bounds(0, 0.999, 0, 0.999)).size());
assertEquals(4000, mTree.search(new Bounds(0.8, 0.9, 0.8, 0.9)).size());
assertEquals(4000, mTree.search(new Bounds(0, 1, 0, 0.01)).size());
assertEquals(16000, mTree.search(new Bounds(0.4, 0.6, 0.4, 0.6)).size());

Also, several tests in the next group fail, but one passes:

// searching bounds that are small / have very exact end points
assertEquals(1, mTree.search(new Bounds(0, .001, 0, .0001)).size());  // Passes
assertEquals(26574, mTree.search(new Bounds(0.356, 0.574, 0.678, 0.987)).size());  // Fails
assertEquals(44622, mTree.search(new Bounds(0.123, 0.456, 0.456, 0.789)).size());  // Fails
assertEquals(4884, mTree.search(new Bounds(0.111, 0.222, 0.333, 0.444)).size());  // Fails

[barbeau]

So my guess (without testing) is that the culprit is:
57201be

...as it changed the implementation of Bounds.contains(), which is used in the QuadTree search. This commit isn't included in 0.3.1, but does appear in 0.3.2 and higher.

[barbeau]

Confirmed - if I revert 57201be in the master branch the tests pass again:
https://github.com/barbeau/android-maps-utils/tree/issue187-failedTest

[broady]

aha!

nice work, @barbeau. sounds like we should update the test, then.

[barbeau]

@broady Yes, after reviewing #71 I think that's the correct coarse of action. Looks like the original tests didn't take inclusivity of max X/Y values into account - and, all returned values I've seen from the failing tests return a number greater than the original expected number, which makes sense if we're now including those border values. I'll submit a PR with this fix shortly.

[barbeau]

PR opened at #206. Yay! All is right with the world, all tests now pass :).