diff --git a/skill-data/data-workflow/references/building-clean-tables.md b/skill-data/data-workflow/references/building-clean-tables.md index 5ad20cd..fe3f813 100644 --- a/skill-data/data-workflow/references/building-clean-tables.md +++ b/skill-data/data-workflow/references/building-clean-tables.md @@ -143,7 +143,7 @@ When refining a built transform _with_ the user, open its inspector so you're lo **Check the output before handing back — the user can't.** Two passes, in order. -**Pass 1 — Correctness (did it run right).** After each transform runs, run quick ad-hoc tests against what Phase 0 led you to expect: row counts in the right ballpark, decoded columns readable (no stray codes), linking ids that resolve to the other tables, no column unexpectedly all-null or blown up in count. Treat surprises as bugs to chase, not noise. A table that can't combine with the others — a dropped id, or the same id named two ways — is a silent failure; catch it here. +**Pass 1 — Correctness (did it run right).** After each transform runs, run quick ad-hoc tests against what Phase 0 led you to expect: row counts in the right ballpark, decoded columns readable (no stray codes), linking ids that resolve to the other tables, no column unexpectedly all-null or blown up in count. Treat surprises as bugs to chase, not noise. A table that can't combine with the others — a dropped id, or the same id named two ways — is a silent failure; catch it here. To go beyond ad-hoc — durable fixture suites, assertions, a coverage matrix, tests the user can re-run — load `transform-test-plan` (`mb skills get transform-test-plan`); its checklist questions double as design questions while you build. **Pass 2 — Fitness (is it nice to use).** Correct isn't the bar; _usable_ is. `SELECT * FROM LIMIT 20` and read every column as if you'd never seen the source: would a business reader find each one readable? Smells that say not-yet, even though nothing errored: diff --git a/skill-data/transform-test-plan/SKILL.md b/skill-data/transform-test-plan/SKILL.md new file mode 100644 index 0000000..fc4fc46 --- /dev/null +++ b/skill-data/transform-test-plan/SKILL.md @@ -0,0 +1,214 @@ +--- +name: transform-test-plan +description: Derive a comprehensive test plan for transforms and their consuming cards — fixture suites, SQL assertions, hand-derived expected CSVs, and a coverage matrix — from the model's declared design. Covers input partitioning (zero-case, multiplicity, dirty rows), grain / conservation / recomputation / conformance checks, error-vs-warn severity conventions, known-red regression suites, and running the same screens against production tables. 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. Running the tests is the transform-test skill; this one decides what to test. +allowed-tools: Read, Write, Edit, Bash, AskUserQuestion +--- + +# Planning transform tests + +Turn a transform chain — and the cards that read it — into fixture suites that +prove the logic on small known data, assertions that state the model's +invariants, and a coverage matrix showing what's checked and what's +deliberately not. Mechanics (the inputs → fixtures → run loop, `--suite`, +`--source`, card targets) live in the `transform-test` skill +(`mb skills get transform-test`) — load it before writing suite files; never +restate it here. + +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 assertion SQL screens production tables +for anomalies. + +## Operating rules + +- **Detect, then derive.** Classify what each 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 test + (assertion file or expected-CSV cell) 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 CSVs are derived by hand** from the fixture story and business + meaning — never captured from pipeline output, which asserts only that the + transform equals itself. +- **Batteries stay off until declared structure switches them on.** No + incremental checks for a full rebuild, 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 each transform's SQL 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), full-rebuild vs. incremental shape. 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` — like `checks.md` below, Read + it from the directory `mb skills path transform-test-plan` prints (or + inline both via `mb skills get transform-test-plan --full --max-bytes 0`; + without `--max-bytes 0` the output is cut). 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 table and column through + `references/checks.md` — declared property → assertion shape, fixture + implication, CSV convention. Read it in full once per plan; it is the + plan's content. +5. **Design the fixture cast**: one small shared cast per chain (≈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 suites + README; downstream suites reuse the cast so expected CSVs stay mutually + derivable. +6. **Hand-derive the expected CSVs**, arithmetic recorded in the README + (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. **Write the suites**, one directory per target: + + ``` + suites/ + README.md # fixture story, arithmetic, conventions, known quirks + run-all.sh + shared/*.sql # assertions reused verbatim by several suites + / + suite.yaml # target, sources, inputs, expected, assertions+severities + fixtures/*.csv + assertions/*.sql + expected.csv + ``` + + Suites are durable deliverables — they live with the project, not in + `.scratch`. Each assertion file opens with a 1–3 line contract comment: + the invariant, the failure modes it catches ("catches both dropped orders + and join fan-out"). + + **Reuse assertions across suites.** Chains test end-to-end cheaply, so one + invariant often applies at several stages — a domain screen on a column + every stage carries, a sign screen valid for a transform and its card. + When the SQL is identical, the file lives once in `suites/shared/`, + referenced as `file: ../shared/.sql` (paths resolve from the suite's + directory, where runs happen). Share only what is truly identical — a + "shared" file needing per-suite column renames is worse than two honest + copies with their own contract comments. +8. **Emit the coverage matrix** in the README: rows = output tables with + grain; columns = check classes (grain, conservation, recomputation, + conformance, domains, referential integrity, temporal, screens); cells + name the covering assertion file or fixture row, or state `gap: `. + Scan shared-attribute columns across rows for cross-table agreement + obligations. Empty cells are honest; silent gaps are not — and that + includes structurally absent batteries: "no cards yet", "no volatile + columns" belong in the deliberate-gaps list with their revival condition + (an end-to-end suite becomes due when the first card is built). +9. **Prove the tests have teeth.** Once per suite: corrupt one expected-CSV + cell, run, confirm the harness points at exactly that cell; revert. Once + per chain: perturb one raw-leaf fixture cell, chain-run, confirm exactly + the declared descendant cells move. +10. **Plan the production screens**: the same assertion SQL with + `test_output` replaced by the real output table (`mb query`, native). + Fixture suites stay deterministic and error-leaning — red means wrong + logic. Live screens — red means the data moved — mirror the suites' + severity split as two directories: `screens/structural/` (must return + zero rows: grain, ties, domains, copy agreement across independently + rebuilt tables) and `screens/anomalies/` (expected to fire: tolerated + oddities, counts recorded in `baselines.tsv`, drift in either direction + failing the run). Keep the tiers separate so neither red pollutes the + other. + +## Severity: error, or warn-that-fires + +**error** = forbidden; failure fails the run. **warn** = tolerated-but- +surfaced: an oddity the owner lives with (orphan rows, ship-before-order +dates) gets a warn assertion **designed to fire on every run**, a fixture row +guaranteeing the fire, and the real-data count in its comment — the tolerance +stays a conscious choice. A firing warn is not "known-red": reserve that term +for a live-bug suite held red pending a fix (below). 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 suite 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 — document a +**known-red suite**: red asserting correct behavior, the minimal fix body in +`fix/` (ready for `mb transform update --file`), damage in the README. +Either way, once green the suite 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 expected-CSV diff is order-insensitive by design. + Flag it as probable dead weight (clustering hints aside); never emit an + ordering assertion. + +## Worked example, condensed + +Chain: `orders` + `customers` → *enriched_orders* (order grain; LEFT JOIN +attaches `customer_name`, `tier`) → *fulfillment_by_tier* (tier grain: +`order_count`, `revenue`, `avg_days_to_ship` over a `shipping` join) → card +*Revenue by 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). + +Derived, per catalog: grain uniqueness on both transforms *and the card*; +row/amount conservation input → output → card; `avg_days_to_ship` recomputed +from components (non-additive — never summed); `tier` domain ⊆ {standard, +premium} at every stage; orphan = keep-with-NULLs → firing warn + expected +row pinning the NULL pass-through; ship-before-order tolerated → second +firing warn with the warehouse count; rounding pinned at the card. Expected +CSVs hand-computed (premium = 3 / 350.50 / 3.0; arithmetic in the README). +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 write suite files before reading `transform-test` — the header + contract, table-id discovery, and `--suite` shape live there. +- Don't capture an expected CSV from the transform's own output — hand-derive + it or it asserts 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 blend fixture suites and production screens into one list — their + reds mean different things and demand different responses. +- 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/skill-data/transform-test-plan/references/checklist.md b/skill-data/transform-test-plan/references/checklist.md new file mode 100644 index 0000000..e45a714 --- /dev/null +++ b/skill-data/transform-test-plan/references/checklist.md @@ -0,0 +1,93 @@ +# 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. **Full rebuild or incremental?** (F1) + Detect: a plain `SELECT` materialization is a full rebuild; merge/append/ + watermark logic means incremental. Incremental switches on a whole extra + fixture battery — never assume it silently. +3. **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 + +4. **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. +5. **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. +6. **Orphan handling per join: drop / keep-with-NULLs / default row — and is + an orphan tolerated (warn) or forbidden (error)?** (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. +7. **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. +8. **Which consuming cards warrant end-to-end suites?** (P4) + Detect: can't — which cards matter is the owner's call. Default to the + cards the chain was built to serve. + +## Stage 3 — per-column declarations + +9. **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"?). +10. **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). +11. **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. +12. **Which columns are stored aggregates, over which detail and filter?** (A4) + Detect: correlated subqueries / joined-aggregate CTEs in the SQL. +13. **Which categorical columns have closed domains — enumerate; is NULL a + member?** (C2) + Detect: profiling distinct values / `mb field values` gives today's set; + the owner confirms it's closed rather than merely small so far. +14. **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). +15. **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. +16. **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. +17. **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? +18. **Which columns are volatile across runs?** (T3) + Detect: `now()` / `current_date` / run-metadata expressions in the SQL. +19. **Per tolerated oddity: warn or error — and is anything currently + tolerated that should become forbidden?** (Q3) + Detect: can't — this is the conscious-choice question the warn convention + exists to keep alive. diff --git a/skill-data/transform-test-plan/references/checks.md b/skill-data/transform-test-plan/references/checks.md new file mode 100644 index 0000000..6db868b --- /dev/null +++ b/skill-data/transform-test-plan/references/checks.md @@ -0,0 +1,299 @@ +# 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 SQL shape or convention), and **fixtures** (what the cast must +contain for the check to have teeth). Assertion SQL reads the target's output +as `test_output` and the seeded input tables under their real names, so +per-row recomputation joins against inputs are always writable. + +Ids (G1, A2, …) are for cross-referencing within the plan documents only — +never surface them to the user bare. + +## 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 (error): `SELECT , COUNT(*) FROM test_output GROUP BY + HAVING COUNT(*) > 1`. NULL grain values form their own bucket. + Emit for every target — transforms *and* cards. +- 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 (error): independent scalar subqueries compared with + `IS DISTINCT FROM`, returning the mismatched pair: + `SELECT (SELECT SUM(t.m) FROM test_output 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. For chains, span raw leaf → final output → card. + Where the source has a published authoritative aggregate (an official total), + also tie the pipeline's headline figure to it — an out-of-band check no + internal bug can satisfy by accident; it validates the semantic reading of + the grain, not just the plumbing. +- 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 (error): `SELECT FROM test_output t WHERE t. + IS DISTINCT FROM `. + Presentation rules (round to n decimals) are the same shape at the card. +- 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 (error), 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 via chain tests.** A layered rollup always agrees +with its base; fixtures live at raw leaves only. +- Applies: any transform reading another transform's output. +- Assert: suite `sources:` names the parent(s); every rollup measure equals + the corresponding aggregation over the (scratch) base (A2 shape); group-set + equality both directions — `SELECT FROM test_output EXCEPT SELECT + DISTINCT FROM ` and the reverse, filtered by the declared + zero-group policy. +- Fixtures: raw-leaf cast only; a group with no contributing rows pins the + zero-group policy. + +## Fact-table type & load mode + +**F1 — Full-rebuild vs. incremental.** One branch switching a whole battery. +- Applies: per transform, always classified. +- Full rebuild: battery off; rerun determinism is free. +- Incremental adds: duplicate-delivery fixture (same source row twice → no + double count), gap fixture (skipped period → declared behavior), + late-arrival fixture (fact before its dimension row → declared behavior), + no-op-change fixture (identical rewrite → no new version), rerun idempotency + (run twice → identical output). + +**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 CSV. + 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 expected CSV; 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 (error): `SELECT t., t., d. FROM test_output t JOIN + d ON t. = d. WHERE t. IS DISTINCT FROM + d.` — one file per owning table, columns UNION ALL'd if preferred. +- Scope: inside a chain test the copies are produced by the very join under + test, so this join-form is near-tautological there — the C4 + functional-dependency form plus expected-CSV cell pinning carries the suite. + The join-form against the independently materialized owning table is the + *drift* check: it belongs in the production screens whenever the tables + rebuild independently. +- 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 / `mb field + values`; confirm the set and whether NULL is a member). +- Assert (error): `SELECT , FROM test_output 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 and surfaced. +- Applies: per join, conditioned on the declared response — drop / + keep-with-NULLs / default row; tolerated or forbidden. +- Assert, strict (error): `SELECT t. FROM test_output t LEFT JOIN d + ON t. = d. WHERE t. IS NOT NULL AND d. IS NULL`. + Tolerated (warn, fires by design): select the orphan rows (`WHERE + t. IS NULL`), plus the expected CSV pinning the pass-through. + 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 (error): `SELECT t., COUNT(DISTINCT t.) FROM test_output 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 (error): 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 a leaf fixture, chain-run, assert every downstream rollup + regrouped — a full rebuild does this for free; assert it. +- 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 surfaced. +- Applies: per declared date pair (ordered ≤ shipped ≤ delivered; signup ≤ + first order), each classified guaranteed vs. tolerated-violated. +- Assert: guaranteed (error) — `SELECT , , FROM + test_output WHERE < `. Tolerated (warn, fires by design) — + same shape, with the real-data count in the contract comment. +- Fixtures: one violating row for every tolerated ordering, its downstream + arithmetic (a negative lag inside an average) pinned in the expected CSV. + +**T3 — Volatile columns.** Values that change across runs can't sit in an +expected CSV. +- Applies: columns derived from `now()`/`current_date` (age), run metadata + (load timestamps, batch ids). +- Assert: route the column to `ignore_columns` in the suite; separately + assert its form where warranted (error): `WHERE age NOT BETWEEN 0 AND 120` + — assertions still see ignored columns. The stable source column + (birth_date) stays exact in the expected CSV. +- Fixtures: none special; the split is the point — ignore the volatile, + assert 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 (error): `SELECT , FROM test_output 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` (error). + Otherwise the policy is enforced by the expected CSV 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 — Severity & contract comments.** Every assertion declares its meaning. +- Applies: always, every assertion. +- Convention: `severity: error` = forbidden; `severity: warn` = tolerated, + surfaced, and — where the oddity exists in real data — *designed to fire on + every run*, with a fixture row guaranteeing it and the warehouse count in + the comment. Every assertion file 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. + +## 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 chain, every row +a named edge, documented as a story table (row → attributes → purpose) in the +suites README; downstream suites reuse it so expected CSVs stay mutually +derivable. + +**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 suites. 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 the matching +warn or error assertion. Which duplicate survives a dedupe is invisible until +a conflicting-duplicate fixture pins it. + +**I6 — Mutation probes.** Run at plan-validation time, not in every suite: +corrupt one expected-CSV cell → the harness must point at that exact cell; +perturb one raw-leaf fixture cell → exactly the declared descendant cells +move. Both guard against vacuous tests. + +## Plan-level artifacts + +**P1 — Coverage matrix.** Output tables (with grain) × check classes; cells +name the covering assertion/fixture or state `gap: `; shared-attribute +columns scanned across rows for agreement obligations (C1). + +**P2 — Hand-derived expected CSVs.** From the fixture story and business +meaning, arithmetic recorded in the README; 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 suite to green; quantify the damage at fixture +and warehouse scale. Then fix now, or document as a known-red suite (red +until the fix lands, minimal fix body in `fix/`, damage in the README) — per +the session's terms. Once green, the suite stays as the regression guard. + +**P4 — Card-level end-to-end suites.** Per key consuming card: `target: +{type: card}`, `sources:` the full chain, fixtures at raw leaves only, +hand-derived expected CSV, end-to-end conservation (A2), grain uniqueness on +the card output (G2), presentation rules (A3). The card is where the user +actually looks; a chain green everywhere but wrong at the card is still wrong. + +**P5 — Anomaly baselines.** Every expected-to-fire live screen records its +current count (`screens/baselines.tsv`: screen → count), and the screen +runner fails on drift in *either* direction — a tolerance silently growing +and one silently vanishing both surface. This machine-checks the known-quirks +counts (I1) and gives warn-fires-by-design (Q3) live-mode teeth: the warn now +fires *at the recorded rate*. A changed baseline is a finding to investigate, +then re-record deliberately. diff --git a/skill-data/transform-test/SKILL.md b/skill-data/transform-test/SKILL.md new file mode 100644 index 0000000..a2fd520 --- /dev/null +++ b/skill-data/transform-test/SKILL.md @@ -0,0 +1,197 @@ +--- +name: transform-test +description: Test a transform — or a card (saved question / model) — against fixture CSVs via `mb transform-test` — seeds scratch tables, runs the target, and checks its output against an expected CSV and/or SQL assertions, never touching real tables. Covers the inputs→fixtures→run loop, the exact-header CSV contract, `--target-type transform|card`, the pass/fail diff, SQL assertions (`--assert` .sql files/globs), YAML suites (`--suite`), chained sub-graph tests (`--source`), and `--ignore-columns`. Load when validating a transform's or card's logic — "test this transform", "test this question against sample data", "assert no negative revenue", "does my transform produce the right output", or anything `mb transform-test …`. +allowed-tools: Read, Write, Edit, Bash +--- + +# Testing transforms + +`mb transform-test` runs a **target** — a transform, or a card (saved question / model) — against **fixture CSVs you supply** instead of the real source tables, then checks its output against an **expected CSV** and/or **SQL assertions**. It seeds throwaway scratch tables, runs the target's query over them, compares, and drops everything — **real tables are never read or written.** Use it to validate a transform's or card's logic on small, known data before trusting it on production rows. + +You check the output two ways, and can combine them: an **expected CSV** (`--expected`, exact-output multiset diff) and/or **assertions** (`--assert`, SQL queries that must return zero rows). At least one of `--expected` or `--assert`/`--suite` is required — `--expected` is **not** mandatory on its own anymore. + +Authoring and running transforms for real is the `transform` skill (`mb skills get transform`); deciding what to build is `data-transformation` (`mb skills get data-transformation`); deciding **what to test** — deriving the fixture cast, assertions, and coverage for a whole chain — is `transform-test-plan` (`mb skills get transform-test-plan`). Flag/profile/output conventions are in `core` (`mb skills get core`). + +## The loop: inputs → fixtures → run + +1. **Discover** which tables need fixtures and their exact columns (`transform-test inputs`). +2. **Write** one CSV per input table, plus an expected-output CSV and/or `.sql` assertion files. +3. **Run** the test; read passed / FAILED + the diff and/or per-assertion results (`transform-test run`). + +The positional argument is the **target** id whose output is diffed. By default that's a transform; pass `--target-type card` to target a saved question or model instead (see "Card targets" below). Everything else — inputs, fixtures, `--source`, the diff — works the same either way. + +## 1. Discover required inputs + +```bash +mb transform-test inputs --profile --json +``` + +Returns one row per input table you must supply a fixture for. Each carries: + +- `table_id` — use it as the key in `--input =`. +- `name` / `schema` — the real table (for your reference; the fixture replaces it). +- `columns` — the **exact** column-name list your CSV header must contain. + +Omit `--source` to test the target transform on its own (its direct input tables). With `--source` it lists the **leaf** tables of the sub-graph (see "Chained tests" below). + +## 2. Write the fixture CSVs — the header contract + +Each input CSV's header must contain **exactly the column names `inputs` reported** — all of them, **case-sensitive**, no extras. Column **order doesn't matter** (matched by name); a missing or unexpected column fails the run with a header-mismatch error. Values are parsed against each column's real warehouse type (integers, dates, etc.), so write values that parse — `2024-01-01` for a date column, an integer for an int column. Leave a cell empty for `NULL`. + +```bash +mkdir -p ./.scratch +cat > ./.scratch/bird_count.csv <<'CSV' +id,date,count +1,2024-01-01,2 +2,2024-01-02,5 +3,2024-01-03,3 +CSV + +# The expected output: header = the columns your transform SELECTs, rows = what it should produce. +cat > ./.scratch/expected.csv <<'CSV' +id,date,count +1,2024-01-01,2 +3,2024-01-03,3 +CSV +``` + +The expected CSV's columns are matched against the transform's **actual output** columns; the comparison is a multiset (row order is ignored, duplicates count). + +## 3. Run the test + +```bash +mb transform-test run \ + --input =./.scratch/bird_count.csv \ + --expected ./.scratch/expected.csv \ + --profile --json +``` + +- `--input` is comma-separated `=` pairs, **one per table `inputs` listed** (e.g. `--input 229=orders.csv,223=people.csv`). The table id is the `table_id` from step 1. +- `--expected` is the path to the expected-output CSV. **Optional** — provide it, `--assert`/`--suite`, or both, but at least one. +- Exits **0 on pass, non-zero on fail** (good for scripting / CI gates). + +Reading the result: + +- The plain (text) summary says `Transform test run passed.` or `Transform test run FAILED …`, followed — whenever assertions ran — by a one-line breakdown (`N assertions — A passed, B FAILED, C warn …`) and a per-assertion table (name / status / failing rows). +- `--json` returns `{status, diff, assertions, test_run_id}`. `status` is `passed` or `failed` (the only two values). `diff` is the expected-CSV comparison (`null` when you ran assertions-only). `assertions` is `null` when none ran, else an array of `{name, status: passed|failed|warn, failing_row_count, sample_rows, columns}`. On `failed`, the `diff` reports missing/extra rows and cell mismatches, and each failing assertion carries its `failing_row_count` + a capped `sample_rows`. Always re-run with `--json` (or start with it) to see why a test failed. +- **Output format follows the usual `core` rules:** `auto` (the default) prints the human summary + table in a terminal but **emits JSON when stdout is piped/redirected** (CI, `| cat`, `$(…)`). Pass `--format text` to force the human table when piping, or `--json` to force JSON in a terminal. +- A run that couldn't complete — bad CSV header, an unsupported transform, etc. — isn't a `failed` status; it surfaces as a thrown error envelope on a non-zero exit, distinct from a clean `failed` diff. + +## Assertions (`--assert`) + +An **assertion** is a SQL query that **passes iff it returns zero rows.** Write each one to a +`.sql` file; `--assert` points at the file (or a glob of them). Inside the SQL, reference the +synthetic relation **`test_output`** (the target's output) and/or the input table names — the +harness redirects every real table to scratch and binds `test_output` to the target. + +```bash +cat > ./.scratch/no_negative_revenue.sql <<'SQL' +SELECT * FROM test_output WHERE revenue < 0 +SQL + +mb transform-test run 173 --source 172 \ + --input 229=orders.csv \ + --assert ./.scratch/no_negative_revenue.sql \ + --profile +``` + +- **`--assert` accepts only `.sql` file paths or globs — inline SQL is NOT supported.** A + non-`.sql` value is rejected with a clear error; write the query to a file instead. +- The **assertion name** is the file basename without `.sql` (`no_negative_revenue` above). +- **Repeatable, and comma-separated.** All of these accumulate: + `--assert a.sql --assert b.sql`, `--assert a.sql,b.sql`, `--assert 'checks/*.sql'` (a glob + expands to one assertion per matching `.sql` file; a glob matching nothing is an error). +- **Severity:** `--assert` files default to **error** severity (a failure fails the run, non-zero + exit). To mark an assertion as a **warn** (reported, but does not fail the run or flip the exit + code), declare it in a `--suite` with `severity: warn`. +- `--assert` and `--expected` compose: provide either or both. With assertions only, the response + `diff` is `null` and the result is driven entirely by the assertions. + +## Test suites (`--suite`) + +A **suite** is a YAML file that declares a whole run — target, sources, inputs, expected, +ignore-columns, and assertions (with optional per-assertion `severity`). It is parsed **entirely +client-side**; the server sees the same request as the equivalent flags. + +```yaml +# suites/orders.yaml +target: + type: transform # or: card + id: 173 +sources: [172] +inputs: + - table: 229 + file: ./.scratch/orders.csv +expected: ./.scratch/expected.csv # optional +ignore_columns: [snapshot_ts] +assertions: + - name: no_negative_revenue + sql: SELECT * FROM test_output WHERE revenue < 0 # inline sql IS allowed in a suite + severity: error + - name: every_state_present + file: ./.scratch/every_state_present.sql # …or point at a .sql file + severity: warn +``` + +```bash +mb transform-test run --suite suites/orders.yaml --profile +``` + +- A suite assertion sets **exactly one** of `sql:` (inline) or `file:` (path to a `.sql` file). + (Inline SQL is fine _here_ — the suite is itself a file you author; only the `--assert` flag is + file-only.) +- **Ad-hoc flags compose with a suite:** `--source`/`--input`/`--expected`/`--ignore-columns` + **override** the suite's values; `--assert` assertions **append** to the suite's. The positional + target id (and `--target-type`) may be omitted when the suite declares `target`. + +## Chained / sub-graph tests (`--source`) + +To test a transform that depends on **other transforms'** outputs, pick boundary `--source` transform ids. Every node on a path from a source to the target runs in dependency order, fed by fixtures only at the **leaves** (raw tables + any sibling outputs not produced inside the selection). `transform-test inputs --source ` tells you exactly which leaves to supply. + +```bash +mb transform-test inputs 173 --source 172 --profile --json # lists the leaf tables +mb transform-test run 173 --source 172 \ + --input 229=orders.csv,223=people.csv \ + --expected expected.csv --profile --json +``` + +`--source` is comma-separated ids. Omitting it == testing the target alone. All transforms in the sub-graph must share one database (a cross-database selection is rejected). + +## Card targets (`--target-type card`) + +The target can be a **card** — a saved question or model — instead of a transform. The card's query is what gets diffed: its producing transforms run in scratch (seeded from your fixtures), the card's query runs over those scratch outputs, and the result is compared to the expected CSV. The inputs → fixtures → run loop is identical; only the positional id and `--target-type` change. + +```bash +mb transform-test inputs --target-type card --source --profile --json +mb transform-test run --target-type card --source \ + --input 229=orders.csv,223=people.csv --expected expected.csv --profile --json +``` + +- The positional is a **card id** (question or model). `--target-type` defaults to `transform`, so transform targets need no flag. +- **Precondition:** the transform output(s) the card reads must be **materialized and synced** — run the producing transform with `mb transform run --sync` first. A card built on an un-materialized output can't be linked to its producer, and you'll get a `sources-not-ancestors` error. +- Native and MBQL cards both work. Native cards carry the same bare-table-qualifier limitation as native transforms (below). + +## Ignoring non-deterministic columns + +For output columns you can't pin in an expected CSV — `now()` timestamps, snapshot dates, random ids — exclude them from the diff: + +```bash +mb transform-test run 42 --input 229=orders.csv --expected out.csv \ + --ignore-columns snapshot_ts,run_id --profile --json +``` + +`--ignore-columns` is comma-separated **output** column names. Naming a column that isn't in the output is an error, so check the transform's SELECT first. + +## Limitations & gotchas + +- **Native SQL (in a transform or a native card) that qualifies columns by the bare table name can't be test-run.** `SELECT orders.id FROM orders` fails with a typed 422 (the fixture-redirect rewrites the table reference but can't safely follow a `orders.`-qualified column). **Alias the table** — `SELECT o.id FROM orders o` — or use unqualified column names. It always fails _safely_ (you get an error, never a wrong-but-green result), and MBQL targets aren't affected. +- **Header must match the real table exactly.** Don't guess columns — copy them from `transform-test inputs`. All columns are required; the test isn't a projection. +- **Provisional / unreleased.** These commands target a dev build of the transforms feature; if `mb transform-test` reports the endpoint is unavailable, the connected instance predates it. +- **Scratch only.** Fixtures seed prefix-guarded scratch tables that are dropped in a `finally` (success, failure, or error). A test run creates no transform-run record and never writes the transform's real output table. + +## Don't + +- Don't hand-guess the input `table_id`s or column headers — always run `transform-test inputs` first; the ids and exact columns come from there. +- Don't treat a `failed` status as a tool error — it's a real result (output ≠ expected, or an error-severity assertion returned rows). Read the `--json` `diff` and `assertions` to fix the transform, the expected CSV, or the assertion. +- Don't supply fixtures for tables `inputs` didn't list, or omit ones it did — the `--input` set must match the required set exactly. +- Don't pass inline SQL to `--assert` — it only accepts `.sql` file paths or globs. Write the query to a file (or put it in a `--suite` under `sql:`). diff --git a/src/commands/parse-id.ts b/src/commands/parse-id.ts index 1bcf31b..1af912c 100644 --- a/src/commands/parse-id.ts +++ b/src/commands/parse-id.ts @@ -3,3 +3,14 @@ import { parseInteger } from "./parse-integer"; export function parseId(value: string, name = "id"): number { return parseInteger(value, { name, min: 1 }); } + +export function parseIdList(value: string | undefined, name = "id"): number[] { + if (value === undefined || value.trim() === "") { + return []; + } + return value + .split(",") + .map((part) => part.trim()) + .filter((part) => part !== "") + .map((part) => parseId(part, name)); +} diff --git a/src/commands/runtime.ts b/src/commands/runtime.ts index 25bcb2c..58cc439 100644 --- a/src/commands/runtime.ts +++ b/src/commands/runtime.ts @@ -36,6 +36,9 @@ export { SKIP_PREFLIGHT_ENV }; export interface MetabaseCommandContext { args: ParsedArgs; + // The unparsed argv for this command. Needed to recover repeated flags, which citty's parser + // collapses to the last value (see runtime/citty.ts `collectRepeatedFlag`). + rawArgs: readonly string[]; ctx: CommonContext; getClient: () => Promise; getResolvedConfig: () => Promise; @@ -109,6 +112,7 @@ export function defineMetabaseCommand( try { await def.run({ args, + rawArgs, ctx, getClient, getResolvedConfig, diff --git a/src/commands/transform-test/assert.test.ts b/src/commands/transform-test/assert.test.ts new file mode 100644 index 0000000..73862c4 --- /dev/null +++ b/src/commands/transform-test/assert.test.ts @@ -0,0 +1,112 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { ConfigError } from "../../core/errors"; + +import { classifyAssertToken, parseAssertFlags, resolveAssertions } from "./assert"; + +describe("classifyAssertToken", () => { + it("classifies a .sql path as a file", () => { + expect(classifyAssertToken("checks/no_negatives.sql")).toEqual({ + kind: "file", + path: "checks/no_negatives.sql", + }); + }); + + it("classifies a *.sql pattern as a glob", () => { + expect(classifyAssertToken("checks/*.sql")).toEqual({ kind: "glob", pattern: "checks/*.sql" }); + }); + + it("throws ConfigError for a non-.sql value (inline SQL is not supported)", () => { + const value = "SELECT * FROM test_output WHERE x < 0"; + expect(() => classifyAssertToken(value)).toThrow(ConfigError); + expect(() => classifyAssertToken(value)).toThrow( + `--assert expects a .sql file path or glob (inline SQL is not supported); got: "${value}"`, + ); + }); + + it("throws ConfigError for a bare name without a .sql extension", () => { + expect(() => classifyAssertToken("no_negatives")).toThrow(ConfigError); + }); +}); + +describe("parseAssertFlags", () => { + it("returns an empty list for undefined or blank values", () => { + expect(parseAssertFlags(undefined)).toEqual([]); + expect(parseAssertFlags([])).toEqual([]); + expect(parseAssertFlags([" "])).toEqual([]); + }); + + it("accepts a single string and splits comma-separated tokens", () => { + expect(parseAssertFlags("a.sql,b.sql")).toEqual([ + { kind: "file", path: "a.sql" }, + { kind: "file", path: "b.sql" }, + ]); + }); + + it("accepts a repeated array of values", () => { + expect(parseAssertFlags(["a.sql", "b/*.sql"])).toEqual([ + { kind: "file", path: "a.sql" }, + { kind: "glob", pattern: "b/*.sql" }, + ]); + }); + + it("throws ConfigError when a value is not a .sql file or glob", () => { + expect(() => parseAssertFlags(["SELECT a, b FROM test_output"])).toThrow(ConfigError); + }); +}); + +describe("resolveAssertions", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "mb-assert-")); + writeFileSync(join(dir, "no_negatives.sql"), "SELECT * FROM test_output WHERE revenue < 0\n"); + writeFileSync(join(dir, "has_rows.sql"), "SELECT * FROM test_output WHERE 1=0"); + writeFileSync(join(dir, "notes.txt"), "ignore me"); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("reads a file token, names it by basename without extension, defaults to error severity", async () => { + const out = await resolveAssertions([{ kind: "file", path: join(dir, "no_negatives.sql") }]); + expect(out).toEqual([ + { + name: "no_negatives", + sql: "SELECT * FROM test_output WHERE revenue < 0", + severity: "error", + }, + ]); + }); + + it("expands a glob to one assertion per matching .sql file, sorted by name", async () => { + const out = await resolveAssertions([{ kind: "glob", pattern: join(dir, "*.sql") }]); + expect(out.map((a) => a.name)).toEqual(["has_rows", "no_negatives"]); + expect(out.every((a) => a.severity === "error")).toBe(true); + }); + + it("resolves multiple file tokens preserving order, each named by basename", async () => { + const out = await resolveAssertions([ + { kind: "file", path: join(dir, "has_rows.sql") }, + { kind: "file", path: join(dir, "no_negatives.sql") }, + ]); + expect(out.map((a) => a.name)).toEqual(["has_rows", "no_negatives"]); + }); + + it("throws ConfigError for a missing file", async () => { + await expect( + resolveAssertions([{ kind: "file", path: join(dir, "nope.sql") }]), + ).rejects.toThrow(ConfigError); + }); + + it("throws ConfigError for a glob that matches nothing", async () => { + await expect( + resolveAssertions([{ kind: "glob", pattern: join(dir, "zzz-*.sql") }]), + ).rejects.toThrow(ConfigError); + }); +}); diff --git a/src/commands/transform-test/assert.ts b/src/commands/transform-test/assert.ts new file mode 100644 index 0000000..c7b2320 --- /dev/null +++ b/src/commands/transform-test/assert.ts @@ -0,0 +1,108 @@ +import { readdir, readFile } from "node:fs/promises"; +import { basename, dirname, extname, join } from "node:path"; + +import { ConfigError, errorMessage, isNotFoundError } from "../../core/errors"; + +export type Severity = "error" | "warn"; + +export interface AssertionDef { + name: string; + sql: string; + severity: Severity; +} + +// An --assert value is a `.sql` file or a glob of them; inline SQL is not supported. +export type AssertToken = { kind: "file"; path: string } | { kind: "glob"; pattern: string }; + +// basename with `*` ⇒ glob; plain `.sql` ⇒ file; anything else rejected. +export function classifyAssertToken(token: string): AssertToken { + if (/\.sql$/i.test(token)) { + return basename(token).includes("*") + ? { kind: "glob", pattern: token } + : { kind: "file", path: token }; + } + throw new ConfigError( + `--assert expects a .sql file path or glob (inline SQL is not supported); got: "${token}"`, + ); +} + +// `--assert` is repeatable (citty hands back a string[] when given more than once) and each +// value may itself be comma-separated (file/glob paths). +export function parseAssertFlags(value: string | string[] | undefined): AssertToken[] { + if (value === undefined) { + return []; + } + const raw = Array.isArray(value) ? value : [value]; + const tokens: AssertToken[] = []; + for (const entry of raw) { + if (entry.trim() === "") { + continue; + } + for (const part of entry.split(",")) { + const trimmed = part.trim(); + if (trimmed !== "") { + tokens.push(classifyAssertToken(trimmed)); + } + } + } + return tokens; +} + +function assertionName(path: string): string { + return basename(path, extname(path)); +} + +export async function readSqlFile(path: string, label: string): Promise { + try { + const contents = await readFile(path, "utf8"); + return contents.trim(); + } catch (error) { + throw new ConfigError(`Cannot read ${label} '${path}': ${errorMessage(error)}`); + } +} + +// A glob here is shallow: a single directory listing filtered by the literal +// prefix/suffix around the one `*` in the basename. Enough for the documented `dir/*.sql` +// form without pulling in a glob dependency; nested `**` is not supported. +async function expandGlob(pattern: string): Promise { + const dir = dirname(pattern); + const base = basename(pattern); + const star = base.indexOf("*"); + const prefix = base.slice(0, star); + const suffix = base.slice(star + 1); + let entries: string[]; + try { + entries = await readdir(dir); + } catch (error) { + if (isNotFoundError(error)) { + throw new ConfigError(`--assert glob '${pattern}' matched nothing (no such directory).`); + } + throw new ConfigError(`Cannot read --assert glob '${pattern}': ${errorMessage(error)}`); + } + const matches = entries + .filter( + (name) => name.startsWith(prefix) && name.endsWith(suffix) && name.length >= base.length - 1, + ) + .toSorted() + .map((name) => join(dir, name)); + if (matches.length === 0) { + throw new ConfigError(`--assert glob '${pattern}' matched no files.`); + } + return matches; +} + +// Each `.sql` file → one assertion, named by basename without extension; severity defaults to error. +export async function resolveAssertions(tokens: AssertToken[]): Promise { + const out: AssertionDef[] = []; + for (const token of tokens) { + const paths = token.kind === "glob" ? await expandGlob(token.pattern) : [token.path]; + for (const path of paths) { + out.push({ + name: assertionName(path), + sql: await readSqlFile(path, "--assert file"), + severity: "error", + }); + } + } + return out; +} diff --git a/src/commands/transform-test/index.ts b/src/commands/transform-test/index.ts new file mode 100644 index 0000000..8eeef86 --- /dev/null +++ b/src/commands/transform-test/index.ts @@ -0,0 +1,13 @@ +import { defineCommand } from "citty"; + +export default defineCommand({ + meta: { + name: "transform-test", + description: + "Test transforms or cards (and sub-graphs) against fixture CSVs without touching real tables", + }, + subCommands: { + inputs: () => import("./inputs").then((mod) => mod.default), + run: () => import("./run").then((mod) => mod.default), + }, +}); diff --git a/src/commands/transform-test/inputs.ts b/src/commands/transform-test/inputs.ts new file mode 100644 index 0000000..1c72b59 --- /dev/null +++ b/src/commands/transform-test/inputs.ts @@ -0,0 +1,49 @@ +import { TestRunInputCompact, testRunInputView } from "../../domain/transform-test-run"; +import { renderList } from "../../output/render"; +import { listEnvelopeSchema, wrapList } from "../../output/types"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId, parseIdList } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +import { fetchSubgraphInputs, parseTargetType, targetLabels, targetTypeFlag } from "./subgraph"; + +export const TestRunInputListEnvelope = listEnvelopeSchema(TestRunInputCompact); + +export default defineMetabaseCommand({ + meta: { + name: "inputs", + description: "List the input tables a transform or card test run requires fixtures for", + }, + details: + "Resolves the sub-graph from the selected --source transforms up to the target (the positional id) and returns its boundary leaf tables — one CSV fixture is required per table for `transform-test run`. The positional id is a transform id by default, or a card id when --target-type card is set. Omit --source to test the target alone. Each row carries the table id (use it in `--input =`) and the exact column headers the fixture CSV must contain.", + // Provisional: the test-run endpoints are unreleased. minVersion mirrors the + // transforms feature baseline so the command runs against a dev build; bump to the actual + // release version before this ships. + capabilities: { minVersion: 59 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...targetTypeFlag, + source: { + type: "string", + description: "Comma-separated boundary source transform ids (omit to test the target alone)", + }, + id: { type: "positional", description: "Target transform or card id", required: true }, + }, + outputSchema: TestRunInputListEnvelope, + examples: [ + "mb transform-test inputs 173 --source 172", + "mb transform-test inputs 173 --source 172 --json", + "mb transform-test inputs 42", + "mb transform-test inputs 88 --target-type card", + ], + async run({ args, ctx, getClient }) { + const targetType = parseTargetType(args["target-type"]); + const target = parseId(args.id, targetLabels(targetType).positionalLabel); + const sources = parseIdList(args.source, "--source"); + const client = await getClient(); + const items = await fetchSubgraphInputs(client, targetType, target, sources); + renderList(wrapList(items), testRunInputView, ctx); + }, +}); diff --git a/src/commands/transform-test/run.ts b/src/commands/transform-test/run.ts new file mode 100644 index 0000000..aea047e --- /dev/null +++ b/src/commands/transform-test/run.ts @@ -0,0 +1,172 @@ +import { readFile } from "node:fs/promises"; + +import { ConfigError, errorMessage } from "../../core/errors"; +import { TestRunResult } from "../../domain/transform-test-run"; +import { collectRepeatedFlag } from "../../runtime/citty"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId, parseIdList } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +import { type AssertionDef, parseAssertFlags, resolveAssertions } from "./assert"; +import { + parseColumnList, + parseInputPairs, + parseTargetType, + runSubgraph, + type SubgraphRunArgs, + targetLabels, + targetTypeFlag, +} from "./subgraph"; +import { parseSuite, type SuiteArgs } from "./suite"; + +const ASSERT_FLAG = { + type: "string", + description: + "Assertion: a .sql file path or a glob of them (dir/*.sql). Repeatable; comma-separates paths. Name = file basename without .sql. Severity defaults to error. Inline SQL is not supported — write the assertion to a .sql file.", +} as const; + +async function loadSuite(path: string): Promise { + let contents: string; + try { + contents = await readFile(path, "utf8"); + } catch (error) { + throw new ConfigError(`Cannot read --suite file '${path}': ${errorMessage(error)}`); + } + return parseSuite(contents, path); +} + +// Merge a YAML suite (if any) with ad-hoc flags: scalar/list flags override the suite when +// provided; --assert assertions append to the suite's. +function compose(suite: SuiteArgs | null, flags: FlagArgs): SubgraphRunArgs { + const targetType = flags.targetType ?? suite?.targetType; + const target = flags.target ?? suite?.target; + if (targetType === undefined || target === undefined) { + throw new ConfigError( + "Missing target. Provide the positional id (and --target-type) or a --suite that defines target.", + ); + } + const sources = flags.sources.length > 0 ? flags.sources : (suite?.sources ?? []); + const inputs = flags.inputs.length > 0 ? flags.inputs : (suite?.inputs ?? []); + const ignoreColumns = + flags.ignoreColumns.length > 0 ? flags.ignoreColumns : (suite?.ignoreColumns ?? []); + const expected = flags.expected ?? suite?.expected; + const assertions: AssertionDef[] = [...(suite?.assertions ?? []), ...flags.assertions]; + + if (expected === undefined && assertions.length === 0) { + throw new ConfigError( + "Provide at least one of --expected or --assert (or define them in --suite).", + ); + } + + const args: SubgraphRunArgs = { + targetType, + target, + sources, + inputs, + ignoreColumns, + assertions, + }; + if (expected !== undefined) { + args.expected = expected; + } + return args; +} + +interface FlagArgs { + targetType?: SubgraphRunArgs["targetType"]; + target?: number; + sources: number[]; + inputs: SubgraphRunArgs["inputs"]; + expected?: string; + ignoreColumns: string[]; + assertions: AssertionDef[]; +} + +export default defineMetabaseCommand({ + meta: { + name: "run", + description: "Test-run a transform or card (or sub-graph) against fixture CSVs and assertions", + }, + details: + "Seeds scratch tables from the --input fixture CSVs (real tables are never touched), runs the sub-graph from the --source transforms up to the target (the positional id) in dependency order, and checks the target's output against the --expected CSV and/or --assert SQL assertions. At least one of --expected or --assert is required. An assertion is a SQL query written to a `.sql` file that passes iff it returns zero rows; it references the synthetic `test_output` relation (the target's output) and/or input table names. A --suite YAML file can declare the whole run (target, inputs, expected, assertions with per-assertion severity) and is parsed entirely client-side; ad-hoc flags compose with it (scalars override, --assert appends). The positional id is a transform id by default, or a card id when --target-type card is set. Use `transform-test inputs` to discover which tables need fixtures. Exits non-zero when the diff fails or any error-severity assertion fails; warn-severity assertion failures are reported but do not affect the exit code.", + // Provisional: the test-run endpoints are unreleased. minVersion mirrors the + // transforms feature baseline so the command runs against a dev build; bump to the actual + // release version before this ships. + capabilities: { minVersion: 59 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...targetTypeFlag, + source: { + type: "string", + description: "Comma-separated boundary source transform ids (omit to test the target alone)", + }, + input: { + type: "string", + description: "Comma-separated = fixtures, one per required input table", + }, + expected: { + type: "string", + description: "Path to the expected-output CSV (optional if --assert is given)", + }, + "ignore-columns": { + type: "string", + description: "Comma-separated output column names to exclude from the diff", + }, + assert: ASSERT_FLAG, + suite: { + type: "string", + description: + "Path to a YAML test-suite file (target, inputs, expected, assertions with optional severity); composed with ad-hoc flags", + }, + // Not required at the citty layer: a --suite can supply the target. `compose` enforces that + // a target comes from one source or the other. + id: { + type: "positional", + required: false, + description: "Target transform or card id (optional when --suite defines the target)", + }, + }, + outputSchema: TestRunResult, + examples: [ + "mb transform-test run 173 --source 172 --input 229=orders.csv --expected expected.csv", + "mb transform-test run 173 --source 172 --input 229=orders.csv --assert checks/no_negatives.sql", + "mb transform-test run 173 --assert checks/no_negatives.sql,checks/has_rows.sql --input 229=orders.csv", + "mb transform-test run 173 --assert 'checks/*.sql' --input 229=orders.csv", + "mb transform-test run --suite suites/orders.yaml", + ], + async run({ args, rawArgs, ctx, getClient }) { + const suite = args.suite !== undefined ? await loadSuite(args.suite) : null; + + const idGiven = typeof args.id === "string" && args.id.trim() !== ""; + const targetType = idGiven || suite === null ? parseTargetType(args["target-type"]) : undefined; + const target = idGiven + ? parseId(args.id, targetLabels(targetType ?? "transform").positionalLabel) + : undefined; + + // `--assert` is repeatable: citty collapses repeats to the last value, so read every + // occurrence from rawArgs. `parseAssertFlags` also comma-splits each value. + const assertValues = collectRepeatedFlag(rawArgs, "assert", { assert: ASSERT_FLAG }); + const flags: FlagArgs = { + sources: parseIdList(args.source, "--source"), + inputs: parseInputPairs(args.input), + ignoreColumns: parseColumnList(args["ignore-columns"]), + assertions: await resolveAssertions(parseAssertFlags(assertValues)), + }; + if (targetType !== undefined && idGiven) { + flags.targetType = targetType; + } + if (target !== undefined) { + flags.target = target; + } + if (args.expected !== undefined && args.expected.trim() !== "") { + flags.expected = args.expected; + } + + const runArgs = compose(suite, flags); + + const client = await getClient(); + await runSubgraph(client, runArgs, ctx); + }, +}); diff --git a/src/commands/transform-test/subgraph.test.ts b/src/commands/transform-test/subgraph.test.ts new file mode 100644 index 0000000..798a58c --- /dev/null +++ b/src/commands/transform-test/subgraph.test.ts @@ -0,0 +1,266 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +import { ConfigError } from "../../core/errors"; +import type { AssertionResult, TestRunResult } from "../../domain/transform-test-run"; +import type { CommonContext } from "../context"; + +import { + assertionsSummaryLine, + buildSubgraphForm, + parseColumnList, + parseInputPairs, + parseTargetType, + renderRunResult, + shouldFail, +} from "./subgraph"; + +function assertion(over: Partial & { name: string }): AssertionResult { + return { + status: "passed", + failing_row_count: 0, + sample_rows: null, + columns: [], + ...over, + }; +} + +function result(over: Partial): TestRunResult { + return { status: "passed", diff: null, ...over }; +} + +function renderCtx(over: Partial): CommonContext { + return { + format: "text", + full: false, + fields: undefined, + maxBytes: 65536, + url: undefined, + apiKey: undefined, + profile: undefined, + skipPreflight: false, + ...over, + }; +} + +describe("parseInputPairs", () => { + it("parses comma-separated = pairs", () => { + expect(parseInputPairs("229=orders.csv,223=people.csv")).toEqual([ + { tableId: 229, path: "orders.csv" }, + { tableId: 223, path: "people.csv" }, + ]); + }); + + it("returns an empty array for undefined or blank input", () => { + expect(parseInputPairs(undefined)).toEqual([]); + expect(parseInputPairs(" ")).toEqual([]); + }); + + it("trims whitespace around entries and around each side of '='", () => { + expect(parseInputPairs(" 1 = a.csv , 2 = b.csv ")).toEqual([ + { tableId: 1, path: "a.csv" }, + { tableId: 2, path: "b.csv" }, + ]); + }); + + it("throws ConfigError with the offending entry when '=' is missing", () => { + expect(() => parseInputPairs("229")).toThrow(ConfigError); + expect(() => parseInputPairs("229")).toThrow( + "Malformed --input entry '229'. Expected = (e.g. 229=orders.csv).", + ); + }); + + it("throws ConfigError when the table id is not a positive integer", () => { + expect(() => parseInputPairs("0=x.csv")).toThrow(ConfigError); + expect(() => parseInputPairs("0=x.csv")).toThrow("invalid --input table id: 0 (must be ≥ 1)"); + }); +}); + +describe("parseColumnList", () => { + it("splits and trims comma-separated names", () => { + expect(parseColumnList("a, b ,c")).toEqual(["a", "b", "c"]); + }); + + it("returns an empty array for undefined or blank input", () => { + expect(parseColumnList(undefined)).toEqual([]); + expect(parseColumnList("")).toEqual([]); + }); +}); + +describe("parseTargetType", () => { + it("accepts the supported target types", () => { + expect(parseTargetType("transform")).toBe("transform"); + expect(parseTargetType("card")).toBe("card"); + }); + + it("throws ConfigError naming the supported types for an unsupported value", () => { + expect(() => parseTargetType("metric")).toThrow(ConfigError); + expect(() => parseTargetType("metric")).toThrow( + 'invalid --target-type: "metric" (expected one of: transform, card)', + ); + }); +}); + +async function formFields(form: FormData): Promise> { + const out: Record = {}; + for (const [key, value] of form.entries()) { + out[key] = typeof value === "string" ? value : await value.text(); + } + return out; +} + +describe("buildSubgraphForm", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "mb-form-")); + writeFileSync(join(dir, "expected.csv"), "id\n1\n"); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("appends an assertions JSON part when assertions are present", async () => { + const form = await buildSubgraphForm({ + targetType: "transform", + target: 173, + sources: [], + inputs: [], + expected: join(dir, "expected.csv"), + ignoreColumns: [], + assertions: [{ name: "a", sql: "SELECT 1", severity: "error" }], + }); + const fields = await formFields(form); + expect(fields["assertions"]).toBe( + JSON.stringify([{ name: "a", sql: "SELECT 1", severity: "error" }]), + ); + expect(fields["expected"]).toBe("id\n1\n"); + }); + + it("omits the expected part when no expected file is given (assertions-only)", async () => { + const form = await buildSubgraphForm({ + targetType: "transform", + target: 173, + sources: [], + inputs: [], + ignoreColumns: [], + assertions: [{ name: "a", sql: "SELECT 1", severity: "error" }], + }); + const fields = await formFields(form); + expect(fields["expected"]).toBeUndefined(); + expect(fields["assertions"]).toBeDefined(); + }); + + it("omits the assertions part when there are no assertions", async () => { + const form = await buildSubgraphForm({ + targetType: "transform", + target: 173, + sources: [], + inputs: [], + expected: join(dir, "expected.csv"), + ignoreColumns: [], + assertions: [], + }); + const fields = await formFields(form); + expect(fields["assertions"]).toBeUndefined(); + }); +}); + +describe("shouldFail", () => { + it("fails when the top-level status is failed", () => { + expect(shouldFail(result({ status: "failed" }))).toBe(true); + }); + + it("passes when the top-level status is passed, even with a failing warn assertion", () => { + const res = result({ + status: "passed", + assertions: [assertion({ name: "w", status: "warn", failing_row_count: 3 })], + }); + expect(shouldFail(res)).toBe(false); + }); +}); + +describe("assertionsSummaryLine", () => { + it("renders the passed/failed/warn breakdown with the first failing assertion", () => { + const res = result({ + status: "failed", + assertions: [ + assertion({ name: "ok", status: "passed" }), + assertion({ name: "neg_rev", status: "failed", failing_row_count: 3 }), + assertion({ name: "warned", status: "warn", failing_row_count: 2 }), + ], + }); + expect(assertionsSummaryLine(res)).toBe( + "3 assertions — 1 passed, 1 FAILED, 1 warn (neg_rev: 3 failing rows)", + ); + }); + + it("returns null when there are no assertions", () => { + expect(assertionsSummaryLine(result({ status: "passed" }))).toBeNull(); + expect(assertionsSummaryLine(result({ status: "passed", assertions: [] }))).toBeNull(); + }); +}); + +describe("renderRunResult", () => { + let stdout: string; + + beforeEach(() => { + stdout = ""; + vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { + stdout += String(chunk); + return true; + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + const failingResult = result({ + status: "failed", + diff: null, + assertions: [ + assertion({ name: "ok_check", status: "passed" }), + assertion({ name: "neg_rev", status: "failed", failing_row_count: 3 }), + ], + }); + + it("renders the per-assertion table on a FAILING run in human/text mode", () => { + renderRunResult("transform", 173, failingResult, renderCtx({ format: "text" })); + // Summary line(s) + expect(stdout).toContain("Transform 173 test run FAILED"); + expect(stdout).toContain("2 assertions — 1 passed, 1 FAILED"); + // The per-assertion table: header + each assertion's name/status/failing rows + expect(stdout).toContain("Name"); + expect(stdout).toContain("Status"); + expect(stdout).toContain("Failing Rows"); + expect(stdout).toContain("ok_check"); + expect(stdout).toContain("neg_rev"); + expect(stdout).toContain("failed"); + }); + + it("emits the full structured JSON (no human table) under --json", () => { + renderRunResult("transform", 173, failingResult, renderCtx({ format: "json" })); + const parsed = JSON.parse(stdout); + expect(parsed.status).toBe("failed"); + expect(parsed.assertions).toHaveLength(2); + expect(parsed.assertions[1].name).toBe("neg_rev"); + // No bordered text table in JSON mode. + expect(stdout).not.toContain("Failing Rows"); + }); + + it("renders the per-assertion table on a PASSING run in text mode", () => { + const passing = result({ + status: "passed", + assertions: [assertion({ name: "ok_check", status: "passed" })], + }); + renderRunResult("transform", 173, passing, renderCtx({ format: "text" })); + expect(stdout).toContain("Transform 173 test run passed."); + expect(stdout).toContain("ok_check"); + expect(stdout).toContain("Failing Rows"); + }); +}); diff --git a/src/commands/transform-test/subgraph.ts b/src/commands/transform-test/subgraph.ts new file mode 100644 index 0000000..44d773c --- /dev/null +++ b/src/commands/transform-test/subgraph.ts @@ -0,0 +1,227 @@ +import { z } from "zod"; + +import { ConfigError } from "../../core/errors"; +import type { Client } from "../../core/http/client"; +import { + type AssertionResult, + assertionResultView, + TestRunInput, + TestRunResult, + testRunResultView, +} from "../../domain/transform-test-run"; +import { renderList, renderSummary, writeText } from "../../output/render"; +import { wrapList } from "../../output/types"; +import { readFilePart } from "../../runtime/upload"; +import type { CommonContext } from "../context"; +import { parseEnumFlag } from "../parse-enum"; +import { parseId } from "../parse-id"; + +import type { AssertionDef } from "./assert"; + +export const TargetType = z.enum(["transform", "card"]); +export type TargetType = z.infer; + +const DEFAULT_TARGET_TYPE: TargetType = "transform"; + +interface TargetLabels { + positionalLabel: string; + summaryNoun: string; +} + +const TARGET_LABELS: Record = { + transform: { positionalLabel: "Target transform id", summaryNoun: "Transform" }, + card: { positionalLabel: "Target card id (saved question or model)", summaryNoun: "Card" }, +}; + +export function targetLabels(targetType: TargetType): TargetLabels { + return TARGET_LABELS[targetType]; +} + +export function parseTargetType(value: string): TargetType { + return parseEnumFlag(value, TargetType, "--target-type"); +} + +export const targetTypeFlag = { + "target-type": { + type: "string", + description: `Test-run target kind: ${TargetType.options.join(" | ")} (default: ${DEFAULT_TARGET_TYPE})`, + default: DEFAULT_TARGET_TYPE, + }, +} as const; + +export interface InputPair { + tableId: number; + path: string; +} + +function parseInputPair(pair: string): InputPair { + const eq = pair.indexOf("="); + if (eq <= 0 || eq === pair.length - 1) { + throw new ConfigError( + `Malformed --input entry '${pair}'. Expected = (e.g. 229=orders.csv).`, + ); + } + const tableId = parseId(pair.slice(0, eq).trim(), "--input table id"); + return { tableId, path: pair.slice(eq + 1).trim() }; +} + +export function parseInputPairs(value: string | undefined): InputPair[] { + if (value === undefined || value.trim() === "") { + return []; + } + return value + .split(",") + .map((part) => part.trim()) + .filter((part) => part !== "") + .map(parseInputPair); +} + +export function parseColumnList(value: string | undefined): string[] { + if (value === undefined || value.trim() === "") { + return []; + } + return value + .split(",") + .map((part) => part.trim()) + .filter((part) => part !== ""); +} + +function subgraphInputsPath(targetType: TargetType, target: number): string { + return `/api/ee/transform-test/${targetType}/${target}/inputs`; +} + +function subgraphPath(targetType: TargetType, target: number): string { + return `/api/ee/transform-test/${targetType}/${target}/run`; +} + +export async function fetchSubgraphInputs( + client: Client, + targetType: TargetType, + target: number, + sources: number[], +): Promise { + return client.requestParsed(z.array(TestRunInput), subgraphInputsPath(targetType, target), { + query: { sources }, + }); +} + +export interface SubgraphRunArgs { + targetType: TargetType; + target: number; + sources: number[]; + inputs: InputPair[]; + // Optional per field; the run requires at least one of expected or assertions. + expected?: string; + ignoreColumns: string[]; + assertions: AssertionDef[]; +} + +export async function buildSubgraphForm(args: SubgraphRunArgs): Promise { + const form = new FormData(); + for (const { tableId, path } of args.inputs) { + const part = await readFilePart(path, `--input ${tableId}`); + form.append(`input-${tableId}`, part.blob, part.filename); + } + if (args.expected !== undefined) { + const expectedPart = await readFilePart(args.expected, "--expected"); + form.append("expected", expectedPart.blob, expectedPart.filename); + } + if (args.sources.length > 0) { + form.append("sources", JSON.stringify(args.sources)); + } + if (args.ignoreColumns.length > 0) { + form.append("options", JSON.stringify({ ignore_columns: args.ignoreColumns })); + } + if (args.assertions.length > 0) { + form.append("assertions", JSON.stringify(args.assertions)); + } + return form; +} + +function assertionList(result: TestRunResult): AssertionResult[] { + return result.assertions ?? []; +} + +// Exit nonzero iff the server's top-level status is `failed`. +export function shouldFail(result: TestRunResult): boolean { + return result.status === "failed"; +} + +export function assertionsSummaryLine(result: TestRunResult): string | null { + const assertions = assertionList(result); + if (assertions.length === 0) { + return null; + } + const passed = assertions.filter((a) => a.status === "passed").length; + const failed = assertions.filter((a) => a.status === "failed").length; + const warned = assertions.filter((a) => a.status === "warn").length; + const parts = [`${passed} passed`, `${failed} FAILED`, `${warned} warn`]; + const firstFailing = assertions.find((a) => a.status === "failed" || a.status === "warn"); + const detail = + firstFailing === undefined + ? "" + : ` (${firstFailing.name}: ${firstFailing.failing_row_count} failing rows)`; + return `${assertions.length} assertions — ${parts.join(", ")}${detail}`; +} + +function summaryLine(targetType: TargetType, target: number, result: TestRunResult): string { + const noun = targetLabels(targetType).summaryNoun; + const lines: string[] = []; + const diffShown = (result.diff ?? null) !== null; + if (result.status === "passed") { + lines.push(`${noun} ${target} test run passed.`); + } else if (diffShown) { + lines.push( + `${noun} ${target} test run FAILED — output did not match expected. Re-run with --json to see the diff.`, + ); + } else { + lines.push(`${noun} ${target} test run FAILED. Re-run with --json to see details.`); + } + const assertions = assertionsSummaryLine(result); + if (assertions !== null) { + lines.push(assertions); + } + return lines.join("\n"); +} + +// Render a run result. Under `--json` (and `--fields`/`--full`) the full structured result is +// emitted; otherwise the human view: the summary line(s) followed — on any run that carried +// assertions, passing OR failing — by the per-assertion table (name / status / failing rows). +export function renderRunResult( + targetType: TargetType, + target: number, + result: TestRunResult, + ctx: CommonContext, +): void { + renderSummary(result, testRunResultView, () => summaryLine(targetType, target, result), ctx); + + const humanView = ctx.format !== "json" && ctx.fields === undefined && !ctx.full; + const assertions = assertionList(result); + if (humanView && assertions.length > 0) { + writeText(""); + renderList(wrapList(assertions), assertionResultView, ctx); + } +} + +export async function runSubgraph( + client: Client, + args: SubgraphRunArgs, + ctx: CommonContext, +): Promise { + const form = await buildSubgraphForm(args); + const result = await client.requestParsed( + TestRunResult, + subgraphPath(args.targetType, args.target), + { + method: "POST", + body: form, + }, + ); + + renderRunResult(args.targetType, args.target, result, ctx); + + if (shouldFail(result)) { + const noun = targetLabels(args.targetType).summaryNoun.toLowerCase(); + throw new Error(`${noun} ${args.target} test run failed`); + } +} diff --git a/src/commands/transform-test/suite.test.ts b/src/commands/transform-test/suite.test.ts new file mode 100644 index 0000000..c52f2a9 --- /dev/null +++ b/src/commands/transform-test/suite.test.ts @@ -0,0 +1,121 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { ConfigError } from "../../core/errors"; +import { ValidationError } from "../../core/errors"; + +import { parseSuite } from "./suite"; + +describe("parseSuite", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "mb-suite-")); + writeFileSync(join(dir, "no_negatives.sql"), "SELECT * FROM test_output WHERE revenue < 0\n"); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("parses a full suite into SubgraphRunArgs-compatible fields", async () => { + const yaml = ` +target: + type: transform + id: 173 +sources: [172] +inputs: + - table: 229 + file: orders.csv +expected: out.csv +ignore_columns: [snapshot_ts] +assertions: + - name: positive_revenue + sql: SELECT * FROM test_output WHERE revenue < 0 + severity: warn + - name: from_file + file: ${join(dir, "no_negatives.sql")} +`; + const out = await parseSuite(yaml, "suite.yaml"); + expect(out.targetType).toBe("transform"); + expect(out.target).toBe(173); + expect(out.sources).toEqual([172]); + expect(out.inputs).toEqual([{ tableId: 229, path: "orders.csv" }]); + expect(out.expected).toBe("out.csv"); + expect(out.ignoreColumns).toEqual(["snapshot_ts"]); + expect(out.assertions).toEqual([ + { + name: "positive_revenue", + sql: "SELECT * FROM test_output WHERE revenue < 0", + severity: "warn", + }, + { + name: "from_file", + sql: "SELECT * FROM test_output WHERE revenue < 0", + severity: "error", + }, + ]); + }); + + it("defaults optional fields (sources, inputs, ignore_columns, assertions) to empty", async () => { + const yaml = ` +target: + type: card + id: 88 +expected: out.csv +`; + const out = await parseSuite(yaml, "suite.yaml"); + expect(out.targetType).toBe("card"); + expect(out.target).toBe(88); + expect(out.sources).toEqual([]); + expect(out.inputs).toEqual([]); + expect(out.ignoreColumns).toEqual([]); + expect(out.assertions).toEqual([]); + expect(out.expected).toBe("out.csv"); + }); + + it("leaves expected undefined when absent", async () => { + const yaml = ` +target: + type: transform + id: 1 +assertions: + - name: a + sql: SELECT 1 +`; + const out = await parseSuite(yaml, "suite.yaml"); + expect(out.expected).toBeUndefined(); + }); + + it("rejects an assertion with neither sql nor file", async () => { + const yaml = ` +target: + type: transform + id: 1 +assertions: + - name: bad +`; + await expect(parseSuite(yaml, "suite.yaml")).rejects.toThrow(ConfigError); + }); + + it("rejects an assertion with both sql and file", async () => { + const yaml = ` +target: + type: transform + id: 1 +assertions: + - name: bad + sql: SELECT 1 + file: x.sql +`; + await expect(parseSuite(yaml, "suite.yaml")).rejects.toThrow(ConfigError); + }); + + it("surfaces a schema ValidationError for a malformed target", async () => { + const yaml = `target: "nope"`; + await expect(parseSuite(yaml, "suite.yaml")).rejects.toThrow(ValidationError); + }); +}); diff --git a/src/commands/transform-test/suite.ts b/src/commands/transform-test/suite.ts new file mode 100644 index 0000000..9505d0a --- /dev/null +++ b/src/commands/transform-test/suite.ts @@ -0,0 +1,78 @@ +import { z } from "zod"; + +import { ConfigError } from "../../core/errors"; +import { parseYaml } from "../../runtime/yaml"; + +import { readSqlFile, type AssertionDef, type Severity } from "./assert"; +import { type InputPair, type TargetType } from "./subgraph"; + +const TargetTypeSchema = z.enum(["transform", "card"]); +const SeveritySchema = z.enum(["error", "warn"]); + +const SuiteAssertion = z + .object({ + name: z.string().min(1), + sql: z.string().optional(), + file: z.string().optional(), + severity: SeveritySchema.optional(), + }) + .strict(); + +const Suite = z + .object({ + target: z.object({ type: TargetTypeSchema, id: z.number().int().positive() }).strict(), + sources: z.array(z.number().int().positive()).optional(), + inputs: z + .array(z.object({ table: z.number().int().positive(), file: z.string() }).strict()) + .optional(), + expected: z.string().optional(), + ignore_columns: z.array(z.string()).optional(), + assertions: z.array(SuiteAssertion).optional(), + }) + .strict(); + +type Suite = z.infer; +type SuiteAssertion = z.infer; + +// The slice of SubgraphRunArgs a suite contributes. +export interface SuiteArgs { + targetType: TargetType; + target: number; + sources: number[]; + inputs: InputPair[]; + expected?: string; + ignoreColumns: string[]; + assertions: AssertionDef[]; +} + +async function resolveSuiteAssertion(entry: SuiteAssertion): Promise { + const severity: Severity = entry.severity ?? "error"; + if (entry.sql !== undefined && entry.file === undefined) { + return { name: entry.name, sql: entry.sql.trim(), severity }; + } + if (entry.file !== undefined && entry.sql === undefined) { + return { + name: entry.name, + sql: await readSqlFile(entry.file, "suite assertion file"), + severity, + }; + } + throw new ConfigError(`Suite assertion '${entry.name}' must set exactly one of 'sql' or 'file'.`); +} + +export async function parseSuite(yamlText: string, source: string): Promise { + const suite: Suite = parseYaml(yamlText, Suite, { source }); + const assertions = await Promise.all((suite.assertions ?? []).map(resolveSuiteAssertion)); + const args: SuiteArgs = { + targetType: suite.target.type, + target: suite.target.id, + sources: suite.sources ?? [], + inputs: (suite.inputs ?? []).map((row) => ({ tableId: row.table, path: row.file })), + ignoreColumns: suite.ignore_columns ?? [], + assertions, + }; + if (suite.expected !== undefined) { + args.expected = suite.expected; + } + return args; +} diff --git a/src/core/http/errors.ts b/src/core/http/errors.ts index 2c48fae..4702d81 100644 --- a/src/core/http/errors.ts +++ b/src/core/http/errors.ts @@ -38,7 +38,9 @@ const STATUS_CLASSIFICATIONS: Record = { const ErrorEnvelope = z .object({ message: z.string().optional(), - error: z.string().optional(), + // `error` may be a structured object, not a string; accept any shape so a non-string + // doesn't break parsing the whole envelope. + error: z.unknown().optional(), "error-message": z.string().optional(), via: z.array(z.object({ message: z.string().optional() }).loose()).optional(), "specific-errors": z.unknown().optional(), @@ -211,7 +213,8 @@ function parseEnvelopeMessage(sanitizedBody: string | null): string | null { return null; } const envelope = result.value; - const topLevel = envelope.message ?? envelope.error ?? envelope["error-message"]; + const errorString = typeof envelope.error === "string" ? envelope.error : undefined; + const topLevel = envelope.message ?? errorString ?? envelope["error-message"]; if (topLevel) { return capLength(topLevel); } diff --git a/src/domain/transform-test-run.test.ts b/src/domain/transform-test-run.test.ts new file mode 100644 index 0000000..7e2b77f --- /dev/null +++ b/src/domain/transform-test-run.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; + +import { TestRunInput, TestRunInputCompact } from "./transform-test-run"; + +describe("TestRunInput", () => { + const base = { + table_id: 229, + schema: "public", + name: "orders", + columns: ["id", "total"], + }; + + it("accepts a schema-qualified input table", () => { + expect(TestRunInput.safeParse(base).success).toBe(true); + }); + + it("accepts a null schema (engines without schemas)", () => { + const parsed = TestRunInput.safeParse({ ...base, schema: null }); + expect(parsed.success).toBe(true); + expect(parsed.data?.schema).toBeNull(); + }); + + it("rejects a missing schema key", () => { + const { schema: _schema, ...rest } = base; + expect(TestRunInput.safeParse(rest).success).toBe(false); + }); + + it("compact pick preserves a null schema", () => { + expect(TestRunInputCompact.parse({ ...base, schema: null }).schema).toBeNull(); + }); +}); diff --git a/src/domain/transform-test-run.ts b/src/domain/transform-test-run.ts new file mode 100644 index 0000000..4d520ea --- /dev/null +++ b/src/domain/transform-test-run.ts @@ -0,0 +1,84 @@ +import { z } from "zod"; + +import type { ResourceView } from "./view"; + +export const TestRunInput = z + .object({ + table_id: z.number().int().positive(), + // null on engines without schemas (e.g. MySQL targets with no schema segment). + schema: z.string().nullable(), + name: z.string(), + columns: z.array(z.string()), + }) + .loose(); +export type TestRunInput = z.infer; + +export const TestRunInputCompact = TestRunInput.pick({ + table_id: true, + schema: true, + name: true, + columns: true, +}).strip(); +export type TestRunInputCompact = z.infer; + +function formatColumns(value: unknown): string { + return Array.isArray(value) ? value.join(", ") : String(value ?? ""); +} + +export const testRunInputView: ResourceView = { + compactPick: TestRunInputCompact, + tableColumns: [ + { key: "table_id", label: "Table ID" }, + { key: "schema", label: "Schema" }, + { key: "name", label: "Name" }, + { key: "columns", label: "Columns", format: formatColumns }, + ], +}; + +export const AssertionResult = z + .object({ + name: z.string(), + status: z.enum(["passed", "failed", "warn"]), + failing_row_count: z.number().int().nonnegative(), + sample_rows: z.array(z.array(z.unknown())).nullable(), + columns: z.array(z.string()), + }) + .loose(); +export type AssertionResult = z.infer; + +export const TestRunResult = z + .object({ + status: z.enum(["passed", "failed"]), + diff: z.unknown(), + assertions: z.array(AssertionResult).nullable().optional(), + }) + .loose(); +export type TestRunResult = z.infer; + +export const TestRunResultCompact = TestRunResult.pick({ + status: true, + diff: true, + assertions: true, +}).strip(); +export type TestRunResultCompact = z.infer; + +export const testRunResultView: ResourceView = { + compactPick: TestRunResultCompact, + tableColumns: [{ key: "status", label: "Status" }], +}; + +export const AssertionResultCompact = AssertionResult.pick({ + name: true, + status: true, + failing_row_count: true, +}).strip(); +export type AssertionResultCompact = z.infer; + +export const assertionResultView: ResourceView = { + compactPick: AssertionResultCompact, + tableColumns: [ + { key: "name", label: "Name" }, + { key: "status", label: "Status" }, + { key: "failing_row_count", label: "Failing Rows" }, + ], +}; diff --git a/src/main.ts b/src/main.ts index 220879b..c5da490 100644 --- a/src/main.ts +++ b/src/main.ts @@ -24,6 +24,7 @@ const main: CommandDef = defineCommand({ document: () => import("./commands/document").then((mod) => mod.default), transform: () => import("./commands/transform").then((mod) => mod.default), "transform-job": () => import("./commands/transform-job").then((mod) => mod.default), + "transform-test": () => import("./commands/transform-test").then((mod) => mod.default), "transform-tag": () => import("./commands/transform-tag").then((mod) => mod.default), setting: () => import("./commands/setting").then((mod) => mod.default), search: () => import("./commands/search").then((mod) => mod.default), diff --git a/src/runtime/citty.test.ts b/src/runtime/citty.test.ts index 3bed035..595bc31 100644 --- a/src/runtime/citty.test.ts +++ b/src/runtime/citty.test.ts @@ -1,7 +1,7 @@ -import type { CommandMeta } from "citty"; +import type { ArgsDef, CommandMeta } from "citty"; import { describe, expect, it } from "vitest"; -import { resolveCitty, toAliasArray } from "./citty"; +import { collectRepeatedFlag, resolveCitty, toAliasArray } from "./citty"; describe("resolveCitty", () => { const meta: CommandMeta = { name: "demo", description: "demo cmd" }; @@ -26,6 +26,37 @@ describe("resolveCitty", () => { }); }); +describe("collectRepeatedFlag", () => { + const argsDef: ArgsDef = { + assert: { type: "string", alias: "a" }, + expected: { type: "string" }, + }; + + it("collects every occurrence of a repeated flag in order", () => { + const raw = ["173", "--assert", "a.sql", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql", "b.sql"]); + }); + + it("handles the --flag=value form", () => { + const raw = ["--assert=a.sql", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql", "b.sql"]); + }); + + it("collects aliases of the flag", () => { + const raw = ["-a", "a.sql", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql", "b.sql"]); + }); + + it("returns an empty array when the flag is absent", () => { + expect(collectRepeatedFlag(["--expected", "out.csv"], "assert", argsDef)).toEqual([]); + }); + + it("ignores tokens after the -- separator", () => { + const raw = ["--assert", "a.sql", "--", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql"]); + }); +}); + describe("toAliasArray", () => { it("returns an empty array for an absent alias", () => { expect(toAliasArray(undefined)).toEqual([]); diff --git a/src/runtime/citty.ts b/src/runtime/citty.ts index 9b3606e..6f43850 100644 --- a/src/runtime/citty.ts +++ b/src/runtime/citty.ts @@ -25,6 +25,45 @@ export function normalizeFlag(value: string): string { return value.replace(/^-+/, "").replace(/-/g, "").toLowerCase(); } +// citty (this version) parses repeated string flags via Node's `util.parseArgs` without +// `multiple`, so `--x a --x b` collapses to the last value. For genuinely repeatable flags we +// recover every occurrence straight from rawArgs. Matches the flag's own name and any aliases, +// the `--flag value` and `--flag=value` (and short `-a value`) forms, and stops at `--`. +export function collectRepeatedFlag( + rawArgs: readonly string[], + flagName: string, + argsDef: ArgsDef, +): string[] { + const def = argsDef[flagName]; + const aliases = def !== undefined && "alias" in def ? toAliasArray(def.alias) : []; + const names = new Set([normalizeFlag(flagName), ...aliases.map(normalizeFlag)]); + const values: string[] = []; + for (let i = 0; i < rawArgs.length; i++) { + const token = rawArgs[i]; + if (token === undefined || token === "--") { + break; + } + if (!token.startsWith("-")) { + continue; + } + const equals = token.indexOf("="); + const head = equals === -1 ? token : token.slice(0, equals); + if (!names.has(normalizeFlag(head))) { + continue; + } + if (equals !== -1) { + values.push(token.slice(equals + 1)); + continue; + } + const next = rawArgs[i + 1]; + if (next !== undefined) { + values.push(next); + i += 1; + } + } + return values; +} + export function flagConsumesValue(token: string, argsDef: ArgsDef): boolean { if (token.includes("=")) { return false; diff --git a/src/runtime/command-help.test.ts b/src/runtime/command-help.test.ts index 565ea2f..962932a 100644 --- a/src/runtime/command-help.test.ts +++ b/src/runtime/command-help.test.ts @@ -312,6 +312,8 @@ const ALL_COMMANDS = [ "transform-job run", "transform-job transforms", "transform-job set-active", + "transform-test inputs", + "transform-test run", "transform-tag list", "transform-tag create", "transform-tag update", diff --git a/src/runtime/upload.ts b/src/runtime/upload.ts index e7a3b4f..c07b85b 100644 --- a/src/runtime/upload.ts +++ b/src/runtime/upload.ts @@ -1,10 +1,24 @@ import { readFile } from "node:fs/promises"; import { basename } from "node:path"; -import { ConfigError, isNotFoundError } from "../core/errors"; +import { ConfigError, errorMessage, isNotFoundError } from "../core/errors"; import { fileNotFoundError } from "./input"; +export interface FilePart { + blob: Blob; + filename: string; +} + +export async function readFilePart(path: string, label: string): Promise { + try { + const bytes = await readFile(path); + return { blob: new Blob([bytes]), filename: basename(path) }; + } catch (error) { + throw new ConfigError(`Cannot read ${label} file '${path}': ${errorMessage(error)}`); + } +} + const CSV_CONTENT_TYPE = "text/csv"; export interface CsvFile { diff --git a/tests/e2e/measure.e2e.test.ts b/tests/e2e/measure.e2e.test.ts index 51fb9ad..68b3f9a 100644 --- a/tests/e2e/measure.e2e.test.ts +++ b/tests/e2e/measure.e2e.test.ts @@ -156,7 +156,7 @@ describe.skipIf(skipReason !== null)("measure e2e", () => { }); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Metabase returned 500"); + expect(result.stderr).toContain("Value does not match schema"); expect(result.stdout).toBe(""); }); diff --git a/tests/e2e/segment.e2e.test.ts b/tests/e2e/segment.e2e.test.ts index c9fce1f..7ff5480 100644 --- a/tests/e2e/segment.e2e.test.ts +++ b/tests/e2e/segment.e2e.test.ts @@ -153,7 +153,7 @@ describe("segment e2e", () => { }); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Metabase returned 500"); + expect(result.stderr).toContain("Value does not match schema"); expect(result.stdout).toBe(""); }); diff --git a/tests/e2e/skills.e2e.test.ts b/tests/e2e/skills.e2e.test.ts index d860df3..5df495a 100644 --- a/tests/e2e/skills.e2e.test.ts +++ b/tests/e2e/skills.e2e.test.ts @@ -17,6 +17,8 @@ const BUNDLED_VISIBLE_NAMES = [ "metadata", "native-sql", "transform", + "transform-test", + "transform-test-plan", "visualization", ] as const; @@ -33,7 +35,7 @@ describe("skills e2e", () => { return dir; } - it("list returns the ten bundled non-hidden skills, sorted by name", async () => { + it("list returns the twelve bundled non-hidden skills, sorted by name", async () => { const result = await runCli({ args: ["skills", "list", "--json"], configHome: await makeIsolatedConfigHome(),