Skip to content

test(cli): cover the domain command (domains, DNS, SSL)#186

Merged
Hydralerne merged 10 commits into
oblien:mainfrom
Rish-it:test/cli-domain-coverage
Jul 24, 2026
Merged

test(cli): cover the domain command (domains, DNS, SSL)#186
Hydralerne merged 10 commits into
oblien:mainfrom
Rish-it:test/cli-domain-coverage

Conversation

@Rish-it

@Rish-it Rish-it commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Problem

The CLI test suite (#182) covered server, token, deployment, project, and the raw api passthrough, but domain — the largest remaining API-shaped command, 9 subcommands spanning custom domains, DNS verification, and SSL — still has no coverage beyond tsc --noEmit. Its contracts are exactly the kind that drift silently: the per-subcommand method/path/body, the verify 200-vs-422 handling, and the non-zero exits that gate scripting (verify/verify-ssl exit 1 when a domain isn't verified). A refactor that repoints a route, reshapes a body, or drops an unverified-exit ships green today.

Follows #181; builds on the harness merged in #182.

Summary

test/e2e/domain.test.ts (27 tests) drives domainCommand over the same stubbed-socket harness the rest of the suite uses — the command action and the real api-client (URL building, /api prefix, auth header, ApiError mapping) run for real; only fetch and the config/caps seams are stubbed.

  • listGET /domains?projectId=… (project id URL-encoded), table vs JSON rows, empty result.
  • addPOST /domains { projectId, hostname, isPrimary }, --primary toggles the flag, the openship domain verify <id> next-step hint, and the { domain, records } JSON shape.
  • previewPOST /domains/preview { hostname }, DNS-record table, JSON.
  • verifyPOST /domains/:id/verify via apiRaw: 200 verified → exit 0; 422 (DNS not propagated) → exit 1 without throwing; json mode prints the body and does not exit; a non-422 (500) surfaces and exits 1.
  • primaryPOST /domains/:id/primary, JSON.
  • recordsGET /domains/:id/records, DNS-record table, JSON.
  • renewPOST /domains/:id/renew, SSL status, JSON.
  • verify-sslPOST /domains/:id/verify-ssl: valid → exit 0; not-valid → exit 1; json mode does not exit.
  • renew-allPOST /domains/renew-all, per-domain results table vs the "Nothing needed renewal" note, JSON.
  • auth + errors — an API {error} is surfaced and exits 1; the bearer token is sent when logged in.

Design notes

  • Offline and fast. No live API/DNS; body and exit assertions read fetchStub.calls and the exit code, so they hold regardless of spinner rendering. Each test is a real regression sentinel, not a coverage number — verified by mutation: removing both unverified-exit lines from domain.ts fails exactly the two exit tests and nothing else.
  • Documented behavior finding (not fixed here): unlike server/mail/service, domain.ts has no pre-flight login guard — when logged out it issues the request with no Authorization header and lets the API reject it, rather than short-circuiting. One test pins that (request still made, no auth header, API 401 surfaced) so adding a pre-flight guard later is a deliberate, visible change. Left as-is; out of scope for a test-only PR.
  • Reuses the merged harness (test/helpers/*) and vitest runner from test(cli): add a vitest suite for the CLI package #182 — no runner or dependency changes, lockfile untouched.

Verification

Run twice, clean, both green:

  • bun run --cwd apps/cli test → 12 files, 76 tests passed (49 prior + 27 domain) ×2
  • bun run --cwd apps/cli lint (tsc --noEmit) → clean (exit 0)
  • bun install --frozen-lockfile → no changes (lockfile matches)
  • mutation check: neutralizing the two !verified → process.exit(1) lines fails exactly the verify 422 and verify-ssl invalid tests, then reverted.

Reproduced against committed HEAD: working tree matches HEAD, full suite green.

Commits

Ten atomic commits, one per subcommand plus the shared auth/error concern; each adds a self-contained describe block and is green in isolation:

test(cli): cover domain list over a mocked API
test(cli): cover domain add and its DNS-record hints
test(cli): cover domain preview DNS-record output
test(cli): cover domain verify 200/422/error exit paths
test(cli): cover domain primary
test(cli): cover domain records
test(cli): cover domain renew
test(cli): cover domain verify-ssl valid/invalid exit paths
test(cli): cover domain renew-all org-wide results
test(cli): cover domain auth header and API-error exit paths

Not in scope

Commands that need a new test seam the harness doesn't yet provide: lib/service (ssh2/dockerode) for stop/service/status, a child_process seam for deploy/doctor/install/update, an interactive @clack stdin seam for up/wizard/reset-admin, and a $HOME config-file seam for context/config/cache/backup/system/login/logout/init. Each seam is a separate follow-up. mail install is covered on its own branch pending #141.

Copilot AI review requested due to automatic review settings July 24, 2026 02:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Rish-it
Rish-it force-pushed the test/cli-domain-coverage branch from 8b95ea3 to f682986 Compare July 24, 2026 02:51
@Hydralerne

Copy link
Copy Markdown
Member

amazing fr, goat these are important tests

@Hydralerne
Hydralerne merged commit 72f6c73 into oblien:main Jul 24, 2026
2 checks passed
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.

3 participants