-
Notifications
You must be signed in to change notification settings - Fork 421
Description
#1788 removes the regression-coverage-report check's code that comments on the PR with the output of the regression test coverage check, because the way it's implemented is unsafe.
Relaying a discussion over email with myself, @jesdaigle, @mcking65, @nschonni:
I wanted to learn why the check is posting a comment to the PR:
In Travis-CI, this was a perma-failing job through “Allowed Failures”, and GitHub does not have an equivalent actions/runner#2347. I think it was also just an ignored, since it was only there if you clicked through
the checks tab in a PR is kind of ugly with screen readers. It is tricky to work. Seems to have some kind of scroll-related issues.
So there are several problems here:
- The regression test coverage report has errors on main.
- The regression test coverage report is being ignored.
- GitHub Actions doesn't allow non-blocking failing checks. (Though, allowing them makes the check easier to ignore.)
I think we should address the first point by fixing the errors.
I think we should address the second point by enforcing the check, so the main branch remains in an evergreen no-error state.
Then the third point is a non-issue, since we want it to be a blocking check to not fall back into the state of having errors and the check being ignored.
So then where and how should we present the output of the check? Here are some options:
- Show the output formatted in the same way as the comment but in the Checks tab. When the check fails, the "Details" link should show the right page.
- Annotate the "Files changed" tab with review comments on the relevant lines in the source.
- Have an external page, and link to it from the Checks page.
These aren't necessarily mutually exclusive. A combination of (1) and (2) could be good, if we can get accessibility issues fixed or if we can work around them somehow.
Possible changes we could propose to GitHub:
- Change GitHub UI to fix accessibility issues with the Checks page. (@mcking65 mentioned that it doesn't work well with screen readers. I'd like to learn more about what the issues are specifically.)
- Change GitHub API to allow bots to comment with a comment-only access token, without full write access.
- Change GitHub Actions to allow non-blocking failing checks. Please support something like "allow-failure" for a given job actions/runner#2347