Skip to content

"Tests pass" blocks PRs that don't need tests #1608

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

Closed
EliahKagan opened this issue Sep 24, 2024 · 0 comments · Fixed by #1607
Closed

"Tests pass" blocks PRs that don't need tests #1608

EliahKagan opened this issue Sep 24, 2024 · 0 comments · Fixed by #1607

Comments

@EliahKagan
Copy link
Member

EliahKagan commented Sep 24, 2024

Current behavior 😯

Tests run on pull requests when changes are made to files in:

https://github.com/Byron/gitoxide/blob/612896d7ec15c153d3d48012c75aaf85d10a5abe/.github/workflows/ci.yml#L28-L37

Otherwise they are intentionally skipped as unnecessary.

That applies to the ci.yml workflow as a whole. The "Tests pass" check, which is defined in that workflow, as well as the actual test jobs it depends on, do not run on pull requests that don't make any of those modifications. An example of such a pull request is #1607.

Expected behavior 🤔

If there is a way to do it that works well, only relevant checks should be required for pull requests.

A possible alternative is to keep things as they are and handle merging of such pull requests manually. Most pull requests do run tests, so auto-merging is still very useful even if this is not fixed. But I suspect there may be a reasonable way to achieve the effect of only requiring checks when they are relevant.

Another consideration, though I'm not sure if it matters, is that PRs are sometimes opened with minimal initial contents, sometimes even empty. Such a PR intentionally does not run unnecessary tests when opened. There might be a disadvantage to such a PR being auto-mergeable, even if it has a merging review from a user whose reviews would otherwise permit that.

Git behavior

As far as I can quickly tell, neither git/git nor git-for-windows/git use the GitHub auto-merging feature. (git/git is also a mirror, and its PRs work via GitGitGadget, so it wouldn't necessarily be comparable in this area.)

Steps to reproduce 🕹

There are two ways to verify this:

@EliahKagan EliahKagan changed the title The "tests pass" CI check waits forever on PRs that don't need tests "Tests pass" blocks PRs that don't need tests Sep 24, 2024
@Byron Byron linked a pull request Sep 24, 2024 that will close this issue
@Byron Byron closed this as completed in bfe363c Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant