ci: check out the code under test, not always the CI branch - #27
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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. Pinningref:was not.What it cost
consumer-smokeon the next deploy in a different repo, which rolled the backend back and skipped the frontend'spromote-dev.CI, so the merge being proposed was never tested. Merged on a red check whose cause went unexamined.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
pull_request, the pushed commit otherwise.github.repositoryis the caller, soCIis still used. The reusable path is unchanged.Applied to all three jobs.
Self-testing
This PR is its own test case.
pull_requestevents 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 thanCI, it works.