Add integration test harness, push/deploy + TUI tests, document two production bugs by l-hellmann · Pull Request #252 · zeropsio/zcli

l-hellmann · 2026-05-13T12:32:27Z

What Type of Change is this?

Description (required)

Builds an in-process integration test harness for zcli commands and adds
unit-level UI tests for the bubbletea models. Includes minimal production-code
seams to enable testing without changing user-facing behavior. Also documents
two confirmed bugs as skipped regression tests (fixes intentionally left for
follow-up PRs to keep this one scoped to test infrastructure).

Production changes (small, backward-compatible)

zeropsRestApiClient.NewAuthorizedClient accepts a full URL or a bare host;
defaults the scheme to https:// when missing.
cmdBuilder.RunRootCmd(rootCmd, RunOptions{Ctx, Args, Stdout, Stderr}) —
new test-friendly entry point that returns an exit code instead of calling
os.Exit. ExecuteRootCmd becomes a thin wrapper around it.
logger.OutputConfig.Out accepts an io.Writer (defaults to os.Stderr),
so uxBlock output can be captured.
flagParams.New(notice io.Writer) routes "Using config file:" notices
through the injected stderr.
New env var ZEROPS_CLI_YAML_FILE_PATH overrides the per-user .zcli.yml
location (parity with ZEROPS_CLI_DATA_FILE_PATH and ZEROPS_CLI_LOG_FILE_PATH).
Empty ZEROPS_TOKEN env value no longer overrides a token persisted by
zcli login — previously export ZEROPS_TOKEN= would silently log out.
yamlReader.ResetCache() — exported so in-process test re-runs aren't
served stale bytes from the package-level cache.

New tests

