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

Disable the red "X" from failing codeCov builds and delay the posting of coverage information to complete test. #7817

Merged
merged 3 commits into from
Apr 27, 2022

Conversation

jamesemery
Copy link
Collaborator

@jamesemery jamesemery commented Apr 26, 2022

Repost of #7815 to test the "after_n_builds" functionality

@jamesemery jamesemery requested a review from droazen April 26, 2022 17:36
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #7817 (0ea8253) into master (4ad1283) will increase coverage by 0.001%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##              master     #7817       +/-   ##
===============================================
+ Coverage     86.941%   86.942%   +0.001%     
- Complexity     36860     36866        +6     
===============================================
  Files           2211      2211               
  Lines         173376    173376               
  Branches       18710     18709        -1     
===============================================
+ Hits          150734    150736        +2     
+ Misses         16055     16052        -3     
- Partials        6587      6588        +1     
Impacted Files Coverage Δ
...itute/hellbender/tools/LocalAssemblerUnitTest.java 92.448% <0.000%> (ø)
.../hellbender/utils/python/PythonUnitTestRunner.java 78.689% <0.000%> (+3.279%) ⬆️

Copy link
Contributor

@droazen droazen left a comment

Choose a reason for hiding this comment

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

👍



comment:
layout: "header, diff, changes, sunburst, uncovered, tree"
branches: null
behavior: default
after_n_builds: 8 # Wait until all 8 of the test suite jacoco test builds have been uploaded before writing a comment to the pr (no more incomplete coverage report emails)
Copy link
Member

Choose a reason for hiding this comment

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

nice find

Copy link
Member

Choose a reason for hiding this comment

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

https://docs.codecov.com/docs/codecovyml-reference#codecovnotifywait_for_ci is another option that isn't tied to the build specifics

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lbergelson I'm testing it over in #7818. If it works there I'll pull that commit into this branch and merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Louis i've tried a few modes of that option. It doesn't seem to work so far as i can tell. There is probably something I misunderstand. I'm going to merge this approach since it does work.

@jamesemery jamesemery closed this Apr 27, 2022
@jamesemery jamesemery reopened this Apr 27, 2022
@jamesemery jamesemery merged commit 8d63275 into master Apr 27, 2022
@jamesemery jamesemery deleted the je_disableCodeCov branch April 27, 2022 18:59
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.

3 participants