Weird JDK8 and/or Travis-CI issue

[anjackson]

Following #139, I tried to enable a JDK8 build on our Travis CI build service. This failed strangely during the test phase:

Running org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndexTest
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex reloadMapFile
INFO: Reloaded assignments from /tmp/range-map8508402699614792229tmp
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(b) for url (bam.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(c) for url (banana.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(c) for url (banana.net/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(b) for url (banana.au/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(a) for url (ape.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(b) for url (apple.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(a) for url (aardvark.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(d) for url (dantheman.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(c) for url (cool.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(d) for url (cups.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(d) for url (zoo.com/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(a) for url (207.241.2.2/)
Jul 07, 2014 12:57:13 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(d) for url (zztop.com/)
Jul 07, 2014 12:57:14 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex reloadMapFile
INFO: Reloaded assignments from /tmp/range-map8539621656330329541tmp
Jul 07, 2014 12:57:14 PM org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndex getRangeGroupForRequest
INFO: Using group(b) for url (apple.com/)
Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.037 sec <<< FAILURE!
testGroupBalance(org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndexTest)  Time elapsed: 0.01 sec  <<< FAILURE!
junit.framework.ComparisonFailure: expected:<...2> but was:<...1>
    at junit.framework.Assert.assertEquals(Assert.java:81)
    at junit.framework.Assert.assertEquals(Assert.java:87)
    at org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndexTest.testGroupBalance(AlphaPartitionedIndexTest.java:91)

Can anyone confirm this issue while running tests under Oracle JDK 8? Or is it something to do with Travis?

/cc @isido

[isido]

I get this also. When I initially did the fix last week, tests passed fine. Either it's some commit after that or Travis-issue. I didn't have time to debug that today, but I'll try to do that tomorrow.

[anjackson]

Thanks @isido!

[ldko]

When I tried to build with Oracle Java 8 yesterday, the build was failing. There seemed to be an issue with the tags in the comments in cdx-server, for example:

[INFO] MavenReportException: Error while creating archive: 

Exit code: 1 - /tmp/coo/openwayback/wayback-cdx-server/src/main/java/org/archive/cdxserver/filter/CollapseFieldFilter.java:16: error: unknown tag: field

* <field> = if <field> matches previous match, then its a dupe <field>:<n> = if
   ^

/tmp/coo/openwayback/wayback-cdx-server/src/main/java/org/archive/cdxserver/filter/CollapseFieldFilter.java:16: error: unknown tag: field

* <field> = if <field> matches previous match, then its a dupe <field>:<n> = if
                ^
...etc...

If I take those out, the build fails with a test failure:
Failed tests:

testGroupBalance(org.archive.wayback.resourceindex.distributed.AlphaPartitionedIndexTest): expected:<...2> but was:<...1>

[anjackson]

From Collections Framework Enhancements in Java SE 8:

In rare situations, this change could introduce a change to the iteration order of HashMap and HashSet. A particular iteration order is not specified for HashMap objects - any code that depends on iteration order should be fixed.

Having had a brief look at the code, the above change this leads me to suspect that something in this part of the code is dependent on the order in which a HashMap or HashSet is iterated. My initial suspicion falls on this part of findBestMember. I'm not really sure what this code does, but if we assume that the weights on all the items in this list are the same, then the particular item returned by this method would depend on the order of iteration.

The 'weights' of each RangeGroup appear to be equal (0, see RangeMember.getWeight()), so the test appears to be a bit nonsensical. It is saying 'which is the best of these two equally weighted options, b1 and b2?', and either is fine unless the order of them matters, so the test should be changed to allow the value to be either 'b1' or b2'.

However, given I don't really understand what this code is supposed to do, I'm not 100% convinced.

[isido]

I looked into that, but couldn't get my head around the code. My working hypothesis was that the testing code was buggy, but I couldn't confirm that since I didn't really understand the bigger picture here.

[johnerikhalse]

I also looked into the code and agree with @isido that the problem is with the testing code. My understanding is that AlphaPartitionedIndex delegates index queries to different remote index partitions. These partitions might be duplicated and the method findBestMember() tries to find the index which has fewest ongoing queries. When the indexes have the same number of ongoing queries, it should not matter which index to pick, but the test has an assumption on which index will be returned in that case. As @anjackson pointed out, this test relies on a particular implementation of HashMap which changed in Java 8.

I've made a pull request (#170) which fixes the test and also adds some javadoc where I describe how I think this thing works.

[PsypherPunk]

Fixed by #170.