github-bot icon indicating copy to clipboard operation
github-bot copied to clipboard

feat: update bot comment when Jenkins finishes

Open mmarchini opened this issue 4 years ago • 5 comments

Note: this is a rough draft of a implementation of the "Edit Comment" feature described in https://github.com/nodejs/github-bot/issues/251. I haven't set up my machine to test the bot yet, so this code is completely untested at this point, but it reflects what I'm thinking.

I'm looking for comments on the flow instead of the code (since the code wasn't tested yet). The flow is as follow:

  • When a Jenkins job starts, the bot will leave a comment on the respective PR
    • The comment will have two placeholders created using html comments
      • One placeholder next to the CI url, for the build status
      • One placeholder below the CI url, for a summary of failures when the build fails
    • The placeholders will hold the build ID so that the bot can identify the respective comment later
  • When a Jenkins job finishes, the bot will:
    • Fetch all comments for the respective PR
    • Find the respective comment (if it can't find, skip the next steps)
    • Replace the first placeholder with the build status
    • If the build failed:
      • Run ncu-ci to get a summary of changes, in markdown format
      • Wrap the output with <details> tag.
      • Replace the second placeholder with the wrapped output
    • The bot updates the comment using GitHub API

The result from this workflow will have minimum visual impact on Pull Requests while also adding a rich summary of failures, and will allow us to search for flaky tests easier (if a test failed on multiple PRs and we search for that test on the PRs tab, all PRs where the test failed will show up, making it easier to determine if a test is flaky or not).

There are some requirements for this workflow to work. We need to change our post-build-status-update on Jenkins to publish the build number as well. We also need to ensure ncu-ci can consume the credentials via environment variables (I don't think it can today). As a bonus, if ncu-ci could expose an API to get the markdown results of ncu-ci that'd be great, as we wouldn't need to spawn another process to do that, but I don't think it'll be possible in the near future.


Update the bot comment on a PR when the corresponding Jenkins build finishes. The updated comment will have the build status, and if the build failed, it will have a summary of failures.

To identify a comment corresponding to a PR, we leave placeholders on the comment as HTML commnets, so those placeholders can be replaced later.

Signed-off-by: Matheus Marchini [email protected]

mmarchini avatar Apr 16 '20 23:04 mmarchini

cc @phillipj

mmarchini avatar Apr 16 '20 23:04 mmarchini

Sorry for the late reply on this @mmarchini! Been wanting to have a looksie, but pretty chaotic lately at home due to covid19 & having to kindergarten my kids most of the day..

I'll get to review this as soon as I can.

phillipj avatar Apr 22 '20 08:04 phillipj

I like what your suggesting here 👍

In case you're not aware, we did edit comment in a short periode in 2019. Back then it was mostly to reduce the amount of noise the github bot brings to the PRs in general. There were split opinions amongst core collaborators whether or not that change was good or bad -- some appreciate new comments from the github bot on builds as a ping, others don't. It also tripped some node-core-utils tools which we didn't consider beforehand. Ended up getting reverted in https://github.com/nodejs/github-bot/pull/240.

You're also aiming for only one comment to be posted (and then edited) from the github bot, right? Or a mix where the first github bot comment seen in a PR contains "last status" from the latest job, in addition to several individual comments representing each and every job that has been kicked off related to that PR that does not get edited?

will allow us to search for flaky tests easier

Sounds really valuable if we could utilise these comments for this purpose!

Have you had the chance to share and get feedback about this with core collaborators yet? As hinted at before, although well intentioned, there might downsides we are not considering. Would be a shame if we ended up having to revert this effort like we did with the last edit comment experiment 😬

phillipj avatar Apr 30 '20 20:04 phillipj

You're also aiming for only one comment to be posted (and then edited) from the github bot, right?

No, it will search for the appropriate comment for that particular CI run.

Have you had the chance to share and get feedback about this with core collaborators yet?

Yes, no objections so far: https://github.com/nodejs/node/issues/32306

I'm working on something else now, but I didn't forget this. I'll try to get back to it at some point, maybe breaking this PR into smaller pieces.

mmarchini avatar Jun 27 '20 05:06 mmarchini

Cool! 👍

Heads up about this PR that changes quite a few files that I'm assuming might be affected by what you're working on: https://github.com/nodejs/github-bot/pull/258. Planning to land that soon-ish.

phillipj avatar Jun 28 '20 19:06 phillipj

I'm going to close this as stagnant but by all means, re-open if anyone plans on picking this up.

Trott avatar Aug 25 '22 03:08 Trott