Feature request: conditional required checks

Related:

https://github.community/t5/How-to-use-Git-and-GitHub/Feature-request-Required-checks-conditioned-on-path/m-p/24097/highlight/false#M6916

Github has a feature for requiring checks to pass before a PR can be merged: https://help.github.com/en/github/administering-a-repository/about-required-status-checks

With the introduction of actions, which can be scoped to file paths, it would be nice for this feature to also take into account file paths, either by explicit configuration or deduction from which actions were applied to the PR. This is especially important in monorepos, where status checks vary between PRs (some may have zero, some may have many).

63 Likes

Hello @solarmosaic-kflorenc

Thank you for your feedback! We’re always working to improve GitHub and the GitHub Community Forum, and we consider every suggestion we receive. I’ve logged your feature request in our internal feature request list. Though I can’t guarantee anything or share a timeline for this, I can tell you that it’s been shared with the appropriate teams for consideration.

Once again, thank you for your input!

Greatly appreciated

MChevy422

Would appreciate a non-robotic reply to this as well.

28 Likes

My understanding is that it’s currently not possible to use on: pull_request: paths with required checks. Currently the check can’t pass if it doesn’t run. It seems that on: pull_request: paths, which would otherwise be a really nice feature, is unusable for required checks. Any hope to improve this situation?

5 Likes

I’m having this exact same problem, and temporarily commented out the paths until this feature becomes a reality. I have no better idea @sjackman 

My workaround works but isn’t super pretty. It looks something like this:

- name: Run Rust tests when pushing to master
        if: github.ref == 'refs/heads/master'
        run: echo "::set-env name=run_tests::true"
      - name: Check for modified Rust files
        run: |
          git fetch origin master
          if test -n "$(git diff --name-only origin/master... -- lib/rust)"; then
            echo "::set-env name=run_tests::true"
          else
            echo "No Rust files have been modified."
          fi
      - name: Install Rust
        if: env.run_tests
        run: |
2 Likes

this is a real pain.

since having less actions to run would reduce the cost for using github actions (because of shorter action queues etc) makes it a must have feature.
having this supported natively would boost developer experience a lot.

1 Like

Our current solution is also very hacky. We have a required check setup, ‘ci’, which must run on every PR. We are using a monorepo, so testing requirements vary by PR. We have one generic ‘ci’ job which just automatically passes, and for any files we actually want tests for, we ignore them in that job and run a different same-named job instead.

e.g.

For the generic ci job:

name: ci
on:
  pull_request:
    branches: 'master'
    paths-ignore:
      - 'my-project/**'

For the ‘my-project’ ci job:

name: ci
on:
  pull_request:
    paths:
    - 'my-project/**'
jobs:
  validate:
    runs-on: ubuntu-latest
    steps:
    - name: start-build
      run: ...

This of course makes it challenging to enforce that multiple ‘ci’ jobs must pass against a PR, so it is not ideal, but it’s better than nothing.

A solution that would work well for us would be to be able to control (ideally in workflow configuration) which status checks should be required to run for a given file path.

2 Likes

This is kinda tricky, because the GitHub PR doesn’t inherently know about any potential check runs happening on whatever service they might be happening on, so the check requirement logic doesn’t seem like it can know whether or not a required check is actually going to be run or not.

One might say, “Well, GitHub Actions ought to be able to talk to PR checks and just handle this behind the scenes,” and one might be right about that, but I bet it’s more complex than we think.

I think a good middle ground or shorter-term solution would be to add an optional parameter to on.paths that tells it to return a success if the paths don’t match, rather than skipping the run entirely. Even better would be if checks could have a “not necessary” result or something, which counts as passing for the purposes of check requirements.

@scotchester It seems like just being able to define what checks should be required for a given path glob should resolve this. For example, something like:

on:
  pull_request:
    paths:
    'my-project/**': ['required-check-1', 'required-check-2']

Or, alternatively

on:
  pull_request:
    paths:
      my-project: 'my-project/**'
    required-checks:
      my-project: ['required-check-1', 'required-check-2']

I’m not sure about that. The current place that required checks are defined is on branch protection settings. Having them be defined in different places, if even possible, seems ripe for introducing confusion.

@scotchester right – they need to refactor that feature and integrate it into Github Actions somehow.

as of today I am using this neat action to achieve this thing, which should be supported natively:

2 Likes

I published an Action that solves all those required-check-problems: https://github.com/marketplace/actions/skip-duplicate-actions

This is similar to GitHub’s path filter, but the underlying algorithm is smarter.
You can give it a try if this is still an issue.

Looks interesting, @fkirc! I took a look at the README, and I admit I’m still a little confused.

Let’s say I have a required check for, say JavaScript unit tests. If my action for that uses your skip entire jobs logic to skip the check if a *.js file is not changed, how does the check pass? I’m not seeing how it avoids the same problem as the traditional paths option that GH provides natively.

Perhaps I should explain the algorithm in more detail.
The crucial point is:
My Action will never ever “guess” that a check passes just because the current commit is ignored.
Instead, my Action will look for the last check where your JavaScript tests were successful.
If the diff between the last successful checks and your current commit is in “skipped paths”, only then it will forward-propagate the successful check to the current commit.

Ahhh, very cool! I will try it out, thanks!

We are also facing this problem with our monorepo.

We went with a complex solution since there doesn’t seem to be any native way to do that.
Our solution consists of a GitHub app that is subscribed to status and check_runs events. When it receives an event, it will do some validation and then send it to a queue. There is a worker that pulls messages from the queue and it will combine the status of all current checks (aside from a list of excluded ones) and push that combined status to GitHub as a check that is called “Combined Status”. That check is required on all PRs. We also have a fake check that runs whenever our other checks don’t run and this is handled via some kind of pre-check/should-build that controls all our build triggering logic, because we needed additional logic more than just checking for paths (think project references in C#)

We’ve had to do some optimization on the worker so we don’t get rate limiting errors and it’s been working fine for a while but we’re starting to hit the limit even with the optimizations.

Our need is simple: the ability to specify checks as “Required to pass when present”. That way, we can defer the path triggering logic to the actual builds/actions themselves. If a service is modified in a PR, a check is triggered and then the PR shouldn’t allow the merge until that check is successful.