Oppiabots response when a PR fails a check suite #110
Conversation
There was a problem hiding this comment.
Hi @jameesjohn, looks good overall, took first pass, PTAL. Thanks!
lib/ciChecks.js
Outdated
| // limitations under the License. | ||
|
|
||
| /** | ||
| * Check for CI test failure and pings PR author. |
There was a problem hiding this comment.
This function checks for...
lib/ciChecks.js
Outdated
| // A checksuite might fail due to a merge to develop. We don't need to | ||
| // check for these since they are not failures related to a pull request. | ||
| if (checkSuite.pull_requests.length > 0) { |
There was a problem hiding this comment.
I did not understand this part - are you talking about the failure which happens when we merge to develop before running tests?
There was a problem hiding this comment.
Some checks fail in develop due to a PR being merged. These checks are not affiliated to any pull request since they happened on the develop branch, and as a result, will lead to an empty pull_requests array from the payload.
There was a problem hiding this comment.
Ah ok - thanks for explaining. Add your reply to the comment in file as well.
lib/ciChecks.js
Outdated
| body: | ||
| 'Hi @' + | ||
| prAuthor + | ||
| ', there are some failing CI checks in your latest push, PTAL' + |
There was a problem hiding this comment.
Can we also add a link to the failing test?
There was a problem hiding this comment.
Also, add in the comment that if you think that the test is a flake please file an issue before restarting.
There was a problem hiding this comment.
Also, add in the comment that if you think that the test is a flake please file an issue before restarting.
Done
Can we also add a link to the failing test?
No, because what we are listening to is the check suite and not the individual check run.
I didn't think it will be okay for oppiabot to comment on every failing test which is why I'm checking for a failure in the check suite.
lib/ciChecks.js
Outdated
| // A checksuite might fail due to a merge to develop. We don't need to | ||
| // check for these since they are not failures related to a pull request. | ||
| if (checkSuite.pull_requests.length > 0) { |
There was a problem hiding this comment.
Ah ok - thanks for explaining. Add your reply to the comment in file as well.
|
Since we are listening on check suite, we will ping the user once (after all the tests are completed). |
|
Hi @jameesjohn. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks! |
There was a problem hiding this comment.
LGTM, Thanks @jameesjohn! Let me know when I should deploy.
Explanation
Comments on the PR and assigns the PR author when the PR fails a check suite.
This feature has been tested at jameesjohn/comment-on-pr#41
Checklist