diff --git a/skill-data/transform-test/SKILL.md b/skill-data/transform-test/SKILL.md new file mode 100644 index 0000000..8dccaff --- /dev/null +++ b/skill-data/transform-test/SKILL.md @@ -0,0 +1,197 @@ +--- +name: transform-test +description: Test a transform — or a card (saved question / model) — against fixture CSVs via `mb transform-test` — seeds scratch tables, runs the target, and checks its output against an expected CSV and/or SQL assertions, never touching real tables. Covers the inputs→fixtures→run loop, the exact-header CSV contract, `--target-type transform|card`, the pass/fail diff, SQL assertions (`--assert` .sql files/globs), YAML suites (`--suite`), chained sub-graph tests (`--source`), and `--ignore-columns`. Load when validating a transform's or card's logic — "test this transform", "test this question against sample data", "assert no negative revenue", "does my transform produce the right output", or anything `mb transform-test …`. +allowed-tools: Read, Write, Edit, Bash +--- + +# Testing transforms + +`mb transform-test` runs a **target** — a transform, or a card (saved question / model) — against **fixture CSVs you supply** instead of the real source tables, then checks its output against an **expected CSV** and/or **SQL assertions**. It seeds throwaway scratch tables, runs the target's query over them, compares, and drops everything — **real tables are never read or written.** Use it to validate a transform's or card's logic on small, known data before trusting it on production rows. + +You check the output two ways, and can combine them: an **expected CSV** (`--expected`, exact-output multiset diff) and/or **assertions** (`--assert`, SQL queries that must return zero rows). At least one of `--expected` or `--assert`/`--suite` is required — `--expected` is **not** mandatory on its own anymore. + +Authoring and running transforms for real is the `transform` skill (`mb skills get transform`); deciding what to build is `data-transformation` (`mb skills get data-transformation`). Flag/profile/output conventions are in `core` (`mb skills get core`). + +## The loop: inputs → fixtures → run + +1. **Discover** which tables need fixtures and their exact columns (`transform-test inputs`). +2. **Write** one CSV per input table, plus an expected-output CSV and/or `.sql` assertion files. +3. **Run** the test; read passed / FAILED + the diff and/or per-assertion results (`transform-test run`). + +The positional argument is the **target** id whose output is diffed. By default that's a transform; pass `--target-type card` to target a saved question or model instead (see "Card targets" below). Everything else — inputs, fixtures, `--source`, the diff — works the same either way. + +## 1. Discover required inputs + +```bash +mb transform-test inputs --profile --json +``` + +Returns one row per input table you must supply a fixture for. Each carries: + +- `table_id` — use it as the key in `--input =`. +- `name` / `schema` — the real table (for your reference; the fixture replaces it). +- `columns` — the **exact** column-name list your CSV header must contain. + +Omit `--source` to test the target transform on its own (its direct input tables). With `--source` it lists the **leaf** tables of the sub-graph (see "Chained tests" below). + +## 2. Write the fixture CSVs — the header contract + +Each input CSV's header must contain **exactly the column names `inputs` reported** — all of them, **case-sensitive**, no extras. Column **order doesn't matter** (matched by name); a missing or unexpected column fails the run with a header-mismatch error. Values are parsed against each column's real warehouse type (integers, dates, etc.), so write values that parse — `2024-01-01` for a date column, an integer for an int column. Leave a cell empty for `NULL`. + +```bash +mkdir -p ./.scratch +cat > ./.scratch/bird_count.csv <<'CSV' +id,date,count +1,2024-01-01,2 +2,2024-01-02,5 +3,2024-01-03,3 +CSV + +# The expected output: header = the columns your transform SELECTs, rows = what it should produce. +cat > ./.scratch/expected.csv <<'CSV' +id,date,count +1,2024-01-01,2 +3,2024-01-03,3 +CSV +``` + +The expected CSV's columns are matched against the transform's **actual output** columns; the comparison is a multiset (row order is ignored, duplicates count). + +## 3. Run the test + +```bash +mb transform-test run \ + --input =./.scratch/bird_count.csv \ + --expected ./.scratch/expected.csv \ + --profile --json +``` + +- `--input` is comma-separated `=` pairs, **one per table `inputs` listed** (e.g. `--input 229=orders.csv,223=people.csv`). The table id is the `table_id` from step 1. +- `--expected` is the path to the expected-output CSV. **Optional** — provide it, `--assert`/`--suite`, or both, but at least one. +- Exits **0 on pass, non-zero on fail** (good for scripting / CI gates). + +Reading the result: + +- The plain (text) summary says `Transform test run passed.` or `Transform test run FAILED …`, followed — whenever assertions ran — by a one-line breakdown (`N assertions — A passed, B FAILED, C warn …`) and a per-assertion table (name / status / failing rows). +- `--json` returns `{status, diff, assertions, test_run_id}`. `status` is `passed` or `failed` (the only two values). `diff` is the expected-CSV comparison (`null` when you ran assertions-only). `assertions` is `null` when none ran, else an array of `{name, status: passed|failed|warn, failing_row_count, sample_rows, columns}`. On `failed`, the `diff` reports missing/extra rows and cell mismatches, and each failing assertion carries its `failing_row_count` + a capped `sample_rows`. Always re-run with `--json` (or start with it) to see why a test failed. +- **Output format follows the usual `core` rules:** `auto` (the default) prints the human summary + table in a terminal but **emits JSON when stdout is piped/redirected** (CI, `| cat`, `$(…)`). Pass `--format text` to force the human table when piping, or `--json` to force JSON in a terminal. +- A run that couldn't complete — bad CSV header, an unsupported transform, etc. — isn't a `failed` status; it surfaces as a thrown error envelope on a non-zero exit, distinct from a clean `failed` diff. + +## Assertions (`--assert`) + +An **assertion** is a SQL query that **passes iff it returns zero rows.** Write each one to a +`.sql` file; `--assert` points at the file (or a glob of them). Inside the SQL, reference the +synthetic relation **`test_output`** (the target's output) and/or the input table names — the +harness redirects every real table to scratch and binds `test_output` to the target. + +```bash +cat > ./.scratch/no_negative_revenue.sql <<'SQL' +SELECT * FROM test_output WHERE revenue < 0 +SQL + +mb transform-test run 173 --source 172 \ + --input 229=orders.csv \ + --assert ./.scratch/no_negative_revenue.sql \ + --profile +``` + +- **`--assert` accepts only `.sql` file paths or globs — inline SQL is NOT supported.** A + non-`.sql` value is rejected with a clear error; write the query to a file instead. +- The **assertion name** is the file basename without `.sql` (`no_negative_revenue` above). +- **Repeatable, and comma-separated.** All of these accumulate: + `--assert a.sql --assert b.sql`, `--assert a.sql,b.sql`, `--assert 'checks/*.sql'` (a glob + expands to one assertion per matching `.sql` file; a glob matching nothing is an error). +- **Severity:** `--assert` files default to **error** severity (a failure fails the run, non-zero + exit). To mark an assertion as a **warn** (reported, but does not fail the run or flip the exit + code), declare it in a `--suite` with `severity: warn`. +- `--assert` and `--expected` compose: provide either or both. With assertions only, the response + `diff` is `null` and the result is driven entirely by the assertions. + +## Test suites (`--suite`) + +A **suite** is a YAML file that declares a whole run — target, sources, inputs, expected, +ignore-columns, and assertions (with optional per-assertion `severity`). It is parsed **entirely +client-side**; the server sees the same request as the equivalent flags. + +```yaml +# suites/orders.yaml +target: + type: transform # or: card + id: 173 +sources: [172] +inputs: + - table: 229 + file: ./.scratch/orders.csv +expected: ./.scratch/expected.csv # optional +ignore_columns: [snapshot_ts] +assertions: + - name: no_negative_revenue + sql: SELECT * FROM test_output WHERE revenue < 0 # inline sql IS allowed in a suite + severity: error + - name: every_state_present + file: ./.scratch/every_state_present.sql # …or point at a .sql file + severity: warn +``` + +```bash +mb transform-test run --suite suites/orders.yaml --profile +``` + +- A suite assertion sets **exactly one** of `sql:` (inline) or `file:` (path to a `.sql` file). + (Inline SQL is fine _here_ — the suite is itself a file you author; only the `--assert` flag is + file-only.) +- **Ad-hoc flags compose with a suite:** `--source`/`--input`/`--expected`/`--ignore-columns` + **override** the suite's values; `--assert` assertions **append** to the suite's. The positional + target id (and `--target-type`) may be omitted when the suite declares `target`. + +## Chained / sub-graph tests (`--source`) + +To test a transform that depends on **other transforms'** outputs, pick boundary `--source` transform ids. Every node on a path from a source to the target runs in dependency order, fed by fixtures only at the **leaves** (raw tables + any sibling outputs not produced inside the selection). `transform-test inputs --source ` tells you exactly which leaves to supply. + +```bash +mb transform-test inputs 173 --source 172 --profile --json # lists the leaf tables +mb transform-test run 173 --source 172 \ + --input 229=orders.csv,223=people.csv \ + --expected expected.csv --profile --json +``` + +`--source` is comma-separated ids. Omitting it == testing the target alone. All transforms in the sub-graph must share one database (a cross-database selection is rejected). + +## Card targets (`--target-type card`) + +The target can be a **card** — a saved question or model — instead of a transform. The card's query is what gets diffed: its producing transforms run in scratch (seeded from your fixtures), the card's query runs over those scratch outputs, and the result is compared to the expected CSV. The inputs → fixtures → run loop is identical; only the positional id and `--target-type` change. + +```bash +mb transform-test inputs --target-type card --source --profile --json +mb transform-test run --target-type card --source \ + --input 229=orders.csv,223=people.csv --expected expected.csv --profile --json +``` + +- The positional is a **card id** (question or model). `--target-type` defaults to `transform`, so transform targets need no flag. +- **Precondition:** the transform output(s) the card reads must be **materialized and synced** — run the producing transform with `mb transform run --sync` first. A card built on an un-materialized output can't be linked to its producer, and you'll get a `sources-not-ancestors` error. +- Native and MBQL cards both work. Native cards carry the same bare-table-qualifier limitation as native transforms (below). + +## Ignoring non-deterministic columns + +For output columns you can't pin in an expected CSV — `now()` timestamps, snapshot dates, random ids — exclude them from the diff: + +```bash +mb transform-test run 42 --input 229=orders.csv --expected out.csv \ + --ignore-columns snapshot_ts,run_id --profile --json +``` + +`--ignore-columns` is comma-separated **output** column names. Naming a column that isn't in the output is an error, so check the transform's SELECT first. + +## Limitations & gotchas + +- **Native SQL (in a transform or a native card) that qualifies columns by the bare table name can't be test-run.** `SELECT orders.id FROM orders` fails with a typed 422 (the fixture-redirect rewrites the table reference but can't safely follow a `orders.`-qualified column). **Alias the table** — `SELECT o.id FROM orders o` — or use unqualified column names. It always fails _safely_ (you get an error, never a wrong-but-green result), and MBQL targets aren't affected. +- **Header must match the real table exactly.** Don't guess columns — copy them from `transform-test inputs`. All columns are required; the test isn't a projection. +- **Provisional / unreleased.** These commands target a dev build of the transforms feature; if `mb transform-test` reports the endpoint is unavailable, the connected instance predates it. +- **Scratch only.** Fixtures seed prefix-guarded scratch tables that are dropped in a `finally` (success, failure, or error). A test run creates no transform-run record and never writes the transform's real output table. + +## Don't + +- Don't hand-guess the input `table_id`s or column headers — always run `transform-test inputs` first; the ids and exact columns come from there. +- Don't treat a `failed` status as a tool error — it's a real result (output ≠ expected, or an error-severity assertion returned rows). Read the `--json` `diff` and `assertions` to fix the transform, the expected CSV, or the assertion. +- Don't supply fixtures for tables `inputs` didn't list, or omit ones it did — the `--input` set must match the required set exactly. +- Don't pass inline SQL to `--assert` — it only accepts `.sql` file paths or globs. Write the query to a file (or put it in a `--suite` under `sql:`). diff --git a/src/commands/parse-id.ts b/src/commands/parse-id.ts index 1bcf31b..1af912c 100644 --- a/src/commands/parse-id.ts +++ b/src/commands/parse-id.ts @@ -3,3 +3,14 @@ import { parseInteger } from "./parse-integer"; export function parseId(value: string, name = "id"): number { return parseInteger(value, { name, min: 1 }); } + +export function parseIdList(value: string | undefined, name = "id"): number[] { + if (value === undefined || value.trim() === "") { + return []; + } + return value + .split(",") + .map((part) => part.trim()) + .filter((part) => part !== "") + .map((part) => parseId(part, name)); +} diff --git a/src/commands/runtime.ts b/src/commands/runtime.ts index 25bcb2c..58cc439 100644 --- a/src/commands/runtime.ts +++ b/src/commands/runtime.ts @@ -36,6 +36,9 @@ export { SKIP_PREFLIGHT_ENV }; export interface MetabaseCommandContext { args: ParsedArgs; + // The unparsed argv for this command. Needed to recover repeated flags, which citty's parser + // collapses to the last value (see runtime/citty.ts `collectRepeatedFlag`). + rawArgs: readonly string[]; ctx: CommonContext; getClient: () => Promise; getResolvedConfig: () => Promise; @@ -109,6 +112,7 @@ export function defineMetabaseCommand( try { await def.run({ args, + rawArgs, ctx, getClient, getResolvedConfig, diff --git a/src/commands/transform-test/assert.test.ts b/src/commands/transform-test/assert.test.ts new file mode 100644 index 0000000..73862c4 --- /dev/null +++ b/src/commands/transform-test/assert.test.ts @@ -0,0 +1,112 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { ConfigError } from "../../core/errors"; + +import { classifyAssertToken, parseAssertFlags, resolveAssertions } from "./assert"; + +describe("classifyAssertToken", () => { + it("classifies a .sql path as a file", () => { + expect(classifyAssertToken("checks/no_negatives.sql")).toEqual({ + kind: "file", + path: "checks/no_negatives.sql", + }); + }); + + it("classifies a *.sql pattern as a glob", () => { + expect(classifyAssertToken("checks/*.sql")).toEqual({ kind: "glob", pattern: "checks/*.sql" }); + }); + + it("throws ConfigError for a non-.sql value (inline SQL is not supported)", () => { + const value = "SELECT * FROM test_output WHERE x < 0"; + expect(() => classifyAssertToken(value)).toThrow(ConfigError); + expect(() => classifyAssertToken(value)).toThrow( + `--assert expects a .sql file path or glob (inline SQL is not supported); got: "${value}"`, + ); + }); + + it("throws ConfigError for a bare name without a .sql extension", () => { + expect(() => classifyAssertToken("no_negatives")).toThrow(ConfigError); + }); +}); + +describe("parseAssertFlags", () => { + it("returns an empty list for undefined or blank values", () => { + expect(parseAssertFlags(undefined)).toEqual([]); + expect(parseAssertFlags([])).toEqual([]); + expect(parseAssertFlags([" "])).toEqual([]); + }); + + it("accepts a single string and splits comma-separated tokens", () => { + expect(parseAssertFlags("a.sql,b.sql")).toEqual([ + { kind: "file", path: "a.sql" }, + { kind: "file", path: "b.sql" }, + ]); + }); + + it("accepts a repeated array of values", () => { + expect(parseAssertFlags(["a.sql", "b/*.sql"])).toEqual([ + { kind: "file", path: "a.sql" }, + { kind: "glob", pattern: "b/*.sql" }, + ]); + }); + + it("throws ConfigError when a value is not a .sql file or glob", () => { + expect(() => parseAssertFlags(["SELECT a, b FROM test_output"])).toThrow(ConfigError); + }); +}); + +describe("resolveAssertions", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "mb-assert-")); + writeFileSync(join(dir, "no_negatives.sql"), "SELECT * FROM test_output WHERE revenue < 0\n"); + writeFileSync(join(dir, "has_rows.sql"), "SELECT * FROM test_output WHERE 1=0"); + writeFileSync(join(dir, "notes.txt"), "ignore me"); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("reads a file token, names it by basename without extension, defaults to error severity", async () => { + const out = await resolveAssertions([{ kind: "file", path: join(dir, "no_negatives.sql") }]); + expect(out).toEqual([ + { + name: "no_negatives", + sql: "SELECT * FROM test_output WHERE revenue < 0", + severity: "error", + }, + ]); + }); + + it("expands a glob to one assertion per matching .sql file, sorted by name", async () => { + const out = await resolveAssertions([{ kind: "glob", pattern: join(dir, "*.sql") }]); + expect(out.map((a) => a.name)).toEqual(["has_rows", "no_negatives"]); + expect(out.every((a) => a.severity === "error")).toBe(true); + }); + + it("resolves multiple file tokens preserving order, each named by basename", async () => { + const out = await resolveAssertions([ + { kind: "file", path: join(dir, "has_rows.sql") }, + { kind: "file", path: join(dir, "no_negatives.sql") }, + ]); + expect(out.map((a) => a.name)).toEqual(["has_rows", "no_negatives"]); + }); + + it("throws ConfigError for a missing file", async () => { + await expect( + resolveAssertions([{ kind: "file", path: join(dir, "nope.sql") }]), + ).rejects.toThrow(ConfigError); + }); + + it("throws ConfigError for a glob that matches nothing", async () => { + await expect( + resolveAssertions([{ kind: "glob", pattern: join(dir, "zzz-*.sql") }]), + ).rejects.toThrow(ConfigError); + }); +}); diff --git a/src/commands/transform-test/assert.ts b/src/commands/transform-test/assert.ts new file mode 100644 index 0000000..c7b2320 --- /dev/null +++ b/src/commands/transform-test/assert.ts @@ -0,0 +1,108 @@ +import { readdir, readFile } from "node:fs/promises"; +import { basename, dirname, extname, join } from "node:path"; + +import { ConfigError, errorMessage, isNotFoundError } from "../../core/errors"; + +export type Severity = "error" | "warn"; + +export interface AssertionDef { + name: string; + sql: string; + severity: Severity; +} + +// An --assert value is a `.sql` file or a glob of them; inline SQL is not supported. +export type AssertToken = { kind: "file"; path: string } | { kind: "glob"; pattern: string }; + +// basename with `*` ⇒ glob; plain `.sql` ⇒ file; anything else rejected. +export function classifyAssertToken(token: string): AssertToken { + if (/\.sql$/i.test(token)) { + return basename(token).includes("*") + ? { kind: "glob", pattern: token } + : { kind: "file", path: token }; + } + throw new ConfigError( + `--assert expects a .sql file path or glob (inline SQL is not supported); got: "${token}"`, + ); +} + +// `--assert` is repeatable (citty hands back a string[] when given more than once) and each +// value may itself be comma-separated (file/glob paths). +export function parseAssertFlags(value: string | string[] | undefined): AssertToken[] { + if (value === undefined) { + return []; + } + const raw = Array.isArray(value) ? value : [value]; + const tokens: AssertToken[] = []; + for (const entry of raw) { + if (entry.trim() === "") { + continue; + } + for (const part of entry.split(",")) { + const trimmed = part.trim(); + if (trimmed !== "") { + tokens.push(classifyAssertToken(trimmed)); + } + } + } + return tokens; +} + +function assertionName(path: string): string { + return basename(path, extname(path)); +} + +export async function readSqlFile(path: string, label: string): Promise { + try { + const contents = await readFile(path, "utf8"); + return contents.trim(); + } catch (error) { + throw new ConfigError(`Cannot read ${label} '${path}': ${errorMessage(error)}`); + } +} + +// A glob here is shallow: a single directory listing filtered by the literal +// prefix/suffix around the one `*` in the basename. Enough for the documented `dir/*.sql` +// form without pulling in a glob dependency; nested `**` is not supported. +async function expandGlob(pattern: string): Promise { + const dir = dirname(pattern); + const base = basename(pattern); + const star = base.indexOf("*"); + const prefix = base.slice(0, star); + const suffix = base.slice(star + 1); + let entries: string[]; + try { + entries = await readdir(dir); + } catch (error) { + if (isNotFoundError(error)) { + throw new ConfigError(`--assert glob '${pattern}' matched nothing (no such directory).`); + } + throw new ConfigError(`Cannot read --assert glob '${pattern}': ${errorMessage(error)}`); + } + const matches = entries + .filter( + (name) => name.startsWith(prefix) && name.endsWith(suffix) && name.length >= base.length - 1, + ) + .toSorted() + .map((name) => join(dir, name)); + if (matches.length === 0) { + throw new ConfigError(`--assert glob '${pattern}' matched no files.`); + } + return matches; +} + +// Each `.sql` file → one assertion, named by basename without extension; severity defaults to error. +export async function resolveAssertions(tokens: AssertToken[]): Promise { + const out: AssertionDef[] = []; + for (const token of tokens) { + const paths = token.kind === "glob" ? await expandGlob(token.pattern) : [token.path]; + for (const path of paths) { + out.push({ + name: assertionName(path), + sql: await readSqlFile(path, "--assert file"), + severity: "error", + }); + } + } + return out; +} diff --git a/src/commands/transform-test/index.ts b/src/commands/transform-test/index.ts new file mode 100644 index 0000000..8eeef86 --- /dev/null +++ b/src/commands/transform-test/index.ts @@ -0,0 +1,13 @@ +import { defineCommand } from "citty"; + +export default defineCommand({ + meta: { + name: "transform-test", + description: + "Test transforms or cards (and sub-graphs) against fixture CSVs without touching real tables", + }, + subCommands: { + inputs: () => import("./inputs").then((mod) => mod.default), + run: () => import("./run").then((mod) => mod.default), + }, +}); diff --git a/src/commands/transform-test/inputs.ts b/src/commands/transform-test/inputs.ts new file mode 100644 index 0000000..1c72b59 --- /dev/null +++ b/src/commands/transform-test/inputs.ts @@ -0,0 +1,49 @@ +import { TestRunInputCompact, testRunInputView } from "../../domain/transform-test-run"; +import { renderList } from "../../output/render"; +import { listEnvelopeSchema, wrapList } from "../../output/types"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId, parseIdList } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +import { fetchSubgraphInputs, parseTargetType, targetLabels, targetTypeFlag } from "./subgraph"; + +export const TestRunInputListEnvelope = listEnvelopeSchema(TestRunInputCompact); + +export default defineMetabaseCommand({ + meta: { + name: "inputs", + description: "List the input tables a transform or card test run requires fixtures for", + }, + details: + "Resolves the sub-graph from the selected --source transforms up to the target (the positional id) and returns its boundary leaf tables — one CSV fixture is required per table for `transform-test run`. The positional id is a transform id by default, or a card id when --target-type card is set. Omit --source to test the target alone. Each row carries the table id (use it in `--input =`) and the exact column headers the fixture CSV must contain.", + // Provisional: the test-run endpoints are unreleased. minVersion mirrors the + // transforms feature baseline so the command runs against a dev build; bump to the actual + // release version before this ships. + capabilities: { minVersion: 59 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...targetTypeFlag, + source: { + type: "string", + description: "Comma-separated boundary source transform ids (omit to test the target alone)", + }, + id: { type: "positional", description: "Target transform or card id", required: true }, + }, + outputSchema: TestRunInputListEnvelope, + examples: [ + "mb transform-test inputs 173 --source 172", + "mb transform-test inputs 173 --source 172 --json", + "mb transform-test inputs 42", + "mb transform-test inputs 88 --target-type card", + ], + async run({ args, ctx, getClient }) { + const targetType = parseTargetType(args["target-type"]); + const target = parseId(args.id, targetLabels(targetType).positionalLabel); + const sources = parseIdList(args.source, "--source"); + const client = await getClient(); + const items = await fetchSubgraphInputs(client, targetType, target, sources); + renderList(wrapList(items), testRunInputView, ctx); + }, +}); diff --git a/src/commands/transform-test/run.ts b/src/commands/transform-test/run.ts new file mode 100644 index 0000000..aea047e --- /dev/null +++ b/src/commands/transform-test/run.ts @@ -0,0 +1,172 @@ +import { readFile } from "node:fs/promises"; + +import { ConfigError, errorMessage } from "../../core/errors"; +import { TestRunResult } from "../../domain/transform-test-run"; +import { collectRepeatedFlag } from "../../runtime/citty"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId, parseIdList } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +import { type AssertionDef, parseAssertFlags, resolveAssertions } from "./assert"; +import { + parseColumnList, + parseInputPairs, + parseTargetType, + runSubgraph, + type SubgraphRunArgs, + targetLabels, + targetTypeFlag, +} from "./subgraph"; +import { parseSuite, type SuiteArgs } from "./suite"; + +const ASSERT_FLAG = { + type: "string", + description: + "Assertion: a .sql file path or a glob of them (dir/*.sql). Repeatable; comma-separates paths. Name = file basename without .sql. Severity defaults to error. Inline SQL is not supported — write the assertion to a .sql file.", +} as const; + +async function loadSuite(path: string): Promise { + let contents: string; + try { + contents = await readFile(path, "utf8"); + } catch (error) { + throw new ConfigError(`Cannot read --suite file '${path}': ${errorMessage(error)}`); + } + return parseSuite(contents, path); +} + +// Merge a YAML suite (if any) with ad-hoc flags: scalar/list flags override the suite when +// provided; --assert assertions append to the suite's. +function compose(suite: SuiteArgs | null, flags: FlagArgs): SubgraphRunArgs { + const targetType = flags.targetType ?? suite?.targetType; + const target = flags.target ?? suite?.target; + if (targetType === undefined || target === undefined) { + throw new ConfigError( + "Missing target. Provide the positional id (and --target-type) or a --suite that defines target.", + ); + } + const sources = flags.sources.length > 0 ? flags.sources : (suite?.sources ?? []); + const inputs = flags.inputs.length > 0 ? flags.inputs : (suite?.inputs ?? []); + const ignoreColumns = + flags.ignoreColumns.length > 0 ? flags.ignoreColumns : (suite?.ignoreColumns ?? []); + const expected = flags.expected ?? suite?.expected; + const assertions: AssertionDef[] = [...(suite?.assertions ?? []), ...flags.assertions]; + + if (expected === undefined && assertions.length === 0) { + throw new ConfigError( + "Provide at least one of --expected or --assert (or define them in --suite).", + ); + } + + const args: SubgraphRunArgs = { + targetType, + target, + sources, + inputs, + ignoreColumns, + assertions, + }; + if (expected !== undefined) { + args.expected = expected; + } + return args; +} + +interface FlagArgs { + targetType?: SubgraphRunArgs["targetType"]; + target?: number; + sources: number[]; + inputs: SubgraphRunArgs["inputs"]; + expected?: string; + ignoreColumns: string[]; + assertions: AssertionDef[]; +} + +export default defineMetabaseCommand({ + meta: { + name: "run", + description: "Test-run a transform or card (or sub-graph) against fixture CSVs and assertions", + }, + details: + "Seeds scratch tables from the --input fixture CSVs (real tables are never touched), runs the sub-graph from the --source transforms up to the target (the positional id) in dependency order, and checks the target's output against the --expected CSV and/or --assert SQL assertions. At least one of --expected or --assert is required. An assertion is a SQL query written to a `.sql` file that passes iff it returns zero rows; it references the synthetic `test_output` relation (the target's output) and/or input table names. A --suite YAML file can declare the whole run (target, inputs, expected, assertions with per-assertion severity) and is parsed entirely client-side; ad-hoc flags compose with it (scalars override, --assert appends). The positional id is a transform id by default, or a card id when --target-type card is set. Use `transform-test inputs` to discover which tables need fixtures. Exits non-zero when the diff fails or any error-severity assertion fails; warn-severity assertion failures are reported but do not affect the exit code.", + // Provisional: the test-run endpoints are unreleased. minVersion mirrors the + // transforms feature baseline so the command runs against a dev build; bump to the actual + // release version before this ships. + capabilities: { minVersion: 59 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...targetTypeFlag, + source: { + type: "string", + description: "Comma-separated boundary source transform ids (omit to test the target alone)", + }, + input: { + type: "string", + description: "Comma-separated = fixtures, one per required input table", + }, + expected: { + type: "string", + description: "Path to the expected-output CSV (optional if --assert is given)", + }, + "ignore-columns": { + type: "string", + description: "Comma-separated output column names to exclude from the diff", + }, + assert: ASSERT_FLAG, + suite: { + type: "string", + description: + "Path to a YAML test-suite file (target, inputs, expected, assertions with optional severity); composed with ad-hoc flags", + }, + // Not required at the citty layer: a --suite can supply the target. `compose` enforces that + // a target comes from one source or the other. + id: { + type: "positional", + required: false, + description: "Target transform or card id (optional when --suite defines the target)", + }, + }, + outputSchema: TestRunResult, + examples: [ + "mb transform-test run 173 --source 172 --input 229=orders.csv --expected expected.csv", + "mb transform-test run 173 --source 172 --input 229=orders.csv --assert checks/no_negatives.sql", + "mb transform-test run 173 --assert checks/no_negatives.sql,checks/has_rows.sql --input 229=orders.csv", + "mb transform-test run 173 --assert 'checks/*.sql' --input 229=orders.csv", + "mb transform-test run --suite suites/orders.yaml", + ], + async run({ args, rawArgs, ctx, getClient }) { + const suite = args.suite !== undefined ? await loadSuite(args.suite) : null; + + const idGiven = typeof args.id === "string" && args.id.trim() !== ""; + const targetType = idGiven || suite === null ? parseTargetType(args["target-type"]) : undefined; + const target = idGiven + ? parseId(args.id, targetLabels(targetType ?? "transform").positionalLabel) + : undefined; + + // `--assert` is repeatable: citty collapses repeats to the last value, so read every + // occurrence from rawArgs. `parseAssertFlags` also comma-splits each value. + const assertValues = collectRepeatedFlag(rawArgs, "assert", { assert: ASSERT_FLAG }); + const flags: FlagArgs = { + sources: parseIdList(args.source, "--source"), + inputs: parseInputPairs(args.input), + ignoreColumns: parseColumnList(args["ignore-columns"]), + assertions: await resolveAssertions(parseAssertFlags(assertValues)), + }; + if (targetType !== undefined && idGiven) { + flags.targetType = targetType; + } + if (target !== undefined) { + flags.target = target; + } + if (args.expected !== undefined && args.expected.trim() !== "") { + flags.expected = args.expected; + } + + const runArgs = compose(suite, flags); + + const client = await getClient(); + await runSubgraph(client, runArgs, ctx); + }, +}); diff --git a/src/commands/transform-test/subgraph.test.ts b/src/commands/transform-test/subgraph.test.ts new file mode 100644 index 0000000..798a58c --- /dev/null +++ b/src/commands/transform-test/subgraph.test.ts @@ -0,0 +1,266 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +import { ConfigError } from "../../core/errors"; +import type { AssertionResult, TestRunResult } from "../../domain/transform-test-run"; +import type { CommonContext } from "../context"; + +import { + assertionsSummaryLine, + buildSubgraphForm, + parseColumnList, + parseInputPairs, + parseTargetType, + renderRunResult, + shouldFail, +} from "./subgraph"; + +function assertion(over: Partial & { name: string }): AssertionResult { + return { + status: "passed", + failing_row_count: 0, + sample_rows: null, + columns: [], + ...over, + }; +} + +function result(over: Partial): TestRunResult { + return { status: "passed", diff: null, ...over }; +} + +function renderCtx(over: Partial): CommonContext { + return { + format: "text", + full: false, + fields: undefined, + maxBytes: 65536, + url: undefined, + apiKey: undefined, + profile: undefined, + skipPreflight: false, + ...over, + }; +} + +describe("parseInputPairs", () => { + it("parses comma-separated = pairs", () => { + expect(parseInputPairs("229=orders.csv,223=people.csv")).toEqual([ + { tableId: 229, path: "orders.csv" }, + { tableId: 223, path: "people.csv" }, + ]); + }); + + it("returns an empty array for undefined or blank input", () => { + expect(parseInputPairs(undefined)).toEqual([]); + expect(parseInputPairs(" ")).toEqual([]); + }); + + it("trims whitespace around entries and around each side of '='", () => { + expect(parseInputPairs(" 1 = a.csv , 2 = b.csv ")).toEqual([ + { tableId: 1, path: "a.csv" }, + { tableId: 2, path: "b.csv" }, + ]); + }); + + it("throws ConfigError with the offending entry when '=' is missing", () => { + expect(() => parseInputPairs("229")).toThrow(ConfigError); + expect(() => parseInputPairs("229")).toThrow( + "Malformed --input entry '229'. Expected = (e.g. 229=orders.csv).", + ); + }); + + it("throws ConfigError when the table id is not a positive integer", () => { + expect(() => parseInputPairs("0=x.csv")).toThrow(ConfigError); + expect(() => parseInputPairs("0=x.csv")).toThrow("invalid --input table id: 0 (must be ≥ 1)"); + }); +}); + +describe("parseColumnList", () => { + it("splits and trims comma-separated names", () => { + expect(parseColumnList("a, b ,c")).toEqual(["a", "b", "c"]); + }); + + it("returns an empty array for undefined or blank input", () => { + expect(parseColumnList(undefined)).toEqual([]); + expect(parseColumnList("")).toEqual([]); + }); +}); + +describe("parseTargetType", () => { + it("accepts the supported target types", () => { + expect(parseTargetType("transform")).toBe("transform"); + expect(parseTargetType("card")).toBe("card"); + }); + + it("throws ConfigError naming the supported types for an unsupported value", () => { + expect(() => parseTargetType("metric")).toThrow(ConfigError); + expect(() => parseTargetType("metric")).toThrow( + 'invalid --target-type: "metric" (expected one of: transform, card)', + ); + }); +}); + +async function formFields(form: FormData): Promise> { + const out: Record = {}; + for (const [key, value] of form.entries()) { + out[key] = typeof value === "string" ? value : await value.text(); + } + return out; +} + +describe("buildSubgraphForm", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "mb-form-")); + writeFileSync(join(dir, "expected.csv"), "id\n1\n"); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("appends an assertions JSON part when assertions are present", async () => { + const form = await buildSubgraphForm({ + targetType: "transform", + target: 173, + sources: [], + inputs: [], + expected: join(dir, "expected.csv"), + ignoreColumns: [], + assertions: [{ name: "a", sql: "SELECT 1", severity: "error" }], + }); + const fields = await formFields(form); + expect(fields["assertions"]).toBe( + JSON.stringify([{ name: "a", sql: "SELECT 1", severity: "error" }]), + ); + expect(fields["expected"]).toBe("id\n1\n"); + }); + + it("omits the expected part when no expected file is given (assertions-only)", async () => { + const form = await buildSubgraphForm({ + targetType: "transform", + target: 173, + sources: [], + inputs: [], + ignoreColumns: [], + assertions: [{ name: "a", sql: "SELECT 1", severity: "error" }], + }); + const fields = await formFields(form); + expect(fields["expected"]).toBeUndefined(); + expect(fields["assertions"]).toBeDefined(); + }); + + it("omits the assertions part when there are no assertions", async () => { + const form = await buildSubgraphForm({ + targetType: "transform", + target: 173, + sources: [], + inputs: [], + expected: join(dir, "expected.csv"), + ignoreColumns: [], + assertions: [], + }); + const fields = await formFields(form); + expect(fields["assertions"]).toBeUndefined(); + }); +}); + +describe("shouldFail", () => { + it("fails when the top-level status is failed", () => { + expect(shouldFail(result({ status: "failed" }))).toBe(true); + }); + + it("passes when the top-level status is passed, even with a failing warn assertion", () => { + const res = result({ + status: "passed", + assertions: [assertion({ name: "w", status: "warn", failing_row_count: 3 })], + }); + expect(shouldFail(res)).toBe(false); + }); +}); + +describe("assertionsSummaryLine", () => { + it("renders the passed/failed/warn breakdown with the first failing assertion", () => { + const res = result({ + status: "failed", + assertions: [ + assertion({ name: "ok", status: "passed" }), + assertion({ name: "neg_rev", status: "failed", failing_row_count: 3 }), + assertion({ name: "warned", status: "warn", failing_row_count: 2 }), + ], + }); + expect(assertionsSummaryLine(res)).toBe( + "3 assertions — 1 passed, 1 FAILED, 1 warn (neg_rev: 3 failing rows)", + ); + }); + + it("returns null when there are no assertions", () => { + expect(assertionsSummaryLine(result({ status: "passed" }))).toBeNull(); + expect(assertionsSummaryLine(result({ status: "passed", assertions: [] }))).toBeNull(); + }); +}); + +describe("renderRunResult", () => { + let stdout: string; + + beforeEach(() => { + stdout = ""; + vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { + stdout += String(chunk); + return true; + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + const failingResult = result({ + status: "failed", + diff: null, + assertions: [ + assertion({ name: "ok_check", status: "passed" }), + assertion({ name: "neg_rev", status: "failed", failing_row_count: 3 }), + ], + }); + + it("renders the per-assertion table on a FAILING run in human/text mode", () => { + renderRunResult("transform", 173, failingResult, renderCtx({ format: "text" })); + // Summary line(s) + expect(stdout).toContain("Transform 173 test run FAILED"); + expect(stdout).toContain("2 assertions — 1 passed, 1 FAILED"); + // The per-assertion table: header + each assertion's name/status/failing rows + expect(stdout).toContain("Name"); + expect(stdout).toContain("Status"); + expect(stdout).toContain("Failing Rows"); + expect(stdout).toContain("ok_check"); + expect(stdout).toContain("neg_rev"); + expect(stdout).toContain("failed"); + }); + + it("emits the full structured JSON (no human table) under --json", () => { + renderRunResult("transform", 173, failingResult, renderCtx({ format: "json" })); + const parsed = JSON.parse(stdout); + expect(parsed.status).toBe("failed"); + expect(parsed.assertions).toHaveLength(2); + expect(parsed.assertions[1].name).toBe("neg_rev"); + // No bordered text table in JSON mode. + expect(stdout).not.toContain("Failing Rows"); + }); + + it("renders the per-assertion table on a PASSING run in text mode", () => { + const passing = result({ + status: "passed", + assertions: [assertion({ name: "ok_check", status: "passed" })], + }); + renderRunResult("transform", 173, passing, renderCtx({ format: "text" })); + expect(stdout).toContain("Transform 173 test run passed."); + expect(stdout).toContain("ok_check"); + expect(stdout).toContain("Failing Rows"); + }); +}); diff --git a/src/commands/transform-test/subgraph.ts b/src/commands/transform-test/subgraph.ts new file mode 100644 index 0000000..44d773c --- /dev/null +++ b/src/commands/transform-test/subgraph.ts @@ -0,0 +1,227 @@ +import { z } from "zod"; + +import { ConfigError } from "../../core/errors"; +import type { Client } from "../../core/http/client"; +import { + type AssertionResult, + assertionResultView, + TestRunInput, + TestRunResult, + testRunResultView, +} from "../../domain/transform-test-run"; +import { renderList, renderSummary, writeText } from "../../output/render"; +import { wrapList } from "../../output/types"; +import { readFilePart } from "../../runtime/upload"; +import type { CommonContext } from "../context"; +import { parseEnumFlag } from "../parse-enum"; +import { parseId } from "../parse-id"; + +import type { AssertionDef } from "./assert"; + +export const TargetType = z.enum(["transform", "card"]); +export type TargetType = z.infer; + +const DEFAULT_TARGET_TYPE: TargetType = "transform"; + +interface TargetLabels { + positionalLabel: string; + summaryNoun: string; +} + +const TARGET_LABELS: Record = { + transform: { positionalLabel: "Target transform id", summaryNoun: "Transform" }, + card: { positionalLabel: "Target card id (saved question or model)", summaryNoun: "Card" }, +}; + +export function targetLabels(targetType: TargetType): TargetLabels { + return TARGET_LABELS[targetType]; +} + +export function parseTargetType(value: string): TargetType { + return parseEnumFlag(value, TargetType, "--target-type"); +} + +export const targetTypeFlag = { + "target-type": { + type: "string", + description: `Test-run target kind: ${TargetType.options.join(" | ")} (default: ${DEFAULT_TARGET_TYPE})`, + default: DEFAULT_TARGET_TYPE, + }, +} as const; + +export interface InputPair { + tableId: number; + path: string; +} + +function parseInputPair(pair: string): InputPair { + const eq = pair.indexOf("="); + if (eq <= 0 || eq === pair.length - 1) { + throw new ConfigError( + `Malformed --input entry '${pair}'. Expected = (e.g. 229=orders.csv).`, + ); + } + const tableId = parseId(pair.slice(0, eq).trim(), "--input table id"); + return { tableId, path: pair.slice(eq + 1).trim() }; +} + +export function parseInputPairs(value: string | undefined): InputPair[] { + if (value === undefined || value.trim() === "") { + return []; + } + return value + .split(",") + .map((part) => part.trim()) + .filter((part) => part !== "") + .map(parseInputPair); +} + +export function parseColumnList(value: string | undefined): string[] { + if (value === undefined || value.trim() === "") { + return []; + } + return value + .split(",") + .map((part) => part.trim()) + .filter((part) => part !== ""); +} + +function subgraphInputsPath(targetType: TargetType, target: number): string { + return `/api/ee/transform-test/${targetType}/${target}/inputs`; +} + +function subgraphPath(targetType: TargetType, target: number): string { + return `/api/ee/transform-test/${targetType}/${target}/run`; +} + +export async function fetchSubgraphInputs( + client: Client, + targetType: TargetType, + target: number, + sources: number[], +): Promise { + return client.requestParsed(z.array(TestRunInput), subgraphInputsPath(targetType, target), { + query: { sources }, + }); +} + +export interface SubgraphRunArgs { + targetType: TargetType; + target: number; + sources: number[]; + inputs: InputPair[]; + // Optional per field; the run requires at least one of expected or assertions. + expected?: string; + ignoreColumns: string[]; + assertions: AssertionDef[]; +} + +export async function buildSubgraphForm(args: SubgraphRunArgs): Promise { + const form = new FormData(); + for (const { tableId, path } of args.inputs) { + const part = await readFilePart(path, `--input ${tableId}`); + form.append(`input-${tableId}`, part.blob, part.filename); + } + if (args.expected !== undefined) { + const expectedPart = await readFilePart(args.expected, "--expected"); + form.append("expected", expectedPart.blob, expectedPart.filename); + } + if (args.sources.length > 0) { + form.append("sources", JSON.stringify(args.sources)); + } + if (args.ignoreColumns.length > 0) { + form.append("options", JSON.stringify({ ignore_columns: args.ignoreColumns })); + } + if (args.assertions.length > 0) { + form.append("assertions", JSON.stringify(args.assertions)); + } + return form; +} + +function assertionList(result: TestRunResult): AssertionResult[] { + return result.assertions ?? []; +} + +// Exit nonzero iff the server's top-level status is `failed`. +export function shouldFail(result: TestRunResult): boolean { + return result.status === "failed"; +} + +export function assertionsSummaryLine(result: TestRunResult): string | null { + const assertions = assertionList(result); + if (assertions.length === 0) { + return null; + } + const passed = assertions.filter((a) => a.status === "passed").length; + const failed = assertions.filter((a) => a.status === "failed").length; + const warned = assertions.filter((a) => a.status === "warn").length; + const parts = [`${passed} passed`, `${failed} FAILED`, `${warned} warn`]; + const firstFailing = assertions.find((a) => a.status === "failed" || a.status === "warn"); + const detail = + firstFailing === undefined + ? "" + : ` (${firstFailing.name}: ${firstFailing.failing_row_count} failing rows)`; + return `${assertions.length} assertions — ${parts.join(", ")}${detail}`; +} + +function summaryLine(targetType: TargetType, target: number, result: TestRunResult): string { + const noun = targetLabels(targetType).summaryNoun; + const lines: string[] = []; + const diffShown = (result.diff ?? null) !== null; + if (result.status === "passed") { + lines.push(`${noun} ${target} test run passed.`); + } else if (diffShown) { + lines.push( + `${noun} ${target} test run FAILED — output did not match expected. Re-run with --json to see the diff.`, + ); + } else { + lines.push(`${noun} ${target} test run FAILED. Re-run with --json to see details.`); + } + const assertions = assertionsSummaryLine(result); + if (assertions !== null) { + lines.push(assertions); + } + return lines.join("\n"); +} + +// Render a run result. Under `--json` (and `--fields`/`--full`) the full structured result is +// emitted; otherwise the human view: the summary line(s) followed — on any run that carried +// assertions, passing OR failing — by the per-assertion table (name / status / failing rows). +export function renderRunResult( + targetType: TargetType, + target: number, + result: TestRunResult, + ctx: CommonContext, +): void { + renderSummary(result, testRunResultView, () => summaryLine(targetType, target, result), ctx); + + const humanView = ctx.format !== "json" && ctx.fields === undefined && !ctx.full; + const assertions = assertionList(result); + if (humanView && assertions.length > 0) { + writeText(""); + renderList(wrapList(assertions), assertionResultView, ctx); + } +} + +export async function runSubgraph( + client: Client, + args: SubgraphRunArgs, + ctx: CommonContext, +): Promise { + const form = await buildSubgraphForm(args); + const result = await client.requestParsed( + TestRunResult, + subgraphPath(args.targetType, args.target), + { + method: "POST", + body: form, + }, + ); + + renderRunResult(args.targetType, args.target, result, ctx); + + if (shouldFail(result)) { + const noun = targetLabels(args.targetType).summaryNoun.toLowerCase(); + throw new Error(`${noun} ${args.target} test run failed`); + } +} diff --git a/src/commands/transform-test/suite.test.ts b/src/commands/transform-test/suite.test.ts new file mode 100644 index 0000000..c52f2a9 --- /dev/null +++ b/src/commands/transform-test/suite.test.ts @@ -0,0 +1,121 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { ConfigError } from "../../core/errors"; +import { ValidationError } from "../../core/errors"; + +import { parseSuite } from "./suite"; + +describe("parseSuite", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "mb-suite-")); + writeFileSync(join(dir, "no_negatives.sql"), "SELECT * FROM test_output WHERE revenue < 0\n"); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("parses a full suite into SubgraphRunArgs-compatible fields", async () => { + const yaml = ` +target: + type: transform + id: 173 +sources: [172] +inputs: + - table: 229 + file: orders.csv +expected: out.csv +ignore_columns: [snapshot_ts] +assertions: + - name: positive_revenue + sql: SELECT * FROM test_output WHERE revenue < 0 + severity: warn + - name: from_file + file: ${join(dir, "no_negatives.sql")} +`; + const out = await parseSuite(yaml, "suite.yaml"); + expect(out.targetType).toBe("transform"); + expect(out.target).toBe(173); + expect(out.sources).toEqual([172]); + expect(out.inputs).toEqual([{ tableId: 229, path: "orders.csv" }]); + expect(out.expected).toBe("out.csv"); + expect(out.ignoreColumns).toEqual(["snapshot_ts"]); + expect(out.assertions).toEqual([ + { + name: "positive_revenue", + sql: "SELECT * FROM test_output WHERE revenue < 0", + severity: "warn", + }, + { + name: "from_file", + sql: "SELECT * FROM test_output WHERE revenue < 0", + severity: "error", + }, + ]); + }); + + it("defaults optional fields (sources, inputs, ignore_columns, assertions) to empty", async () => { + const yaml = ` +target: + type: card + id: 88 +expected: out.csv +`; + const out = await parseSuite(yaml, "suite.yaml"); + expect(out.targetType).toBe("card"); + expect(out.target).toBe(88); + expect(out.sources).toEqual([]); + expect(out.inputs).toEqual([]); + expect(out.ignoreColumns).toEqual([]); + expect(out.assertions).toEqual([]); + expect(out.expected).toBe("out.csv"); + }); + + it("leaves expected undefined when absent", async () => { + const yaml = ` +target: + type: transform + id: 1 +assertions: + - name: a + sql: SELECT 1 +`; + const out = await parseSuite(yaml, "suite.yaml"); + expect(out.expected).toBeUndefined(); + }); + + it("rejects an assertion with neither sql nor file", async () => { + const yaml = ` +target: + type: transform + id: 1 +assertions: + - name: bad +`; + await expect(parseSuite(yaml, "suite.yaml")).rejects.toThrow(ConfigError); + }); + + it("rejects an assertion with both sql and file", async () => { + const yaml = ` +target: + type: transform + id: 1 +assertions: + - name: bad + sql: SELECT 1 + file: x.sql +`; + await expect(parseSuite(yaml, "suite.yaml")).rejects.toThrow(ConfigError); + }); + + it("surfaces a schema ValidationError for a malformed target", async () => { + const yaml = `target: "nope"`; + await expect(parseSuite(yaml, "suite.yaml")).rejects.toThrow(ValidationError); + }); +}); diff --git a/src/commands/transform-test/suite.ts b/src/commands/transform-test/suite.ts new file mode 100644 index 0000000..9505d0a --- /dev/null +++ b/src/commands/transform-test/suite.ts @@ -0,0 +1,78 @@ +import { z } from "zod"; + +import { ConfigError } from "../../core/errors"; +import { parseYaml } from "../../runtime/yaml"; + +import { readSqlFile, type AssertionDef, type Severity } from "./assert"; +import { type InputPair, type TargetType } from "./subgraph"; + +const TargetTypeSchema = z.enum(["transform", "card"]); +const SeveritySchema = z.enum(["error", "warn"]); + +const SuiteAssertion = z + .object({ + name: z.string().min(1), + sql: z.string().optional(), + file: z.string().optional(), + severity: SeveritySchema.optional(), + }) + .strict(); + +const Suite = z + .object({ + target: z.object({ type: TargetTypeSchema, id: z.number().int().positive() }).strict(), + sources: z.array(z.number().int().positive()).optional(), + inputs: z + .array(z.object({ table: z.number().int().positive(), file: z.string() }).strict()) + .optional(), + expected: z.string().optional(), + ignore_columns: z.array(z.string()).optional(), + assertions: z.array(SuiteAssertion).optional(), + }) + .strict(); + +type Suite = z.infer; +type SuiteAssertion = z.infer; + +// The slice of SubgraphRunArgs a suite contributes. +export interface SuiteArgs { + targetType: TargetType; + target: number; + sources: number[]; + inputs: InputPair[]; + expected?: string; + ignoreColumns: string[]; + assertions: AssertionDef[]; +} + +async function resolveSuiteAssertion(entry: SuiteAssertion): Promise { + const severity: Severity = entry.severity ?? "error"; + if (entry.sql !== undefined && entry.file === undefined) { + return { name: entry.name, sql: entry.sql.trim(), severity }; + } + if (entry.file !== undefined && entry.sql === undefined) { + return { + name: entry.name, + sql: await readSqlFile(entry.file, "suite assertion file"), + severity, + }; + } + throw new ConfigError(`Suite assertion '${entry.name}' must set exactly one of 'sql' or 'file'.`); +} + +export async function parseSuite(yamlText: string, source: string): Promise { + const suite: Suite = parseYaml(yamlText, Suite, { source }); + const assertions = await Promise.all((suite.assertions ?? []).map(resolveSuiteAssertion)); + const args: SuiteArgs = { + targetType: suite.target.type, + target: suite.target.id, + sources: suite.sources ?? [], + inputs: (suite.inputs ?? []).map((row) => ({ tableId: row.table, path: row.file })), + ignoreColumns: suite.ignore_columns ?? [], + assertions, + }; + if (suite.expected !== undefined) { + args.expected = suite.expected; + } + return args; +} diff --git a/src/core/http/errors.ts b/src/core/http/errors.ts index 2c48fae..4702d81 100644 --- a/src/core/http/errors.ts +++ b/src/core/http/errors.ts @@ -38,7 +38,9 @@ const STATUS_CLASSIFICATIONS: Record = { const ErrorEnvelope = z .object({ message: z.string().optional(), - error: z.string().optional(), + // `error` may be a structured object, not a string; accept any shape so a non-string + // doesn't break parsing the whole envelope. + error: z.unknown().optional(), "error-message": z.string().optional(), via: z.array(z.object({ message: z.string().optional() }).loose()).optional(), "specific-errors": z.unknown().optional(), @@ -211,7 +213,8 @@ function parseEnvelopeMessage(sanitizedBody: string | null): string | null { return null; } const envelope = result.value; - const topLevel = envelope.message ?? envelope.error ?? envelope["error-message"]; + const errorString = typeof envelope.error === "string" ? envelope.error : undefined; + const topLevel = envelope.message ?? errorString ?? envelope["error-message"]; if (topLevel) { return capLength(topLevel); } diff --git a/src/domain/transform-test-run.test.ts b/src/domain/transform-test-run.test.ts new file mode 100644 index 0000000..7e2b77f --- /dev/null +++ b/src/domain/transform-test-run.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; + +import { TestRunInput, TestRunInputCompact } from "./transform-test-run"; + +describe("TestRunInput", () => { + const base = { + table_id: 229, + schema: "public", + name: "orders", + columns: ["id", "total"], + }; + + it("accepts a schema-qualified input table", () => { + expect(TestRunInput.safeParse(base).success).toBe(true); + }); + + it("accepts a null schema (engines without schemas)", () => { + const parsed = TestRunInput.safeParse({ ...base, schema: null }); + expect(parsed.success).toBe(true); + expect(parsed.data?.schema).toBeNull(); + }); + + it("rejects a missing schema key", () => { + const { schema: _schema, ...rest } = base; + expect(TestRunInput.safeParse(rest).success).toBe(false); + }); + + it("compact pick preserves a null schema", () => { + expect(TestRunInputCompact.parse({ ...base, schema: null }).schema).toBeNull(); + }); +}); diff --git a/src/domain/transform-test-run.ts b/src/domain/transform-test-run.ts new file mode 100644 index 0000000..4d520ea --- /dev/null +++ b/src/domain/transform-test-run.ts @@ -0,0 +1,84 @@ +import { z } from "zod"; + +import type { ResourceView } from "./view"; + +export const TestRunInput = z + .object({ + table_id: z.number().int().positive(), + // null on engines without schemas (e.g. MySQL targets with no schema segment). + schema: z.string().nullable(), + name: z.string(), + columns: z.array(z.string()), + }) + .loose(); +export type TestRunInput = z.infer; + +export const TestRunInputCompact = TestRunInput.pick({ + table_id: true, + schema: true, + name: true, + columns: true, +}).strip(); +export type TestRunInputCompact = z.infer; + +function formatColumns(value: unknown): string { + return Array.isArray(value) ? value.join(", ") : String(value ?? ""); +} + +export const testRunInputView: ResourceView = { + compactPick: TestRunInputCompact, + tableColumns: [ + { key: "table_id", label: "Table ID" }, + { key: "schema", label: "Schema" }, + { key: "name", label: "Name" }, + { key: "columns", label: "Columns", format: formatColumns }, + ], +}; + +export const AssertionResult = z + .object({ + name: z.string(), + status: z.enum(["passed", "failed", "warn"]), + failing_row_count: z.number().int().nonnegative(), + sample_rows: z.array(z.array(z.unknown())).nullable(), + columns: z.array(z.string()), + }) + .loose(); +export type AssertionResult = z.infer; + +export const TestRunResult = z + .object({ + status: z.enum(["passed", "failed"]), + diff: z.unknown(), + assertions: z.array(AssertionResult).nullable().optional(), + }) + .loose(); +export type TestRunResult = z.infer; + +export const TestRunResultCompact = TestRunResult.pick({ + status: true, + diff: true, + assertions: true, +}).strip(); +export type TestRunResultCompact = z.infer; + +export const testRunResultView: ResourceView = { + compactPick: TestRunResultCompact, + tableColumns: [{ key: "status", label: "Status" }], +}; + +export const AssertionResultCompact = AssertionResult.pick({ + name: true, + status: true, + failing_row_count: true, +}).strip(); +export type AssertionResultCompact = z.infer; + +export const assertionResultView: ResourceView = { + compactPick: AssertionResultCompact, + tableColumns: [ + { key: "name", label: "Name" }, + { key: "status", label: "Status" }, + { key: "failing_row_count", label: "Failing Rows" }, + ], +}; diff --git a/src/main.ts b/src/main.ts index 220879b..c5da490 100644 --- a/src/main.ts +++ b/src/main.ts @@ -24,6 +24,7 @@ const main: CommandDef = defineCommand({ document: () => import("./commands/document").then((mod) => mod.default), transform: () => import("./commands/transform").then((mod) => mod.default), "transform-job": () => import("./commands/transform-job").then((mod) => mod.default), + "transform-test": () => import("./commands/transform-test").then((mod) => mod.default), "transform-tag": () => import("./commands/transform-tag").then((mod) => mod.default), setting: () => import("./commands/setting").then((mod) => mod.default), search: () => import("./commands/search").then((mod) => mod.default), diff --git a/src/runtime/citty.test.ts b/src/runtime/citty.test.ts index 3bed035..595bc31 100644 --- a/src/runtime/citty.test.ts +++ b/src/runtime/citty.test.ts @@ -1,7 +1,7 @@ -import type { CommandMeta } from "citty"; +import type { ArgsDef, CommandMeta } from "citty"; import { describe, expect, it } from "vitest"; -import { resolveCitty, toAliasArray } from "./citty"; +import { collectRepeatedFlag, resolveCitty, toAliasArray } from "./citty"; describe("resolveCitty", () => { const meta: CommandMeta = { name: "demo", description: "demo cmd" }; @@ -26,6 +26,37 @@ describe("resolveCitty", () => { }); }); +describe("collectRepeatedFlag", () => { + const argsDef: ArgsDef = { + assert: { type: "string", alias: "a" }, + expected: { type: "string" }, + }; + + it("collects every occurrence of a repeated flag in order", () => { + const raw = ["173", "--assert", "a.sql", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql", "b.sql"]); + }); + + it("handles the --flag=value form", () => { + const raw = ["--assert=a.sql", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql", "b.sql"]); + }); + + it("collects aliases of the flag", () => { + const raw = ["-a", "a.sql", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql", "b.sql"]); + }); + + it("returns an empty array when the flag is absent", () => { + expect(collectRepeatedFlag(["--expected", "out.csv"], "assert", argsDef)).toEqual([]); + }); + + it("ignores tokens after the -- separator", () => { + const raw = ["--assert", "a.sql", "--", "--assert", "b.sql"]; + expect(collectRepeatedFlag(raw, "assert", argsDef)).toEqual(["a.sql"]); + }); +}); + describe("toAliasArray", () => { it("returns an empty array for an absent alias", () => { expect(toAliasArray(undefined)).toEqual([]); diff --git a/src/runtime/citty.ts b/src/runtime/citty.ts index 9b3606e..6f43850 100644 --- a/src/runtime/citty.ts +++ b/src/runtime/citty.ts @@ -25,6 +25,45 @@ export function normalizeFlag(value: string): string { return value.replace(/^-+/, "").replace(/-/g, "").toLowerCase(); } +// citty (this version) parses repeated string flags via Node's `util.parseArgs` without +// `multiple`, so `--x a --x b` collapses to the last value. For genuinely repeatable flags we +// recover every occurrence straight from rawArgs. Matches the flag's own name and any aliases, +// the `--flag value` and `--flag=value` (and short `-a value`) forms, and stops at `--`. +export function collectRepeatedFlag( + rawArgs: readonly string[], + flagName: string, + argsDef: ArgsDef, +): string[] { + const def = argsDef[flagName]; + const aliases = def !== undefined && "alias" in def ? toAliasArray(def.alias) : []; + const names = new Set([normalizeFlag(flagName), ...aliases.map(normalizeFlag)]); + const values: string[] = []; + for (let i = 0; i < rawArgs.length; i++) { + const token = rawArgs[i]; + if (token === undefined || token === "--") { + break; + } + if (!token.startsWith("-")) { + continue; + } + const equals = token.indexOf("="); + const head = equals === -1 ? token : token.slice(0, equals); + if (!names.has(normalizeFlag(head))) { + continue; + } + if (equals !== -1) { + values.push(token.slice(equals + 1)); + continue; + } + const next = rawArgs[i + 1]; + if (next !== undefined) { + values.push(next); + i += 1; + } + } + return values; +} + export function flagConsumesValue(token: string, argsDef: ArgsDef): boolean { if (token.includes("=")) { return false; diff --git a/src/runtime/command-help.test.ts b/src/runtime/command-help.test.ts index 565ea2f..962932a 100644 --- a/src/runtime/command-help.test.ts +++ b/src/runtime/command-help.test.ts @@ -312,6 +312,8 @@ const ALL_COMMANDS = [ "transform-job run", "transform-job transforms", "transform-job set-active", + "transform-test inputs", + "transform-test run", "transform-tag list", "transform-tag create", "transform-tag update", diff --git a/src/runtime/upload.ts b/src/runtime/upload.ts index e7a3b4f..c07b85b 100644 --- a/src/runtime/upload.ts +++ b/src/runtime/upload.ts @@ -1,10 +1,24 @@ import { readFile } from "node:fs/promises"; import { basename } from "node:path"; -import { ConfigError, isNotFoundError } from "../core/errors"; +import { ConfigError, errorMessage, isNotFoundError } from "../core/errors"; import { fileNotFoundError } from "./input"; +export interface FilePart { + blob: Blob; + filename: string; +} + +export async function readFilePart(path: string, label: string): Promise { + try { + const bytes = await readFile(path); + return { blob: new Blob([bytes]), filename: basename(path) }; + } catch (error) { + throw new ConfigError(`Cannot read ${label} file '${path}': ${errorMessage(error)}`); + } +} + const CSV_CONTENT_TYPE = "text/csv"; export interface CsvFile { diff --git a/tests/e2e/measure.e2e.test.ts b/tests/e2e/measure.e2e.test.ts index 51fb9ad..68b3f9a 100644 --- a/tests/e2e/measure.e2e.test.ts +++ b/tests/e2e/measure.e2e.test.ts @@ -156,7 +156,7 @@ describe.skipIf(skipReason !== null)("measure e2e", () => { }); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Metabase returned 500"); + expect(result.stderr).toContain("Value does not match schema"); expect(result.stdout).toBe(""); }); diff --git a/tests/e2e/segment.e2e.test.ts b/tests/e2e/segment.e2e.test.ts index c9fce1f..7ff5480 100644 --- a/tests/e2e/segment.e2e.test.ts +++ b/tests/e2e/segment.e2e.test.ts @@ -153,7 +153,7 @@ describe("segment e2e", () => { }); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Metabase returned 500"); + expect(result.stderr).toContain("Value does not match schema"); expect(result.stdout).toBe(""); }); diff --git a/tests/e2e/skills.e2e.test.ts b/tests/e2e/skills.e2e.test.ts index d860df3..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(),