Introduce custom mechanism for exposing coverage information#4146
Introduce custom mechanism for exposing coverage information#4146ScottDugas wants to merge 4 commits into
Conversation
This kind of avoids displaying context around the fiel header
ScottDugas
changed the title
alecgrieser
left a comment
alecgrieser
left a comment
There was a problem hiding this comment.
Overall, this looks good. I guess I had one or two questions, but this also seems like it would be good to merge
|
|
||
| annotation_line = max(1, first_changed_line - 1) | ||
| message = f'File coverage: {file_pct_str} | Changed lines: {changed_pct_str}' | ||
| return f'::notice file={repo_path},line={annotation_line}::{message}' |
There was a problem hiding this comment.
Should this upgrade from notice to warning if the changed percentage is below some threshold (say, 80%)? I could also see skipping this annotation if the coverage is high enough (say, 100%). I believe it's a bit harder, but also getting branch coverage may be nice
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| gh pr diff ${{ github.event.pull_request.number }} > pr_diff.patch | ||
| - name: Annotate Coverage |
There was a problem hiding this comment.
You mention in the PR description that this doesn't remove the JaCoCo report (yet). Is the idea that these annotations are preferable to the old report because they're more noticeable and have the same information? Or would we want to have a markdown summary get generated and put that into the GitHub step summary?
2 participants
This introduces a new mechanism to our pull request builds to add annotations to the diff indicating the coverage of the changed file.
You can see an example of this in: #4147 OR #4164 (same changes but at different points in the history)
To each modified java file, it adds an annotation above the first changed line indicating the total coverage for that file, and the coverage of the modified lines.
We already have teamscale validating that no methods are completely uncovered, but hopefully this will provide some additional information that could expose situations where a code change is under-covered. Further information would need to be gathered by looking at the report directly.
Note: We currently also use https://github.com/Madrapps/jacoco-report which adds similar information to the build, but, we never got the commenting back to the PR to work, and it runs on Node 20, which is no longer going to be supported. This PR does not remove that dependency, but that will probably come later.