Count the number of warnings and errors during SAM file validation #855

Merged
merged 1 commit into from Apr 19, 2017

Conversation

Projects
None yet
3 participants
Contributor

ronlevine commented Apr 17, 2017

Description

Count the number of warnings and errors so Picard's ValidateSamFile can determine the appropriate exit code described in broadinstitute/picard#794.

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)

ronlevine self-assigned this Apr 17, 2017

ronlevine requested a review from tfenne Apr 17, 2017

Contributor

ronlevine commented Apr 17, 2017

@tfenne Please review.

codecov-io commented Apr 17, 2017 edited

Codecov Report

Merging #855 into master will increase coverage by 0.018%.
The diff coverage is 66.667%.

@@              Coverage Diff               @@
##              master     #855       +/-   ##
==============================================
+ Coverage     64.922%   64.94%   +0.018%     
- Complexity      7203     7209        +6     
==============================================
  Files            527      527               
  Lines          31789    31800       +11     
  Branches        5425     5426        +1     
==============================================
+ Hits           20638    20651       +13     
  Misses          9017     9017               
+ Partials        2134     2132        -2
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/htsjdk/samtools/SamFileValidator.java 85.132% <66.667%> (-0.336%) 73 <2> (+3)
...dk/samtools/seekablestream/SeekableHTTPStream.java 56.061% <0%> (+1.515%) 10% <0%> (+1%) ⬆️
...a/htsjdk/samtools/cram/encoding/rans/Decoding.java 85.714% <0%> (+3.571%) 12% <0%> (+1%) ⬆️
...samtools/util/AsyncBlockCompressedInputStream.java 76% <0%> (+4%) 13% <0%> (+1%) ⬆️

ronlevine requested a review from yfarjoun Apr 18, 2017

Contributor

ronlevine commented Apr 18, 2017

@yfarjoun Could you take a look?

@yfarjoun

tiny request 👍 after that.

+ return;
+ }
+ this.numWarnings++;
+ } else {
@yfarjoun

yfarjoun Apr 18, 2017

Contributor

can you protect this against a new Severity level?

@ronlevine

ronlevine Apr 18, 2017

Contributor

Changed to a switch statement that handles each known error type and throws a SAMException if it's an unknown error.

Contributor

ronlevine commented Apr 18, 2017

@yfarjoun Squashing and merging once tests pass if that's fine with you.

Contributor

yfarjoun commented Apr 18, 2017

yup. go ahead.

@ronlevine ronlevine Count the number of warnings and errors during SAM file validation
6fb6cca

@ronlevine ronlevine merged commit ecd7df2 into master Apr 19, 2017

5 checks passed

codecov/changes No unexpected coverage changes found.
Details
codecov/patch 66.667% of diff hit (target 64.922%)
Details
codecov/project 64.94% (+0.018%) compared to da2b76b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

ronlevine deleted the rhl_return_error_codes branch Apr 19, 2017

ronlevine referenced this pull request in broadinstitute/picard May 29, 2017

Merged

ValidateSamFile returns informative error codes #822

3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment