-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: include coverage message #159
feat: include coverage message #159
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add tests for the changes?
Yes, I will try to add tests. Although, it will probably be quite tricky to test something like the Before I continue to write tests I will refactor. Anyways, are we happy with the generated formats? -- Update: 11 August 2022 I haven't forgotten about this. I have been on some time off. Will resume one prioritised. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored code as said in the previous comment. Designed unit tests for the new added functions.
Missing an integration test that checks the run method acts as expected. Needs further discussion to whether we want to test with a real endpoint or we want to fully mock.
(cc: @felangel )
}); | ||
}); | ||
|
||
test('formatCoverageAsMessage returns normally', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More test can be added to verify the formatting is correct. I'll wait until an agreed format is decided. Currently working on the formatting with @scarletteliza 👀
// TODO(alestiago): Figure out how we want to test this, since | ||
// we are mocking the github context and octokit but we seem to | ||
// be unable to do so within the cp.execSync call. | ||
// expect(createComment).toHaveBeenCalled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Automatically adds a comment to the pull request with coverage information. If a comment already exists it automatically updates the comment with the new information to avoid polluting the conversation history.
Includes two new parameters:
report_coverage_comment
which reports coverage information as a pull request comment.github_token
token required to comment on a pull request.Aims to resolve #101 .
Comment formats
Successful comment looks like:
✅ VeryGoodCoverage
Coverage: 100% (100 of 100 lines)
Min coverage difference: 0% (100% / 100%)
Unsuccessful comment looks like:
❌ VeryGoodCoverage
Coverage: 95% (95 of 100 lines)
Min coverage difference: -1% (95% / 96%)
Lines not covered
- /Users/felix/Development/github.com/felangel/bloc/packages/bloc/lib/src/bloc_observer.dart: 20, 27, 36, 43, 51Type of Change