diff --git a/README.md b/README.md index 30bd0654..ad334412 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,85 @@ mb transform-tag delete 5 --yes | ------- | --------------------------------------------------------------------------------------------------------------------------------- | | `--yes` | Skip the interactive confirmation prompt. In non-TTY contexts the prompt is skipped automatically (kubectl/gh/docker convention). | +## Transform tests + +CRUD and run on `/api/ee/transform-test`. Requires Metabase v65 or newer with the `transforms-testing` premium feature. A transform test pins a transform's behaviour: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. The transform and the expectations read only those temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, on Postgres, MySQL, H2, ClickHouse, Redshift or SQL Server. + +An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each a `name` and a `cast_type` the warehouse accepts as a `CAST` target) and `rows`; every row carries exactly the declared columns. + +An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. Expectation names are unique within a test. Expectation SQL may name only the transform's target table and its declared input tables, which are rewritten to the run's temp tables; any other table, or a column qualified by a table name rather than an alias, is refused with `transform-test.unremapped-reference`. + +Create and update bodies are closed at every level, so a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent back: + +```sh +mb transform-test get 1 --full --json \ + | jq 'del(.id, .entity_id, .creator_id, .created_at, .updated_at)' \ + | mb transform-test update 1 +``` + +### `mb transform-test list` + +```sh +mb transform-test list --json +mb transform-test list --transform-id 1 +``` + +| Flag | Description | +| --------------------- | ------------------------------------ | +| `--transform-id ` | Only the tests of this transform id. | + +### `mb transform-test get ` + +The compact form carries the id, transform, name and description; `--full` adds the `inputs` and `expectations` themselves. + +```sh +mb transform-test get 1 +mb transform-test get 1 --full --json +``` + +### `mb transform-test create` + +```sh +mb transform-test create --file transform-test.json +``` + +| Flag | Description | +| --------------- | ----------------------- | +| `--body ` | Inline JSON body. | +| `--file ` | Path to JSON body file. | + +### `mb transform-test update ` + +Only the fields the body carries are patched; `inputs` and `expectations` replace what is stored rather than merging into it. + +```sh +mb transform-test update 1 --body '{"name":"renamed"}' +``` + +### `mb transform-test delete ` + +```sh +mb transform-test delete 1 --yes +``` + +| Flag | Description | +| ------- | --------------------------------------------------------------------------------------------------------------------------------- | +| `--yes` | Skip the interactive confirmation prompt. In non-TTY contexts the prompt is skipped automatically (kubectl/gh/docker convention). | + +### `mb transform-test run ` + +Runs the transform against the fixtures and reports what each expectation found. Exits 1 when the test does not pass, so it drops straight into CI. + +```sh +mb transform-test run 1 +mb transform-test run 1 --json +mb transform-test run 1 --timeout 300000 +``` + +| Flag | Description | +| ---------------- | -------------------------------------------------------------------------- | +| `--timeout ` | Request timeout in ms (default 30000); the run is one synchronous request. | + ## Databases Read warehouse metadata from `/api/database`. The `db` group exposes the full database list, the per-database record with optional table/field hydration, schema and table inspection, and the two manual-sync triggers. diff --git a/packages/cli/package.json b/packages/cli/package.json index adac9969..ec69bd84 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@metabase/cli", - "version": "0.4.0", + "version": "0.4.1", "description": "Metabase CLI", "license": "AGPL-3.0", "repository": { diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index 2e20af60..940974d7 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -12,7 +12,7 @@ Top-level command groups (run `mb --help` to discover verbs): ``` auth | db | table | field | upload | content-translation | query | card | dashboard | snippet | segment | measure | collection | library -document | glossary | timeline | timeline-event | transform | transform-job | transform-tag | alert | subscription | setting +document | glossary | timeline | timeline-event | transform | transform-job | transform-tag | transform-test | alert | subscription | setting search | dependency | git-sync | setup | eid | uuid | upgrade | skills ``` @@ -171,6 +171,9 @@ This file is enough for any single-command task. For anything deeper, load the r - **`notification`** — scheduled delivery: question alerts (`mb alert`) and dashboard subscriptions (`mb subscription`). Choosing between them, the two schedule/recipient contracts, channel prerequisites, testing a send. - **`transform`** — transform body JSON, create + run-with-wait, run inspection, tags, jobs. + + +- **`transform-test-plan`** — planning transform tests. - **`document`** — Metabase documents (TipTap body, embedding cards). diff --git a/packages/cli/skill-data/transform-test-plan/SKILL.md b/packages/cli/skill-data/transform-test-plan/SKILL.md new file mode 100644 index 00000000..082bdc40 --- /dev/null +++ b/packages/cli/skill-data/transform-test-plan/SKILL.md @@ -0,0 +1,82 @@ +--- +name: transform-test-plan +description: Derive a comprehensive test plan for a transform — the fixture cast, the expectations, hand-derived expected rows, and a coverage matrix — from the model's declared design. Covers input partitioning (zero-case, multiplicity, dirty rows), grain / conservation / recomputation / conformance checks, and known-quirk conventions. Load when the user wants tests planned or written for transforms — "write tests for my transforms", "is my model right", "test plan for this pipeline", "add data quality checks" — whether the model is mid-build or already deployed. The `mb transform-test` command and body shapes live in the `transform` skill; this one decides what to test. +allowed-tools: Read, Write, Edit, Bash, AskUserQuestion +requires: [transformTests] +--- + +# Planning transform tests + +Turn a transform into a fixture cast that proves the logic on small known rows, expectations that state the model's invariants, and a coverage matrix showing what's checked and what's deliberately not. Mechanics — the `inputs`/`expectations` body shape and every `mb transform-test` verb — live in the `transform` skill (`mb skills get transform`); load it before authoring, and never restate it here. The two references this skill routes through, `references/checklist.md` and `references/checks.md`, come with `mb skills get transform-test-plan --full`, or read them from `mb skills path transform-test-plan`. + +Every check derives from what the model **declares** — detected from its SQL, confirmed with its owner — never from conformance to a modeling doctrine. One plan serves two moments: while the model is **built**, checks pin each design decision; once **deployed**, the same SQL screens production tables for anomalies. + +## Operating rules + +- **Detect, then derive.** Classify what the transform is and which conventions it uses (the checklist); derive checks only from that. Star, one-big-table, partial denormalization — all fine; never flag a style. +- **Judgment calls go through the checklist.** The session's autonomy setting governs which answers you supply yourself and which you bring to the user — it never makes the checklist a formality. Every answer you supply yourself is recorded in the plan as a stated assumption, paired with the named expectation that enforces it — reversing the decision then breaks a test, not a paragraph. And regardless of setting, when genuinely unsure, ask — a wrong-but-confident grain poisons every downstream check. +- **Expected rows are derived by hand** from the fixture story and business meaning — never captured from the transform's output, which asserts only that the transform equals itself. +- **Batteries stay off until declared structure switches them on.** No snapshot-density checks without a snapshot, no version-history checks without effective/end/current columns. An empty section beats a speculative one. + +## The procedure + +1. **Profile the real inputs**: per table, row count; per column, min/max/distinct-count/null incidence; orphan counts across declared links (`mb field summary`, `mb query`). Profiling feeds domains, bounds, null partitions, and key candidates — and every fixture edge cites the real-data condition that warrants it, with its count ("the warehouse has 67 ship-before-order rows"). +2. **Detect.** Read the transform's SQL (`mb transform get --full --json`) for: grain candidates (`GROUP BY` keys, the joins' driving table), join types (orphan handling), correlated aggregates (stored aggregates), `_` naming (copies), effective/end/current columns (version history), `now()`/`current_date` (volatile columns). A transform whose grain you cannot state in one phrase is itself a finding — raise it before writing any test. +3. **Confirm.** Walk [references/checklist.md](references/checklist.md). Three stages: classify the model, per-table declarations, per-column declarations. Each question carries its detection hint; answer autonomously where the hint resolves, ask where it doesn't. In build-along mode these are design questions — treat an undecided answer as a decision to make together, not a blocker. +4. **Derive.** Route every output column through [references/checks.md](references/checks.md) — declared property → expectation shape, fixture implication, expected-row convention. Read it in full once per plan; it is the plan's content. +5. **Design the fixture cast**: one small cast per transform (≈5–10 rows per table), human-named rows ("Alice Premium"), every row a named edge — zero-case entities for every outer join and aggregation, ≥2-member groups for every grouping and join, one dirty row per screenable defect, boundary dates. Document it as a table (row → attributes → purpose) in the plan. + + Express the cast as literal rows rather than as a query, so the story stays legible in the test itself; a query is worth it only when the rows are mechanical to generate. A cast is written against one warehouse and does not carry to another. + +6. **Hand-derive the expected rows**, arithmetic recorded in the plan (premium: 3 orders / 350.50 / 3.0). Every fixture row's fate appears in some expected cell. Pin NULL-vs-0-vs-empty for every zero-case row — that cell is the null policy's only enforcement. +7. **Author the test.** One test per transform, per coherent story: an `equals` pinning the output, declaring every output column, since `equals` compares only the columns it declares, and `empty` expectations stating the invariants that survive a change to the cast. + + Give each expectation a name that states the invariant, because the name is what a failure leads with ("revenue never negative", not "check 3"), and open each `empty` expectation's SQL with a `--` contract comment: the invariant, and the failure modes it catches ("catches both dropped orders and join fan-out"). + + Where one invariant applies to several transforms, duplicate the SQL. Nothing is shared between tests, so give each copy its own name and comment rather than one that only makes sense next to its twin. + +8. **Emit the coverage matrix** in the plan: rows = output columns with the table's grain; columns = check classes (grain, conservation, recomputation, conformance, domains, referential integrity, temporal, screens); cells name the covering expectation or expected-row cell, or state `gap: `. Scan shared-attribute columns across transforms for cross-table agreement obligations. Empty cells are honest; silent gaps are not. +9. **Prove the tests have teeth.** Once per test: corrupt one expected cell, `run`, confirm `cell-mismatches` names exactly that column; revert. Then perturb one input cell and confirm exactly the declared output cells move. An `empty` expectation that only selects output rows passes against an empty output, so a green test can still be vacuous. + +## Severity: error, or the tolerated oddity + +Every expectation is pass or fail, and one failure fails the run. **error** = forbidden, and it becomes an `empty` expectation. There is no warn severity, so **never author an expectation you expect to fire**: a permanently red test trains everyone to ignore the result, and a red run gates everyone else's work. + +A tolerated-but-surfaced oddity — one the owner lives with, like orphan rows or ship-before-order dates — gets encoded the two ways that hold: **pin it in the `equals` rows** (an orphan passing through with NULLs is a cell in the expected output, so reversing the tolerance breaks the test), and **record it in the plan's known-quirks list** with its real-data count, so the tolerance stays a conscious choice. + +Under budget pressure cut business-rule checks first, then cross-table structure checks; never single-column screens (domains, ranges, nulls) — cheapest, and the last line. + +## When a check exposes a live bug + +Non-negotiable: **never soften the test to green** — expected values state correct behavior; matching them to buggy output documents the bug as intended — and **surface the finding with its blast radius at both scales**, fixture ("1530.24 of 1600.74 fixture dollars survive") and warehouse ("908 of 2,050 orders dropped"). What happens next follows the session's terms, not a fixed protocol: propose and apply the fix now (when the user wants it or the autonomy setting covers it), or — when the fix must wait — hold the correct expectation and record the red in the plan with the minimal fix body. A stored test has no red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. Either way, once green the test stays as the regression guard. + +## When the doctrine doesn't apply + +The vocabulary follows Kimball's dimensional modeling (grain, additivity, conformed attributes, slowly changing dimensions) — precise, widely understood terms. Real models are Kimball-inspired at most; no check may score adherence: + +- Full calendar date dimensions are rare. Never demand one; test date _semantics_ — ranges, orderings, volatile derivations. +- Surrogate keys are doctrine, natural-key joins are practice. Test whichever key the model declares; never flag natural-key joins. +- "No NULL FKs / no NULL attributes" is doctrine routinely dropped. Null policy is three independent declarations — measures, attributes, FKs — each detected and confirmed, never presumed. +- One-big-table is legitimate: it still has a grain, its copies still need agreement checks, its functional dependencies still hold. +- A transform-level `ORDER BY` has no testable effect — output tables carry no row order and the `equals` comparison is a multiset. Flag it as probable dead weight (clustering hints aside); never write an ordering expectation. + +## One transform at a time + +Each test covers one transform. When the transform under test reads another transform's output, that target table is an input like any other: declare it and fake it. Derive those rows from the base transform's own expected output, so the two tests tell one story, and note the coupling in both plans — changing the base's expected rows means changing this test's input. + +## Worked example, condensed + +`orders` + `customers` → _enriched_orders_ (order grain; LEFT JOIN attaches `customer_name`, `tier`). + +Cast, 7 orders: two tiers; Alice and Carol with 2 orders each (multiplicity); two never shipped (zero-case for the shipping join); order 106 shipped before ordered (real oddity, count cited); order 107's customer_id matches no customer (orphan). Two `rows` inputs, one per source table. + +Derived, per catalog: grain uniqueness on `order_id`; row and amount conservation from input to output, as scalar subqueries over the target and the seeded input; `tier` domain ⊆ {standard, premium}; orphan = keep-with-NULLs → an expected row pinning the NULL pass-through; ship-before-order tolerated → the plan's known-quirks list with the warehouse count. The whole output, every column, pinned in one `equals`, hand-computed, arithmetic in the plan. The inner-vs-LEFT-join bug this cast catches — unshipped orders silently dropped from revenue — is what the zero-case rows exist for. + +## Don't + +- Don't author expectations before loading the `transform` skill — the body shape, the closed create/update contract, and the verb flags live there. +- Don't capture expected rows from the transform's own output — hand-derive them or they assert nothing. +- Don't emit checks for structure the model doesn't declare (snapshot density, version history, bridge weights) — an inapplicable battery buries real findings. +- Don't let a fixture cast go all-clean — no zero-case, no orphan, no dirty row proves the happy path and nothing else; the bugs live in the edges. +- Don't write an expectation you expect to fail. +- Don't surface bare check-ids ("per C3…") to the user — name the check in plain words; the ids are for your cross-referencing, not their reading. diff --git a/packages/cli/skill-data/transform-test-plan/references/checklist.md b/packages/cli/skill-data/transform-test-plan/references/checklist.md new file mode 100644 index 00000000..3f5609e9 --- /dev/null +++ b/packages/cli/skill-data/transform-test-plan/references/checklist.md @@ -0,0 +1,32 @@ +# Confirmation checklist + +The judgment calls a plan depends on, in derivation order. Each question carries a **detect** hint — how to answer it from the SQL, the metadata, or profiling. Answer autonomously where the hint resolves cleanly; ask where it doesn't. Every answer you supply yourself goes into the emitted plan as a stated assumption, phrased so the owner can falsify it at a glance ("Assuming `discount` can never be negative — correct?"). + +In build-along mode these are design questions: the owner is deciding, not recalling. An undecided answer is a decision to make together — surface the options and the check each implies (the `checks.md` entry named in parentheses). + +## Stage 1 — classify the model (per transform) + +1. **Grain: one row per what?** (G1, G2) Detect: `GROUP BY` keys; otherwise the driving table of the joins. Confirm whenever detection and any written description disagree — that disagreement is itself a finding. +2. **Fact-table type — and if periodic snapshot, dense or sparse?** (F2) Detect: event-grain with a single event date → transaction; entity×period grain → periodic snapshot (density is the one yes/no that flips its whole invariant set — always confirm); one row per pipeline occurrence with milestone-date columns → accumulating snapshot. + +## Stage 2 — per-table declarations + +3. **Conservation ties: output rows/sums tie to which input, with which declared exclusions?** (A2) Detect: `WHERE` clauses and join types name the exclusions (filtered rows, dropped duplicates). Exclusions must be declared, or conservation appears to fail. +4. **Zero-group policy: does a group with no contributing rows appear (with zeros) or stay absent?** (A5) Detect: driving table of the rollup — grouping the detail can't produce empty groups; joining a dimension first can. +5. **Orphan handling per join: drop / keep-with-NULLs / default row — and is an orphan tolerated or forbidden?** (C3, I5) Detect: join type. INNER = drop, LEFT = keep-with-NULLs, COALESCE to a sentinel = default row. Tolerated-vs-forbidden is the owner's call — profiling says whether orphans exist today, not whether they're acceptable. Forbidden becomes an `empty` expectation; tolerated is pinned in the expected rows and recorded in the known-quirks list. +6. **Version history: effective/end/current columns anywhere? If not, confirm overwrite-everywhere.** (T1) Detect: column names. Absence means history is silently rewritten in rollups — state that consequence when confirming, not just the mechanism. +7. **Does this transform read another transform's output?** (A5) Detect: another transform's target table in this one's SQL. It becomes a declared input like any other, faked from that transform's own expected output — confirm which rows, and record the coupling in both plans. + +## Stage 3 — per-column declarations + +8. **Each measure: additive, semi-additive, or non-additive?** (A1) Detect: sums and counts are additive; balances and levels are semi-additive; ratios, rates, and unit prices are non-additive. Confirm the ambiguous ones (a "score"? a "quantity on hand"?). +9. **Legal bounds per measure?** (Q1) Detect: profiling min/max suggests, business meaning decides — can `discount` be negative? can `quantity` be zero? Derived bounds are free (a sum of non-negatives is non-negative). +10. **Each derived measure: the exact recomputation rule?** (A3) Detect: the SELECT expression is the rule — but rounding, business-day adjustments, and NULL handling are conventions to confirm, not read. +11. **Which columns are stored aggregates, over which detail and filter?** (A4) Detect: correlated subqueries / joined-aggregate CTEs in the SQL. +12. **Which categorical columns have closed domains — enumerate; is NULL a member?** (C2) Detect: profiling distinct values gives today's set; the owner confirms it's closed rather than merely small so far. +13. **Which columns are denormalized copies, of which owning attribute?** (C1) Detect: `_` naming; any column functionally dependent on a non-grain key. Every hit must be classified: copy (C1), stored aggregate (A4), or smuggled coarser-grain fact — a finding. +14. **Which many-to-one edges hold within one output table?** (C4) Detect: hierarchy-shaped column pairs (product/category, zip/state). Applies where no owning table is in scope; otherwise C1 covers it. +15. **Which date orderings are business-guaranteed vs. known-violated and tolerated (with real-data counts)?** (T2) Detect: profiling counts the violations that exist; the owner decides tolerated vs. bug. +16. **Null policy per column — measures, attributes, FKs separately.** (Q2) Detect: the SQL shows what's produced (COALESCE, CASE); the owner confirms intent — "count of nothing": 0 or NULL? empty date: NULL or sentinel? +17. **Which columns are volatile across runs?** (T3) Detect: `now()` / `current_date` / run-metadata expressions in the SQL. A volatile column is left out of the `equals` columns; its form can still be asserted with an `empty`. +18. **Per tolerated oddity: does it stay tolerated, and what is its current count?** (Q3) Detect: can't — this is the conscious-choice question, and the plan's known-quirks list is where it survives. diff --git a/packages/cli/skill-data/transform-test-plan/references/checks.md b/packages/cli/skill-data/transform-test-plan/references/checks.md new file mode 100644 index 00000000..8714787a --- /dev/null +++ b/packages/cli/skill-data/transform-test-plan/references/checks.md @@ -0,0 +1,156 @@ +# Check catalog + +Every check the plan can derive, grouped by theme. Per entry: **when it applies** (the declared property that switches it on — blank means always), **assert** (the expectation shape), and **fixtures** (what the input cast must contain for the check to have teeth). + +Expectation SQL names the transform's **target table** and its **declared input tables** under their real names; both are rewritten to the run's temp tables, so per-row recomputation joins against inputs are writable. Anything else you name is refused (`transform-test.unremapped-reference`), so no check can read a real table. + +Ids (G1, A2, …) are for cross-referencing within the plan documents only — never surface them to the user bare. + +Throughout, `` stands for the transform's target table as written in the transform's own definition, and `` for a declared input table. + +`IS DISTINCT FROM` is the null-safe inequality; MySQL spells it `NOT (a <=> b)`, and where neither parses, `a <> b OR (a IS NULL) <> (b IS NULL)`. + +## Grain & keys + +**G1 — Declared grain.** Every output table states "one row per X"; the declaration anchors every other check. + +- Applies: always. Elicit or detect (GROUP BY keys; the join's driving table). +- Assert: nothing directly — G1 is the plan's opening move. A table with no statable grain, or a grain stated inconsistently between docs and SQL, is a finding before any SQL runs. +- Fixtures: the grain declaration decides the cast's row structure. + +**G2 — Grain-key uniqueness.** The grain key stays a key; doubles as the fan-out guard for every enriching join. + +- Applies: always — one-big-table, rollups, and stars all have a grain. +- Assert (`empty`): `SELECT , COUNT(*) FROM GROUP BY HAVING COUNT(*) > 1`. NULL grain values form their own bucket. +- Fixtures: a parent with ≥2 children on every join (I4) is what makes this check able to fail. + +## Additivity & reconciliation + +**A1 — Measure additivity classification.** Additive / semi-additive (balances) / non-additive (ratios, rates, unit prices) decides which aggregations are valid tests. + +- Applies: every numeric measure; per-column declaration. +- Assert: routes the measure — additive → A2; semi-additive → sum across non-time slices only (a test summing a balance over time is a bug in the plan); non-additive → A3 recomputation, never reconciled by summing. +- Fixtures: none directly; one coverage-matrix axis (measure × valid aggregation set). + +**A2 — Conservation reconciliation.** Row counts and additive-measure sums tie from input to output; catches dropped rows and join double-counting at once. + +- Applies: per declared tie (which input, which declared exclusions). +- Assert (`empty`): independent scalar subqueries compared with `IS DISTINCT FROM`, returning the mismatched pair: `SELECT (SELECT SUM(t.m) FROM t) AS output_sum, (SELECT SUM(s.m) FROM s) AS input_sum WHERE (…) IS DISTINCT FROM (…)`. Never reconcile via a fact-to-fact join — cardinality is uncontrollable and wrong results are silent. +- Fixtures: amounts chosen so partial survival is visible (distinct values, odd cents). + +**A3 — Derived-measure recomputation.** Averages, lags, ratios, rounded presentations recompute from their components per row. + +- Applies: every derived column, with its declared rule (rounding, business-day adjustment, NULL handling). +- Assert (`empty`): `SELECT FROM t WHERE t. IS DISTINCT FROM `. +- Fixtures: component values whose derivation is non-trivial (a NULL in the AVG, a negative lag). + +**A4 — Stored aggregates on entity tables.** Lifetime/rollup stats carried on an entity-grain table (`lifetime_orders`, `review_count`, `first_order_date`) equal recomputation from detail — per row _and_ in total. + +- Applies: columns detected as correlated aggregates over a detail table. +- Assert (`empty`), two per column: per-row — `WHERE t. IS DISTINCT FROM (SELECT COUNT(*)/SUM(…)/MIN(…) FROM d WHERE d. = t.)`; total — the A2 scalar-pair shape. Totals alone cancel offsetting per-row errors; the per-row form is the one that catches them. +- Fixtures: an entity with several detail rows and an entity with none (I3/I4). + +**A5 — Rollup consistency.** A layered rollup always agrees with its base. + +- Applies: any transform reading another transform's output. The base is a declared input like any other, faked with rows taken from the base transform's own expected output. +- Assert (`empty`): every rollup measure equals the corresponding aggregation over the base (A2 shape); group-set equality both directions — `SELECT FROM EXCEPT SELECT DISTINCT FROM ` and the reverse, filtered by the declared zero-group policy. +- Fixtures: a group with no contributing rows pins the zero-group policy. + +## Fact-table type + +**F2 — Fact-table type bundle.** Transaction / periodic snapshot / accumulating snapshot each carry a distinct invariant set. + +- Applies: per fact-shaped output, always classified; **transaction** needs nothing beyond G2 + A2 (sparsity is legitimate). +- Periodic snapshot, if declared _dense_: `COUNT(*) = |entities| × |periods|` (or the declared subset); per-entity gap detection in the period series; inactive-period representation (zero vs NULL) pinned in the expected rows. If sparse, density checks off — the one yes/no flips the whole set. +- Accumulating snapshot: milestone dates monotone in pipeline order where set (`WHERE < ` per adjacent pair); unset-milestone default (NULL vs sentinel) pinned in the expected rows; completion flags ∈ {0,1} and consistent with their date's set-ness; lags via A3. +- Fixtures (accumulating): occurrences at every completion stage — none, some, all milestones. + +## Conformance & domains + +**C1 — Denormalized-copy agreement.** Every copied attribute agrees with the owning table's value for that key. + +- Applies: columns declared as copies (detect: `_` naming; any column functionally dependent on a non-grain key). Each such column must be a declared copy (this check), a stored aggregate (A4), or it's a smuggled coarser-grain fact — a finding: it double-counts under summation. +- Assert (`empty`): `SELECT t., t., d. FROM t JOIN d ON t. = d. WHERE t. IS DISTINCT FROM d.`. +- Scope: in a test, the copies are produced by the very join under test, so this join-form is near- tautological — the C4 functional-dependency form plus expected-row cell pinning carries the test. The join-form against an independently materialized owning table is the _drift_ check, which belongs to whatever screens the deployed tables. +- Fixtures: copies with distinct values per entity so a crossed join is visible. + +**C2 — Closed-domain screen.** A categorical column's values stay inside the declared enumeration. + +- Applies: per column declared closed (detect from profiling; confirm the set and whether NULL is a member). +- Assert (`empty`): `SELECT , FROM WHERE IS NOT NULL AND NOT IN ()`. +- Fixtures: every domain value represented where practical; one out-of-domain input row if the source can produce one (I5). + +**C3 — Referential integrity / orphan policy.** Every FK resolves, or the declared orphan handling is pinned. + +- Applies: per join, conditioned on the declared response — drop / keep-with-NULLs / default row; tolerated or forbidden. +- Assert, forbidden (`empty`): `SELECT t. FROM t LEFT JOIN d ON t. = d. WHERE t. IS NOT NULL AND d. IS NULL`. Tolerated: not an expectation — pin the orphan's pass-through as a row in the `equals` (NULL in the copied attributes), and record the tolerance with its warehouse count in the plan. Default-row convention adds: distinct unknown keys must not collapse into one output row. +- Fixtures: one orphan row (I5). Without it the join direction is untested — an inner join silently dropping unmatched rows is the classic bug this catches. + +**C4 — Many-to-one consistency.** Each declared many-to-one edge holds within the output (product → one category; zip → one state). + +- Applies: per declared edge; where an owning table is in scope, C1 subsumes it — this is the one-big-table variant with nothing to join against. +- Assert (`empty`): `SELECT t., COUNT(DISTINCT t.) FROM t GROUP BY t. HAVING COUNT(DISTINCT t.) > 1`. +- Fixtures: a violating input row if the source can produce one (I5), pinning the transform's behavior on dirty input. + +## Temporal & version history + +**T1 — Change handling per attribute.** How the model treats a changed source attribute decides the temporal fixtures. + +- Applies: detect effective/end/current housekeeping columns. +- Present → version-history battery (`empty` each): per durable key exactly one current row; `effective < end` per row; intervals contiguous and non-overlapping; current row's end = the declared far-future default; fact rows join the version whose interval contains the fact date. +- Absent → confirm overwrite-everywhere as a stated assumption (history is silently rewritten in rollups), and derive the propagation probe: change an attribute in an input, run, assert the output regrouped. +- Fixtures: a before/after change pair; for version history, an entity with ≥2 versions and a fact row dated inside each interval. + +**T2 — Date-pair ordering.** Business-guaranteed orderings asserted; known violations recorded. + +- Applies: per declared date pair (ordered ≤ shipped ≤ delivered; signup ≤ first order), each classified guaranteed vs. tolerated-violated. +- Assert: guaranteed (`empty`) — `SELECT , , FROM WHERE < `. Tolerated — pin the violating row's downstream arithmetic (a negative lag inside an average) in the expected rows, and record the real-data count in the plan. +- Fixtures: one violating row for every tolerated ordering. + +**T3 — Volatile columns.** Values that change across runs can't be pinned. + +- Applies: columns derived from `now()`/`current_date` (age), run metadata (load timestamps, batch ids). +- Assert: leave the column out of the `equals` `columns` list — an undeclared column never enters the comparison. Separately assert its form where warranted (`empty`): `WHERE age NOT BETWEEN 0 AND 120`. The stable source column (`birth_date`) stays exact in the expected rows. +- Fixtures: none special; the split is the point — omit the volatile, pin the stable. + +## Screens & severity + +**Q1 — Range / sign screens.** Each measure's declared bounds hold. + +- Applies: per bounded measure; bounds from business meaning plus profiling (derived bounds are free: a sum of non-negatives is non-negative). +- Assert (`empty`): `SELECT , FROM WHERE < OR > ` (one-sided where only one bound exists). +- Fixtures: boundary values where the bound is business-set. + +**Q2 — Null policy, three ways.** Measures, descriptive attributes, and FKs carry independent null policies; never presume one from another. + +- Applies: per column, asked separately — "count of nothing": 0 or NULL? empty date: NULL or sentinel? FK: see C3. +- Assert: declared non-null columns get `WHERE IS NULL` (`empty`). Otherwise the policy is enforced by the expected-row cell of a zero-case fixture row — NULL vs 0 vs empty is invisible until a fixture forces the choice into a cell. +- Fixtures: the zero-case row (I3) is the enforcement mechanism. + +**Q3 — Naming & the tolerated oddity.** Every expectation declares its meaning. + +- Applies: always, every expectation. +- Convention: the expectation's `name` states the invariant in plain words, because a failure leads with it. Each `empty` SQL opens with 1–3 comment lines naming the invariant and the failure modes it catches. Budget cuts drop business-rule checks first, then cross-table structure checks, never single-column screens. +- A forbidden condition is an `empty` expectation. A tolerated one is pinned in the expected rows and recorded in the plan's known-quirks list with its warehouse count. Never author an expectation designed to fire. + +## Input modeling (fixture-design rules) + +**I1 — Profiling-derived partitions.** Fixture edges and tolerated non-ties cite profiled reality with counts; the plan's known-quirks section carries each tolerated oddity with its reason and count. + +**I2 — Shared fixture cast.** One small human-named cast per transform, every row a named edge, documented as a story table (row → attributes → purpose) in the plan. Where one transform's output is another's input, derive the faked rows from the first's expected output so the two tests tell one story. + +**I3 — Zero-case partitions.** For every outer join and aggregation relation: one entity with zero matches. Its expected row pins the absence representation (Q2) and the join direction (C3) — the all-clean cast is the single most common cause of vacuous tests. When the schema distinguishes states the profiled data never exhibits (a literal zero in a source that only has NULLs and positives), fixture the missing state — nothing else forces it onto an expected cell. + +**I4 — Multiplicity partitions.** Every aggregation gets a >1-member group and an exactly-1 group (the 0 case is I3); every join gets a parent with ≥2 children. Expected values then differ from any single row's, so copy-through bugs can't pass. + +**I5 — Dirty-input pinning.** Per screenable defect the source can carry (orphan FK, out-of-domain value, duplicate natural key, hierarchy violation, ordering violation): one fixture row exhibiting it, an expected row pinning the transform's response (drop / pass-through / default), and — where the defect is forbidden — the matching `empty` expectation. Which duplicate survives a dedupe is invisible until a conflicting-duplicate fixture pins it. + +**I6 — Mutation probes.** Run at plan-validation time, not on every run: corrupt one expected cell → `cell-mismatches` must name that exact column; perturb one input cell → exactly the declared output cells move. Both guard against vacuous tests: an `empty` expectation that only selects output rows passes against an empty output. + +## Plan-level artifacts + +**P1 — Coverage matrix.** Output columns (with the table's grain) × check classes; cells name the covering expectation or expected-row cell, or state `gap: `; shared-attribute columns scanned across transforms for agreement obligations (C1). + +**P2 — Hand-derived expected rows.** From the fixture story and business meaning, arithmetic recorded in the plan; never captured from output; every fixture row's fate appears in some expected cell. + +**P3 — Live-bug handling.** A check failing against a deployed transform is a real finding: never soften the test to green; quantify the damage at fixture and warehouse scale. Then fix now, or record the red in the plan with the minimal fix body, per the session's terms. A stored test has no red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index cb9e5bba..bf8d9662 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -1,6 +1,6 @@ --- name: transform -description: Author and run Metabase transforms via `mb` — body shape (native SQL or structured MBQL), create + run-with-wait, run inspection, dependencies, cancel, the `update`-vs-recreate iteration rule, the writable-keys-only PATCH contract, plus transform tags and tag-driven transform-job schedules. Load when the user touches transforms — "create a transform", "run a transform", "fix a failing transform", "list transform runs", "cancel a running transform", "manage transform tags", "run a transform job", or anything `mb transform …` / `mb transform-job …` / `mb transform-tag …`. +description: Author and run Metabase transforms via `mb` — body shape (native SQL or structured MBQL), create + run-with-wait, run inspection, dependencies, cancel, the `update`-vs-recreate iteration rule, the writable-keys-only PATCH contract, transform tests (fixtures and expectations run against temp tables), plus transform tags and tag-driven transform-job schedules. Load when the user touches transforms — "create a transform", "run a transform", "fix a failing transform", "test a transform", "list transform runs", "cancel a running transform", "manage transform tags", "run a transform job", or anything `mb transform …` / `mb transform-job …` / `mb transform-tag …` / `mb transform-test …`. allowed-tools: Read, Write, Edit, Bash, AskUserQuestion requires: [transforms] --- @@ -173,6 +173,68 @@ mb transform run "$ID" --wait --profile --json # → succeeded If you really must `create + delete` instead, do the `delete` **before** the first `git-sync export` so the failed entity never lands in git history — an export of a soft-failed state is noise that needs a follow-up cleanup commit. See `git-sync`, "Read state before mutating", for the ordering rule. + + +## Transform tests + +A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. The transform and the expectations read only the run's temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, and only on Postgres, MySQL, H2, ClickHouse, Redshift or SQL Server; otherwise `create`/`update`/`run` refuse with `transform-test.unsupported-transform` / `unsupported-driver` (422). + +```bash +mb transform-test list --transform-id --profile --json # --transform-id is optional +mb transform-test get --full --profile --json # --full for inputs/expectations +mb transform-test create --file ./.scratch/test.json --profile --json +mb transform-test update --file ./.scratch/patch.json --profile --json +mb transform-test delete --yes --profile +mb transform-test run --profile --json # exits 1 unless it passes +``` + +**`inputs`** — one per table the transform reads, each naming a `table` plus either `format: "sql"` with `sql`, or `format: "rows"` with `columns` (each a `name` and a `cast_type`) and `rows`. + +**`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `BIGINT`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. + +**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. `equals` compares the rows as a multiset over the declared columns only, so an output column it does not declare is never checked. An `equals` without a `format` is refused, and one with `format: "sql"` is saved but reports `error` on every run, so its test can never pass. + +**Expectation SQL may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference` (400), at `create`/`update` as well as `run`. A column qualified by its table name (`people.id`) is refused the same way, in expectation SQL and in the transform's own SQL alike: qualify by an alias (`FROM people p … p.id`) or leave the column bare — for the transform's SQL that means editing the transform. Expectation names are unique within a test (refused with "expectation names must be unique within a test"), and every `rows` row carries exactly its declared columns (refused naming the missing or undeclared columns). + +```json +{ + "transform_id": 1, + "name": "adults only", + "inputs": [ + { + "table": { "schema": "public", "name": "people" }, + "format": "rows", + "columns": [ + { "name": "id", "cast_type": "INTEGER" }, + { "name": "age", "cast_type": "INTEGER" } + ], + "rows": [ + { "id": 1, "age": 30 }, + { "id": 2, "age": 12 } + ] + } + ], + "expectations": [ + { + "type": "equals", + "name": "exactly one row, id 1", + "format": "rows", + "columns": [{ "name": "id", "cast_type": "INTEGER" }], + "rows": [{ "id": 1 }] + }, + { + "type": "empty", + "name": "no null ids", + "sql": "SELECT * FROM public.adults WHERE id IS NULL" + } + ] +} +``` + +Create and update bodies are closed at every level (the top level is checked before sending; a stray nested key is a 400) — strip `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` from a `get --full` body before sending it back. + + + ## Drop the materialized table (keep the transform) ```bash diff --git a/packages/cli/src/commands/git-sync/sync-task.ts b/packages/cli/src/commands/git-sync/sync-task.ts index e4cb8503..9085db3e 100644 --- a/packages/cli/src/commands/git-sync/sync-task.ts +++ b/packages/cli/src/commands/git-sync/sync-task.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { isSyncTaskFailed, SyncTask } from "@metabase/client/domain/git-sync"; +import { FailedResultError } from "@metabase/client/errors"; import type { PollOptions } from "@metabase/client/poll"; import type { ResourceView } from "../../output/view"; @@ -38,9 +39,9 @@ export function throwIfFailedTask(final: SyncTask | null, verb: string): void { } const failure = withDetail(`git-sync ${verb} ${final.status}`, taskDetail(final)); if (final.status === "conflict") { - throw new Error(`${sentence(failure)} ${CONFLICT_REMEDY}`); + throw new FailedResultError(`${sentence(failure)} ${CONFLICT_REMEDY}`); } - throw new Error(failure); + throw new FailedResultError(failure); } // A task that ends in conflict carries no error message: the labels of the entities it conflicted diff --git a/packages/cli/src/commands/skills/path.ts b/packages/cli/src/commands/skills/path.ts index 014321a5..db070cda 100644 --- a/packages/cli/src/commands/skills/path.ts +++ b/packages/cli/src/commands/skills/path.ts @@ -20,7 +20,7 @@ const SkillPath = z.object({ }); type SkillPathJson = z.infer; -export const SkillPathListEnvelope = listEnvelopeSchema(SkillPath); +const SkillPathListEnvelope = listEnvelopeSchema(SkillPath); const skillPathView: ResourceView = { compactPick: SkillPath, diff --git a/packages/cli/src/commands/transform-test/create.ts b/packages/cli/src/commands/transform-test/create.ts new file mode 100644 index 00000000..eec06f7c --- /dev/null +++ b/packages/cli/src/commands/transform-test/create.ts @@ -0,0 +1,38 @@ +import { TransformTest, TransformTestCreateInput } from "@metabase/client/domain/transform-test"; + +import { renderSummary } from "../../output/render"; +import { transformTestView } from "../../output/views/transform-test"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "create", description: "Create a transform test from JSON" }, + details: + 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. The body is closed at every level: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.', + requires: ["transformTest.create"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + }, + inputSchema: TransformTestCreateInput, + outputSchema: TransformTest, + examples: [ + "cat transform-test.json | mb transform-test create", + "mb transform-test create --file transform-test.json", + ], + async run({ args, ctx, getClient }) { + const body = await readBody({ flag: args.body, file: args.file }, TransformTestCreateInput); + const client = await getClient(); + const created = await client.transformTest.create(body); + renderSummary( + created, + transformTestView, + `Created transform test ${created.id} "${created.name}".`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/transform-test/delete.ts b/packages/cli/src/commands/transform-test/delete.ts new file mode 100644 index 00000000..93e25085 --- /dev/null +++ b/packages/cli/src/commands/transform-test/delete.ts @@ -0,0 +1,31 @@ +import { confirmAndDelete, DeleteResult } from "../delete-runtime"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "delete", description: "Delete a transform test by id" }, + requires: ["transformTest.delete"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + yes: { type: "boolean", description: "Skip confirmation", default: false }, + id: { type: "positional", description: "Transform test id", required: true }, + }, + outputSchema: DeleteResult, + examples: ["mb transform-test delete 1 --yes", "mb transform-test delete 1"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + await confirmAndDelete({ + id, + yes: args.yes, + promptMessage: `Delete transform test ${id}?`, + successMessage: `Deleted transform test ${id}.`, + abortMessage: `Aborted; transform test ${id} was not deleted.`, + deleteResource: () => client.transformTest.delete(id), + ctx, + }); + }, +}); diff --git a/packages/cli/src/commands/transform-test/get.ts b/packages/cli/src/commands/transform-test/get.ts new file mode 100644 index 00000000..f4a4ce63 --- /dev/null +++ b/packages/cli/src/commands/transform-test/get.ts @@ -0,0 +1,28 @@ +import { TransformTest } from "@metabase/client/domain/transform-test"; + +import { renderItem } from "../../output/render"; +import { transformTestView } from "../../output/views/transform-test"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "get", description: "Get a transform test by id" }, + details: + "The compact form carries the id, transform, name and description. Pass --full for the `inputs` and `expectations` themselves, which is also the body to edit and send back to `update` once `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` are shed.", + requires: ["transformTest.get"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + id: { type: "positional", description: "Transform test id", required: true }, + }, + outputSchema: TransformTest, + examples: ["mb transform-test get 1", "mb transform-test get 1 --full --json"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + const transformTest = await client.transformTest.get(id); + renderItem(transformTest, transformTestView, ctx); + }, +}); diff --git a/packages/cli/src/commands/transform-test/index.ts b/packages/cli/src/commands/transform-test/index.ts new file mode 100644 index 00000000..757ae02b --- /dev/null +++ b/packages/cli/src/commands/transform-test/index.ts @@ -0,0 +1,14 @@ +import { defineCommandGroup } from "../group"; + +export default defineCommandGroup({ + name: "transform-test", + description: "Manage transform tests — fixtures and expectations run against temp tables", + subCommands: { + list: () => import("./list").then((mod) => mod.default), + get: () => import("./get").then((mod) => mod.default), + create: () => import("./create").then((mod) => mod.default), + update: () => import("./update").then((mod) => mod.default), + delete: () => import("./delete").then((mod) => mod.default), + run: () => import("./run").then((mod) => mod.default), + }, +}); diff --git a/packages/cli/src/commands/transform-test/list.ts b/packages/cli/src/commands/transform-test/list.ts new file mode 100644 index 00000000..09dc6603 --- /dev/null +++ b/packages/cli/src/commands/transform-test/list.ts @@ -0,0 +1,34 @@ +import { TransformTestCompact } from "@metabase/client/domain/transform-test"; + +import { renderList } from "../../output/render"; +import { listEnvelopeSchema } from "../../output/types"; +import { windowList } from "../../output/window"; +import { transformTestView } from "../../output/views/transform-test"; +import { connectionFlags, listFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export const TransformTestListEnvelope = listEnvelopeSchema(TransformTestCompact); + +export default defineMetabaseCommand({ + meta: { name: "list", description: "List transform tests" }, + requires: ["transformTest.list"], + args: { + ...outputFlags, + ...listFlags, + ...profileFlag, + ...connectionFlags, + "transform-id": { type: "string", description: "Only the tests of this transform id" }, + }, + outputSchema: TransformTestListEnvelope, + examples: ["mb transform-test list", "mb transform-test list --transform-id 1 --json"], + async run({ args, ctx, getClient }) { + const client = await getClient(); + const transformId = + args["transform-id"] === undefined + ? undefined + : parseId(args["transform-id"], "--transform-id"); + const { data } = await client.transformTest.list({ "transform-id": transformId }); + renderList(windowList(data, ctx.range), transformTestView, ctx); + }, +}); diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts new file mode 100644 index 00000000..540db90d --- /dev/null +++ b/packages/cli/src/commands/transform-test/run.ts @@ -0,0 +1,77 @@ +import { + type TransformTestExpectationResult, + TransformTestRunResult, +} from "@metabase/client/domain/transform-test"; +import { ConfigError, FailedResultError } from "@metabase/client/errors"; +import { DEFAULT_TIMEOUT_MS } from "@metabase/client/http/transport"; + +import { renderSummaryWithinMaxBytes } from "../../output/render"; +import { transformTestRunResultView } from "../../output/views/transform-test"; +import { interruptSignal } from "../../runtime/interrupt"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +type ExpectationStatus = TransformTestExpectationResult["status"]; + +function namesOf( + expectations: TransformTestExpectationResult[], + status: ExpectationStatus, +): string[] { + return expectations.filter((expectation) => expectation.status === status).map((e) => e.name); +} + +function summaryLine(id: number, result: TransformTestRunResult): string { + if (result.status === "passed") { + return `Transform test ${id} passed ${result.expectations.length} expectation(s).`; + } + const failed = namesOf(result.expectations, "failed"); + const errored = namesOf(result.expectations, "error"); + const parts = [ + failed.length > 0 ? `failed: ${failed.join(", ")}` : null, + errored.length > 0 ? `could not run: ${errored.join(", ")}` : null, + ].filter((part) => part !== null); + return `Transform test ${id} ${result.status} — ${parts.join("; ")}.`; +} + +export default defineMetabaseCommand({ + meta: { name: "run", description: "Run a transform test by id" }, + details: + "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. The transform and the expectations read only those temp tables; a `format: \"sql\"` input runs verbatim against the transform's source database. The run is one synchronous request, so --timeout bounds the whole of it. --json reports what each expectation found, including the rows a comparison disagreed on. A test that does not pass exits 1, also when its report is over --max-bytes and is not printed; the error then names the expectations that did not pass.", + requires: ["transformTest.run"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + timeout: { + type: "string", + description: "Request timeout in ms", + default: String(DEFAULT_TIMEOUT_MS), + }, + id: { type: "positional", description: "Transform test id", required: true }, + }, + outputSchema: TransformTestRunResult, + examples: [ + "mb transform-test run 1", + "mb transform-test run 1 --json", + "mb transform-test run 1 --timeout 300000", + ], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const timeoutMs = parseId(args.timeout, "timeout"); + const client = await getClient(); + const result = await client.transformTest.run(id, { timeoutMs, signal: interruptSignal }); + const summary = summaryLine(id, result); + const oversize = renderSummaryWithinMaxBytes(result, transformTestRunResultView, summary, ctx); + if (result.status === "passed") { + if (oversize !== null) { + throw new ConfigError(oversize); + } + return; + } + // The run has already happened, so a report too big to print still owes the caller its outcome. + const failure = + oversize === null ? `transform test ${id} ${result.status}` : `${summary} Its ${oversize}`; + throw new FailedResultError(failure); + }, +}); diff --git a/packages/cli/src/commands/transform-test/update.ts b/packages/cli/src/commands/transform-test/update.ts new file mode 100644 index 00000000..dadf9d70 --- /dev/null +++ b/packages/cli/src/commands/transform-test/update.ts @@ -0,0 +1,36 @@ +import { TransformTest, TransformTestUpdateInput } from "@metabase/client/domain/transform-test"; + +import { renderSummary } from "../../output/render"; +import { transformTestView } from "../../output/views/transform-test"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "update", description: "Update a transform test from JSON" }, + details: + "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it. The body is closed at every level: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.", + requires: ["transformTest.update"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + id: { type: "positional", description: "Transform test id", required: true }, + }, + inputSchema: TransformTestUpdateInput, + outputSchema: TransformTest, + examples: [ + "cat patch.json | mb transform-test update 1", + "mb transform-test update 1 --file patch.json", + ], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const body = await readBody({ flag: args.body, file: args.file }, TransformTestUpdateInput); + const client = await getClient(); + const updated = await client.transformTest.update(id, body); + renderSummary(updated, transformTestView, `Updated transform test ${updated.id}.`, ctx); + }, +}); diff --git a/packages/cli/src/commands/transform/run.ts b/packages/cli/src/commands/transform/run.ts index 32f321a5..083a677a 100644 --- a/packages/cli/src/commands/transform/run.ts +++ b/packages/cli/src/commands/transform/run.ts @@ -1,4 +1,5 @@ import { isTransformRunFailed, TransformRunResult } from "@metabase/client/domain/transform"; +import { FailedResultError } from "@metabase/client/errors"; import { renderSummary } from "../../output/render"; import type { ResourceView } from "../../output/view"; @@ -19,7 +20,7 @@ const transformRunResultView: ResourceView = { export default defineMetabaseCommand({ meta: { name: "run", description: "Trigger a transform run by id" }, details: - "Starts a run and returns immediately. --wait polls the run to a terminal status. --sync additionally waits until the run's output table is registered and returns its `target_table_id`, so you can build MBQL cards against it — the run registers the table itself, so no separate `db sync-schema` is needed; --sync implies waiting for the run.", + "Starts a run and returns immediately. --wait polls the run to a terminal status. --sync additionally waits until the run's output table is registered and returns its `target_table_id`, so you can build MBQL cards against it — the run registers the table itself, so no separate `db sync-schema` is needed; --sync implies waiting for the run. While waiting, a run that does not succeed exits 1.", requires: ["transform.run"], args: { ...outputFlags, @@ -63,7 +64,7 @@ export default defineMetabaseCommand({ // Waiting was asked for, so the absence of a run to report on means none was started. if (result.run_id === null || result.final === null) { renderSummary(result, transformRunResultView, result.message, ctx); - throw new Error(`transform run did not start: ${result.message}`); + throw new FailedResultError(`transform run did not start: ${result.message}`); } const status = result.final.status; @@ -75,7 +76,7 @@ export default defineMetabaseCommand({ ); if (isTransformRunFailed(status)) { - throw new Error(`transform run ${result.run_id} ${status}`); + throw new FailedResultError(`transform run ${result.run_id} ${status}`); } }, }); diff --git a/packages/cli/src/core/skills.test.ts b/packages/cli/src/core/skills.test.ts index 9911b161..4cfa1f29 100644 --- a/packages/cli/src/core/skills.test.ts +++ b/packages/cli/src/core/skills.test.ts @@ -682,6 +682,7 @@ describe("the shipped skills", () => { "data-action": ["dataActionsWithoutModel"], "git-sync": ["remoteSync"], transform: ["transforms"], + "transform-test-plan": ["transformTests"], }); const withMarkersLeft = all .filter((skill) => { @@ -702,6 +703,7 @@ describe("the shipped skills", () => { "native-sql", "notification", "transform", + "transform-test-plan", "visualization", ]); expect(withMarkersLeft).toEqual([]); diff --git a/packages/cli/src/main.ts b/packages/cli/src/main.ts index 2340c5e3..8636fb12 100644 --- a/packages/cli/src/main.ts +++ b/packages/cli/src/main.ts @@ -31,6 +31,7 @@ const main: CommandDef = defineCommand({ transform: () => import("./commands/transform").then((mod) => mod.default), "transform-job": () => import("./commands/transform-job").then((mod) => mod.default), "transform-tag": () => import("./commands/transform-tag").then((mod) => mod.default), + "transform-test": () => import("./commands/transform-test").then((mod) => mod.default), setting: () => import("./commands/setting").then((mod) => mod.default), search: () => import("./commands/search").then((mod) => mod.default), "git-sync": () => import("./commands/git-sync").then((mod) => mod.default), diff --git a/packages/cli/src/output/error.test.ts b/packages/cli/src/output/error.test.ts index 5c02f3af..db5c2ab0 100644 --- a/packages/cli/src/output/error.test.ts +++ b/packages/cli/src/output/error.test.ts @@ -73,6 +73,7 @@ const CATEGORIES: ReadonlyArray = [ "capability", "abort", "internal", + "failed", "unknown", ]; @@ -88,6 +89,7 @@ const EXPECTED_EXIT_CODES: Record = { capability: 2, abort: 130, internal: 1, + failed: 1, unknown: 1, }; diff --git a/packages/cli/src/output/error.ts b/packages/cli/src/output/error.ts index ac405588..99e2be7d 100644 --- a/packages/cli/src/output/error.ts +++ b/packages/cli/src/output/error.ts @@ -42,6 +42,7 @@ export function exitCodeFor(category: ErrorCategory): number { case "response-shape": case "timeout": case "internal": + case "failed": case "unknown": { return FAILURE_EXIT_CODE; } diff --git a/packages/cli/src/output/render.ts b/packages/cli/src/output/render.ts index 2801c295..9dbee55e 100644 --- a/packages/cli/src/output/render.ts +++ b/packages/cli/src/output/render.ts @@ -34,10 +34,12 @@ export function writeText(text: string): void { type KeyValuePair = readonly [label: string, value: string]; export function renderItem(item: T, view: ResourceView, opts: RenderOptions): void { + writeWithinMaxBytes(itemBody(item, view, opts), opts); +} + +function itemBody(item: T, view: ResourceView, opts: RenderOptions): string { const projected = applyProjection(item, view, opts.full, opts.fields); - const body = renderItemBody(item, view, projected, opts) + "\n"; - assertItemWithinMaxBytes(body, opts); - process.stdout.write(body); + return renderItemBody(item, view, projected, opts) + "\n"; } // Default text/human view prints `summaryText` — a bare scalar for single-value lookups @@ -53,13 +55,35 @@ export function renderSummary( summaryText: string | (() => string), opts: RenderOptions, ): void { + writeWithinMaxBytes(summaryBody(item, view, summaryText, opts), opts); +} + +// renderSummary for a command whose result outranks the cap: over it, nothing is written and the +// refusal comes back as a message, so the caller can still report what the result was. +export function renderSummaryWithinMaxBytes( + item: T, + view: ResourceView, + summaryText: string | (() => string), + opts: RenderOptions, +): string | null { + const body = summaryBody(item, view, summaryText, opts); + const oversize = oversizeMessage(body, opts); + if (oversize === null) { + process.stdout.write(body); + } + return oversize; +} + +function summaryBody( + item: T, + view: ResourceView, + summaryText: string | (() => string), + opts: RenderOptions, +): string { if (opts.format === "json" || opts.fields !== undefined || opts.full) { - renderItem(item, view, opts); - return; + return itemBody(item, view, opts); } - const body = (typeof summaryText === "function" ? summaryText() : summaryText) + "\n"; - assertItemWithinMaxBytes(body, opts); - process.stdout.write(body); + return (typeof summaryText === "function" ? summaryText() : summaryText) + "\n"; } export function renderList( @@ -199,13 +223,21 @@ function renderKeyValueLines(pairs: ReadonlyArray): string { return pairs.map(([label, value]) => `${label.padEnd(padding)} ${value}`).join("\n"); } -function assertItemWithinMaxBytes(body: string, opts: RenderOptions): void { +function writeWithinMaxBytes(body: string, opts: RenderOptions): void { + const oversize = oversizeMessage(body, opts); + if (oversize !== null) { + throw new ConfigError(oversize); + } + process.stdout.write(body); +} + +function oversizeMessage(body: string, opts: RenderOptions): string | null { if (opts.maxBytes <= 0) { - return; + return null; } const bytes = Buffer.byteLength(body, "utf8"); if (bytes <= opts.maxBytes) { - return; + return null; } - throw new ConfigError(itemOversizeMessage(bytes, opts.maxBytes, opts.oversizeHint)); + return itemOversizeMessage(bytes, opts.maxBytes, opts.oversizeHint); } diff --git a/packages/cli/src/output/views/transform-test.ts b/packages/cli/src/output/views/transform-test.ts new file mode 100644 index 00000000..3d221478 --- /dev/null +++ b/packages/cli/src/output/views/transform-test.ts @@ -0,0 +1,22 @@ +import { + type TransformTest, + TransformTestCompact, + TransformTestRunResult, +} from "@metabase/client/domain/transform-test"; + +import type { ResourceView } from "../view"; + +export const transformTestView: ResourceView = { + compactPick: TransformTestCompact, + tableColumns: [ + { key: "id", label: "ID" }, + { key: "transform_id", label: "Transform" }, + { key: "name", label: "Name" }, + { key: "description", label: "Description" }, + ], +}; + +export const transformTestRunResultView: ResourceView = { + compactPick: TransformTestRunResult, + tableColumns: [{ key: "status", label: "Status" }], +}; diff --git a/packages/cli/src/runtime/command-help.test.ts b/packages/cli/src/runtime/command-help.test.ts index 7319e381..d645c834 100644 --- a/packages/cli/src/runtime/command-help.test.ts +++ b/packages/cli/src/runtime/command-help.test.ts @@ -417,6 +417,12 @@ const ALL_COMMANDS = [ "transform-tag create", "transform-tag update", "transform-tag delete", + "transform-test list", + "transform-test get", + "transform-test create", + "transform-test update", + "transform-test delete", + "transform-test run", "setting list", "setting get", "setting set", @@ -506,6 +512,8 @@ const BODY_COMMANDS = [ "transform-job update", "transform-tag create", "transform-tag update", + "transform-test create", + "transform-test update", "setup", "snippet create", "snippet update", @@ -652,6 +660,23 @@ describe("command tree contract", () => { }); }); + it("declares each transform-test command's single client method", async () => { + const entries = await allEntries(); + const requires = Object.fromEntries( + entries + .filter((entry) => entry.command.startsWith("transform-test ")) + .map((entry) => [entry.command, entry.requires]), + ); + expect(requires).toEqual({ + "transform-test list": requiresOf("transformTest.list"), + "transform-test get": requiresOf("transformTest.get"), + "transform-test create": requiresOf("transformTest.create"), + "transform-test update": requiresOf("transformTest.update"), + "transform-test delete": requiresOf("transformTest.delete"), + "transform-test run": requiresOf("transformTest.run"), + }); + }); + it("advertises a --limit default on search alone, so the shared flag description stays true", async () => { const entries = await allEntries(); const withDefault = Object.fromEntries( diff --git a/packages/client/README.md b/packages/client/README.md index 61869800..33e268c3 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -374,8 +374,8 @@ Types: `MetabaseClient`, `Transport`, `ClientCredentials`, `ClientOptions`, `Req ### Errors `MetabaseError` is the abstract root of the taxonomy: `NetworkError`, `TimeoutError`, -`ValidationError`, `ResponseShapeError`, `ConfigError`, `InternalError`, `AbortError`, -`ChainedRequestError`, `UnknownError`, and `HttpError`. Every one carries a `category` +`ValidationError`, `ResponseShapeError`, `ConfigError`, `InternalError`, `FailedResultError`, +`AbortError`, `ChainedRequestError`, `UnknownError`, and `HttpError`. Every one carries a `category` (`ErrorCategory`), an `isRetryable`, a `userMessage`, and a `developerDetail` typed per class — `NetworkErrorDetail`, `TimeoutErrorDetail` (the `HttpTimeoutDetail | PollingTimeoutDetail` union), `ValidationErrorDetail`, `ResponseShapeErrorDetail` (the @@ -387,7 +387,9 @@ carries the request context and the Zod issues, and a hand-decoded body or heade `metabase-…` response header — carries the source it was read from and the raw value. `ConfigError` and `InternalError` split blame: the first is input a caller could correct, the second -is a caller that violated a function's contract, which is a bug in the calling code. `HttpError` +is a caller that violated a function's contract, which is a bug in the calling code. +`FailedResultError` is neither: the operation ran and its result is a failure — a test that did not +pass, a run that ended errored — with nothing to debug beyond the result itself. `HttpError` carries the status and response body, redacted of known secrets at construction, plus a `kind` (`HttpErrorKind`) separating a route Metabase does not serve from a row that is gone; `isHttpNotFound(value)` answers the coarser question of whether a thrown value is an `HttpError` with diff --git a/packages/client/src/domain/transform-test.test.ts b/packages/client/src/domain/transform-test.test.ts new file mode 100644 index 00000000..025a811e --- /dev/null +++ b/packages/client/src/domain/transform-test.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; + +import { + TransformTest, + TransformTestCreateInput, + TransformTestUpdateInput, +} from "./transform-test"; + +const STORED = { + id: 3, + entity_id: "AbCdEfGhIjKlMnOpQrStU", + transform_id: 7, + creator_id: 1, + name: "people summary holds one row", + description: null, + inputs: [{ table: { schema: "public", name: "people" }, format: "sql", sql: "SELECT 1 AS id" }], + expectations: [{ type: "empty", name: "no null ids", sql: "SELECT * FROM out" }], + created_at: "2026-09-15T12:00:00Z", + updated_at: "2026-09-15T12:00:00Z", +}; + +const READ_ONLY_KEYS = ["id", "entity_id", "creator_id", "created_at", "updated_at"] as const; + +function withoutReadOnlyKeys(stored: TransformTest): Record { + const body: Record = { ...stored }; + for (const key of READ_ONLY_KEYS) { + delete body[key]; + } + return body; +} + +describe("TransformTestCreateInput", () => { + it("names the read-only keys a body cloned from a stored test still carries", () => { + const result = TransformTestCreateInput.safeParse(TransformTest.parse(STORED)); + + expect(result.success).toBe(false); + expect(result.error?.issues).toEqual([ + { + code: "unrecognized_keys", + keys: [...READ_ONLY_KEYS], + path: [], + message: 'Unrecognized keys: "id", "entity_id", "creator_id", "created_at", "updated_at"', + }, + ]); + }); + + it("accepts that same body once those keys are stripped", () => { + const body = withoutReadOnlyKeys(TransformTest.parse(STORED)); + + expect(TransformTestCreateInput.parse(body)).toEqual({ + transform_id: 7, + name: "people summary holds one row", + description: null, + inputs: [ + { table: { schema: "public", name: "people" }, format: "sql", sql: "SELECT 1 AS id" }, + ], + expectations: [{ type: "empty", name: "no null ids", sql: "SELECT * FROM out" }], + }); + }); +}); + +describe("TransformTestUpdateInput", () => { + it("names a read-only key rather than sending it for the server to refuse", () => { + const result = TransformTestUpdateInput.safeParse({ id: 3, name: "renamed" }); + + expect(result.success).toBe(false); + expect(result.error?.issues).toEqual([ + { + code: "unrecognized_keys", + keys: ["id"], + path: [], + message: 'Unrecognized key: "id"', + }, + ]); + }); + + it("patches a single field", () => { + expect(TransformTestUpdateInput.parse({ name: "renamed" })).toEqual({ name: "renamed" }); + }); +}); diff --git a/packages/client/src/domain/transform-test.ts b/packages/client/src/domain/transform-test.ts index 7f095ead..4ae2d5d0 100644 --- a/packages/client/src/domain/transform-test.ts +++ b/packages/client/src/domain/transform-test.ts @@ -106,6 +106,11 @@ export const TransformTestCompact = TransformTest.pick({ }).strip(); export type TransformTestCompact = z.infer; +/** + * The server closes this map, so an unrecognized key is a 400 there. Refusing it here names the key + * instead: a body cloned from `get` has to shed `id`, `entity_id`, `creator_id`, `created_at` and + * `updated_at` before it can be sent back. + */ export const TransformTestCreateInput = z .object({ transform_id: z.number().int().positive(), @@ -114,9 +119,10 @@ export const TransformTestCreateInput = z inputs: z.array(TransformTestInput), expectations: z.array(TransformTestExpectation), }) - .loose(); + .strict(); export type TransformTestCreateInput = z.infer; +/** Closed for the same reason as [[TransformTestCreateInput]]; every field is optional. */ export const TransformTestUpdateInput = z .object({ transform_id: z.number().int().positive().optional(), @@ -125,7 +131,7 @@ export const TransformTestUpdateInput = z inputs: z.array(TransformTestInput).optional(), expectations: z.array(TransformTestExpectation).optional(), }) - .loose(); + .strict(); export type TransformTestUpdateInput = z.infer; export const TransformTestResultColumn = z diff --git a/packages/client/src/errors.ts b/packages/client/src/errors.ts index a9ab2f9c..9c652fef 100644 --- a/packages/client/src/errors.ts +++ b/packages/client/src/errors.ts @@ -14,6 +14,7 @@ export type ErrorCategory = | "capability" | "abort" | "internal" + | "failed" | "unknown"; export interface NetworkErrorDetail { @@ -219,6 +220,20 @@ export class InternalError extends MetabaseError { } } +// The operation ran to completion and what it reports is a negative result — a test that failed, a +// run that ended in error. Nothing went wrong in the client or its caller, so there is no detail to +// debug: the report is in the result the caller already has. +export class FailedResultError extends MetabaseError { + readonly category = "failed"; + readonly isRetryable = false; + readonly developerDetail = null; + + constructor(message: string) { + super(message); + this.name = "FailedResultError"; + } +} + export class AbortError extends MetabaseError { readonly category = "abort"; readonly isRetryable = false; diff --git a/packages/client/src/http/transport.ts b/packages/client/src/http/transport.ts index a2aa54d7..80d6ddfd 100644 --- a/packages/client/src/http/transport.ts +++ b/packages/client/src/http/transport.ts @@ -32,7 +32,7 @@ export type ExpectedContentType = "json" | "text" | "binary"; export const DEFAULT_METHOD: HttpMethod = "GET"; -const DEFAULT_TIMEOUT_MS = 30_000; +export const DEFAULT_TIMEOUT_MS = 30_000; const OCTET_STREAM_CONTENT_TYPE = "application/octet-stream"; const TEXT_CONTENT_TYPE_PREFIX = "text/"; const ERROR_BODY_BYTE_CAP = 64 * 1024; diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 5019fc8f..5a1c0245 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -21,6 +21,7 @@ export { ChainedRequestError, ConfigError, errorMessage, + FailedResultError, InternalError, isFileNotFoundError, MetabaseError, diff --git a/tests/e2e/skills.e2e.test.ts b/tests/e2e/skills.e2e.test.ts deleted file mode 100644 index 704a6368..00000000 --- a/tests/e2e/skills.e2e.test.ts +++ /dev/null @@ -1,389 +0,0 @@ -import { readFileSync } from "node:fs"; -import { join, resolve } from "node:path"; - -import { afterEach, describe, expect, it } from "vitest"; -import type { z } from "zod"; - -import { parseJson } from "@metabase/client/json"; - -import { SkillGetEnvelope } from "../../packages/cli/src/commands/skills/get"; -import { SkillListEnvelope } from "../../packages/cli/src/commands/skills/list"; -import { SkillPathListEnvelope } from "../../packages/cli/src/commands/skills/path"; -import { - discoverSkills, - type SkillContent, - type SkillInfo, -} from "../../packages/cli/src/core/skills"; -import { cliErrorMessage } from "./cli-error"; -import { cleanupConfigHome, mkTempConfigHome, runCli } from "./run-cli"; -import { seedProbedProfile } from "./seed-profile"; - -// The shipped skills are the expected values: the binary under test bundles this very directory. -const SKILL_DATA_DIR = resolve(import.meta.dirname, "..", "..", "packages", "cli", "skill-data"); -const BUNDLED_VISIBLE = discoverSkills([SKILL_DATA_DIR]).filter((skill) => !skill.hidden); - -const BUNDLED_VISIBLE_NAMES = [ - "core", - "dashboard", - "data-action", - "data-workflow", - "document", - "git-sync", - "mbql", - "metadata", - "native-sql", - "notification", - "transform", - "visualization", -] as const; - -const UNFILTERED_NOTE = - 'Skills are unfiltered: there is no profile "default" (run `mb auth login` to create one and record its server).'; -const SKILL_OVERSIZE_HINT = - "a skill body is indivisible — pass --max-bytes 0 to print it whole, or `mb skills path ` to read it from disk"; - -function bundledSkill(name: string): SkillInfo { - const skill = BUNDLED_VISIBLE.find((candidate) => candidate.name === name); - if (skill === undefined) { - throw new Error(`no bundled skill named "${name}" under ${SKILL_DATA_DIR}`); - } - return skill; -} - -function listRow(skill: SkillInfo): z.infer["data"][number] { - return { name: skill.name, description: skill.description }; -} - -// A skill as `skills get` prints it with nothing to resolve against: SKILL.md verbatim, no extras. -function asWritten(skill: SkillInfo): SkillContent { - return { - name: skill.name, - description: skill.description, - body: readFileSync(join(skill.dir, "SKILL.md"), "utf8"), - references: [], - templates: [], - }; -} - -function fullList(rows: readonly SkillInfo[]): z.infer { - return { - data: rows.map(listRow), - returned: rows.length, - offset: 0, - total: rows.length, - has_more: false, - next_offset: null, - unavailable: null, - }; -} - -// The names a text listing prints at column zero; descriptions sit indented beneath them. -function namesInTextListing(stdout: string): string[] { - return stdout.split("\n").filter((line) => line !== "" && !line.startsWith(" ")); -} - -const TRANSFORM_UNAVAILABLE_ON_58 = { - name: "transform", - failure: { - reason: "version-too-old", - detail: - "This operation requires Metabase v59+ (this server is v0.58.0). Upgrade Metabase to use it.", - feature: "transforms", - since: 59, - tokenFeature: null, - serverVersion: "v0.58.0", - }, -}; - -const DATA_ACTION_UNAVAILABLE_ON_58 = { - name: "data-action", - failure: { - reason: "version-too-old", - detail: - "This operation requires Metabase v65+ (this server is v0.58.0). Upgrade Metabase to use it.", - feature: "dataActionsWithoutModel", - since: 65, - tokenFeature: null, - serverVersion: "v0.58.0", - }, -}; - -const GIT_SYNC_UNAVAILABLE_ON_58 = { - name: "git-sync", - failure: { - reason: "version-too-old", - detail: - "This operation requires Metabase v60+ (this server is v0.58.0). Upgrade Metabase to use it.", - feature: "remoteSync", - since: 60, - tokenFeature: "remote_sync", - serverVersion: "v0.58.0", - }, -}; - -const FEATURE_BOUND_ON_58 = new Set(["data-action", "git-sync", "transform"]); - -describe("skills e2e", () => { - const tempDirs: string[] = []; - - afterEach(async () => { - await Promise.all(tempDirs.splice(0).map(cleanupConfigHome)); - }); - - async function makeIsolatedConfigHome(): Promise { - const dir = await mkTempConfigHome(); - tempDirs.push(dir); - return dir; - } - - it("ships exactly the twelve visible skills this suite names", () => { - expect(BUNDLED_VISIBLE.map((skill) => skill.name)).toEqual([...BUNDLED_VISIBLE_NAMES]); - }); - - it("list returns the twelve bundled non-hidden skills, sorted by name, with `unavailable: null` and a clean stderr when there is no cached probe", async () => { - const result = await runCli({ - args: ["skills", "list", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillListEnvelope)).toEqual(fullList(BUNDLED_VISIBLE)); - expect(result.stderr).toBe(""); - }); - - it("list in text mode notes that nothing was filtered when there is no cached probe", async () => { - const result = await runCli({ - args: ["skills", "list", "--format", "text"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode).toBe(0); - expect(namesInTextListing(result.stdout)).toEqual([...BUNDLED_VISIBLE_NAMES]); - expect(result.stderr).toBe(UNFILTERED_NOTE); - }); - - it("list against a v58 profile omits the feature-bound skills and reports them under `unavailable`", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const result = await runCli({ args: ["skills", "list", "--json"], configHome }); - - expect(result.exitCode, result.stderr).toBe(0); - const usable = BUNDLED_VISIBLE.filter((skill) => !FEATURE_BOUND_ON_58.has(skill.name)); - expect(parseJson(result.stdout, SkillListEnvelope)).toEqual({ - ...fullList(usable), - unavailable: [ - DATA_ACTION_UNAVAILABLE_ON_58, - GIT_SYNC_UNAVAILABLE_ON_58, - TRANSFORM_UNAVAILABLE_ON_58, - ], - }); - expect(result.stderr).toBe(""); - }); - - it("list --unfiltered against a v58 profile lists every skill and reports `unavailable: null`", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const result = await runCli({ - args: ["skills", "list", "--unfiltered", "--json"], - configHome, - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillListEnvelope)).toEqual(fullList(BUNDLED_VISIBLE)); - expect(result.stderr).toBe(""); - }); - - it("list in text mode against a v58 profile names each skipped skill on stderr", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const result = await runCli({ args: ["skills", "list", "--format", "text"], configHome }); - - expect(result.exitCode).toBe(0); - expect(namesInTextListing(result.stdout)).toEqual( - BUNDLED_VISIBLE_NAMES.filter((name) => !FEATURE_BOUND_ON_58.has(name)), - ); - expect(result.stderr).toBe( - [ - `Skipped skill "data-action": ${DATA_ACTION_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, - `Skipped skill "git-sync": ${GIT_SYNC_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, - `Skipped skill "transform": ${TRANSFORM_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, - ].join("\n"), - ); - }); - - it("get transform against a v58 profile withholds the body, and --unfiltered prints it", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const withheld = await runCli({ args: ["skills", "get", "transform", "--json"], configHome }); - expect(withheld.exitCode, withheld.stderr).toBe(0); - expect(parseJson(withheld.stdout, SkillGetEnvelope)).toEqual({ - data: [], - returned: 0, - offset: 0, - total: 0, - has_more: false, - next_offset: null, - unavailable: [TRANSFORM_UNAVAILABLE_ON_58], - }); - - const printed = await runCli({ - args: ["skills", "get", "transform", "--unfiltered", "--json", "--max-bytes", "0"], - configHome, - }); - expect(printed.exitCode, printed.stderr).toBe(0); - expect(parseJson(printed.stdout, SkillGetEnvelope)).toEqual({ - data: [asWritten(bundledSkill("transform"))], - returned: 1, - offset: 0, - total: 1, - has_more: false, - next_offset: null, - unavailable: null, - }); - }); - - it("get core returns the SKILL.md body with frontmatter intact and no references unless --full", async () => { - const result = await runCli({ - args: ["skills", "get", "core", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: [asWritten(bundledSkill("core"))], - returned: 1, - offset: 0, - total: 1, - has_more: false, - next_offset: null, - unavailable: null, - }); - }); - - it("get --all returns every non-hidden skill (with --max-bytes 0 to opt out of the list cap)", async () => { - const result = await runCli({ - args: ["skills", "get", "--all", "--json", "--max-bytes", "0"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: BUNDLED_VISIBLE.map(asWritten), - returned: BUNDLED_VISIBLE.length, - offset: 0, - total: BUNDLED_VISIBLE.length, - has_more: false, - next_offset: null, - unavailable: null, - }); - expect(result.stderr).toBe(""); - }); - - it("get answers a cap too small for even one skill with an empty window and no resumption point", async () => { - const result = await runCli({ - args: ["skills", "get", "core", "--json", "--max-bytes", "200"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - // What the answer would have measured is the answer itself, uncapped: the notice quotes it so - // a caller can size the cap that would carry it. - const uncapped = await runCli({ - args: ["skills", "get", "core", "--json", "--max-bytes", "0"], - configHome: await makeIsolatedConfigHome(), - }); - const fullBytes = Buffer.byteLength(uncapped.stdout.trimEnd(), "utf8"); - - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: [], - returned: 0, - offset: 0, - total: 1, - has_more: true, - next_offset: null, - truncated: { reason: "max_bytes", bytes: fullBytes }, - unavailable: null, - }); - expect(result.stderr).toBe(`… cut at ${fullBytes} bytes; ${SKILL_OVERSIZE_HINT}`); - }); - - it("get accepts comma-separated names", async () => { - const result = await runCli({ - args: ["skills", "get", "git-sync,transform", "--json", "--max-bytes", "0"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: [asWritten(bundledSkill("git-sync")), asWritten(bundledSkill("transform"))], - returned: 2, - offset: 0, - total: 2, - has_more: false, - next_offset: null, - unavailable: null, - }); - }); - - it("get rejects an unknown skill name with exit 2 and a ConfigError message listing available names", async () => { - const result = await runCli({ - args: ["skills", "get", "does-not-exist"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode).toBe(2); - expect(cliErrorMessage(result.stderr)).toBe( - `unknown skill name(s): does-not-exist (available: ${BUNDLED_VISIBLE_NAMES.join(", ")})`, - ); - }); - - it("get without a name or --all errors with exit 2", async () => { - const result = await runCli({ - args: ["skills", "get"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode).toBe(2); - expect(cliErrorMessage(result.stderr)).toBe( - "provide a skill name (comma-separated for multiple) or --all", - ); - }); - - it("path with no name lists every non-hidden skill's directory", async () => { - const result = await runCli({ - args: ["skills", "path", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillPathListEnvelope)).toEqual({ - data: BUNDLED_VISIBLE.map((skill) => ({ name: skill.name, dir: skill.dir })), - returned: BUNDLED_VISIBLE.length, - offset: 0, - total: BUNDLED_VISIBLE.length, - has_more: false, - next_offset: null, - }); - }); - - it("path returns a single-item envelope", async () => { - const result = await runCli({ - args: ["skills", "path", "core", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillPathListEnvelope)).toEqual({ - data: [{ name: "core", dir: bundledSkill("core").dir }], - returned: 1, - offset: 0, - total: 1, - has_more: false, - next_offset: null, - }); - }); -});