Skip to content
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

Automerge despite failing or pending checks #31783

Open
rarkins opened this issue Oct 4, 2024 · 4 comments
Open

Automerge despite failing or pending checks #31783

rarkins opened this issue Oct 4, 2024 · 4 comments
Labels
core:automerge Relating to Renovate's automerge capabilities priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented Oct 4, 2024

Describe the proposed change(s).

For GitHub in particular, it's possible to define required status checks for base branches, which means that merging is allowed even if non-required checks are pending or even failed. Today Renovate is conservative and does not attempt to automerge unless all checks have passed.

We could support a new configuration option automergeWhen, with these values:

  • "all-passed": this matches todays behavior. Only attempt to automerge if all checks are passed
  • "auto": if Renovate detects required checks, it will try automerging if those specifically have all passed, even if other checks are pending or failed. Otherwise default to all-passed
  • "always": Renovate will ignore checks, and always try to automerge (equivalent to today's ignoreTests=true. This means that either (a) users want to immediately automerge without checks, or (b) they rely on branch protection

We'll need to do some research to decide if this logic is best implemented in the worker layer or implemented separately per-platform. We already have a related concept of internalChecksAsSuccess which is implemented completely at the platform layer.

@rarkins rarkins added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others core:automerge Relating to Renovate's automerge capabilities labels Oct 4, 2024
@HonkingGoose
Copy link
Collaborator

Default setting for automergeWhen?

  • "auto": if Renovate detects required checks, it will try automerging if those specifically have all passed, even if other checks are pending or failed. Otherwise default to all-passed

What is the planned default setting for the new automergeWhen config option?

Bad user experience, if default is set to "auto"

  1. User sets Required status checks on GitHub.
  2. Renovate gets new feature automergeWhen, which defaults to "auto".
  3. User updates to new Renovate version.
  4. Renovate automerges a PR with skipped, or failed tests.
  5. User angry at Renovate for automerging a PR with skipped, or failed tests. "This is obviously broken, why did Renovate automerge this?!?"
  6. The user complains on our Discussion forum, and then learns they should have updated their Required status checks on GitHub first, so all their tests are marked as required.
  7. User updates their Required status checks on GitHub.

User must regularly update Required status checks

The user should update the Required status checks when:

  • The name of the test string changes, for example from Build Node 20 to Build Node 22
  • They add new tests
  • They remove outdated tests

It's easy to forget to update the tests. Then users may only notice the problem when Renovate starts automerging things that should not be automerged.

@jontg
Copy link

jontg commented Oct 9, 2024

There's a difference between a test failed and a required status check failed — most of the time developers update the tests that inform a status_check, and not the status_checks themselves.

What's more likely is that developers will add a new type of check [for example, "end-to-end tests" or a new "linting" status_check, or a PR name formatter], and forget to mark that new type of check as required.

@jontg
Copy link

jontg commented Oct 9, 2024

I would expect there to be a 4th option required which will merge if all (zero or more) required status checks have passed, rather than trying to be clever in auto.

@rarkins
Copy link
Collaborator Author

rarkins commented Oct 11, 2024

You've convinced me. No "auto", instead that option can be required-passed. Default can remain as all-passed and users can opt into required-passed. If someone configures required-passed on a platform which doesn't support branch protection (e.g. bitbucket) or on a repo or branch without branch protections, it will behave the same as all-passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core:automerge Relating to Renovate's automerge capabilities priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)
Projects
None yet
Development

No branches or pull requests

3 participants