Skip to content

ci: check out the code under test, not always the CI branch - #27

Merged
saqibmanan merged 1 commit into
CIfrom
ci-checkout-pr-content
Sep 3, 2026
Merged

ci: check out the code under test, not always the CI branch#27
saqibmanan merged 1 commit into
CIfrom
ci-checkout-pr-content

Conversation

@saqibmanan

Copy link
Copy Markdown
Contributor

A pull request's checks currently test CI, not the pull request. That has now caused three distinct failures in a single day.

The problem

Every job pins both:

repository: CivicDataLab/CivicDataSpace-test
ref: CI

Pinning repository: is necessary — this workflow is called from DataSpaceBackend and DataSpaceFrontend, and without it a checkout there would fetch the caller's code rather than this suite. Pinning ref: was not.

What it cost

What happened
#24 Added tests that never executed on their own PR. Green, merged — then failed consumer-smoke on the next deploy in a different repo, which rolled the backend back and skipped the frontend's promote-dev.
#20 Checks reported on CI, so the merge being proposed was never tested. Merged on a red check whose cause went unexamined.
#26 The fix for the above could not go green — its checks ran the broken code it was deleting. Needed an override to merge.

None of these were bad luck. A PR whose checks do not test the PR is reporting on something else, and its green means nothing.

The change

ref: >-
  ${{ github.repository == 'CivicDataLab/CivicDataSpace-test'
      && (github.event.pull_request.head.sha || github.sha)
      || 'CI' }}
  • In this repo — check out whatever triggered the run: the PR head for pull_request, the pushed commit otherwise.
  • Called from another repogithub.repository is the caller, so CI is still used. The reusable path is unchanged.

Applied to all three jobs.

Self-testing

This PR is its own test case. pull_request events use the workflow file from the PR head, so the checks on this PR should exercise this change — if they run against this branch rather than CI, it works.

Every job pinned both `repository:` and `ref: CI`. Pinning the
repository is necessary - this workflow is called from
DataSpaceBackend and DataSpaceFrontend, and without it a checkout there
would fetch the caller's code rather than this suite. Pinning the ref was
not, and it meant a pull request's checks ran against `CI` instead of
against the pull request.

That went wrong three separate ways in one day:

- #24 added tests that never executed on their own PR. They passed,
  merged, and then failed consumer-smoke on the next deploy in a
  different repo - which rolled the backend back and skipped the
  frontend's promote step.
- #20's checks reported on `CI`, so the merge being proposed was never
  actually tested. It was merged on a red check whose cause was
  unexamined.
- #26, the fix for the breakage above, could not go green at all: its
  checks ran the broken code it was deleting. It needed an override to
  merge.

None of those were bad luck. A PR whose checks do not test the PR is
reporting on something else entirely, and the green means nothing.

When this workflow runs in its own repository it now checks out whatever
triggered it - the PR head for a pull_request, the pushed commit
otherwise. When called from another repository, `github.repository` is
the caller and `CI` is still correct, so the reusable path is unchanged.

This commit is its own test: if the checks that run on this PR exercise
this change, the fix works.
@saqibmanan
saqibmanan merged commit 651ee12 into CI Sep 3, 2026
3 checks passed
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 this pull request may close these issues.

1 participant