diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9faabe7..7f08f82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,8 +13,30 @@ permissions: contents: read jobs: - # Keep this job id: main requires the matrix checks by name, and calling through a - # reusable workflow prefixes each with it. + # Keep this job id: it prefixes every check name that calling through a reusable workflow + # produces, so renaming it renames the checks. ci: uses: ./.github/workflows/run_tests.yml secrets: inherit + + # The context to require on main for the suite. Requiring the matrix legs by name instead + # would block every Release PR, because a job-level `if:` is evaluated before the matrix + # expands and a skipped job yields one check run rather than six. This job is not a matrix, + # so it reports either way, and `skipped` counts as satisfied, which is what lets a Release + # PR through. Only a real run of this workflow can republish it, which is why the run-tests + # escape hatch lives in label_tests.yml: a label event reaching here would republish the + # context having tested nothing, turning a red gate green. + tests-passed: + name: 👷 Tests + needs: ci + if: always() + runs-on: ubuntu-latest + steps: + - name: Check the test job + env: + RESULT: ${{ needs.ci.result }} + run: | + case "$RESULT" in + success|skipped) echo "test job: $RESULT" ;; + *) echo "::error::test job reported $RESULT"; exit 1 ;; + esac diff --git a/.github/workflows/label_tests.yml b/.github/workflows/label_tests.yml new file mode 100644 index 0000000..560c2aa --- /dev/null +++ b/.github/workflows/label_tests.yml @@ -0,0 +1,26 @@ +name: label-tests + +# The escape hatch for a Release PR, which build skips. It lives in its own workflow rather +# than as a `labeled` trigger on build, because types: cannot filter by label name, so every +# label would start a build run, and that run republishes the 👷 Tests context having run +# nothing. A red gate would go green the moment anyone added a label. +on: + pull_request: + types: [labeled] + branches: + - main + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # Job id `forced` keeps these check names off `ci / …`, so they never stand in for the + # build run's own checks. + forced: + if: github.event.label.name == 'run-tests' + uses: ./.github/workflows/run_tests.yml + secrets: inherit diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index d3a31c4..abc3741 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -17,26 +17,32 @@ concurrency: permissions: contents: read -env: - # True only for a Release PR opened by release-please and last pushed by it. Keying on the - # branch name alone would let any PR, a fork's included, name its branch release-please--x - # and skip every required check, which branch protection then counts as satisfied. The - # github.actor clause is the pusher rather than the PR author, so a human commit pushed - # onto the Release PR is tested like any other instead of riding the skip into main. - RELEASE_PR: >- - ${{ github.actor == 'github-actions[bot]' - && github.event.pull_request.user.login == 'github-actions[bot]' - && github.event.pull_request.head.repo.full_name == github.repository - && startsWith(github.head_ref, 'release-please--') }} - -# The guard sits on the steps here rather than on the job, unlike the other five SDKs. main -# requires the six matrix legs of test-build by name, and a job-level `if:` is evaluated -# before the matrix expands, which is why the matrix context is not available there. A -# skipped job would produce one check run instead of six, so those required contexts would -# never appear and the Release PR would sit blocked forever. Guarding the steps keeps every -# leg reporting while the 20-minute integration leg is the part that goes away. +# If main ever requires the matrix legs of test-build by name again, this guard must move +# back to the steps. A job-level `if:` is evaluated before the matrix expands, which is why +# the matrix context is not available there, so a skipped job produces ONE check run and not +# six, and those six required contexts would never appear: every Release PR would block +# forever. main currently requires only the PR-title check, which is what makes the +# job-level form safe. Keep the guard on the job rather than on the calling job in ci.yml +# either way, because a job skipped by `if:` reports success and satisfies a required status +# check, while a reusable workflow that is never called produces no check at all. jobs: test-build: + # Skip a Release PR that release-please opened on a branch in this repository. The three + # clauses are all needed: the PR author cannot be forged, but on its own the branch name + # would let any PR, a fork's included, call itself release-please--x and skip everything. + # + # github.actor is deliberately not a clause. CONTRIBUTING tells maintainers to click + # Update branch, which attributes that merge commit to them, so keying on the pusher + # would mean the skip never fires on the documented release path. The cost is that a + # human commit pushed onto a Release PR is not tested here. release.yml runs this same + # workflow against the merge commit before it tags, and that is what gates the release. + # + # Label a Release PR `run-tests` to force the suite to run anyway. + if: >- + ${{ contains(github.event.pull_request.labels.*.name, 'run-tests') + || !(github.event.pull_request.user.login == 'github-actions[bot]' + && github.event.pull_request.head.repo.full_name == github.repository + && startsWith(github.head_ref, 'release-please--')) }} name: 👷 Test & Build ${{ matrix.goVer }}${{ matrix.label }} environment: feeds-enabled-shard runs-on: ubuntu-latest @@ -57,7 +63,7 @@ jobs: go-version: ${{ matrix.goVer }} - name: Unit tests via ${{ matrix.goVer }} - if: matrix.goVer != '1.19' && env.RELEASE_PR != 'true' + if: matrix.goVer != '1.19' env: STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} @@ -65,7 +71,7 @@ jobs: run: go test -short -v -race ./... - name: Integration tests via ${{ matrix.goVer }} - if: matrix.goVer == '1.19' && env.RELEASE_PR != 'true' + if: matrix.goVer == '1.19' env: STREAM_BASE_URL: ${{ vars.STREAM_BASE_URL }} STREAM_API_KEY: ${{ vars.STREAM_API_KEY }} @@ -75,7 +81,7 @@ jobs: go tool cover -func=cover.out - name: Upload coverage to Codecov - if: matrix.goVer == '1.19' && env.RELEASE_PR != 'true' + if: matrix.goVer == '1.19' uses: codecov/codecov-action@v5 with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 413e04b..6047108 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,7 +52,8 @@ Releases are driven by [release-please](https://github.com/googleapis/release-pl - Merge PRs to `main` with conventional-commit titles. The PR title becomes the commit subject and is what determines the next version, so a non-conventional title ships nothing. - release-please keeps a Release PR open with the version bump and the generated changelog. Review it. - The Release PR is opened by `github-actions[bot]`, so its CI runs are held at "action required". If the PR is behind `main`, clicking **Update branch** also releases them, because that commit is attributed to you; otherwise click **Approve and run**. It needs a code-owner approval like any other PR. -- Merge the Release PR. That creates the tag and the GitHub Release, and `proxy.golang.org` picks the tag up. There is no separate publish step. +- The test suite does not run on a Release PR, whichever of those two paths you take. A Release PR only bumps the version and rewrites the changelog, and every commit in it already passed on the PR it came from, so all it can do is fail on the shared integration app and stall the release. `Lint`, `reviewdog`, CodeQL and the PR-title check still run. Label the PR `run-tests` if you want the suite anyway. +- Merge the Release PR. That creates the tag and the GitHub Release, and `proxy.golang.org` picks the tag up. There is no separate publish step. The suite does run before the tag: `release.yml` calls it on the merge commit, and a failure there leaves `autorelease: pending` set and needs the manual recovery below. Only `feat`, `fix`, `perf` and breaking changes produce a release (`revert` may also). A window of only `chore`, `ci`, `docs`, `test`, `refactor`, `style` or `build` commits produces no Release PR, which is intended.