Skip to content

ENT-5541: Publish PR coverage#3200

Merged
jirihnidek merged 1 commit intocandlepin:mainfrom
m-horky:mhorky/5541_coverage
Feb 2, 2023
Merged

ENT-5541: Publish PR coverage#3200
jirihnidek merged 1 commit intocandlepin:mainfrom
m-horky:mhorky/5541_coverage

Conversation

@m-horky
Copy link
Contributor

@m-horky m-horky commented Feb 1, 2023

  • Card ID: ENT-5541

Publish coverage report as a comment under the PR.

  • To send the comment only once, not for every matrix system, only
    'Fedora latest' is used as a data source.
  • Because of how GitHub permissions work, the comment is only sent if
    the PR originates from a feature branch; nothing will be sent for a PR
    originating from some fork. Full coverage is still displayed in CI
    output, it is just not sent as a comment.
  • Package 'pytest-cov' is used instead of 'coverage'. It is still using
    coverage in a background, but it runs it as a pytest addon instead,
    allowing us to pass its arguments into pytest, instead of wrapping
    whole pytest call inside of a coverage invocation.

@m-horky m-horky force-pushed the mhorky/5541_coverage branch 12 times, most recently from 331e0a9 to f889f16 Compare February 2, 2023 10:34
@m-horky m-horky marked this pull request as ready for review February 2, 2023 10:37
* Card ID: ENT-5541

Publish coverage report as a comment under the PR.
- To send the comment only once, not for every matrix system, only
  'Fedora latest' is used as a data source.
- Because of how GitHub permissions work, the comment is only sent if
  the PR originates from a feature branch; nothing will be sent for a PR
  originating from some fork. Full coverage is still displayed in CI
  output, it is just not sent as a comment.
- Package 'pytest-cov' is used instead of 'coverage'. It is still using
  coverage in a background, but it runs it as a pytest addon instead,
  allowing us to pass its arguments into pytest, instead of wrapping
  whole pytest call inside of a coverage invocation.
@m-horky m-horky force-pushed the mhorky/5541_coverage branch from f889f16 to ea495a6 Compare February 2, 2023 12:41
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I'll leave it to @jirihnidek for an additional review, as I know he uses/checks the coverage details often when working on sub-man.

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

ACK

@jirihnidek jirihnidek merged commit 703c212 into candlepin:main Feb 2, 2023
@m-horky m-horky deleted the mhorky/5541_coverage branch February 2, 2023 15:28
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