Skipping a matrix job hangs the Pull Request when inner-matrix jobs are required status checks

[billyvg]

Originally posted by @minut1bc at actions/runner#952 ...

Jobs with a build matrix configuration can be skipped from the top level, before expanding the matrix. This is problematic when the inner-matrix jobs are set as required status checks for a branch protection rule, resulting in the Pull Request hanging forever.

The current dirty fix for this is copying the top level if conditional, for each and every step inside the job, which results in a lot of duplicated code, as well as still having to create and run each inner-matrix job, even though every step is skipped. Additionally, this has the downside of setting the job result to success, instead of skipped. This may be unwanted behaviour in cases where another job would depend on this one, and on it's result being skipped for some conditions.

It would be possible to instead only require the whole matrix as the single status check, but this is not ideal in cases where only some of the inner-matrix jobs are actually a requirement. Again, splitting off into two jobs, with required checks and optional checks, would present a lot of duplicated code, as well as extra internal work for creating the additional top level job.

It would be a lot more efficient to internally expand the job matrix, without running all the jobs, and instantly skipping them in such cases.

The Pull Request that demonstrates this behaviour can be found here, with the workflow code here.

[ethomson]

Thanks for the feedback - obviously there's a lot of considerations that we need to make when we think about required status checks, as many people really expect required status checks to really be required and that not running them (even due to configuration) is not sufficient. We'll have to think carefully about any changes, but it's definitely something that we'll consider. Thanks again.

[chuckatkins]

That's already the current behavior for jobs without the matrix strategy. Currently if you have a required non-matrix job then it can be skipped and branch protection requirement is still be satisfied. So really it's just asking that matrix jobs be given the same capability.

As for an implementation at the branch protection level, what if we had the additional ability to specify an entire matrix job as required instead of only being able to require each matrix expansion individually? If the job runs then the protection rule would only be satisfied when all expanded matrix entries pass, and if it's skipped then it's still satisfied like it is now.

[billyvg]

It would be nice if we could end a job early with a specified conclusion (e.g. skipped). That way, instead of requiring an if condition for each step, we can check a condition early on in the workflow and end early.

[oshaughnessy]

Would it be possible to handle this at the branch protection level, by allowing users to define the conditions of their branch protection? Instead of assuming it's always "success", how about allowing us to override that by allowing "success" or "skipped" for a given workflow requirement? In my situation, I see required jobs stuck in pending status because some of their steps were skipped, even though all the steps that did run were successful.

[mwaddell]

One way to solve this problem that would also provide additional flexibility is to execute the strategy before the if for the job (instead of the other way around like it is now) and then execute the if once for each matrix entry. This way, the if could refer to ${{ matrix.os }} to provide the ability to have things like this:

# Run on all OSes for pushes, but only run on ubuntu for PRs
if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && matrix.os == 'ubuntu-latest') }}

This way, if an innner-matrix job is a required status check, it will be individually skipped and not block the build.

[hanseartic]

exactly the case i was even expecting to work; have all the expanded jobs skip that don't have changed files. would be genius if we had that.

[chuckatkins]

Another way to work around this currently is to use a dummy collector job that depends on the matrix jobs. Then in your branch protection rules just set the dummy collector job as required. When the matrix jobs are skipped so will the collector job and the protection rule will be satisfied. When the matrix jobs are not skipped then the collector job will only run and pass once all matrix entries for each job in the collector job's needs list succeed. I'm currently implementing this to skip build and test jobs for documentation-only changes.

[SimonHeimberg]

What happens if a matrix job fails? 🤔 The dummy collector job is skipped, which looks the same as with skipped matrix job. 😕
Do you have a working example?

[anantakrishna]

What happens if a matrix job fails? 🤔 The dummy collector job is skipped, which looks the same as with skipped matrix job. 😕

You can avoid skipping it with if: always(). See https://github.com/orgs/community/discussions/26822#discussioncomment-3305794

[lars-reimann]

I just ran into this issue when I wanted to skip certain jobs that use a matrix if no related files were changed. If the job is run, it has the name build (17) and when it's skipped the check is called build (entire job is skipped, matrix is not evaluated). This makes it impossible to require either of them easily. The current workaround of collecting the individual results in another job is not ideal, since it clutters the check window on PRs unnecessarily.

I'd rather be able to create all jobs specified by the matrix but then skip them individually or all at once. Maybe there should be a step to skip the remainder of a job like so:

    steps:
      - name: Skip check
        if: # code is unchanged
        run: # skip remaining steps

Though I guess that'll cause issue with billing, so maybe @mwaddell's solution is better.

[marcinkubica]

I'd agree the current if evaluation for the matrix is done too late in the game, spawning matix jobs just to skip the steps. Then the jobs result is basically incorrect. IMHO this problem should be treated as a bug to fix, not a new feature to provide.

As you see the only way to tell if I have jobs skipped in the matrix it to look at the execution time :) while "the normal" job result is skipped as expected.

[indiVar0508]

++

[mwaddell]

actions/runner#662 would be another way to solve this (where you could exit a workflow early but successfully)