DEV Community

Sergey Shinder
Sergey Shinder

Posted on

Our integration tests skipped themselves on every Dependabot pull request

In April our checkout started failing for every customer who paid by bank transfer. The cause was a minor version of our payment provider's SDK, merged automatically two days earlier, which had renamed a field in the response we read to get the transfer reference. Our integration suite calls the provider's sandbox and reads exactly that field. It had passed on the pull request. Or rather, it had not failed.

We let Dependabot open pull requests for dependency updates, and a workflow enables auto-merge for patch and minor versions once the required checks are green. The integration tests need a sandbox API key, stored as an Actions secret. Years ago somebody made them skip when the key is missing, with a pytest skipif on the environment variable, so that developers without a key could still run the suite on their laptops.

GitHub runs workflows triggered by Dependabot as if the pull request came from a fork. The GITHUB_TOKEN is read only, and Actions secrets are not passed to the run at all. Dependabot has its own secret store for exactly this reason, and we had never put anything in it. So on every Dependabot pull request the key was empty, all forty two integration tests reported skipped, pytest exited with zero and the check was green. It had been that way for as long as auto-merge had existed, eighteen months. We had been merging every dependency update on the strength of the unit tests alone.

Nobody noticed because a skipped test is drawn in grey, on a page nobody opens when the tick is green.

We pinned the SDK back that morning and had transfers working again within the hour. The sandbox key is now also a Dependabot secret. The skip only applies outside CI: when the CI variable is set and the key is missing, the suite fails at collection with a message saying which secret is absent. The integration job writes the number of tests it ran and the number it skipped to the job summary, and a final step fails the job if more than two were skipped. And auto-merge now requires that job by name, not just a green status on the pull request.

A test that skips itself has decided, on your behalf, that its answer does not matter today. Ours had decided that for every pull request we no longer read.

– Sergey Shinder

Top comments (1)

Collapse
 
howcani_howcani_77e786a89 profile image
howcani howcani •

The green tick was telling the truth, which is the uncomfortable part. On protected branches, GitHub's own rule is that a required status check has to be successful, skipped, or neutral — so a check whose tests all skipped satisfies the gate by design: "Required status checks must have a successful, skipped, or neutral status before collaborators can make changes to a protected branch." Your tick wasn't lying; it was answering a narrower question than the one you were asking.

The word also does opposite things depending on where the skip lands, which is what makes the green feel safe:

  • a workflow skipped by branch filtering, path filtering, or a commit message stays Pending, and a PR requiring it is blocked from merging;
  • a test or a job that skips itself reports skipped, and the required check is satisfied.

Same grey word in the UI. Only the first one stops you.

I rebuilt your shape locally to see where the information actually survives — three tests behind skipif on an env var, no key set: 3 skipped in 0.01s, exit code 0. And the JUnit XML a dashboard would parse reads <testsuite errors="0" failures="0" skipped="3" tests="3">. So any gate that checks failures == 0 reproduces your eighteen months silently, with not even a grey row to look at. With the key present the suite caught it immediately (KeyError: 'transfer_reference'). That is the part worth saying out loud: the tests were never wrong, they were never allowed to speak.

One door left after your fix — the guard itself. If the job that counts the skips can be skipped (a job-level if:, or a guard step without if: always()), the count never gets written, and a skipped check still satisfies the required check. Give the count its own job, so the required name cannot be produced by a skipped run:

  skipped-guard:
    needs: [integration]
    if: always()
    runs-on: ubuntu-latest
    steps:
      - uses: actions/download-artifact@v4
        with: { name: junit, path: . }
      - run: |
          n=$(grep -o 'skipped="[0-9]*"' report.xml | grep -o '[0-9]*')
          echo "skipped=$n"; test "$n" -le 2
Enter fullscreen mode Exit fullscreen mode

"Nobody noticed because a skipped test is drawn in grey, on a page nobody opens when the tick is green" is the line I would steal. The version of it that keeps me honest: at the API level, the grey and the green are the same status.