From 955e04addcec0c3d6a6ea9fc2e48cdc4e6925de9 Mon Sep 17 00:00:00 2001 From: Ed Snible Date: Thu, 13 Aug 2026 13:35:08 -0400 Subject: [PATCH] Run the repo checks on pull requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds .github/workflows/ci.yml: one job running gofmt, a go.mod tidy check, vet, build, and the test suite on every pull request and on pushes to main. The steps mirror the Makefile targets a contributor runs locally, so a CI failure means the same command fails on their machine. gofmt is checked with -l rather than `make fmt`, which rewrites files — CI must report formatting, not silently fix it and then test code the author never wrote. Tests run three times: plain, under -race, and with -shuffle=on. The shuffle is not redundant. The suite mutates process state (HOME, XDG_CONFIG_HOME, cobra flag values), and cmd/root_test.go documents a pflag hazard where a slice flag's first Set in a later test appends rather than replaces, so an order-dependent test is a real risk. Better surfaced here than on an unrelated PR. All three passes together take about 25 seconds. Pushes to main are included so a merge that breaks something is attributed to the merge rather than to the next pull request. Concurrency cancels superseded pull-request runs but never cancels main-branch runs, so consecutive merges each get a result. Conventions follow release.yml: checkout@v7, setup-go@v7, and go-version-file: go.mod so the Go version stays in one place. Permissions are contents: read, since nothing here writes. No services, credentials, or network egress are needed: tests bind ephemeral localhost ports and point HOME at a temp directory, and the container tests assert on the command strings they would run rather than invoking a real runtime. Assisted by Claude. Signed-off-by: Ed Snible --- .github/workflows/ci.yml | 89 ++++++++++++++++++++++++++++++++++++++++ README.md | 19 +++++++++ 2 files changed, 108 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..c1c7db7 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,89 @@ +name: ci + +# Run the repo's checks on pull requests, and on pushes to main so a merge that +# breaks something is attributed to the merge rather than to the next PR. +# +# The checks mirror the Makefile targets a contributor runs locally (fmt, vet, +# test), so CI failing means the same command fails on their machine. gofmt is +# checked with -l rather than `make fmt`, which rewrites files: CI must report +# formatting, not silently fix it and test something the author never wrote. +# +# Tests are self-contained — they bind ephemeral localhost ports and set HOME to +# a temp dir, and the container tests assert on generated command strings without +# invoking a real runtime — so no services, credentials, or network egress are +# needed. + +on: + pull_request: + push: + branches: [main] + +# Read-only: nothing here writes to the repo, and the default token grants more. +permissions: + contents: read + +# A force-push or a quick follow-up commit makes an in-flight run obsolete; +# cancel it rather than paying for a result nobody reads. Pushes to main are +# keyed per-ref, so consecutive merges do not cancel each other. +concurrency: + group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + test: + name: test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-go@v7 + with: + # go.mod is the single source of the Go version, as in release.yml. + go-version-file: go.mod + + # Before the build, so a formatting failure is reported in seconds rather + # than after the slowest step. + - name: Check formatting + run: | + unformatted="$(gofmt -l .)" + if [ -n "$unformatted" ]; then + echo "These files need gofmt:" >&2 + echo "$unformatted" >&2 + exit 1 + fi + + # `go mod tidy` writing nothing proves go.mod and go.sum match the imports. + # Checked against the real files rather than a copy so the diff names them. + - name: Check go.mod is tidy + run: | + go mod tidy + if ! git diff --quiet -- go.mod go.sum; then + echo "go.mod/go.sum are not tidy; run 'make tidy' and commit the result:" >&2 + git diff -- go.mod go.sum >&2 + exit 1 + fi + + - name: Vet + run: make vet + + - name: Build + run: go build ./... + + # -count=1 defeats the test cache, which would otherwise let a green result + # stand in for a run that never happened on this commit. + - name: Test + run: go test ./... -count=1 + + # A second pass under the race detector. Separate from the run above so a + # plain failure is not reported as a race, and because the suite is fast + # enough (a few seconds) that running it twice costs little. + - name: Test with the race detector + run: go test ./... -race -count=1 + + # Test order is a real source of flakes here: the suite mutates process + # state (HOME, XDG_CONFIG_HOME, cobra flag values), and cmd/root_test.go + # documents a pflag hazard where a flag's first Set in a later test can + # append rather than replace. Shuffling surfaces that in CI instead of on + # someone's unrelated PR. + - name: Test in a shuffled order + run: go test ./... -count=1 -shuffle=on diff --git a/README.md b/README.md index 2b36abb..cd2768b 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,25 @@ rossoctl namespaces list --json rossoctl -v agents list ``` +## Tests + +```sh +make test # go test ./... +make vet +gofmt -l . # prints files needing formatting; make fmt rewrites them +go test ./... -race -count=1 +go test ./... -count=1 -shuffle=on +``` + +The suite needs no services, credentials, or network access: tests bind ephemeral +localhost ports and point `HOME` at a temp directory, and the container tests +assert on the command strings they would run rather than invoking a real runtime. + +`.github/workflows/ci.yml` runs exactly these on every pull request and on pushes +to `main`, plus a `go mod tidy` check. Shuffled order is included because the +suite mutates process state (`HOME`, cobra flag values), so an order-dependent +test is a real risk — see the pflag hazard documented in `cmd/root_test.go`. + ## Full docs See [the documentation](./docs)