Repository navigation
ci: add test workflow, split offline and live CLI tests - #17
Conversation
The only workflow was release.yml, so nothing ran on pull requests. The integration suite could not fill that gap: one `integration` build tag covered both offline tests and tests needing a live Cloud org, and `skipIfNoCredentials` partitioned them invisibly at runtime. Split the suite into two tiers by build tag — `cli` (builds the binary, no network, no credentials) and `cloud` (live org) — and add test.yml running unit plus `cli` on PRs and pushes to main. Moving the credential gate down to the tests that need it recovers 19 subtests that were skipping for no reason, including every blocked-command test: the "not yet available" guard runs before the auth check, so the exit-code-2 behaviour is now enforced in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
actions/checkout leaves GITHUB_TOKEN in the local git config by default, where any later step or third-party action can read it. Neither test job runs git operations, so there is nothing to keep it for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
m1n0
left a comment
There was a problem hiding this comment.
Checked the split rather than the summary. Build tags line up with the filename suffixes, and no _cli_test.go calls skipIfNoCredentials, so there's no runtime credential gating left in the offline tier to quietly skip things. Ran both jobs locally with SODA_TEST_* unset: unit green, offline tier green, 47 subtests, exit 0.
Nothing lost in the move either. Three names vanish from the suite, TestDatasourceCreateBadFile, TestRunnerCreateMissingName and TestSecretCreateMissingFlags, and all three turn up as subtests of the new table-driven TestFlagValidation. They were credential-gated before and now run on every PR, which is the part I like most here.
Workflow hygiene is right: pinned action SHAs matching release.yml, contents: read, persist-credentials: false, cancel-in-progress scoped to PRs. AGENTS.md matches what the workflow actually does and no -tags integration reference survives anywhere.
One to decide separately, not here: the cloud tier now has no automated home at all. Correct call for a PR workflow given it creates and deletes real resources, but tests that only run when someone remembers will rot. Worth a nightly with the secrets eventually.
The offline CLI suite fails on main: TestContractLint/invalid_contract expects `bogus_field` at the contract root to exit 2, but lint accepts it and exits 0. Two PRs merged a minute apart caused this. #16 synced the contract JSON schema from the backend, which flips the root from "additionalProperties": false to true — unknown top-level keys are now allowed by design — and dropped its own TestLintFile_InvalidProperty accordingly. #17 branched before that landed and added a CLI test asserting the old behavior. Both were green against their own base; the merge of the two is not. The schema is generated backend-side, so pinning the root closed again would be reverted by the next sync and would reject contracts the backend accepts. Point the subtest at a check object instead: those are still closed, so it keeps CLI-level coverage of the "additional properties not allowed" path rather than deleting it. Refs: #16, #17 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing runs on pull requests.
release.ymlis the only workflow and it fires on tags, sogo buildandgo testhappen only on a contributor’s machine — a PR that breaks the build gets no signal.The integration suite could not simply be pointed at CI. A single
integrationbuild tag covered both tests that need a live Cloud org and tests that need nothing at all, andskipIfNoCredentialspartitioned them at runtime, per function. On a machine without credentials 54 of 82 subtests vanished silently, so "the suite passed" meant very little.Change
The suite is split into two tiers by build tag, sharing one harness:
cli— builds the binary and drives it as a subprocess. No network, no credentials, no skips. This runs on every PR.cloud— the same harness against a real Cloud org. Creates and deletes real resources, so it stays out of CI and keeps its credential gate.test.ymlruns unit tests and theclitier on pull requests and pushes tomain.Pushing the credential gate down to the tests that actually need it recovers 19 subtests that were skipping for no reason — the offline tier goes from 28 to 47 passing. Most valuable among them: every blocked-command test. The "not yet available" guard runs before the auth check, so the exit-code-2 contract that CI consumers depend on is now enforced on each PR instead of only on a developer machine with live credentials.
No assertions were changed. Seven flag-validation tests were consolidated into one table-driven test, and the invariant they encode is now stated explicitly: validation precedes auth, so a bad flag exits 2 (execution error), not 3 (authentication error). Two candidates turned out to violate that —
contract verify <missing file>andcontract pullresolve the profile before reading arguments, so they exit 3 without credentials and stayed in thecloudtier.Caveats for review
gofmtis deliberately not a CI step: 12 files onmainare already unformatted, so a format gate would fail on arrival. Only files touched here were formatted. Worth a follow-up.The
cloudtier now runs nowhere automatically. It was never in CI, so this is not a regression, but a scheduled job with theSODA_TEST_*secrets is the obvious next step.Testing
go test ./...,go test -tags cli ./tests/integration/...(47 pass, 0 skip), andgo test -tags cloud ./tests/integration/...(skips cleanly with no credentials) all pass, andgo vetis clean under both tags. To confirm nothing was lost in the move, the full before/after test-name sets were diffed: the four names that disappear all map to renames.