From 591de9378e4d01825a6cf7449193abc2ccf85e17 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Mon, 22 Jun 2026 15:48:41 -0600 Subject: [PATCH 1/9] Test ye the transforms --- src/commands/parse-id.ts | 11 +++ src/commands/transform-test/index.ts | 12 +++ src/commands/transform-test/inputs.ts | 54 ++++++++++ src/commands/transform-test/run.test.ts | 51 ++++++++++ src/commands/transform-test/run.ts | 125 ++++++++++++++++++++++++ src/core/http/errors.ts | 8 +- src/domain/transform-test-run.ts | 59 +++++++++++ src/main.ts | 1 + src/runtime/upload.ts | 18 ++++ 9 files changed, 337 insertions(+), 2 deletions(-) create mode 100644 src/commands/transform-test/index.ts create mode 100644 src/commands/transform-test/inputs.ts create mode 100644 src/commands/transform-test/run.test.ts create mode 100644 src/commands/transform-test/run.ts create mode 100644 src/domain/transform-test-run.ts create mode 100644 src/runtime/upload.ts 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/transform-test/index.ts b/src/commands/transform-test/index.ts new file mode 100644 index 0000000..31e7814 --- /dev/null +++ b/src/commands/transform-test/index.ts @@ -0,0 +1,12 @@ +import { defineCommand } from "citty"; + +export default defineCommand({ + meta: { + name: "transform-test", + description: "Test transforms (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..596b314 --- /dev/null +++ b/src/commands/transform-test/inputs.ts @@ -0,0 +1,54 @@ +import { z } from "zod"; + +import { + TestRunInput, + 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"; + +export const TestRunInputListEnvelope = listEnvelopeSchema(TestRunInputCompact); + +export default defineMetabaseCommand({ + meta: { + name: "inputs", + description: "List the input tables a transform 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`. Omit --source to test the target transform 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/subgraph 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, + source: { + type: "string", + description: "Comma-separated boundary source transform ids (omit to test the target alone)", + }, + id: { type: "positional", description: "Target transform 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", + ], + async run({ args, ctx, getClient }) { + const target = parseId(args.id); + const sources = parseIdList(args.source, "--source"); + const client = await getClient(); + const items = await client.requestParsed( + z.array(TestRunInput), + `/api/transform/${target}/test-run/subgraph-inputs`, + { query: { sources } }, + ); + renderList(wrapList(items), testRunInputView, ctx); + }, +}); diff --git a/src/commands/transform-test/run.test.ts b/src/commands/transform-test/run.test.ts new file mode 100644 index 0000000..102146f --- /dev/null +++ b/src/commands/transform-test/run.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; + +import { ConfigError } from "../../core/errors"; + +import { parseColumnList, parseInputPairs } from "./run"; + +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([]); + }); +}); diff --git a/src/commands/transform-test/run.ts b/src/commands/transform-test/run.ts new file mode 100644 index 0000000..00f725a --- /dev/null +++ b/src/commands/transform-test/run.ts @@ -0,0 +1,125 @@ +import { ConfigError } from "../../core/errors"; +import { TestRunResult, testRunResultView } from "../../domain/transform-test-run"; +import { renderSummary } from "../../output/render"; +import { readFilePart } from "../../runtime/upload"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId, parseIdList } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +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 summaryLine(target: number, result: TestRunResult): string { + if (result.status === "passed") { + return `Transform ${target} test run passed.`; + } + return `Transform ${target} test run FAILED — output did not match expected. Re-run with --json to see the diff.`; +} + +export default defineMetabaseCommand({ + meta: { + name: "run", + description: "Test-run a transform (or sub-graph) against fixture CSVs", + }, + 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 diffs the target's output against the --expected CSV. Use `transform-test inputs` to discover which tables need fixtures. Omit --source to test the target transform alone. Exits non-zero when the output does not match.", + // PROVISIONAL: the test-run/subgraph 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, + 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" }, + "ignore-columns": { + type: "string", + description: "Comma-separated output column names to exclude from the diff", + }, + id: { type: "positional", description: "Target transform id", required: true }, + }, + outputSchema: TestRunResult, + examples: [ + "mb transform-test run 173 --source 172 --input 229=orders.csv,223=people.csv --expected expected.csv", + "mb transform-test run 42 --input 229=orders.csv --expected out.csv --ignore-columns snapshot_ts", + ], + async run({ args, ctx, getClient }) { + const target = parseId(args.id); + const sources = parseIdList(args.source, "--source"); + const inputs = parseInputPairs(args.input); + const expected = args.expected; + if (expected === undefined || expected.trim() === "") { + throw new ConfigError("Missing required --expected ."); + } + const ignoreColumns = parseColumnList(args["ignore-columns"]); + + const form = new FormData(); + for (const { tableId, path } of inputs) { + const part = await readFilePart(path, `--input ${tableId}`); + form.append(`input-${tableId}`, part.blob, part.filename); + } + const expectedPart = await readFilePart(expected, "--expected"); + form.append("expected", expectedPart.blob, expectedPart.filename); + if (sources.length > 0) { + form.append("sources", JSON.stringify(sources)); + } + if (ignoreColumns.length > 0) { + form.append("options", JSON.stringify({ ignore_columns: ignoreColumns })); + } + + const client = await getClient(); + const result = await client.requestParsed( + TestRunResult, + `/api/transform/${target}/test-run/subgraph`, + { method: "POST", body: form }, + ); + + renderSummary(result, testRunResultView, () => summaryLine(target, result), ctx); + + if (result.status === "failed") { + throw new Error(`transform ${target} test run failed: output did not match expected`); + } + }, +}); diff --git a/src/core/http/errors.ts b/src/core/http/errors.ts index 2c48fae..e197bcc 100644 --- a/src/core/http/errors.ts +++ b/src/core/http/errors.ts @@ -38,7 +38,10 @@ const STATUS_CLASSIFICATIONS: Record = { const ErrorEnvelope = z .object({ message: z.string().optional(), - error: z.string().optional(), + // `error` is usually a string, but some endpoints send a structured object + // (e.g. {type, message}); accept any shape so a non-string never fails the + // whole-envelope parse and drops us to a bare status-code message. + 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 +214,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.ts b/src/domain/transform-test-run.ts new file mode 100644 index 0000000..3e4b5eb --- /dev/null +++ b/src/domain/transform-test-run.ts @@ -0,0 +1,59 @@ +import { z } from "zod"; + +import type { ResourceView } from "./view"; + +export const TestRunInput = z + .object({ + table_id: z.number().int().positive(), + schema: z.string(), + 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 TestRunResult = z + .object({ + status: z.enum(["passed", "failed"]), + diff: z.unknown(), + test_run_id: z.number().int().positive().nullable(), + }) + .loose(); +export type TestRunResult = z.infer; + +export const TestRunResultCompact = TestRunResult.pick({ + status: true, + test_run_id: true, + diff: true, +}).strip(); +export type TestRunResultCompact = z.infer; + +export const testRunResultView: ResourceView = { + compactPick: TestRunResultCompact, + tableColumns: [ + { key: "status", label: "Status" }, + { key: "test_run_id", label: "Run ID" }, + ], +}; diff --git a/src/main.ts b/src/main.ts index 0be18aa..13db99a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -20,6 +20,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), setting: () => import("./commands/setting").then((mod) => mod.default), search: () => import("./commands/search").then((mod) => mod.default), "git-sync": () => import("./commands/git-sync").then((mod) => mod.default), diff --git a/src/runtime/upload.ts b/src/runtime/upload.ts new file mode 100644 index 0000000..8dd0a75 --- /dev/null +++ b/src/runtime/upload.ts @@ -0,0 +1,18 @@ +import { readFile } from "node:fs/promises"; +import { basename } from "node:path"; + +import { ConfigError, errorMessage } from "../core/errors"; + +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)}`); + } +} From 4a1e12c82f53b0090008bc40e34d4c2820a121c7 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Wed, 24 Jun 2026 14:52:26 -0600 Subject: [PATCH 2/9] Draft skill --- skill-data/transform-test/SKILL.md | 112 +++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 skill-data/transform-test/SKILL.md diff --git a/skill-data/transform-test/SKILL.md b/skill-data/transform-test/SKILL.md new file mode 100644 index 0000000..56e5f51 --- /dev/null +++ b/skill-data/transform-test/SKILL.md @@ -0,0 +1,112 @@ +--- +name: transform-test +description: Test a transform (or a connected sub-graph of transforms) against fixture CSVs via `mb transform-test` — seeds scratch tables, runs the transform, and diffs its output against an expected CSV, never touching real tables. Covers the inputs→fixtures→run loop, the exact-header CSV contract, reading the pass/fail diff, chained sub-graph tests (`--source`), `--ignore-columns`, and the native-SQL limitations. Load when the user wants to validate a transform's logic before running it for real — "test this transform", "check the transform against sample data", "does my transform produce the right output", "write fixtures for a transform", or anything `mb transform-test …`. +allowed-tools: Read, Write, Edit, Bash +--- + +# Testing transforms + +`mb transform-test` runs a transform against **fixture CSVs you supply** instead of the real source tables, then diffs the output against an **expected CSV**. It seeds throwaway scratch tables, runs the transform's query into a scratch output, compares, and drops everything — **real tables are never read or written.** Use it to validate a transform's logic on small, known data before trusting it on production rows. + +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`). 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 + one expected-output CSV. +3. **Run** the test; read passed / FAILED + the diff (`transform-test run`). + +The positional argument is always the **target** transform id (the one whose output is diffed). + +## 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 (required). +- Exits **0 on pass, non-zero on fail** (good for scripting / CI gates). + +Reading the result: + +- The plain summary says `Transform test run passed.` or `Transform test run FAILED — output did not match expected. Re-run with --json to see the diff.` +- `--json` returns `{status, diff, test_run_id}` where `status` is `passed` or `failed` (those are the only two values). On `failed`, **`diff` is where the truth is** — it reports missing rows (expected but not produced), extra rows (produced but not expected), and cell-level mismatches. Always re-run with `--json` (or start with it) to see why a test failed. +- 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. + +## 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). + +## 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 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 transforms aren't affected. This is a known, accepted limitation. +- **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). Read the `--json` `diff` to fix either the transform or the expected CSV. +- Don't supply fixtures for tables `inputs` didn't list, or omit ones it did — the `--input` set must match the required set exactly. From 6f5cb480c79970b71a3726ad3df1a961273b7bf2 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Fri, 26 Jun 2026 16:03:17 -0600 Subject: [PATCH 3/9] Update to catch up --- src/commands/transform-test/index.ts | 3 +- src/commands/transform-test/inputs.ts | 27 ++- src/commands/transform-test/run.ts | 94 +++-------- .../{run.test.ts => subgraph.test.ts} | 20 ++- src/commands/transform-test/subgraph.ts | 159 ++++++++++++++++++ 5 files changed, 208 insertions(+), 95 deletions(-) rename src/commands/transform-test/{run.test.ts => subgraph.test.ts} (69%) create mode 100644 src/commands/transform-test/subgraph.ts diff --git a/src/commands/transform-test/index.ts b/src/commands/transform-test/index.ts index 31e7814..8eeef86 100644 --- a/src/commands/transform-test/index.ts +++ b/src/commands/transform-test/index.ts @@ -3,7 +3,8 @@ import { defineCommand } from "citty"; export default defineCommand({ meta: { name: "transform-test", - description: "Test transforms (and sub-graphs) against fixture CSVs without touching real tables", + description: + "Test transforms or cards (and sub-graphs) against fixture CSVs without touching real tables", }, subCommands: { inputs: () => import("./inputs").then((mod) => mod.default), diff --git a/src/commands/transform-test/inputs.ts b/src/commands/transform-test/inputs.ts index 596b314..f618e51 100644 --- a/src/commands/transform-test/inputs.ts +++ b/src/commands/transform-test/inputs.ts @@ -1,25 +1,21 @@ -import { z } from "zod"; - -import { - TestRunInput, - TestRunInputCompact, - testRunInputView, -} from "../../domain/transform-test-run"; +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 test run requires fixtures for", + 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`. Omit --source to test the target transform alone. Each row carries the table id (use it in `--input =`) and the exact column headers the fixture CSV must contain.", + "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/subgraph 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. @@ -28,27 +24,26 @@ export default defineMetabaseCommand({ ...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 id", required: true }, + 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 target = parseId(args.id); + 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 client.requestParsed( - z.array(TestRunInput), - `/api/transform/${target}/test-run/subgraph-inputs`, - { query: { sources } }, - ); + 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 index 00f725a..bc8a86c 100644 --- a/src/commands/transform-test/run.ts +++ b/src/commands/transform-test/run.ts @@ -1,62 +1,25 @@ import { ConfigError } from "../../core/errors"; -import { TestRunResult, testRunResultView } from "../../domain/transform-test-run"; -import { renderSummary } from "../../output/render"; -import { readFilePart } from "../../runtime/upload"; +import { TestRunResult } from "../../domain/transform-test-run"; import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { parseId, parseIdList } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; -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 summaryLine(target: number, result: TestRunResult): string { - if (result.status === "passed") { - return `Transform ${target} test run passed.`; - } - return `Transform ${target} test run FAILED — output did not match expected. Re-run with --json to see the diff.`; -} +import { + parseColumnList, + parseInputPairs, + parseTargetType, + runSubgraph, + targetLabels, + targetTypeFlag, +} from "./subgraph"; export default defineMetabaseCommand({ meta: { name: "run", - description: "Test-run a transform (or sub-graph) against fixture CSVs", + description: "Test-run a transform or card (or sub-graph) against fixture CSVs", }, 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 diffs the target's output against the --expected CSV. Use `transform-test inputs` to discover which tables need fixtures. Omit --source to test the target transform alone. Exits non-zero when the output does not match.", + "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 diffs the target's output against the --expected CSV. The positional id is a transform id by default, or a card id when --target-type card is set (a card target uses no --source). Use `transform-test inputs` to discover which tables need fixtures. Omit --source to test the target alone. Exits non-zero when the output does not match.", // PROVISIONAL: the test-run/subgraph 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. @@ -65,6 +28,7 @@ export default defineMetabaseCommand({ ...outputFlags, ...profileFlag, ...connectionFlags, + ...targetTypeFlag, source: { type: "string", description: "Comma-separated boundary source transform ids (omit to test the target alone)", @@ -78,15 +42,17 @@ export default defineMetabaseCommand({ type: "string", description: "Comma-separated output column names to exclude from the diff", }, - id: { type: "positional", description: "Target transform id", required: true }, + id: { type: "positional", description: "Target transform or card id", required: true }, }, outputSchema: TestRunResult, examples: [ "mb transform-test run 173 --source 172 --input 229=orders.csv,223=people.csv --expected expected.csv", "mb transform-test run 42 --input 229=orders.csv --expected out.csv --ignore-columns snapshot_ts", + "mb transform-test run 88 --target-type card --input 229=orders.csv --expected out.csv", ], async run({ args, ctx, getClient }) { - const target = parseId(args.id); + const targetType = parseTargetType(args["target-type"]); + const target = parseId(args.id, targetLabels(targetType).positionalLabel); const sources = parseIdList(args.source, "--source"); const inputs = parseInputPairs(args.input); const expected = args.expected; @@ -95,31 +61,11 @@ export default defineMetabaseCommand({ } const ignoreColumns = parseColumnList(args["ignore-columns"]); - const form = new FormData(); - for (const { tableId, path } of inputs) { - const part = await readFilePart(path, `--input ${tableId}`); - form.append(`input-${tableId}`, part.blob, part.filename); - } - const expectedPart = await readFilePart(expected, "--expected"); - form.append("expected", expectedPart.blob, expectedPart.filename); - if (sources.length > 0) { - form.append("sources", JSON.stringify(sources)); - } - if (ignoreColumns.length > 0) { - form.append("options", JSON.stringify({ ignore_columns: ignoreColumns })); - } - const client = await getClient(); - const result = await client.requestParsed( - TestRunResult, - `/api/transform/${target}/test-run/subgraph`, - { method: "POST", body: form }, + await runSubgraph( + client, + { targetType, target, sources, inputs, expected, ignoreColumns }, + ctx, ); - - renderSummary(result, testRunResultView, () => summaryLine(target, result), ctx); - - if (result.status === "failed") { - throw new Error(`transform ${target} test run failed: output did not match expected`); - } }, }); diff --git a/src/commands/transform-test/run.test.ts b/src/commands/transform-test/subgraph.test.ts similarity index 69% rename from src/commands/transform-test/run.test.ts rename to src/commands/transform-test/subgraph.test.ts index 102146f..3bf4e09 100644 --- a/src/commands/transform-test/run.test.ts +++ b/src/commands/transform-test/subgraph.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { ConfigError } from "../../core/errors"; -import { parseColumnList, parseInputPairs } from "./run"; +import { parseColumnList, parseInputPairs, parseTargetType } from "./subgraph"; describe("parseInputPairs", () => { it("parses comma-separated = pairs", () => { @@ -33,9 +33,7 @@ describe("parseInputPairs", () => { 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)", - ); + expect(() => parseInputPairs("0=x.csv")).toThrow("invalid --input table id: 0 (must be ≥ 1)"); }); }); @@ -49,3 +47,17 @@ describe("parseColumnList", () => { 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)', + ); + }); +}); diff --git a/src/commands/transform-test/subgraph.ts b/src/commands/transform-test/subgraph.ts new file mode 100644 index 0000000..995f4bc --- /dev/null +++ b/src/commands/transform-test/subgraph.ts @@ -0,0 +1,159 @@ +import { z } from "zod"; + +import { ConfigError } from "../../core/errors"; +import type { Client } from "../../core/http/client"; +import { TestRunInput, TestRunResult, testRunResultView } from "../../domain/transform-test-run"; +import { renderSummary } from "../../output/render"; +import { readFilePart } from "../../runtime/upload"; +import type { CommonContext } from "../context"; +import { parseEnumFlag } from "../parse-enum"; +import { parseId } from "../parse-id"; + +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/transform-test/${targetType}/${target}/subgraph-inputs`; +} + +function subgraphPath(targetType: TargetType, target: number): string { + return `/api/transform-test/${targetType}/${target}/subgraph`; +} + +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[]; + expected: string; + ignoreColumns: string[]; +} + +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); + } + 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 })); + } + return form; +} + +function summaryLine(targetType: TargetType, target: number, result: TestRunResult): string { + const noun = targetLabels(targetType).summaryNoun; + if (result.status === "passed") { + return `${noun} ${target} test run passed.`; + } + return `${noun} ${target} test run FAILED — output did not match expected. Re-run with --json to see the diff.`; +} + +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, + }, + ); + + renderSummary( + result, + testRunResultView, + () => summaryLine(args.targetType, args.target, result), + ctx, + ); + + if (result.status === "failed") { + const noun = targetLabels(args.targetType).summaryNoun.toLowerCase(); + throw new Error(`${noun} ${args.target} test run failed: output did not match expected`); + } +} From 29c7fdc2a8f0aa2051ba5ba75a2bfbd7b297783f Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Fri, 26 Jun 2026 16:14:16 -0600 Subject: [PATCH 4/9] Update skill --- skill-data/transform-test/SKILL.md | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/skill-data/transform-test/SKILL.md b/skill-data/transform-test/SKILL.md index 56e5f51..796f3ea 100644 --- a/skill-data/transform-test/SKILL.md +++ b/skill-data/transform-test/SKILL.md @@ -1,12 +1,12 @@ --- name: transform-test -description: Test a transform (or a connected sub-graph of transforms) against fixture CSVs via `mb transform-test` — seeds scratch tables, runs the transform, and diffs its output against an expected CSV, never touching real tables. Covers the inputs→fixtures→run loop, the exact-header CSV contract, reading the pass/fail diff, chained sub-graph tests (`--source`), `--ignore-columns`, and the native-SQL limitations. Load when the user wants to validate a transform's logic before running it for real — "test this transform", "check the transform against sample data", "does my transform produce the right output", "write fixtures for a transform", or anything `mb 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 diffs its output against an expected CSV, never touching real tables. Covers the inputs→fixtures→run loop, the exact-header CSV contract, `--target-type transform|card`, reading the pass/fail diff, chained sub-graph tests (`--source`), `--ignore-columns`, and the native-SQL limitations. Load when the user wants to validate a transform's or card's logic before trusting it — "test this transform", "test this question/model against sample data", "does my transform produce the right output", "write fixtures for a transform", or anything `mb transform-test …`. allowed-tools: Read, Write, Edit, Bash --- # Testing transforms -`mb transform-test` runs a transform against **fixture CSVs you supply** instead of the real source tables, then diffs the output against an **expected CSV**. It seeds throwaway scratch tables, runs the transform's query into a scratch output, compares, and drops everything — **real tables are never read or written.** Use it to validate a transform's logic on small, known data before trusting it on production rows. +`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 diffs its output against an **expected CSV**. 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. 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`). Flag/profile/output conventions are in `core` (`mb skills get core`). @@ -16,7 +16,7 @@ Authoring and running transforms for real is the `transform` skill (`mb skills g 2. **Write** one CSV per input table + one expected-output CSV. 3. **Run** the test; read passed / FAILED + the diff (`transform-test run`). -The positional argument is always the **target** transform id (the one whose output is diffed). +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 @@ -87,6 +87,20 @@ mb transform-test run 173 --source 172 \ `--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: @@ -100,7 +114,7 @@ mb transform-test run 42 --input 229=orders.csv --expected out.csv \ ## Limitations & gotchas -- **Native SQL 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 transforms aren't affected. This is a known, accepted limitation. +- **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. This is a known, accepted limitation. - **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. From 192f5ef210cecd3aa22478bde3c163ee08b3fc30 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Tue, 30 Jun 2026 11:56:31 -0600 Subject: [PATCH 5/9] Add SQL assertions and YAML suite support to transform-test run --- skill-data/transform-test/SKILL.md | 4 +- src/commands/transform-test/assert.test.ts | 112 +++++++++++++++ src/commands/transform-test/assert.ts | 108 ++++++++++++++ src/commands/transform-test/inputs.ts | 2 +- src/commands/transform-test/run.ts | 139 ++++++++++++++++--- src/commands/transform-test/subgraph.test.ts | 131 ++++++++++++++++- src/commands/transform-test/subgraph.ts | 81 +++++++++-- src/commands/transform-test/suite.test.ts | 121 ++++++++++++++++ src/commands/transform-test/suite.ts | 78 +++++++++++ src/core/http/errors.ts | 5 +- src/domain/transform-test-run.ts | 29 ++++ 11 files changed, 770 insertions(+), 40 deletions(-) create mode 100644 src/commands/transform-test/assert.test.ts create mode 100644 src/commands/transform-test/assert.ts create mode 100644 src/commands/transform-test/suite.test.ts create mode 100644 src/commands/transform-test/suite.ts diff --git a/skill-data/transform-test/SKILL.md b/skill-data/transform-test/SKILL.md index 796f3ea..581cdc9 100644 --- a/skill-data/transform-test/SKILL.md +++ b/skill-data/transform-test/SKILL.md @@ -71,7 +71,7 @@ mb transform-test run \ Reading the result: - The plain summary says `Transform test run passed.` or `Transform test run FAILED — output did not match expected. Re-run with --json to see the diff.` -- `--json` returns `{status, diff, test_run_id}` where `status` is `passed` or `failed` (those are the only two values). On `failed`, **`diff` is where the truth is** — it reports missing rows (expected but not produced), extra rows (produced but not expected), and cell-level mismatches. Always re-run with `--json` (or start with it) to see why a test failed. +- `--json` returns `{status, diff, test_run_id}` where `status` is `passed` or `failed` (those are the only two values). On `failed`, **the `diff` carries the detail** — it reports missing rows (expected but not produced), extra rows (produced but not expected), and cell-level mismatches. Always re-run with `--json` (or start with it) to see why a test failed. - 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. ## Chained / sub-graph tests (`--source`) @@ -114,7 +114,7 @@ mb transform-test run 42 --input 229=orders.csv --expected out.csv \ ## 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. This is a known, accepted limitation. +- **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. 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/inputs.ts b/src/commands/transform-test/inputs.ts index f618e51..a7c3700 100644 --- a/src/commands/transform-test/inputs.ts +++ b/src/commands/transform-test/inputs.ts @@ -16,7 +16,7 @@ export default defineMetabaseCommand({ }, 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/subgraph endpoints are unreleased. minVersion mirrors the + // Provisional: the test-run/subgraph 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 }, diff --git a/src/commands/transform-test/run.ts b/src/commands/transform-test/run.ts index bc8a86c..f548408 100644 --- a/src/commands/transform-test/run.ts +++ b/src/commands/transform-test/run.ts @@ -1,26 +1,88 @@ -import { ConfigError } from "../../core/errors"; +import { readFile } from "node:fs/promises"; + +import { ConfigError, errorMessage } from "../../core/errors"; import { TestRunResult } from "../../domain/transform-test-run"; 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"; + +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", + 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 diffs the target's output against the --expected CSV. The positional id is a transform id by default, or a card id when --target-type card is set (a card target uses no --source). Use `transform-test inputs` to discover which tables need fixtures. Omit --source to test the target alone. Exits non-zero when the output does not match.", - // PROVISIONAL: the test-run/subgraph endpoints are unreleased. minVersion mirrors the + "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/subgraph 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 }, @@ -37,35 +99,68 @@ export default defineMetabaseCommand({ type: "string", description: "Comma-separated = fixtures, one per required input table", }, - expected: { type: "string", description: "Path to the expected-output CSV" }, + 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", }, - id: { type: "positional", description: "Target transform or card id", required: true }, + assert: { + 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.", + }, + 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,223=people.csv --expected expected.csv", - "mb transform-test run 42 --input 229=orders.csv --expected out.csv --ignore-columns snapshot_ts", - "mb transform-test run 88 --target-type card --input 229=orders.csv --expected out.csv", + "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, ctx, getClient }) { - const targetType = parseTargetType(args["target-type"]); - const target = parseId(args.id, targetLabels(targetType).positionalLabel); - const sources = parseIdList(args.source, "--source"); - const inputs = parseInputPairs(args.input); - const expected = args.expected; - if (expected === undefined || expected.trim() === "") { - throw new ConfigError("Missing required --expected ."); + 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; + + const flags: FlagArgs = { + sources: parseIdList(args.source, "--source"), + inputs: parseInputPairs(args.input), + ignoreColumns: parseColumnList(args["ignore-columns"]), + assertions: await resolveAssertions(parseAssertFlags(args.assert)), + }; + 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 ignoreColumns = parseColumnList(args["ignore-columns"]); + + const runArgs = compose(suite, flags); const client = await getClient(); - await runSubgraph( - client, - { targetType, target, sources, inputs, expected, ignoreColumns }, - ctx, - ); + await runSubgraph(client, runArgs, ctx); }, }); diff --git a/src/commands/transform-test/subgraph.test.ts b/src/commands/transform-test/subgraph.test.ts index 3bf4e09..b8201a7 100644 --- a/src/commands/transform-test/subgraph.test.ts +++ b/src/commands/transform-test/subgraph.test.ts @@ -1,8 +1,34 @@ -import { describe, expect, it } from "vitest"; +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 type { AssertionResult, TestRunResult } from "../../domain/transform-test-run"; + +import { + assertionsSummaryLine, + buildSubgraphForm, + parseColumnList, + parseInputPairs, + parseTargetType, + shouldFail, +} from "./subgraph"; -import { parseColumnList, parseInputPairs, parseTargetType } 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, test_run_id: null, ...over }; +} describe("parseInputPairs", () => { it("parses comma-separated = pairs", () => { @@ -61,3 +87,104 @@ describe("parseTargetType", () => { ); }); }); + +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(); + }); +}); diff --git a/src/commands/transform-test/subgraph.ts b/src/commands/transform-test/subgraph.ts index 995f4bc..c479a09 100644 --- a/src/commands/transform-test/subgraph.ts +++ b/src/commands/transform-test/subgraph.ts @@ -2,13 +2,22 @@ import { z } from "zod"; import { ConfigError } from "../../core/errors"; import type { Client } from "../../core/http/client"; -import { TestRunInput, TestRunResult, testRunResultView } from "../../domain/transform-test-run"; -import { renderSummary } from "../../output/render"; +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; @@ -101,33 +110,78 @@ export interface SubgraphRunArgs { target: number; sources: number[]; inputs: InputPair[]; - expected: string; + // Optional per field; the run requires at least one of expected or assertions. + expected?: string; ignoreColumns: string[]; + assertions: AssertionDef[]; } -async function buildSubgraphForm(args: SubgraphRunArgs): Promise { +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); } - const expectedPart = await readFilePart(args.expected, "--expected"); - form.append("expected", expectedPart.blob, expectedPart.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") { - return `${noun} ${target} test run 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.`); } - return `${noun} ${target} test run FAILED — output did not match expected. Re-run with --json to see the diff.`; + const assertions = assertionsSummaryLine(result); + if (assertions !== null) { + lines.push(assertions); + } + return lines.join("\n"); } export async function runSubgraph( @@ -152,8 +206,15 @@ export async function runSubgraph( ctx, ); - if (result.status === "failed") { + // Per-assertion table for the human view only; --json/--fields/--full already carried the full result. + const assertions = assertionList(result); + if (assertions.length > 0 && ctx.format !== "json" && ctx.fields === undefined && !ctx.full) { + writeText(""); + renderList(wrapList(assertions), assertionResultView, ctx); + } + + if (shouldFail(result)) { const noun = targetLabels(args.targetType).summaryNoun.toLowerCase(); - throw new Error(`${noun} ${args.target} test run failed: output did not match expected`); + 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 e197bcc..4702d81 100644 --- a/src/core/http/errors.ts +++ b/src/core/http/errors.ts @@ -38,9 +38,8 @@ const STATUS_CLASSIFICATIONS: Record = { const ErrorEnvelope = z .object({ message: z.string().optional(), - // `error` is usually a string, but some endpoints send a structured object - // (e.g. {type, message}); accept any shape so a non-string never fails the - // whole-envelope parse and drops us to a bare status-code message. + // `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(), diff --git a/src/domain/transform-test-run.ts b/src/domain/transform-test-run.ts index 3e4b5eb..4217c40 100644 --- a/src/domain/transform-test-run.ts +++ b/src/domain/transform-test-run.ts @@ -34,10 +34,22 @@ export const testRunInputView: ResourceView = { ], }; +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(), test_run_id: z.number().int().positive().nullable(), }) .loose(); @@ -47,6 +59,7 @@ export const TestRunResultCompact = TestRunResult.pick({ status: true, test_run_id: true, diff: true, + assertions: true, }).strip(); export type TestRunResultCompact = z.infer; @@ -57,3 +70,19 @@ export const testRunResultView: ResourceView = { { key: "test_run_id", label: "Run ID" }, ], }; + +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" }, + ], +}; From 980ccd3b057b24204451ebe5549f13ddc47c3f57 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Tue, 30 Jun 2026 15:34:28 -0600 Subject: [PATCH 6/9] Docs update --- skill-data/transform-test/SKILL.md | 89 ++++++++++++++++++-- src/commands/runtime.ts | 4 + src/commands/transform-test/run.ts | 20 +++-- src/commands/transform-test/subgraph.test.ts | 78 ++++++++++++++++- src/commands/transform-test/subgraph.ts | 33 +++++--- src/runtime/citty.test.ts | 35 +++++++- src/runtime/citty.ts | 39 +++++++++ 7 files changed, 266 insertions(+), 32 deletions(-) diff --git a/skill-data/transform-test/SKILL.md b/skill-data/transform-test/SKILL.md index 581cdc9..8dccaff 100644 --- a/skill-data/transform-test/SKILL.md +++ b/skill-data/transform-test/SKILL.md @@ -1,20 +1,22 @@ --- 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 diffs its output against an expected CSV, never touching real tables. Covers the inputs→fixtures→run loop, the exact-header CSV contract, `--target-type transform|card`, reading the pass/fail diff, chained sub-graph tests (`--source`), `--ignore-columns`, and the native-SQL limitations. Load when the user wants to validate a transform's or card's logic before trusting it — "test this transform", "test this question/model against sample data", "does my transform produce the right output", "write fixtures for a transform", or anything `mb 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 diffs its output against an **expected CSV**. 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. +`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`). 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 + one expected-output CSV. -3. **Run** the test; read passed / FAILED + the diff (`transform-test run`). +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. @@ -65,15 +67,83 @@ mb transform-test run \ ``` - `--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 (required). +- `--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 summary says `Transform test run passed.` or `Transform test run FAILED — output did not match expected. Re-run with --json to see the diff.` -- `--json` returns `{status, diff, test_run_id}` where `status` is `passed` or `failed` (those are the only two values). On `failed`, **the `diff` carries the detail** — it reports missing rows (expected but not produced), extra rows (produced but not expected), and cell-level mismatches. Always re-run with `--json` (or start with it) to see why a test failed. +- 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. @@ -114,7 +184,7 @@ mb transform-test run 42 --input 229=orders.csv --expected out.csv \ ## 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. +- **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. @@ -122,5 +192,6 @@ mb transform-test run 42 --input 229=orders.csv --expected out.csv \ ## 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). Read the `--json` `diff` to fix either the transform or the expected CSV. +- 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/runtime.ts b/src/commands/runtime.ts index 2019e80..d02c5be 100644 --- a/src/commands/runtime.ts +++ b/src/commands/runtime.ts @@ -35,6 +35,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; @@ -106,6 +109,7 @@ export function defineMetabaseCommand( try { await def.run({ args, + rawArgs, ctx, getClient, getResolvedConfig, diff --git a/src/commands/transform-test/run.ts b/src/commands/transform-test/run.ts index f548408..3337622 100644 --- a/src/commands/transform-test/run.ts +++ b/src/commands/transform-test/run.ts @@ -2,6 +2,7 @@ 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"; @@ -18,6 +19,12 @@ import { } 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 { @@ -107,11 +114,7 @@ export default defineMetabaseCommand({ type: "string", description: "Comma-separated output column names to exclude from the diff", }, - assert: { - 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.", - }, + assert: ASSERT_FLAG, suite: { type: "string", description: @@ -133,7 +136,7 @@ export default defineMetabaseCommand({ "mb transform-test run 173 --assert 'checks/*.sql' --input 229=orders.csv", "mb transform-test run --suite suites/orders.yaml", ], - async run({ args, ctx, getClient }) { + 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() !== ""; @@ -142,11 +145,14 @@ export default defineMetabaseCommand({ ? 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(args.assert)), + assertions: await resolveAssertions(parseAssertFlags(assertValues)), }; if (targetType !== undefined && idGiven) { flags.targetType = targetType; diff --git a/src/commands/transform-test/subgraph.test.ts b/src/commands/transform-test/subgraph.test.ts index b8201a7..4979d2e 100644 --- a/src/commands/transform-test/subgraph.test.ts +++ b/src/commands/transform-test/subgraph.test.ts @@ -2,10 +2,11 @@ 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 { 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, @@ -13,6 +14,7 @@ import { parseColumnList, parseInputPairs, parseTargetType, + renderRunResult, shouldFail, } from "./subgraph"; @@ -30,6 +32,20 @@ function result(over: Partial): TestRunResult { return { status: "passed", diff: null, test_run_id: 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([ @@ -188,3 +204,63 @@ describe("assertionsSummaryLine", () => { 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 index c479a09..3939743 100644 --- a/src/commands/transform-test/subgraph.ts +++ b/src/commands/transform-test/subgraph.ts @@ -184,6 +184,25 @@ function summaryLine(targetType: TargetType, target: number, result: TestRunResu 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, @@ -199,19 +218,7 @@ export async function runSubgraph( }, ); - renderSummary( - result, - testRunResultView, - () => summaryLine(args.targetType, args.target, result), - ctx, - ); - - // Per-assertion table for the human view only; --json/--fields/--full already carried the full result. - const assertions = assertionList(result); - if (assertions.length > 0 && ctx.format !== "json" && ctx.fields === undefined && !ctx.full) { - writeText(""); - renderList(wrapList(assertions), assertionResultView, ctx); - } + renderRunResult(args.targetType, args.target, result, ctx); if (shouldFail(result)) { const noun = targetLabels(args.targetType).summaryNoun.toLowerCase(); 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; From 3f61f6f4273b358ab41acaf39d7f5f04dc1fa867 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Wed, 1 Jul 2026 15:47:47 -0600 Subject: [PATCH 7/9] Endpoints moved to EE --- src/commands/transform-test/subgraph.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/transform-test/subgraph.ts b/src/commands/transform-test/subgraph.ts index 3939743..4ae6795 100644 --- a/src/commands/transform-test/subgraph.ts +++ b/src/commands/transform-test/subgraph.ts @@ -87,11 +87,11 @@ export function parseColumnList(value: string | undefined): string[] { } function subgraphInputsPath(targetType: TargetType, target: number): string { - return `/api/transform-test/${targetType}/${target}/subgraph-inputs`; + return `/api/ee/transform-test/${targetType}/${target}/subgraph-inputs`; } function subgraphPath(targetType: TargetType, target: number): string { - return `/api/transform-test/${targetType}/${target}/subgraph`; + return `/api/ee/transform-test/${targetType}/${target}/subgraph`; } export async function fetchSubgraphInputs( From 70350d1a6ce490ecde7bdbe71fb4c5491687c9a1 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Tue, 7 Jul 2026 16:40:28 -0600 Subject: [PATCH 8/9] Catch up with metabase/metabase --- src/commands/transform-test/inputs.ts | 2 +- src/commands/transform-test/run.ts | 2 +- src/commands/transform-test/subgraph.test.ts | 2 +- src/commands/transform-test/subgraph.ts | 4 +-- src/domain/transform-test-run.test.ts | 31 ++++++++++++++++++++ src/domain/transform-test-run.ts | 10 ++----- 6 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 src/domain/transform-test-run.test.ts diff --git a/src/commands/transform-test/inputs.ts b/src/commands/transform-test/inputs.ts index a7c3700..1c72b59 100644 --- a/src/commands/transform-test/inputs.ts +++ b/src/commands/transform-test/inputs.ts @@ -16,7 +16,7 @@ export default defineMetabaseCommand({ }, 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/subgraph endpoints are unreleased. minVersion mirrors the + // 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 }, diff --git a/src/commands/transform-test/run.ts b/src/commands/transform-test/run.ts index 3337622..aea047e 100644 --- a/src/commands/transform-test/run.ts +++ b/src/commands/transform-test/run.ts @@ -89,7 +89,7 @@ export default defineMetabaseCommand({ }, 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/subgraph endpoints are unreleased. minVersion mirrors the + // 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 }, diff --git a/src/commands/transform-test/subgraph.test.ts b/src/commands/transform-test/subgraph.test.ts index 4979d2e..798a58c 100644 --- a/src/commands/transform-test/subgraph.test.ts +++ b/src/commands/transform-test/subgraph.test.ts @@ -29,7 +29,7 @@ function assertion(over: Partial & { name: string }): Assertion } function result(over: Partial): TestRunResult { - return { status: "passed", diff: null, test_run_id: null, ...over }; + return { status: "passed", diff: null, ...over }; } function renderCtx(over: Partial): CommonContext { diff --git a/src/commands/transform-test/subgraph.ts b/src/commands/transform-test/subgraph.ts index 4ae6795..44d773c 100644 --- a/src/commands/transform-test/subgraph.ts +++ b/src/commands/transform-test/subgraph.ts @@ -87,11 +87,11 @@ export function parseColumnList(value: string | undefined): string[] { } function subgraphInputsPath(targetType: TargetType, target: number): string { - return `/api/ee/transform-test/${targetType}/${target}/subgraph-inputs`; + return `/api/ee/transform-test/${targetType}/${target}/inputs`; } function subgraphPath(targetType: TargetType, target: number): string { - return `/api/ee/transform-test/${targetType}/${target}/subgraph`; + return `/api/ee/transform-test/${targetType}/${target}/run`; } export async function fetchSubgraphInputs( 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 index 4217c40..4d520ea 100644 --- a/src/domain/transform-test-run.ts +++ b/src/domain/transform-test-run.ts @@ -5,7 +5,8 @@ import type { ResourceView } from "./view"; export const TestRunInput = z .object({ table_id: z.number().int().positive(), - schema: z.string(), + // 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()), }) @@ -50,14 +51,12 @@ export const TestRunResult = z status: z.enum(["passed", "failed"]), diff: z.unknown(), assertions: z.array(AssertionResult).nullable().optional(), - test_run_id: z.number().int().positive().nullable(), }) .loose(); export type TestRunResult = z.infer; export const TestRunResultCompact = TestRunResult.pick({ status: true, - test_run_id: true, diff: true, assertions: true, }).strip(); @@ -65,10 +64,7 @@ export type TestRunResultCompact = z.infer; export const testRunResultView: ResourceView = { compactPick: TestRunResultCompact, - tableColumns: [ - { key: "status", label: "Status" }, - { key: "test_run_id", label: "Run ID" }, - ], + tableColumns: [{ key: "status", label: "Status" }], }; export const AssertionResultCompact = AssertionResult.pick({ From 218731b0edd7ec171518a9cbb2d3669d3b0b8eaa Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Wed, 8 Jul 2026 14:02:54 -0600 Subject: [PATCH 9/9] Fix non-conflict conflicts after merging in main --- src/runtime/command-help.test.ts | 2 ++ tests/e2e/measure.e2e.test.ts | 2 +- tests/e2e/segment.e2e.test.ts | 2 +- tests/e2e/skills.e2e.test.ts | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) 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/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..8cb3bf6 100644 --- a/tests/e2e/skills.e2e.test.ts +++ b/tests/e2e/skills.e2e.test.ts @@ -17,6 +17,7 @@ const BUNDLED_VISIBLE_NAMES = [ "metadata", "native-sql", "transform", + "transform-test", "visualization", ] as const; @@ -33,7 +34,7 @@ describe("skills e2e", () => { return dir; } - it("list returns the ten bundled non-hidden skills, sorted by name", async () => { + it("list returns the eleven bundled non-hidden skills, sorted by name", async () => { const result = await runCli({ args: ["skills", "list", "--json"], configHome: await makeIsolatedConfigHome(),