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

HaplotypeCallerIntegrationTest: add a boolean toggle to update the expected outputs for all exact-match-based tests #5324

Merged
merged 1 commit into from
Oct 19, 2018

Conversation

droazen
Copy link
Collaborator

@droazen droazen commented Oct 18, 2018

No description provided.

@droazen
Copy link
Collaborator Author

droazen commented Oct 18, 2018

@ldgauthier please review

@nh13
Copy link
Contributor

nh13 commented Oct 18, 2018

Thanks @droazen, this will be super helpful

@codecov-io
Copy link

codecov-io commented Oct 18, 2018

Codecov Report

Merging #5324 into master will increase coverage by <.001%.
The diff coverage is 92.857%.

@@               Coverage Diff               @@
##              master     #5324       +/-   ##
===============================================
+ Coverage     86.795%   86.795%   +<.001%     
- Complexity     30125     30126        +1     
===============================================
  Files           1843      1843               
  Lines         139485    139487        +2     
  Branches       15376     15376               
===============================================
+ Hits          121066    121068        +2     
+ Misses         12827     12826        -1     
- Partials        5592      5593        +1
Impacted Files Coverage Δ Complexity Δ
...aplotypecaller/HaplotypeCallerIntegrationTest.java 88.155% <92.857%> (+0.054%) 83 <1> (+1) ⬆️
...oadinstitute/hellbender/utils/gcs/BucketUtils.java 81.098% <0%> (ø) 42% <0%> (ø) ⬇️

Copy link
Contributor

@ldgauthier ldgauthier left a comment

Choose a reason for hiding this comment

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

Very helpful indeed. I've been doing this with debug mode and there's just so much clicking.

Do you have any thoughts on a safety mechanism to make sure something doesn't get merged with this turned on? Of course a conscientious reviewer like yourself would catch it, but I wouldn't want you to have to review all the HaplotypeCaller PRs. Maybe Travis should have a check for the number of integration tests run in this class?

@droazen
Copy link
Collaborator Author

droazen commented Oct 19, 2018

@ldgauthier Good point! I'll add a test that fails if the boolean toggle is left turned on.

…pected outputs for all exact-match-based tests
@droazen droazen force-pushed the dr_haplotypecallerintegrationtest_toggle branch from 4a3aef1 to 7a1d05d Compare October 19, 2018 15:40
@droazen
Copy link
Collaborator Author

droazen commented Oct 19, 2018

Added a new test to assert that the toggle is off -- back to you @ldgauthier

* Make sure that someone didn't leave the UPDATE_EXACT_MATCH_EXPECTED_OUTPUTS toggle turned on
*/
@Test
public void assertThatExpectedOutputUpdateToggleIsDisabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

So elegantly simple.

@droazen droazen merged commit 64cfe3a into master Oct 19, 2018
@droazen droazen deleted the dr_haplotypecallerintegrationtest_toggle branch October 19, 2018 18:28
EdwardDixon pushed a commit to EdwardDixon/gatk that referenced this pull request Nov 9, 2018
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