Add integration test harness, push/deploy + TUI tests, document two production bugs#252
Merged
Conversation
…lean filter, deploy-git-folder
…ntext() in TUI tests, checked type assertions
8 tasks
tikinang
reviewed
May 18, 2026
tikinang
reviewed
May 18, 2026
tikinang
reviewed
May 18, 2026
tikinang
reviewed
May 18, 2026
tikinang
reviewed
May 18, 2026
Contributor
tikinang
left a comment
There was a problem hiding this comment.
Great improvement, I like the test harness setup. I found just some small things to make it nicer.
tikinang
reviewed
May 18, 2026
Contributor
|
And maybe adding the integration tests to the automatic checks? Similar to: in |
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.
Collaborator
Author
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.
tikinang
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Type of Change is this?
Description (required)
Builds an in-process integration test harness for
zclicommands and addsunit-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.NewAuthorizedClientaccepts 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.ExecuteRootCmdbecomes a thin wrapper around it.logger.OutputConfig.Outaccepts anio.Writer(defaults toos.Stderr),so uxBlock output can be captured.
flagParams.New(notice io.Writer)routes "Using config file:" noticesthrough the injected stderr.
ZEROPS_CLI_YAML_FILE_PATHoverrides the per-user.zcli.ymllocation (parity with
ZEROPS_CLI_DATA_FILE_PATHandZEROPS_CLI_LOG_FILE_PATH).ZEROPS_TOKENenv value no longer overrides a token persisted byzcli login— previouslyexport ZEROPS_TOKEN=would silently log out.yamlReader.ResetCache()— exported so in-process test re-runs aren'tserved stale bytes from the package-level cache.
New tests
Integration (
src/cmd/,//go:build devel, run viamake test-integration):httptest.Server, per-test tempdir, login/scope seeding,archive decompression, and git fixture helpers.
clean/staged/all, --deploy-git-folder).
TUI (
src/uxBlock/models/, viagithub.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)--setupflag ignored when the service name happens to match a setupname in
zerops.yaml(servicePush.go:85,serviceDeploy.go:72).Reproduced from a real pipeline running
--setup showcase-backendon aservice named "backend".
apiProcess.AppVersion.Id/.Build(
servicePush.go:235,:251) when a process poll returnsstatus=RUNNINGwithappVersion: null. Crashes the binary from thespinner goroutine. Confirmed by removing the
t.Skipin isolation; stacktrace 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.make lintreports zero issues insrc/andcmd/(after runningtools/install.sh). The lint binary itself reports a pre-existingtoolchain mismatch in
golang.org/x/crypto/.../fips140only_go1.26.gowhen the developer has Go ≥1.26 installed; unrelated to this diff.
Related issues & labels (optional)