Skip to content

acceptance: add cmd/lakebox tests#5474

Merged
akshaysingla-db merged 1 commit into
databricks:demo-lakeboxfrom
akshaysingla-db:akshay/lakebox-acceptance-tests
Jun 8, 2026
Merged

acceptance: add cmd/lakebox tests#5474
akshaysingla-db merged 1 commit into
databricks:demo-lakeboxfrom
akshaysingla-db:akshay/lakebox-acceptance-tests

Conversation

@akshaysingla-db
Copy link
Copy Markdown
Collaborator

Summary

Bootstraps acceptance/cmd/lakebox/ — lakebox had zero acceptance tests today. Six leaf scenarios cover the rendered output of every subcommand surface that's exercised by user-visible flows.

Test What it locks down
list/empty Empty result → No lakeboxes found. hint
list/with-entries Multi-row table: NAME column always present, mixed running/stopped/creating… statuses, autostop label including the noAutostop=true "never" case
status/not-found 404 → friendly no lakebox named "X" — databricks lakebox list shows available IDs (the message added in #5460)
ssh-key/list/empty Empty result → "Run databricks lakebox register to add one" hint
config/no-flags No flags → nothing to update error
config/idle-timeout-bounds Out-of-range values (30s, 48h) → client-side bounds error before any API call, with the duration-formatted bounds from #5372

Each test stubs the relevant API endpoint via the testserver framework's [[Server]] entries. The two config tests need no stub because validation errors fire before any API call. Parent acceptance/cmd/lakebox/test.toml adds .databricks to Ignore so the CLI's local state file (lakebox.json) doesn't bleed into the test directory's diff.

All six pass under both DATABRICKS_BUNDLE_ENGINE=terraform and =direct with identical output, which is correct — lakebox is engine-independent.

These would have caught the FQDN-displayed-in-status inconsistency and the start.go 5-vs-10-minute help-text mismatch that the bugfix-bundle PR (#5460) caught manually.

Test plan

  • go test ./acceptance -run TestAccept/cmd/lakebox passes (12 subtests, both engine variants)
  • -update produces stable golden files (re-run without -update is clean)

Out of scope (potential follow-ups)

  • Acceptance coverage for write-path commands (create, delete, start, stop, default, register) — these are interactive or have stateful side effects worth a separate, more involved test design.
  • Coverage for the ssh command — that path execs ssh directly so it isn't testable through this framework without an SSH server stub.

This pull request and its description were written by Isaac.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 8, 2026

Waiting for approval

Could not determine reviewers from git history.
Round-robin suggestion: @anton-107

Eligible reviewers: @andrewnester, @anton-107, @denik, @pietern, @renaudhartert-db, @shreyas-goenka, @simonfaltum

Suggestions based on git history. See OWNERS for ownership rules.

Six leaf scenarios covering the rendered output of every lakebox
subcommand surface exercised by user-visible flows:

- list/empty               — empty result → "No lakeboxes found." hint
- list/with-entries        — multi-row table: NAME column always present,
                             mixed running/stopped/creating states,
                             autostop label including the noAutostop=true
                             "never" case
- status/not-found         — 404 → friendly "no lakebox named X — `databricks
                             lakebox list` shows available IDs"
- ssh-key/list/empty       — empty result → register hint
- config/no-flags          — no flags → "nothing to update" error
- config/idle-timeout-bounds — out-of-range values → client-side bounds
                             error before any API call

Each test stubs the relevant API endpoint via the testserver framework's
[[Server]] entries (status/not-found uses a 404 response; the two
config tests need no stub because validation errors fire before any
API call). The parent acceptance/cmd/lakebox/test.toml adds
.databricks to Ignore so the CLI's local state file doesn't bleed into
the test directory's diff.

All six pass under both DATABRICKS_BUNDLE_ENGINE=terraform and =direct
with identical output, which is correct: lakebox is engine-independent.

These would have caught the FQDN-displayed-in-status inconsistency and
the start.go 5-vs-10 minute help-text mismatch that the bugfix-bundle
PR landed earlier this week.

Co-authored-by: Isaac
@akshaysingla-db akshaysingla-db force-pushed the akshay/lakebox-acceptance-tests branch from bd42bc1 to 53a6605 Compare June 8, 2026 15:59
@akshaysingla-db akshaysingla-db merged commit 00946fd into databricks:demo-lakebox Jun 8, 2026
13 of 20 checks passed
akshaysingla-db added a commit that referenced this pull request Jun 8, 2026
)

## Summary

Second batch of lakebox acceptance tests, following #5474. Covers 14
leaf scenarios on the write/connection-path commands and JSON output
shapes that weren't in the first batch.

| Test | What it locks down |
|---|---|
| `list/json` | `-o json` shape (no FQDN field, gatewayHost present,
idleTimeout serialized as Duration string) |
| `status/running` | Happy-path text output with all rendered fields |
| `status/json` | JSON shape — verifies the FQDN-removal in #5460 didn't
get reverted |
| `ssh-key/list/with-entries` | Multi-row table including unnamed key
falling back to `(unset)` |
| `ssh-key/delete/success` | DELETE wire route + confirmation message |
| `delete/success` | Happy path with `--auto-approve` to bypass the
prompt |
| `delete/not-found` | 404 → friendly `no lakebox named X` (the
consistency fix from #5460) |
| `delete/no-tty-no-auto-approve` | Non-interactive context fast-fails
pointing at the `--auto-approve` flag |
| `default/set` | Set default → confirmation message |
| `default/not-found` | 404 → friendly error (same message as delete) |
| `start/already-running` | API returns Running → CLI short-circuits
with "Already running X" (skips waitForRunning) |
| `stop/success` | Happy path with refreshed status |
| `config/update-name` | PATCH wire route + post-update output rendering
|
| `create/with-name` | POST wire route + stdout ID + default-set side
effect |

## Two test-infrastructure additions

1. **`acceptance/cmd/lakebox/script.prepare`** sets `HOME=$TEST_TMP_DIR`
so each test's `~/.databricks/lakebox.json` is isolated. Without this,
parallel lakebox tests race each other writing to the shared HOME-rooted
state file and one read sees the other's half-written content. Auth is
unaffected — the framework passes `DATABRICKS_HOST` / `DATABRICKS_TOKEN`
explicitly via env, so the CLI doesn't fall back to `~/.databrickscfg`.

2. **`EnvMatrix.DATABRICKS_BUNDLE_ENGINE = []`** in the parent
`test.toml` overrides the root matrix to skip the per-engine variants.
Lakebox is engine-independent and running both variants in parallel was
the original symptom of the HOME race.

Two of the ssh-key tests also pin the test key hash via a high-priority
`[[Repls]]` entry so the framework's default 3+-digit-run → `[NUMID]`
regex doesn't shred it mid-hash.

## Test plan

- [x] `go test ./acceptance -run TestAccept/cmd/lakebox` — all 20 tests
pass (6 from #5474 + 14 new)
- [x] `./task lint` clean
- [x] Re-running with `-update` and then without is idempotent (no
drift)

## Still out of scope

- `register` — generates and reads real SSH keys via `ssh-keygen`; needs
a heavier fixture / process stub
- `ssh` — `execv`s into ssh; not testable through this framework

This pull request and its description were written by Isaac.
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