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

CRAM: some structure test cleanup #1312

Merged
merged 1 commit into from
Mar 2, 2019
Merged

CRAM: some structure test cleanup #1312

merged 1 commit into from
Mar 2, 2019

Conversation

jmthibault79
Copy link
Contributor

@jmthibault79 jmthibault79 commented Mar 1, 2019

Move common code from ContainerTest, ContainerFactoryTest, ContainerParserTest, SliceTests, CRAMBAIIndexerTest, and CramCompressionRecordUtil into new CRAMStructureTestUtil

Description

Test cleanup, as requested

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)

@jmthibault79
Copy link
Contributor Author

git diffs are a mess, so it's probably easiest to view the whole files locally.

@codecov-io
Copy link

codecov-io commented Mar 1, 2019

Codecov Report

Merging #1312 into master will increase coverage by 0.006%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             master     #1312       +/-   ##
==============================================
+ Coverage     67.75%   67.756%   +0.006%     
- Complexity     8235      8236        +1     
==============================================
  Files           562       562               
  Lines         33538     33538               
  Branches       5636      5636               
==============================================
+ Hits          22722     22724        +2     
+ Misses         8634      8632        -2     
  Partials       2182      2182
Impacted Files Coverage Δ Complexity Δ
...ain/java/htsjdk/samtools/cram/structure/Slice.java 60.234% <0%> (+1.17%) 30% <0%> (+1%) ⬆️

Copy link
Collaborator

@cmnbroad cmnbroad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor comments, but overall this looks really good. Thx.


final int slice2AlignmentStart = 20;
final int slice2AlignmentSpan = 20;
final Slice slice2 = new Slice(mappedReferenceContext);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion to improve the readability of the code below: use mappedSlice as a prefix for these variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

new Slice(ReferenceContext.UNMAPPED_UNPLACED_CONTEXT),
new Slice(ReferenceContext.MULTIPLE_REFERENCE_CONTEXT)
},
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice.

- ContainerTest, ContainerFactoryTest, ContainerParserTest, SliceTests, and CRAMBAIIndexerTest
@jmthibault79 jmthibault79 merged commit 1509dcc into master Mar 2, 2019
@jmthibault79 jmthibault79 deleted the jt_test_cleanup branch March 2, 2019 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants