Skip to content

ci: run the test suite with the shipping build tags - #516

Closed
xe-nvdk wants to merge 1 commit into
mainfrom
ci/test-with-shipping-tags
Closed

ci: run the test suite with the shipping build tags#516
xe-nvdk wants to merge 1 commit into
mainfrom
ci/test-with-shipping-tags

Conversation

@xe-nvdk

@xe-nvdk xe-nvdk commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

Adds a CI workflow that runs the Go test suite on every PR and push to main with the same build tags the release uses, so what we test is what we ship.

  • standard: go test -tags=duckdb_arrow ./...
  • FIPS variant: go test -tags=duckdb_arrow,fips ./... with GOFIPS140=v1.0.0
  • go vet -tags=duckdb_arrow ./... (once, on the standard variant)

Why

The only existing workflow (release-build.yml) builds and signs binaries but runs no tests. Worse, a bare go test ./... (no tag) does not compile the Arrow query path at all — the DuckDB driver (duckdb-go/v2) gates its Arrow CGO API behind //go:build duckdb_arrow:

$ go build ./internal/database/      # no tag
internal/database/duckdb_arrow.go:27:26: undefined: duckdb.NewArrowFromConn

So automated testing of the code that actually ships was missing entirely. This is the gap that let the PR #489 Arrow-handler RBAC bug through (caught only by a manual tagged build). This workflow makes tested == shipped.

Design notes

  • No -race: bcrypt-based auth tests hard-code 1000ms request timeouts that the race detector's CPU overhead exceeds. These are not real races (0 DATA RACE markers; the suite passes clean without -race). Race coverage can be added later behind a carve-out for the timeout-sensitive internal/auth / internal/api packages.
  • No gofmt gate: Go 1.26's gofmt rewrites doc-comment content (e.g. mangles the 'it''s' SQL-escape example in internal/sql/mask.go into a smart-quote), so enforcing it in CI would propagate comment corruption. Formatting is left to local tooling / review.
  • Mirrors the build-binaries matrix in release-build.yml (runner, setup-go@v6 / Go 1.26.4, CGO_ENABLED=1, GOFIPS140).

Relation to #490

Issue #490 proposed dropping the duckdb_arrow tag so default go test exercises shipped code. That's not feasible — the tag is required by the DuckDB driver (above), and there's no native Go way to bake a default tag into a repo. #490 is closed; this PR delivers the underlying goal (test == ship) the correct way: run CI with the tag rather than removing it.

Test plan

  • YAML validates (yaml.safe_load)
  • Reproduced both jobs locally:
    • standard go test -tags=duckdb_arrow ./... → 32 ok, 0 fail
    • FIPS GOFIPS140=v1.0.0 go test -tags=duckdb_arrow,fips ./... → 32 ok, 0 fail
    • go vet -tags=duckdb_arrow ./... → clean
  • CI green on this PR (the workflow runs on itself)

🤖 Generated with Claude Code

Add a CI workflow that runs `go test` on every PR and push to main with the
same build tags the release uses — duckdb_arrow (standard) and
duckdb_arrow,fips with GOFIPS140=v1.0.0 (FIPS variant) — plus go vet.

Previously the only workflow (release-build.yml) built and signed binaries but
ran NO tests, and a bare `go test` without -tags=duckdb_arrow does not even
compile the Arrow query path: the DuckDB driver (duckdb-go/v2) gates its Arrow
CGO API behind that tag. So automated testing of the code that actually ships
was missing entirely — the gap that let the PR #489 Arrow-handler bug through.
This makes tested == shipped.

No -race: bcrypt-based auth tests hard-code 1000ms request timeouts that the
race detector's overhead exceeds (the suite is race-clean — 0 DATA RACE). No
gofmt gate: Go 1.26's gofmt rewrites doc-comment content (mangles the `'it''s'`
SQL-escape example in internal/sql/mask.go), so enforcing it would propagate
comment corruption; formatting is left to local tooling.

Closes #490 (proposed dropping the tag; not feasible — the driver requires it).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the release notes to document that the CI workflow now runs the test suite against the shipped build using the appropriate build tags. The reviewer suggested splitting a long, run-on sentence in the new paragraph to improve readability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.


The deliverable is signed scan evidence attached to each release, not "zero findings" — transitive OS CVEs from the Debian base layer outside Arc's control are reported but do not gate the release.

**CI now runs the test suite against the shipped build.** A new `CI` workflow runs `go test` on every pull request and push to `main` with the same build tags the release uses — `duckdb_arrow` (standard) and `duckdb_arrow,fips` with `GOFIPS140=v1.0.0` (FIPS variant) — plus `go vet`. Previously the only workflow built and signed binaries but ran no tests, and a bare `go test` (without `-tags=duckdb_arrow`) does not even compile the Arrow query path, because the DuckDB driver gates its Arrow API behind that tag — so automated testing of the code that actually ships was missing. This closes that gap; what CI tests is now what the release builds. (Issue #490, which proposed dropping the tag, was closed: the tag is required by the driver and cannot be removed without a driver change.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The newly added paragraph contains a very long, run-on sentence that describes the previous state of the CI workflow and the compilation behavior of go test. Splitting this into two sentences would significantly improve the readability and flow of the release notes.\n\nFor example:\n\nmarkdown\n**CI now runs the test suite against the shipped build.** A new `CI` workflow runs `go test` on every pull request and push to `main` with the same build tags the release uses — `duckdb_arrow` (standard) and `duckdb_arrow,fips` with `GOFIPS140=v1.0.0` (FIPS variant) — plus `go vet`. Previously, the only workflow built and signed binaries but ran no tests. Additionally, a bare `go test` (without `-tags=duckdb_arrow`) does not compile the Arrow query path because the DuckDB driver gates its Arrow API behind that tag, meaning automated testing of the code that actually ships was missing. This closes that gap; what CI tests is now what the release builds. (Issue #490, which proposed dropping the tag, was closed: the tag is required by the driver and cannot be removed without a driver change.)\n

@xe-nvdk

xe-nvdk commented Jun 22, 2026

Copy link
Copy Markdown
Member Author

Closing. This PR surfaced that no CI runs the test suite, but the first run exposed a non-CI-portable test (TestScopedSecretsCoexist depends on ambient AWS config for DuckDB credential-chain validation). Not pursuing the CI workflow right now.

@xe-nvdk xe-nvdk closed this Jun 22, 2026
@xe-nvdk
xe-nvdk deleted the ci/test-with-shipping-tags branch June 22, 2026 21:22
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