-
Notifications
You must be signed in to change notification settings - Fork 760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes for apparent build errors #154
Conversation
@anjackson I'm getting a failed build as well on master, full output here. Ubuntu 15.10, 64-bit
And, your branch isn't building for me either. Output here. I also setup TravisCI for this repo here. More than happy to put in a pull request as well. Maybe it'll help out for issues like this in the future. |
Thanks @ruebot - your builds hit heap space problems, but this bit looks like there are unresolved issues
Note that IA have their own build server which seems perfectly happy, so perhaps this a subtle platform/locale issue? I've attached the arc.gz from the last selftest run (from /tmp/heritrix-junit-tests/selftest/StatisticsSelfTest/jobs/selftest-job/arcs/) - can you attach yours so we can compare the contents? WEB-20160402195540108-00000-551 And here's the WARC version from /tmp/heritrix-junit-tests/selftest/StatisticsSelfTest/jobs/selftest-job/20160402195538/warcs/ |
@anjackson ah, cool. Didn't know they had a Jenkins setup. I just realized my mvn output said it was using Java 7 instead of Java 8, so that might be worth noting. Here is the arc from my last selftest run, which would have been your branch: |
Oh that's weird. Yours doesn't have a DNS response record, and indeed no results from 'localhost' at all. Quite confused now. Maybe running out of heap space killed it midway through!? |
I'll do a |
Hrm. Both still fail, and I don't see a |
Here is a second build with Java 7, and more memory. It looks like it got further along and failed at ...and the sample arc: https://www.dropbox.com/s/gcnvq15s0wsdxl9/WEB-20160403002631073-00000-25360~simian~8443.warc.gz?dl=0 |
Okay, I'm totally confused now. That build should not have failed like that. Maybe I should also set up a Travis build with lots of JVM combinations? |
👍 |
TravisCI - master - https://travis-ci.org/ruebot/heritrix3/builds/120423334 Looks like openjdk8 isn't setup on the TravisCI machines. |
I eventually got Travis builds running (e.g.). Java 7 is fine although I kept hitting a race condition in one test and had to introduce a delay. Java 8 hits more problems due to relying on comparing the serialised binary forms of objects. This is probably something we should try to work out how to avoid, as the serialisation format is not a fixed part of the spec, and can be expected to change from time to time and across Java implementations. However, I'm not sure how best to do this. |
I'm kinda letting you guys sort out these issues :) No .travis.yml in the pull request? |
I thought you'd rather not have one, as you already run Jenkins? Very happy to patch it in if you don't mind, as it would make it much easier for me to run Travis CI on our fork. |
I think it's fine to have both jenkins and travis builds. Jenkins makes it easier to download build artifacts and it populates our maven repo. We can use travis for testing with different versions of java and stuff like that. |
Setup TravisCI
Thanks to @ruebot for finishing off the Travis config. |
np! @nlevitt do you know how to setup the TravisCI hook in GitHub? |
Been using the travis build against one of my forks. The OpenJDK build seems to occasionally fail (different unit tests) for no (seemingly) good reason? The OracleJDK, however, has only failed when it should have. |
Sorry this slipped off my radar. @ruebot I cherry-picked the .travis.yml commit so we can have a baseline before merging these fixes. https://travis-ci.org/internetarchive/heritrix3 |
Cool. You know how to enable it on your end in GitHub settings? Also, do you want a README badge for it? Happy to toss in another pull request for that if need be. |
I didn't have to do anything in github settings. (Maybe there was some account level setting that was already enabled though.) I just enabled it in travis. Sure feel free to send a PR for a the readme badge. |
Yeah, you can get at it two different ways, so sounds like we're all good. |
After running into more issues with those byte count checks in StatisticsSelfTest.java I finally took the time to examine the issue. Turns out the size of the dns record depends on the local environment (dns server, I guess). Specifically the TTL varies. For example
vs.
On #164 I extended @anjackson's branch to exclude dns records from the size checks. |
Fixes for apparent build errors (extends #154)
Under Java 1.7 (u51), on a Mac (see full details below), I find the current build to be broken.
One set of errors arise because the unit test assumes it knows how the JVM will serialised HashMaps, and how this relates to the ordering of the HashMap. However, this ordering is not guaranteed and since some point on the 1.7 line these tests will now fail.
The other discrepancy is strange - one of the selftest crawls run during the build checks the number of novel bytes is as expected. For some reason, this value is 1 lower than it was before. In particular, the novel bytes associated with the localhost dns request appears to be 48 rather than the expected 49. Perhaps this is some kind of odd platform dependence?