Integration (src/cmd/, //go:build devel, run via make test-integration):

Harness with httptest.Server, per-test tempdir, login/scope seeding,
archive decompression, and git fixture helpers.
14 service-push tests (happy paths, error paths, polling, scope, archive-file-path).
7 git-archive tests (not-init / zero-commits errors, workspace-state
clean/staged/all, --deploy-git-folder).
1 service-deploy variant.
1 project-list, 1 login, 1 version.

TUI (src/uxBlock/models/, via github.com/charmbracelet/x/exp/teatest):

selector (10): single + multi-select, filter-mode, esc, clamping, label.
prompt (7): cursor movement, clamping, esc, initial position.
input (5): typing, backspace, esc, empty submit, label render.
table (11): renderer output, body indexing, deep clone, cell pretty-flag.

Confirmed bugs documented as skipped tests (pushDeploy_bugs_test.go)

--setup flag ignored when the service name happens to match a setup
name in zerops.yaml (servicePush.go:85, serviceDeploy.go:72).
Reproduced from a real pipeline running --setup showcase-backend on a
service named "backend".
SIGSEGV nil-deref on apiProcess.AppVersion.Id / .Build
(servicePush.go:235, :251) when a process poll returns
status=RUNNING with appVersion: null. Crashes the binary from the
spinner goroutine. Confirmed by removing the t.Skip in isolation; stack
trace is in the test comment.

Both tests are committed as t.Skip'd reproductions so CI stays green;
removing the skip after each fix will lock the regression in.

Test plan

make test — unit tests pass.
make test-integration — 24 PASS + 2 SKIP, ~16 s.
TUI tests pass — 33 across 4 packages, <1 s combined.
Branch builds for darwin/arm64, linux/amd64, windows/amd64.
make lint reports zero issues in src/ and cmd/ (after running
tools/install.sh). The lint binary itself reports a pre-existing
toolchain mismatch in golang.org/x/crypto/.../fips140only_go1.26.go
when the developer has Go ≥1.26 installed; unrelated to this diff.

Related issues & labels (optional)

…jected stderr

…riant

…lean filter, deploy-git-folder

…dFunc rejection

…ntext() in TUI tests, checked type assertions

tikinang · 2026-05-18T12:26:47Z

@@ -0,0 +1,150 @@
+//go:build devel


Why devel? I would anticipate something like integration instead.

// All integration tests build under the `devel` build tag (so the // production-version-check HTTP call from src/version is stubbed to a no-op) // and run via `make test-integration`

reference

Honestly the devel tag will go when the refactor of the self-update lands, then there will be no reason to keep that around.

tikinang · 2026-05-18T12:41:54Z

+	logPath := filepath.Join(dir, "zcli.log")
+	t.Setenv(constants.CliDataFilePathEnvVar, dataPath)
+	t.Setenv(constants.CliLogFilePathEnvVar, logPath)
+	t.Setenv(constants.CliTokenEnvVar, "")


What is the reason for this?

Override for already defined ZEROPS_TOKEN env in shell, so you don't one when running tests

tikinang · 2026-05-18T12:46:20Z

+	// region named "prg1" by default.
+	f.HandleJSON("/regions", 200, map[string]any{
+		"items": []map[string]any{{
+			"name":      "prg1",


Hmm, I wonder about the map[string]any approach. What if we used the structures from generated SDK?

I considered it, but I think map[string]any is the right call here — the contract under test is the JSON wire format, not the Go types.

Canary on SDK drift. If a zerops-go bump silently renames a field or changes a JSON tag, shared types move both sides together and the test keeps passing against the new contract. Literal JSON fails until I consciously update the fixture — what I want from an integration test.

Catches tag bugs on the consumer side. A mis-tagged decoding struct would round-trip through itself and pass with typed fixtures; map[string]any writes keys literally, so assertions catch it.

Minimal fixtures. Many response fields aren't read by the CLI (see the GetUserInfo comment). Typed structs force zero values or omitempty, obscuring what the test pins; optional.Null[T] is also awkward to build inline.

The one real win for typed structs would be catching field-type changes at compile time on SDK bump — narrow, and we'd catch it the first test run anyway. If we want stronger SDK-drift guarantees, a separate contract test that decodes a captured real response through zerops-go is the better tool,
orthogonal to these.

tikinang · 2026-05-18T12:58:15Z

+// test. See integration_harness_test.go and pushDeploy_helpers_test.go for
+// the shared scaffolding.
+//
+// # Tests by command


I would much rather see these tests descriptions in the individual test files, it allows this doc file comment drifting appart from what the tests do more easily, which will be confusing.

Please leave only the description above, if you deem it necessary.

done ea5ddf1

tikinang

Great improvement, I like the test harness setup. I found just some small things to make it nicer.

tikinang · 2026-05-18T13:02:24Z

+type RunOptions struct {
+	// Ctx is the root context. If nil, a fresh context.Background() is used
+	// and OS signals are wired to cancel it.
+	//nolint:containedctx // intentional: callers (tests, ExecuteRootCmd) pass the root ctx by value
+	Ctx context.Context
+	// Args overrides os.Args[1:] when non-nil. Useful for tests.
+	Args []string
+	// Stdout receives command output. Defaults to os.Stdout when nil.
+	Stdout io.Writer
+	// Stderr receives error/log output (including uxBlock messages).
+	// Defaults to os.Stderr when nil.
+	Stderr io.Writer
+}


Hmm, private fields with option funcs and sane defaults (as done in the RunRootCmd) would be nice? It may remove the need for //nolint. But this is fine too.

reworked per your comment a9a77d9

tikinang · 2026-05-18T13:11:12Z

+			"id":          "00000000-0000-0000-0000-0000000000bb",
+			"clientId":    "00000000-0000-0000-0000-0000000000aa",


It probably doesn't matter now, but this wouldn't happen in case of Zerops public API, it uses UUID short (shortened 22-char-base64 version of UUID V4).

wouldn't matter but switched to 22-char-base64 ids to keep parity ea00ca3

tikinang · 2026-05-18T13:14:08Z

+	require.NotEqualf(t, 0, res.ExitCode,
+		"expected non-zero exit\n--- stderr ---\n%s\n--- stdout ---\n%s", res.Stderr, res.Stdout)


nit: weird formatting, maybe include in the CLAUDE.md, that it shouldn't produce these weird half line breaks without , on the end... but whatever

done and noted in CLAUDE.md 6ad3eee

tikinang · 2026-05-18T13:21:56Z

+		}},
+	})
+
+	f.HandleJSON("/api/rest/public/project/search", 200, map[string]any{


Just note: I would love to eventually move from the /search endpoint to the standard read API endpoints that we are rolling out now (just waiting for to publish the new SDK, the read endpoints are already deployed to prod).

That's just for future reference.

tikinang · 2026-05-18T13:30:11Z

And maybe adding the integration tests to the automatic checks? Similar to:

      - name: test
        if: ${{ matrix.runTests }}
        run: env ${{ matrix.osEnv }} go test -v ./cmd/... ./src/...

in .github

Move per-test descriptions from the central doc_test.go file onto each test function as a godoc-style comment starting with the function name. Lift the package-wide scaffolding paragraph onto integration_harness_test.go as a file comment. doc_test.go is removed.

Document the multiline-call style rule in CLAUDE.md and retrofit existing violations: every multiline function call now puts the open paren on its own line with each argument on its own line and a trailing comma. Printf-style format-string + args still ride one line as a logical group.

Match zerops-go ID format (16 raw UUID bytes → URL-safe base64 without padding = 22 chars). Values are derived deterministically from UUIDv5(DNS, "zcli-test-fixture-<role>") so the constants stay stable across runs.

l-hellmann · 2026-05-18T22:17:48Z

@tikinang

And maybe adding the integration tests to the automatic checks?

Will add in #254

Move the root context off the RunOptions struct and onto RunRootCmd as the first argument, matching Go convention and dropping the containedctx nolint. ExecuteRootCmd wires SIGINT/SIGTERM via signal.NotifyContext, replacing the hand-rolled regSignals goroutine. Convert RunOptions to the functional-options pattern: private runOptions struct with sane defaults (os.Stdout/os.Stderr) plus public WithArgs, WithStdout, WithStderr helpers. Variadic RunRootCmd lets ExecuteRootCmd call without any options.

Threading ctx through RunRootCmd as a real parameter let contextcheck follow the call graph it couldn't before, and the new signal.NotifyContext wiring introduced an exitAfterDefer. - Move stop() before os.Exit so it actually runs (gocritic). - Thread ctx into version.GetVersionCheckMismatch and printMessageData; precompute Latest/LatestUrl in messageData so the template no longer swallows the caller's context via context.Background(). - Suppress contextcheck on buildCobraCmd with a note: cobra threads ctx to the run-func via ExecuteContext, which is the canonical pattern here.

l-hellmann added 25 commits May 13, 2026 12:17

accept full URL in NewAuthorizedClient, default scheme to https

2c6fcc5

add CLAUDE.md

b66c389

add RunRootCmd entry with injectable stdout/stderr and exit code return

8478239

add integration harness with version and login tests

33e52b1

allow zcli.yml path override and route config-file notices through in…

3fd1a6a

…jected stderr

isolate integration tests from user zcli.yml

1c77055

ignore empty ZEROPS_TOKEN env override

aac7b2f

add project list integration test

7168824

expose yamlReader.ResetCache for in-process test isolation

e03b769

reset yamlReader cache per integration test

d04ccdc

add service push happy-path integration test

5eceaea

extend service push tests with setup flag and version-name cases

49a2242

document --setup precedence bug via skipped regression test

2b6250c

add tier 1 push/deploy tests: error paths, process failure, deploy va…

bc63092

…riant

document confirmed nil-deref bug in push log-streaming callback

eed4c37

split push/deploy integration tests across files and add tier 2 cases

5c032aa

switch integration tests to testify assert/require

70faee9

add tier 3 git archive tests: not-init/no-commits errors, workspace-c…

902d614

…lean filter, deploy-git-folder

add workspace-state staged/all, project-flag, and saved-scope tests

c4b1c79

document all integration tests in a package-doc test file

a4c18d6

test interactive selector model via teatest

d5356d2

test prompt, input, and selector filter via teatest

01ce12f

test multi-select selector: spacebar toggles, ctrl+a/d, GetOneSelecte…

64e2ea9

…dFunc rejection

test table render, body/row indexing, and cell pretty-flag

4df5878

fix CI lint: gofmt constants, nolint containedctx on RunOptions, t.Co…

3b46af0

…ntext() in TUI tests, checked type assertions

l-hellmann requested review from MichalSalon and tikinang May 13, 2026 12:42

l-hellmann mentioned this pull request May 13, 2026

Fix --setup handling and nil-deref crash in service push #253

Merged

8 tasks

l-hellmann self-assigned this May 15, 2026

tikinang reviewed May 18, 2026

View reviewed changes

l-hellmann added 3 commits May 18, 2026 23:55

use realistic 22-char base64 UUID IDs in push/deploy test fixtures

ea00ca3

Match zerops-go ID format (16 raw UUID bytes → URL-safe base64 without padding = 22 chars). Values are derived deterministically from UUIDv5(DNS, "zcli-test-fixture-<role>") so the constants stay stable across runs.

l-hellmann and others added 6 commits May 19, 2026 00:31

add section about comments the CLAUDE.md

9ab7e39

fix: honor --setup flag over auto-match by service name

c31c14c

fix: nil-guard apiProcess.AppVersion in push log-streaming callback

e17e0ad

reject unknown --setup client-side before any API call

da1f3ed

l-hellmann requested a review from tikinang May 18, 2026 22:57

		"id": "00000000-0000-0000-0000-0000000000bb",
		"clientId": "00000000-0000-0000-0000-0000000000aa",

		require.NotEqualf(t, 0, res.ExitCode,
		"expected non-zero exit\n--- stderr ---\n%s\n--- stdout ---\n%s", res.Stderr, res.Stdout)

Conversation

l-hellmann commented May 13, 2026

What Type of Change is this?

Description (required)

Related issues & labels (optional)

Uh oh!

tikinang May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tikinang May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tikinang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tikinang commented May 18, 2026

Uh oh!

l-hellmann commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tikinang May 18, 2026 •

edited

Loading

tikinang May 18, 2026 •

edited

Loading

l-hellmann commented May 18, 2026 •

edited

Loading