Feature request: alert PR author in case of CI failure #27178

1. 
   stickies-v commented at 4:00 pm on February 28, 2023: contributor

   Would be nice to have a means of quickly getting alerted about the CI failing on a PR you authored. It’s a bit cumbersome to keep an eye out on it manually, and a freshly created PR typically gets more eyes on it so having it in failing state is a bit of a waste of everyone’s time. Creating this issue after brief IRC discussion.

   I think the main requirements are:

   • be able to opt out or easily ignore/hide
   • only get a single notification per run, not for every single job failing
   • low-touch, easy to maintain

   Some potential approaches I see:

   1. Use GitHub actions as recommended by Cirrus CI documention, e.g. here’s a sample email workflow

   • can we get this to work so the PR author is notified?

   2. Use bitcoin-git IRC bot to DM the nickname that corresponds to the PR author’s GitHub nickname

   • people with a different username on IRC and GitHub would not get notifications
   • to opt-out, I think users could simply use /ignore bitcoin-git?

   3. Use drahtbot to tag the author as assignee when CI fails (and remove them as assignee upon force push)

   • haven’t come up with an elegant way for people to opt-out of this, but arguably it’s also the least intrusive option and e.g. could be filtered on the mailbox level?
2. stickies-v added the label Feature on Feb 28, 2023
3. 
   maflcko commented at 4:44 pm on February 28, 2023: member

   Two notes:

   • There are still false positives, so in the end a human has to look at the result and figure out whether it needs to be fixed on master or the pull, or none (re-run due to network issue) .
   • One can keep a background tab open until it shows the green checkmark.
4. 
   maflcko commented at 2:14 pm on March 3, 2023: member

   Some more notes:

   • I think we don’t want to send out notifications for each failed task, only one if any task failed.

   So what about adding a new label (yes moar labels) “CI failed” (or so) with the same process as “Needs rebase”? This would notify ~everyone, not only the author, but I think this is what we want. Because notifying the author in case of a intermittent CI network error is entirely pointless if they can’t re-run the task anyway. For example, I can’t re-run tasks anymore, so sending a notification to me seems just a needless hop. For intermittent issues, it should be fine to ask the author to open/search for the issue report. However, they’ll still need to re-run after sending the report, which again isn’t something they can do?

5. maflcko added the label Tests on Mar 3, 2023
6. 
   stickies-v commented at 1:34 pm on March 15, 2023: contributor

   Because notifying the author in case of a intermittent CI network error is entirely pointless if they can’t re-run the task anyway.

   I think having a “CI failed” label is not a bad idea, but I don’t agree that notifying everyone is desirable. Afaik everyone can re-run the CI on their own PRs, if they connect their github account to cirrus? At least I’m able to, and I don’t think I have any special privileges. For example, on https://cirrus-ci.com/task/5819454748098560 I have this option:

   Unless I’m missing anything there, an author-only notification would be preferred imo.

7. 
   maflcko commented at 2:02 pm on March 15, 2023: member

   I am mostly thinking that merely assigning the author or applying a label does not provide enough information. Only pressing the re-run button is rarely the right choice (correct me if I am wrong):

   • Intermittent Network failure (should be rare) -> Re-run (if “author” and “member”), else ask a maintainer for re-run.
   • Pre-existing test failure -> Search/File issue, then Re-run.
   • Test failure introduced in the pull -> Author needs to fix it.

   But I guess, if the Label workflow is chosen, the info can be attached to the label description?

8. 
   stickies-v commented at 3:38 pm on March 15, 2023: contributor

   Drahtbot could also update its comment to prefix it with instructions on what to do in case of CI failure? Also, I don’t think I’ve actually ever received an email from label changes and can’t find any settings or documentation for it?

   So, perhaps the combination of DrahtBot:

   1. assigning author to the PR to trigger a notification (the notification itself won’t really contain any useful info, but I don’t think it’ll necessarily be too confusing either)
   2. adding the “CI failed” label to make the problem more visible as well as searchable to everyone
   3. adding instructions on what to do in case of CI failure to the DrahtBot comment which nowadays always comes as the first comment on the PR and thus should be properly visible

   I think this would adequately address all 3 scenarios?

9. 
   maflcko commented at 12:52 pm on March 31, 2023: member

   Can an admin enable check suites for the webhook in all repos, please?

   

10. 
    fanquake commented at 12:55 pm on March 31, 2023: member

    enable check suites for the webhook in all repos, please?

    Should be available now.

11. 
    fanquake commented at 12:56 pm on March 31, 2023: member

    Use drahtbot to tag the author as assignee when CI fails (and remove them as assignee upon force push)

    Let’s not do this, because it makes assignment potentially useless otherwise.

12. DrahtBot added the label CI failed on Mar 31, 2023
13. 
    maflcko commented at 2:25 pm on March 31, 2023: member

    Can I also get check runs? Looks like no events are sent out for check suites on a re-run.
14. 
    fanquake commented at 3:53 pm on March 31, 2023: member

    Can I also get check runs?

    Should be available now.

15. maflcko added the label Brainstorming on Apr 11, 2023
16. 
    maflcko commented at 2:29 pm on April 11, 2023: member

    I don’t see how this should be implemented. The Check suites don’t link back to the pull request. Example:

    • https://api.github.com/repos/bitcoin/bitcoin/check-suites/12155872750

    And the pulls or status API doesn’t link to the check suite. Example:

    • https://api.github.com/repos/bitcoin/bitcoin/commits/fa4a46de0b3c1a5895e95dba7e95278932fbfc2c/status
17. maflcko added the label Upstream on Apr 11, 2023
18. 
    maflcko commented at 12:46 pm on April 19, 2023: member

    Ok, I found a hacky way to implement this:

    GitHub check_suite event -> list check runs via GitHub API -> call cirrus CI API to get the task’s build’s pullRequest :smiling_face_with_tear:

    Let me know if there are any bugs, questions or feature requests. Other than that, I think this can be closed?

19. 
    stickies-v commented at 1:07 pm on April 19, 2023: contributor

    Nice! Thank you for adding this to Drahtbot. Looks good, just looked at a PR with a recent CI failure and seeing this:

    Don’t think this needs anything more - closing.

20. stickies-v closed this on Apr 19, 2023

    ---

21. fanquake removed the label CI failed on Apr 19, 2023
22. bitcoin locked this on Apr 18, 2024

Contributors
stickies-v maflcko fanquake

Labels
Feature Brainstorming Tests Upstream