Skip to content
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

Test on Java 11 #1269

Closed
wants to merge 4 commits into from
Closed

Test on Java 11 #1269

wants to merge 4 commits into from

Conversation

tomwhite
Copy link
Contributor

Description

Runs htsjdk's unit tests on Java 11.

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

@yfarjoun
Copy link
Contributor

did you only open this to get the tests? is this a serious candidate for merging? in the current state, of course that would not be possible...

@codecov-io
Copy link

codecov-io commented Feb 12, 2019

Codecov Report

Merging #1269 into master will decrease coverage by 1.843%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##              master     #1269       +/-   ##
===============================================
- Coverage     69.358%   67.515%   -1.843%     
+ Complexity      8302      8151      -151     
===============================================
  Files            555       558        +3     
  Lines          33118     33397      +279     
  Branches        5572      5632       +60     
===============================================
- Hits           22970     22548      -422     
- Misses          7886      8665      +779     
+ Partials        2262      2184       -78
Impacted Files Coverage Δ Complexity Δ
...ain/java/htsjdk/samtools/util/MergingIterator.java 85.714% <ø> (ø) 11 <0> (ø) ⬇️
.../main/java/htsjdk/samtools/sra/ReferenceCache.java 0% <0%> (-81.818%) 0% <0%> (-4%)
src/main/java/htsjdk/samtools/sra/SRAUtils.java 0% <0%> (-81.818%) 0% <0%> (-3%)
src/main/java/htsjdk/samtools/SRAIndex.java 0% <0%> (-77.273%) 0% <0%> (-18%)
src/main/java/htsjdk/samtools/SRAIterator.java 0% <0%> (-72.857%) 0% <0%> (-18%)
...java/htsjdk/samtools/sra/SRAAlignmentIterator.java 0% <0%> (-72%) 0% <0%> (-15%)
...c/main/java/htsjdk/samtools/sra/SRALazyRecord.java 0% <0%> (-71.591%) 0% <0%> (-130%)
src/main/java/htsjdk/samtools/BinList.java 0% <0%> (-70.588%) 0% <0%> (-2%)
...va/htsjdk/samtools/sra/SRAUnalignmentIterator.java 0% <0%> (-65.574%) 0% <0%> (-14%)
...va/htsjdk/samtools/sra/SRAIndexedSequenceFile.java 0% <0%> (-64.865%) 0% <0%> (-7%)
... and 54 more

@tomwhite
Copy link
Contributor Author

@yfarjoun the goal here is to check we can build and run htsjdk tests with Java 11. The source and target Java versions are not changed - they are still Java 8 - so this is just to ensure that we can use a Java 11 compiler and runtime with the current htsjdk source. It does not replace the Java 8-based build.

There were a few things I had to change to get this to work:

This is a serious candidate for merging, since none of these changes interfere with the current Java 8 build. I also added a openjdk11 target to Travis so that every change is checked on Java 11. This will mean that htsjdk doesn't regress in its ability to be built with and run on Java 11.

@lbergelson
Copy link
Member

Wow, those two javadoc comments were the issue? I assumed it would be much worse than that. Bizarre. That shouldn't cause a javadoc null point exception...

@lbergelson
Copy link
Member

@tomwhite This still had some issues with it, I pushed lb_java11 which should resolve them. Would you like to incorporate those changes into yours or should I create a new pr?

@tomwhite
Copy link
Contributor Author

@lbergelson thanks for looking at this. I had a look at your branch, but it wasn't clear how the changes related to Java 11. That said, please go ahead and merge as you see fit.

@lbergelson
Copy link
Member

@tomwhite They fix problems introduce by moving to gradle 5. It broke the maven publication so I switched it to the newer maven-publish which seemed to fix the problem. I bumped a few versions of things and did a little cleanup to remove newly deprecated stuff as well.

@lbergelson lbergelson mentioned this pull request Feb 19, 2019
5 tasks
@lbergelson
Copy link
Member

closing this since I've taken it over in #1291

@lbergelson lbergelson closed this Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants