/htsjdk

Small patches to custom InflaterFactory support left over from code review #794

Merged
merged 1 commit into from Feb 1, 2017

Conversation

Reviewers

@lbergelson lbergelson

Assignees

@lbergelson lbergelson

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@droazen @codecov-io @lbergelson
@droazen
Contributor

droazen commented Feb 1, 2017

A few remaining tweaks to the custom InflaterFactory support left over from the
final review round of #771
@droazen
Contributor

droazen commented Feb 1, 2017

@lbergelson or @gspowley please review

lbergelson was assigned by droazen Feb 1, 2017

droazen requested a review from lbergelson Feb 1, 2017

@codecov-io

codecov-io commented Feb 1, 2017 • edited

Codecov Report

Merging #794 into master will increase coverage by -0.003%.

@@               Coverage Diff               @@
##              master      #794       +/-   ##
===============================================
- Coverage     64.559%   64.556%   -0.003%     
===============================================
  Files            524       524               
  Lines          31664     31667        +3     
  Branches        5415      5415               
===============================================
+ Hits           20442     20443        +1     
- Misses          9077      9080        +3     
+ Partials        2145      2144        -1
Impacted Files Coverage Δ Complexity Δ
...main/java/htsjdk/samtools/util/BlockGunzipper.java 52% <ø> (-3.319%) 0 <ø> (-7)
...a/htsjdk/samtools/cram/encoding/rans/Decoding.java 85.714% <ø> (+3.571%) 0% <ø> (-11%) ❌

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 173bccd...939bf1e. Read the comment docs.
src/test/java/htsjdk/samtools/util/BlockCompressedInputStreamTest.java
+ {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(new FileInputStream(tempFile), false, countingInflaterFactory), linesWritten, 4},
+ {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(tempFile, countingInflaterFactory), linesWritten, 4},
+ {(CheckedExceptionInputStreamSupplier) () -> new AsyncBlockCompressedInputStream(tempFile, countingInflaterFactory), linesWritten, 4},
+ {(CheckedExceptionInputStreamSupplier) () -> new BlockCompressedInputStream(new URL("http://broadinstitute.github.io/picard/testdata/index_test.bam"), countingInflaterFactory), null, 21}
};
}
@Test(dataProvider = "customInflaterInput")
@lbergelson

lbergelson Feb 1, 2017

Contributor

Could you make this @Test(dataProvider = "customInflaterInput" singleThreaded = true) since it's known to be not safe.

@droazen

droazen Feb 1, 2017

Contributor

Done

src/test/java/htsjdk/samtools/util/BlockCompressedInputStreamTest.java
return new Object[][]{
// use default InflaterFactory
- {new BlockCompressedInputStream(new FileInputStream(tempFile), false), linesWritten, 4},
@lbergelson

lbergelson Feb 1, 2017

Contributor

gross but I'm not sure I see a better way...

@lbergelson

lbergelson Feb 1, 2017

Contributor

although, since it's creating a temp file anyway, which is probably the most dangerous part, I'm not sure how much this is actually buying you.

@lbergelson

lbergelson requested changes Feb 1, 2017
View changes

one minor request then 👍

@droazen droazen Small patches to custom InflaterFactory support left over from code r… 
…eview

A few remaining tweaks to the custom InflaterFactory support from the
final review round of #771

Some checks were not successful

2 failing and 3 successful checks
codecov/patch — 0% of diff hit (target 64.559%)
Details
codecov/project — 64.553% (-0.006%) compared to 173bccd
Details
codecov/changes — No unexpected coverage changes found.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
939bf1e

@droazen droazen merged commit 34440b7 into master Feb 1, 2017
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

droazen deleted the dr_small_patches_to_inflaterfactory_support branch Feb 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment