Skip to content

ci: move Go test pipeline from CircleCI to GitHub Actions#4250

Merged
reinkrul merged 4 commits into
masterfrom
feature/4249-circleci-to-gha
May 12, 2026
Merged

ci: move Go test pipeline from CircleCI to GitHub Actions#4250
reinkrul merged 4 commits into
masterfrom
feature/4249-circleci-to-gha

Conversation

@reinkrul

@reinkrul reinkrul commented May 12, 2026

Copy link
Copy Markdown
Member

Summary

  • Replaces .circleci/config.yml with .github/workflows/go-test.yaml: go test ./... -race on PRs and pushes to master / V*.
  • Uploads filtered coverage to Qlty Cloud via qltysh/qlty-action/coverage (SHA-pinned to v2.2.0) with OIDC — no token secret.
  • actions/checkout runs with persist-credentials: false (zizmor finding).
  • Updates the build-status badge in README.rst / README_template.rst to point at the new workflow.

Single-runner port — drops the 8-way circleci tests split + coverage merge. Shard via matrix later only if wall-clock becomes a problem.

Drive-by fixes required to keep the new pipeline green

The GHA ubuntu-latest runner has different filesystem readdir order and uses systemd-resolved (different DNS error strings) than CircleCI's cimg/go image, which exposed two existing brittlenesses. Happy to split these into separate PRs if preferred.

  • policy/local.go: dir.Readdir(0)os.ReadDir(directory). Directory entries now load in lexical order, so duplicate-scope detection reports the same file deterministically across platforms.
  • crypto/storage/vault/vault_test.go, didman/api/v1/client_test.go: drop assert.Regexp against no such host|Temporary failure in name resolution. These matched on Go's net package error text and verified nothing about the code under test; assert.Error(t, err) + nil-result checks remain. Matches the pattern other "wrong address" subtests in the same didman file already use.

Closes #4249.

Test plan

  • Go test workflow green on this PR.
  • qlty check (zizmor) green.
  • After merge to master: coverage shows up on the Qlty project (requires Qlty GitHub App + OIDC trust; fallback is a QLTY_COVERAGE_TOKEN secret with token: \${{ secrets.QLTY_COVERAGE_TOKEN }}).
  • Disable CircleCI for the repo once master build is green.

Assisted by AI

Replace .circleci/config.yml with a go-test.yaml workflow that runs
go test -race and uploads coverage to Qlty Cloud via the official
qlty-action with OIDC. Update README build-status badges.

Closes #4249

Assisted by AI
@qltysh

qltysh Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

All good ✅

reinkrul added 2 commits May 12, 2026 09:40
- Run tests inside golang:1.26.3 container so DNS errors come from glibc
  ("no such host"), matching the regex used by tests in
  crypto/storage/vault, didman/api/v1, policy and others — the GHA
  ubuntu-latest runner uses systemd-resolved which returns
  "server misbehaving".
- Set persist-credentials: false on actions/checkout (zizmor:
  credential persistence through GitHub Actions artifacts).
- Pin qltysh/qlty-action/coverage to v2.2.0 commit SHA (zizmor:
  unpinned action reference).

