Repository navigation
CI: required check "Build, Unit and Functional Tests Successful" is skipped (not failed) when functional_tests or upgrade_tests fail #2153
Description
The result job in .github/workflows/build.yaml ("Build, Unit and Functional Tests Successful", which the step comment identifies as the check required for pull requests) has needs: [functional_tests, upgrade_tests] and no if::
VFSForGit/.github/workflows/build.yaml
Lines 344 to 351 in 9f524a6
With no status function in if, GitHub applies success() implicitly, so when functional_tests or upgrade_tests fails, times out or is cancelled, result is skipped rather than failed. GitHub's documentation states that a job skipped by a conditional reports "Success" and "will not prevent a pull request from merging, even if it is a required check" (about status checks). So if this is the only required check, a PR with failing functional tests could be merged (or auto-merged). If the test jobs are also required in the branch rules, this is moot.
The validate job already avoids job-level skips (it runs the jobs but skips their steps), so result can be strict. Suggested change:
result:
runs-on: ubuntu-latest
name: Build, Unit and Functional Tests Successful
needs: [functional_tests, upgrade_tests]
if: ${{ !cancelled() }}
steps:
- name: Check needed jobs
if: ${{ needs.functional_tests.result != 'success' || needs.upgrade_tests.result != 'success' }}
run: exit 1
- name: Success! # for easier identification of successful runs in the Checks Required for Pull Requests
run: echo "Workflow run is successful!"Happy to open a PR if that's useful.
Found by static analysis of the public workflow file as part of a research study on CI merge gates; nothing was run against this repository. Prepared with AI assistance (analysis and write-up).
Activity
dscho commentedon Nov 4, 2026
How certain are you about this? Honestly, it's been too long that I worked on this workflow job to remember whether I verified that a failure in one of the prerequisites would automatically fail that required check.
Since you already used AI to prepare this analysis, maybe you can also use AI to generate a quick proof that one failing check will still mark the entire thing as a success because the required check is not failing?
sotomayorlucas commentedon Nov 4, 2026
Fair question, so we measured it directly rather than relying on the docs. Minimal reproduction in a public throwaway repo: sotomayorlucas/ci-investigation#1 (workflow).
testsrunsexit 1;gate_dflthasneeds: testsand noif:, i.e. the same shape asresulthere.testsfails andgate_dfltends skipped.gate_dfltas the only required check (tried both a ruleset and classic branch protection), the PR stays mergeable: the API reportsmergeable: true, mergeable_state: "unstable", and the merge box shows the skipped check tagged Required with the Merge button enabled.mergeable_state: "blocked"; a job skipped by its ownif: falsebehaves likegate_dflt(mergeable).This matches GitHub's docs ("A job that is skipped will report its status as Success. It will not prevent a pull request from merging, even if it is a required check."). The comment in
validateabout skipped jobs not meeting the requirement is right for a different case: when a whole workflow doesn't run (e.g. path filters), its check never reports and the PR waits for it. A job skipped inside a workflow that ran reports Success.So this only matters if
Build, Unit and Functional Tests Successfulis the required check andfunctional_tests/upgrade_testsare not required themselves. If that's your setup, the change suggested above makesresultfail when either need fails or is cancelled. Happy to open a PR.dscho commentedon Nov 4, 2026
Lucas (@sotomayorlucas) thanks for verifying. The idea of that
resultjob was to have a single required check in lieu of the sorely missing option to mark a workflow as required to pass.To be honest, I did not even expect a job that is skipped due to failing prerequisites to be eligible for qualifying as a "passing required check". So that's two black marks on the design of GitHub Actions here.
So yes, I would love to approve and merge a PR to work around these problems. I want a single job with a defined and non-mutable name to be used as required check, and I don't want that job to pretend to pass when anything in that workflow run failed. Thank you for identifying this as a problem that needs to be fixed.
tyrielv commentedon Nov 4, 2026
I'm surprised by this to. I would have sworn that it is working correctly and blocking PRs from merging if they have failing tests.
tyrielv commentedon Nov 4, 2026
Johannes Schindelin (@dscho) quick test - I marked a draft "Ready for Review" #2147 which is failing one of the test jobs - can you approve it and see if it becomes merge eligible?
build: make the required `result` check fail instead of skip
dscho commentedon Nov 4, 2026
Tyrie Vella (@tyrielv) done. Unfortunately: