[ANCHOR-413] Add jacoco test report to PR workflow #1062
Conversation
JiahuiWho
force-pushed
the
anchor-413
branch
5 times, most recently
from
ac2c734
to
f94c902
Compare
JiahuiWho
force-pushed
the
anchor-413
branch
2 times, most recently
from
69fd466
to
14d926a
Compare
|
Reference Server Preview is available here: |
|
Reference Server Preview is available here: |
|
Reference Server Preview is available here: |
1 similar comment
|
Reference Server Preview is available here: |
|
Reference Server Preview is available here: |
1 similar comment
|
Reference Server Preview is available here: |
JiahuiWho
force-pushed
the
anchor-413
branch
from
8a3ab76
to
d08292c
Compare
|
Reference Server Preview is available here: |
JiahuiWho
changed the title
JiahuiWho
force-pushed
the
anchor-413
branch
from
d08292c
to
b6e41c7
Compare
|
Reference Server Preview is available here: |
JiahuiWho
changed the title
| min-coverage-overall: 40 | ||
| min-coverage-changed-files: 60 |
There was a problem hiding this comment.
Can this check be moved into the gradle build instead? That way, the developer can get feedback locally without publishing a PR.
There was a problem hiding this comment.
Also, do we want to set coverage minimums by project? I think it would make sense to have the core project require higher coverage than the reference server, for example.
There was a problem hiding this comment.
Can this check be moved into the gradle build instead? That way, the developer can get feedback locally without publishing a PR.
Yes The JacocoCoverageVerification task can be used to verify if code coverage metrics are met based on configured rules. However the build fails if any of the configured rules are not met and JaCoCo only reports the first violated rule. I will gradually implemented in the following PR.
Right now you can simply run jacocoTestReport in local to get an report but without the delta.
Also, do we want to set coverage minimums by project? I think it would make sense to have the
coreproject require higher coverage than the reference server, for example.
I think that's part of the first question where we want enforce more granular test coverage rules.
There was a problem hiding this comment.
Yes The JacocoCoverageVerification task can be used to verify if code coverage metrics are met based on configured rules. However the build fails if any of the configured rules are not met and JaCoCo only reports the first violated rule. I will gradually implemented in the following PR.
So once we enable JacocoCoverageVerification, do we still want to set min-coverage-overall and min-coverage-changed-files in the Github action?
|
Reference Server Preview is available here: |
|
Reference Server Preview is available here: |
|
Reference Server Preview is available here: |
Description
This is to add jacococ test report to PR workflow, to monitor test coverage change
Testing
After PR being published, there should be a github-action bot comment in your PR with test coverage.
Please see #1081 (comment) for example.
Note that this PR is published from a branch on original repo instead of a forked repo.
Known limitations
For now Github only grant apps write permission (which is needed to make comment) to original repo, and read permission to forked repo, that mean this action will only works on PR created from original repo. I have created a ticket https://stellarorg.atlassian.net/browse/ANCHOR-433 to follow up on this and try to find a work around for forked repo
For more info about github permission please refer to https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token
Currently the min-coverage-overall is set to 40 and min-coverage-changed-files is 60, this is just for testing, will be changed accordingly.