Assisted by AI
- policy: switch loadFromDirectory to os.ReadDir so .json files load in
  lexical order. dir.Readdir(0) returned entries in filesystem-defined
  order, making duplicate-scope detection non-deterministic across
  platforms (the duplicate-detection test asserts which file is reported
  as the dupe, which only held on CircleCI's filesystem).
- vault, didman client tests: drop assert.Regexp on DNS error strings —
  these matched against Go net package output ("no such host" /
  "Temporary failure in name resolution") and didn't verify any
  behavior of the code under test. The "got an error + nil result"
  assertions cover the actual contract. Aligns with the other
  "wrong address" subtests in the same didman file.
- Drop the golang:1.26.3 container workaround from go-test.yaml now
  that the underlying brittleness is gone.

Assisted by AI
@qltysh

qltysh Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Qlty


Coverage Impact

⬆️ Merging this pull request will increase total coverage on master by 0.07%.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: D Coverage rating: C
policy/local.go100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Comment thread .github/workflows/go-test.yaml Outdated
- 'master'
- 'V*'
pull_request:
branches:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why only on these?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped the pull_request.branches filter — PRs targeting any base now trigger the workflow. Kept the push filter on master/V* so we don't double-run on feature-branch pushes (the pull_request event covers those).

@stevenvegt stevenvegt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like the test to run on every PR, so we have tests on PRs on feature branches as well.

Per review feedback: tests should fire on PRs to feature branches too.

Assisted by AI
@reinkrul
reinkrul merged commit 05a7056 into master May 12, 2026
11 checks passed
@reinkrul
reinkrul deleted the feature/4249-circleci-to-gha branch May 12, 2026 11:32
reinkrul added a commit that referenced this pull request May 18, 2026
Three independent CI tweaks targeting the ~13 min PR wall-clock:

1. go-test.yaml: drop `-p 1`. Inherited from CircleCI per-shard config
   during PR #4250 migration; the new GHA pipeline isn't sharded so the
   flag just serialized 137 packages on a single 2-core runner. Audit
   shows no inter-package parallelism hazards in the current codebase
   (no fixed-port net.Listen, no os.Chdir, no TestMain, no shared
   on-disk paths). Local empirical: -p 1 takes 7m04s, -p 2 takes 3m31s,
   both pass. Expected CI: 12m15s -> ~6m.

2. e2e-tests.yaml: add gha cache to both Build and push steps. Dockerfile
   already orders go.mod/go.sum download before COPY ., so the cache is
   effective on PRs that don't touch go.sum. Saves ~1-2 min after warm-up.

3. build-images.yaml: build amd64 only on PRs, multi-arch on push/tag.
   arm64 under QEMU is ~5-8x slower; PRs are push: false anyway, so we
   only verify build doesn't break - arm64 adds no signal. Saves ~20
   runner-min/PR (not on critical path but reduces resource use).

Also: add .scratch/ and .claude/scheduled_tasks.lock to .gitignore
(per global preference - keeps local agent state out of commits).

Refs #4258.

Assisted by AI
@reinkrul reinkrul mentioned this pull request May 18, 2026
4 tasks
reinkrul added a commit that referenced this pull request May 19, 2026
Three independent CI tweaks targeting the ~13 min PR wall-clock:

1. go-test.yaml: drop `-p 1`. Inherited from CircleCI per-shard config
   during PR #4250 migration; the new GHA pipeline isn't sharded so the
   flag just serialized 137 packages on a single 2-core runner. Audit
   shows no inter-package parallelism hazards in the current codebase
   (no fixed-port net.Listen, no os.Chdir, no TestMain, no shared
   on-disk paths). Local empirical: -p 1 takes 7m04s, -p 2 takes 3m31s,
   both pass. Expected CI: 12m15s -> ~6m.

2. e2e-tests.yaml: add gha cache to both Build and push steps. Dockerfile
   already orders go.mod/go.sum download before COPY ., so the cache is
   effective on PRs that don't touch go.sum. Saves ~1-2 min after warm-up.

3. build-images.yaml: build amd64 only on PRs, multi-arch on push/tag.
   arm64 under QEMU is ~5-8x slower; PRs are push: false anyway, so we
   only verify build doesn't break - arm64 adds no signal. Saves ~20
   runner-min/PR (not on critical path but reduces resource use).

Also: add .scratch/ and .claude/scheduled_tasks.lock to .gitignore
(per global preference - keeps local agent state out of commits).

Refs #4258.

Assisted by AI
reinkrul added a commit that referenced this pull request Jun 5, 2026
Backport of #4250 and #4259 from master, limited to CI:

- add .github/workflows/go-test.yaml, remove .circleci/config.yml
- build-images: build only linux/amd64 on PRs, multi-arch on push/tag
- e2e-tests: enable GHA layer cache
- drop brittle DNS-error / readdir-order test assertions exposed by the
  ubuntu-latest runner (same fixes as #4250) so go-test passes

Assisted by AI
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.

CI: switch from CircleCI to GitHub Actions

2 participants