diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 4998ccfbf..5a984b569 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -401,4 +401,7 @@ extracting `OffsetExpression`/`LimitExpression`. | `ALTER MODULE X ADD JAR DEPENDENCY (…)` succeeds, `list jar dependencies` reports it, the build is **green** — and the runtime throws `SQLException: No JDBC driver found in app for URL`. `deployment/build.gradle` has no dependencies block and `find deployment -iname '**'` returns nothing | Not a bad write. Declaring and resolving are **separate steps**: the model records the coordinate, and `mx sync-java-dependencies ` is what downloads it into `vendorlib/`. Studio Pro runs that when you edit Module Settings; nothing headless was running it. Confirmed on 11.12.1 — a full `mxbuild --target=deploy` resolves nothing, and the sync command then fetches the jar | `cmd/mxcli/docker/javadeps.go` (`SyncJavaDependencies`, `UnvendoredJarDependencies`), `cmd/mxcli/cmd_sync_java_deps.go` (`mxcli sync-java-deps [--check]`), `cmd/mxcli/docker/runlocal.go` (vendors before boot), `mdl/executor/cmd_modules.go` (`warnUnvendoredJarDependencies`) | **How to find the missing step**: the reporter's open question was "does mxbuild skip Maven resolution, or does mxcli write it somewhere MxBuild cannot read?" — neither. `strings mx.dll | grep -i dependenc` surfaced `ISyncJavaDependenciesRunner`/`SkipManagedDependencySync`, and `mx --help` listed `sync-java-dependencies`. When a model-level write "works" but the artefact never appears, check whether the **toolset** has a separate command for it before suspecting the write. Wired at three levels so the gap cannot stay silent: the executor says so the moment it writes an unvendored coordinate, `run --local` resolves it before boot, and `--check` exits non-zero as a build gate. Resolution needs network, so every call site is best-effort with an actionable message. Tests `cmd/mxcli/docker/javadeps_test.go`. mxcli-formula1 #12 | | `$Total = 5;` does not parse — `no viable alternative at input '$Total=5'` — while `DECLARE $Total Integer = 0;` does, and so do `$X = HEAD($List)`, `$X = create M.E (…)` and `$X = execute database query …`. The error names the token, not the missing keyword | Assignment existed only as a **prefix** on specific activity statements (`(VARIABLE EQUALS)?` on CALL/CREATE/RETRIEVE/…), plus a `SET $Var = expression` statement. A plain value therefore required `SET`, which nothing in the error or the surrounding syntax suggested | `mdl/grammar/domains/MDLMicroflow.g4` (`setStatement : SET? …`), `cmd/mxcli/syntax/features_microflow.go` | Make the guessable form work rather than improve the error: `SET` is now optional and both spellings produce the same `MfSetStmt`/`ChangeVariableAction`. **Prove a grammar relaxation causes no regressions with a control binary, not by reading**: `git stash` the `.g4`, `make grammar`, build `bin/mxcli-control`, sweep every `mdl-examples/**/*.mdl` with both — 13 scripts fail, the *same* 13, all pre-existing. ANTLR's adaptive prediction picks the activity-prefixed alternatives over `setStatement` on its own; no ordering change was needed. Executed against a real .mpr, mxbuild reports 0 errors. Tests `mdl/visitor/visitor_microflow_bare_assign_test.go` (bare and keyword forms must agree on the AST, not merely both parse). mxcli-formula1 #13 | | `mxcli test tests/ -p app/App.mpr` fails with "no such file or directory" for a `tests/` that sits right next to the `.mpr` | Test paths resolved against the process CWD only. Defensible in isolation, but mxcli otherwise encourages naming the project (`-p`) rather than standing in its directory, and project auto-discovery searches outward — so the two conventions collide and the failure looks like a missing directory | `cmd/mxcli/cmd_test_run.go` (`resolveTestPaths`) | Fall back to project-relative **only when the CWD-relative path does not exist**: a `tests/` in both places must resolve to the one the user is standing in, since silently preferring the project's copy would run the wrong suite. A path that exists in neither is passed through unchanged so the error names what was typed, not a rewritten path the user never mentioned. Tests `cmd/mxcli/cmd_test_run_paths_test.go`. mxcli-formula1 #13 | +| A test annotated `@cleanup rollback` (or with no `@cleanup` at all — rollback is the documented default) still leaves its rows in the database; a misspelled strategy like `@cleanup rollbak` does the same, silently, while the run reports PASS | `TestCase.Cleanup` was parsed and then used nowhere. The after-startup runner had no seam to implement it — tests run inside the startup action, so there is no context the runner owns. The test endpoint creates that seam: it builds the `IContext` each test runs on | `cmd/mxcli/testrunner/endpoint.go` (the handler's execute block), `cmd/mxcli/testrunner/cleanup_strategy.go` | Wrap the call in `ctx.startTransaction()` … `ctx.rollbackTransaction()` in a **finally** (a throwing test is the one most likely to leave half-written data), gated on a `rollback=1` query parameter the client sends per test. Report `rolledBack`/`rollbackError` in the response and warn per test — a rollback that fails silently is worse than none. Reject an unknown `@cleanup` value at **parse** time so `--list` catches it too. Verify against the database, not the endpoint's own claim: run one test with rollback and one with `@cleanup none` in the same suite and query Postgres — the `none` row must be the only survivor | +| A suite passes under `mxcli test --attach` and fails under `--local`, with assertions that depend on startup state (a loaded cache, seeded reference data) seeing zero rows | The `--local` runner pointed after-startup at its own registration microflow and did **not** chain the project's own, so the app's startup logic never ran. It was a deliberate choice (a known baseline) but was invisible: the run printed only `After-startup set to MxTest.RegisterEndpoint`, never that the user's microflow had been displaced | `cmd/mxcli/testrunner/runner.go` (`runEndpoint`), `cmd/mxcli/testrunner/cleanup_strategy.go` (`describeStartup`) | Capture project state **before** generating the endpoint MDL, and pass `state.afterStartup` to `GenerateEndpointMDL` so the generated flow chains it — the hosted `--test-endpoint` path already did this, and the mismatch between the two was the bug. Add `--skip-app-startup` for a deterministic empty baseline, and always print which of the two happened. Note the startup microflow's writes run at boot, outside any test transaction, so `@cleanup rollback` does not undo them. mxcli-formula1 findings #19 | +| `mxcli test tests/ -p app/App.mpr --list` fails with `stat tests/: no such file or directory` while the same command without `--list` runs fine | The `--list` branch passed raw `args` to `ListTests`, bypassing `resolveTestPaths` — so a path relative to the project (rather than the working directory) resolved for execution but not for listing | `cmd/mxcli/cmd_test_run.go` (the `if list` branch) | Pass `resolveTestPaths(args, projectPath)` there too. When a command has two entry points into the same input, check both go through the same path resolution. mxcli-formula1 findings #15 | | After `SET` became optional, `mx check` on a project built from `02-microflow-examples.mdl` reports six errors that no MDL change caused: `[CE0109] "Undefined variable 'ProductList.Price'."` at four Aggregate list activities, and `[CE0015] "Aggregate function must specify a valid attribute."` at the expression-based one. Identical on both engines. `mxcli check` on the same script is silent | Two conversions existed for one syntax. `$Sum = sum($List.Price)` used to reach the dedicated `aggregateListStatement` rule; making `SET` optional put `setStatement` — alternative 5 of ~50 — in front of it, so ANTLR matched the lower-numbered alternative and the statement fell through to `buildSetStatement`'s fallback conversion, which joined list and attribute into one name and dropped the per-item expression entirely. Underneath, `buildListAggregateAsFunction` never appended the expression argument, so the SET path could not have seen it either | `mdl/grammar/domains/MDLMicroflow.g4` (`setStatement` moved LAST in `microflowStatement`), `mdl/visitor/visitor_microflow_statements.go` (`buildSetAggregate` replaces `extractVariableAndAttribute`), `mdl/visitor/visitor_microflow_expression.go` (`buildListAggregateAsFunction` appends the expression argument) | **A permissive alternative belongs last.** `$X = ` overlaps every `VARIABLE EQUALS ` statement in the rule — aggregates, list operations, RANGE — and ANTLR's ALL(*) picks the lowest-numbered alternative that matches, so a new general form silently steals from every specific one above it. **The measurement trap that let this ship**: the `SET?` change was swept with a control binary over every `mdl-examples/**/*.mdl` and found no difference — but with `mxcli check`, which parses and validates and never serializes. This defect lives between the AST and the BSON, where only `exec` + `mx check` can see it. Sweeping with `check` proves the grammar still *parses*; it proves nothing about what the visitor *builds*. For a grammar change, the control sweep must run the integration gate (`go test -tags integration -run TestMxCheck_DoctypeScripts`), not `check`. Fix proven by reverting both halves and watching the new tests fail with the reported symptom. Tests `mdl/visitor/visitor_microflow_aggregate_test.go`. Upstream CI on the ako→mendixlabs sync PR | diff --git a/.claude/skills/mendix/test-microflows.md b/.claude/skills/mendix/test-microflows.md index 8d1534bab..2a2f90ff8 100644 --- a/.claude/skills/mendix/test-microflows.md +++ b/.claude/skills/mendix/test-microflows.md @@ -93,6 +93,48 @@ The markdown format turns your tests into living documentation. | `@throws` | Expect error | `@throws 'validation failed'` | | `@cleanup` | Rollback strategy | `@cleanup rollback` (default) or `@cleanup none` | +### `@cleanup` — what happens to a test's data + +**`rollback` is the default**, so by default a test's database writes do not +survive it. The endpoint opens a transaction around the call and rolls it back +afterwards, including when the test throws. + +```mdl +/** + * @test creating an order does not leak + * @expect $result = 'ok' + */ +$result = CALL MICROFLOW Sales.CreateOrder(Amount = 100); +/ + +/** + * @test seed data the next test needs + * @cleanup none + */ +$result = CALL MICROFLOW Sales.SeedCatalogue(); +/ +``` + +Use `@cleanup none` when the writes are the point — seeding a fixture, or +inspecting the result in the running app afterwards. + +Two things worth knowing: + +- **`--local` only.** Rollback needs the test endpoint, which owns the context + the test runs in. The Docker / `--legacy-runner` path executes tests inside + the after-startup action and has no such seam, so it always commits. +- **A rollback that fails is reported, loudly.** The run prints a `WARNING` per + affected test and a summary line, because the alternative — data left behind + while the suite still says PASS — is the failure mode this annotation exists + to prevent. `--verbose` tags every test with `[rolled back]`, `[committed]` or + `[ROLLBACK FAILED]`. + +A misspelled strategy (`@cleanup rollbak`) is a **parse error**, not a silent +fallback to committing. + +Rollback matters most under `--attach`, where the database is the one your dev +app is using. + --- ## Running Tests @@ -130,8 +172,9 @@ older **after-startup microflow** pattern. 2. Records the project's current after-startup microflow, and whether an `MxTest` module already exists 3. Generates **one `MxTest.Test_` microflow per test**, plus a Java action - that registers an HTTP endpoint, and points after-startup at a microflow whose - only job is to call it — **no test runs during startup** + that registers an HTTP endpoint, and points after-startup at a microflow that + registers it and then **chains your own after-startup microflow** — + **no test runs during startup** 4. Builds and boots the app once 5. Invokes each test by name over HTTP; each returns its own verdict in the response @@ -150,6 +193,34 @@ Two consequences worth knowing when reading a failing run: Each test is a separate microflow with its own variable scope, so `$result` in one test never collides with `$result` in another. +#### Your app's after-startup microflow still runs + +The generated startup flow registers the endpoint and then calls the project's +own after-startup microflow, so tests see the app in the state it actually boots +into — a loaded cache, seeded reference data, whatever your app does. The run +says which happened: + +``` +After-startup set to MxTest.RegisterEndpoint (registers the endpoint; runs no tests, then runs your MyModule.ASU_Startup) +``` + +Pass `--skip-app-startup` when you want an empty, deterministic baseline +instead — the app seeds demo data and your tests assert on counts, say: + +``` +After-startup set to MxTest.RegisterEndpoint (… --skip-app-startup, so MyModule.ASU_Startup will NOT run) +``` + +This is why a suite behaves the same under `--local` and `--attach`. Before it +chained, `--local` ran with the app's startup logic suppressed, and a suite that +depended on startup state passed under `--attach` and failed under `--local` for +reasons unrelated to the code. + +One thing rollback does **not** cover: whatever the startup microflow writes +happens at boot, outside any test's transaction, so `@cleanup rollback` does not +undo it. Under `--local` that lands in the scratch `_test` database; +under `--attach` your app wrote it at its own boot regardless. + #### `--watch`: keep the runtime warm ```bash diff --git a/CLAUDE.md b/CLAUDE.md index 088cd61be..f39bf63e3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -610,7 +610,7 @@ Full syntax tables for all MDL statements (microflows, pages, security, navigati - LSP server with hover, go-to-definition, completion, diagnostics, symbols, folding - VS Code extension (`vscode-mdl`) with context menu commands (Run/Check/Selection) - Docker build integration (`mxcli docker build`) with PAD patching (Phase 1) -- Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`): local test runs go through a **token-guarded HTTP endpoint** registered by a generated Java custom request handler, instead of compiling the suite into the project's after-startup microflow. One microflow per test, resolved by name at request time from `Core.getMicroflowNames()` and invoked with `Core.microflowCall(...).execute(...)` — so a throwing test fails only itself (not the boot), results are returned rather than scraped from the runtime log, and each test has its own variable scope. The handler **survives `reload_model`** (after-startup does not re-run, the JVM is unchanged), which is what makes `--watch` possible: ~30s first run, then ~2s from an edit — to a test *or* to the microflow under test — to a verdict. `--attach` skips even that boot by running against an app already up under `run --local --test-endpoint`, driving that process's serve + admin APIs over loopback; it uses **that app's database**, only ever adds/removes its own test microflows, and refuses a change needing a restart. Security: the handler is **not registered at all** without `MXCLI_TEST_TOKEN` in the runtime env (so a project that kept the `MxTest` module through a failed cleanup is inert in production), the token is constant-time compared, non-loopback callers are refused, `/list` is clamped to the test namespace, and only `MxTest.Test_*` may be invoked. The token reaches the runtime via its environment and is never written into the project. Docker keeps the after-startup runner (`--legacy-runner` selects it locally). Packages: `cmd/mxcli/testrunner/` (`endpoint.go`, `client.go`, `watch.go`, `host.go`, `handshake.go`). See `docs/15-testing/SPIKE_test_endpoint_request_handler.md` +- Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`): local test runs go through a **token-guarded HTTP endpoint** registered by a generated Java custom request handler, instead of compiling the suite into the project's after-startup microflow. Boot registers the endpoint and then **chains the project's own after-startup microflow**, so tests see the app as it really boots (`--skip-app-startup` opts out) — without that, a suite depending on startup state passed under `--attach` and failed under `--local`. One microflow per test, resolved by name at request time from `Core.getMicroflowNames()` and invoked with `Core.microflowCall(...).execute(...)` — so a throwing test fails only itself (not the boot), results are returned rather than scraped from the runtime log, and each test has its own variable scope. Owning the `IContext` is also what finally makes **`@cleanup rollback`** (the annotation's documented default, previously parsed and ignored) real: the handler wraps the call in `startTransaction()`/`rollbackTransaction()`, so a test's writes do not survive it — verified against Postgres, with `@cleanup none` as the in-run control. A rollback that fails is reported per test and summarised, never silent; an unknown strategy is a parse error. The handler **survives `reload_model`** (after-startup does not re-run, the JVM is unchanged), which is what makes `--watch` possible: ~30s first run, then ~2s from an edit — to a test *or* to the microflow under test — to a verdict. `--attach` skips even that boot by running against an app already up under `run --local --test-endpoint`, driving that process's serve + admin APIs over loopback; it uses **that app's database**, only ever adds/removes its own test microflows, and refuses a change needing a restart. Security: the handler is **not registered at all** without `MXCLI_TEST_TOKEN` in the runtime env (so a project that kept the `MxTest` module through a failed cleanup is inert in production), the token is constant-time compared, non-loopback callers are refused, `/list` is clamped to the test namespace, and only `MxTest.Test_*` may be invoked. The token reaches the runtime via its environment and is never written into the project. Docker keeps the after-startup runner (`--legacy-runner` selects it locally). Packages: `cmd/mxcli/testrunner/` (`endpoint.go`, `client.go`, `watch.go`, `host.go`, `handshake.go`). See `docs/15-testing/SPIKE_test_endpoint_request_handler.md` - Warm local dev loop (`mxcli run --local [--watch] [--screenshot]`): Docker-free `mxbuild --serve` + standalone runtime, hot `reload_model` for behavioural changes and restart+DDL for structural ones (chosen from the serve build's `restartRequired`). Bundles the browser client (`web/dist/` via mxbuild's rollup runner, which the serve Deploy target skips) so Mendix 11.x apps render in a browser. `--watch` keeps an incremental rollup bundler hot (CHOKIDAR_USEPOLLING for container fs; ~3-4s page re-bundle, skipped for model-only edits) and watches only model source (`.mpr`+`mprcontents/`). `--ensure-db` provisions the local Postgres + app database if missing; `--setup` does the non-blocking prerequisites (cache mxbuild+runtime, ensure DB) and exits — `mxcli init` wires it into a Claude Code SessionStart hook so a fresh/reaped web session self-bootstraps, and `docs-site/src/tools/bootstrap-prompt.md` is the empty-repo seed prompt. `--screenshot` captures a Playwright PNG each change (pixel-perfect page loop), with `--screenshot-url` deep links (repeatable for multi-page sets, one PNG per page) and `--screenshot-user`/`--screenshot-password` form login (session saved as Playwright storage state, reused via `screenshot --load-storage`). See `docs/11-proposals/PROPOSAL_mxcli_dev_warm_loop.md` - External browser preview (`mxcli run --hub ` + `mxcli tunnel-hub`): the app stays local and reverse-tunnels out over a single 443 connection (embedded chisel) to a static relay, so it is reachable in a browser at a public URL — works from egress-only environments (Claude Code web), verified live through the session's MITM egress proxy. `run --hub` implies `--local`, boots the runtime with `ApplicationRootUrl` set to the assigned URL (so the SPA/`originURI` work under the public origin), resolves the control proxy honouring `NO_PROXY`, and retries forever. `mxcli tunnel-hub --domain ` is the **multi-tenant** relay: a registry keyed by prefix/project/solution/branch/worktree (stable URLs on reconnect) fronts many previews at per-subdomain hosts (`[prefix-]project[-branch].`; main collapses to the project) over one 443 with per-subdomain autocert, a registration API (`/api/register|status|deregister|backends|sessions`), and an availability overview at `hub./` **grouped by Claude Code session** (`/api/sessions`): each session lists the endpoints it exposed and links back to its `claude.ai/code` conversation. Client identity flags: `--hub-prefix`/`--hub-project`/`--hub-solution`/`--hub-branch`/`--hub-worktree` (project + branch auto-detected); `--hub-session` groups a session's endpoints (auto-detected from `CLAUDE_CODE_REMOTE_SESSION_ID`). Past sessions are retained: a durable per-session endpoint history (`--sessions-file`, default `~/.mxcli/hub-sessions.json`) survives restarts and reaping, and is pruned after `--session-retention` (default 30d) — so the overview shows offline sessions too (`SessionLog` in `cmd/mxcli/tunnelhub/sessions.go`). Package: `cmd/mxcli/tunnelhub/`. See `docs/11-proposals/PROPOSAL_mxcli_dev_warm_loop.md` (slices 3–4) - Tunnel-hub GitHub authentication (opt-in, gated on `--github-oauth-client-id`; absent = today's open hub): **viewer plane** — GitHub OAuth web flow + HMAC-signed SSO session cookie (`Domain=.`), owner-checked previews (`--require-auth` default on → 302 to login / 403 non-owner; soft mode filters the listing only), `/api/backends` filtered to the viewer (unauthenticated → 401), admin "signed in as" via `/api/whoami`. **Registration plane** — durable, hashed hub API keys (`--keys-file`, default `~/.mxcli/hub-keys.json`, survive restarts) presented as `X-Hub-Key` → stamps `Backend.Owner`; shared `X-Hub-Secret` still works as an owner-less fallback. **Key issuance** — the hub's `/cli` browser page mints a key from the session cookie (no PAT; the device flow was removed as Claude Code containers block GitHub's device endpoints), rotate-by-default + count + revoke-all; `mxcli auth hub login --token ` is the headless path; `run --hub` reads `MXCLI_HUB_KEY` (env → `~/.mxcli/auth.json`) and degrades to local-only if registration fails. Append-only JSONL audit trail (`--audit-log`, no secrets). Packages: `cmd/mxcli/tunnelhub/` (+`audit/`), `cmd/mxcli/hubauth/`. See `docs/11-proposals/PROPOSAL_hub_authentication.md` diff --git a/cmd/mxcli/cmd_test_run.go b/cmd/mxcli/cmd_test_run.go index 56f059b1c..46b7abcdd 100644 --- a/cmd/mxcli/cmd_test_run.go +++ b/cmd/mxcli/cmd_test_run.go @@ -38,14 +38,26 @@ loop can keep serving the same project while tests run. 1. Parses test files and extracts test blocks with @test/@expect annotations 2. Generates one microflow per test, plus a Java action that registers a token-guarded HTTP endpoint - 3. Boots the app once — startup only registers the endpoint, it runs no tests + 3. Boots the app once — startup registers the endpoint and then runs your own + after-startup microflow, so tests see the app as it really boots. No test + runs during startup 4. Invokes each test by name over HTTP; the verdict comes back in the response 5. Restores original project settings +Your after-startup microflow running is what makes a suite behave the same under +--local and --attach. Pass --skip-app-startup for an empty, deterministic +baseline instead — the run always prints which of the two it did. + Because each test is its own microflow invoked on its own, a test that throws fails only itself instead of ending the run, and results are returned rather than recovered from the runtime log. +It also makes @cleanup real. By default (@cleanup rollback) each test runs in a +transaction the endpoint rolls back afterwards, so its database writes do not +survive — use @cleanup none when the writes are the point. The Docker path +always commits: it runs tests inside the after-startup action and has no +context of its own to roll back. + The endpoint is only reachable from loopback, only with a per-run token passed to the runtime through its environment (never written into your project), and will only ever invoke the generated MxTest.Test_* microflows. With no token in @@ -121,6 +133,7 @@ Examples: legacyRunner, _ := cmd.Flags().GetBool("legacy-runner") watch, _ := cmd.Flags().GetBool("watch") attach, _ := cmd.Flags().GetBool("attach") + skipAppStartup, _ := cmd.Flags().GetBool("skip-app-startup") verbose, _ := cmd.Flags().GetBool("verbose") color, _ := cmd.Flags().GetBool("color") timeoutStr, _ := cmd.Flags().GetString("timeout") @@ -132,8 +145,10 @@ Examples: } if list { - // Just list tests, no execution needed - if err := testrunner.ListTests(args, os.Stdout); err != nil { + // resolveTestPaths here too: listing that cannot find a path execution + // finds is a confusing split, and `mxcli test tests/ -p app/App.mpr + // --list` hit exactly that (mxcli-formula1 findings #15). + if err := testrunner.ListTests(resolveTestPaths(args, projectPath), os.Stdout); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } @@ -147,19 +162,20 @@ Examples: } opts := testrunner.RunOptions{ - ProjectPath: projectPath, - TestFiles: resolveTestPaths(args, projectPath), - SkipBuild: skipBuild, - Local: local, - LegacyRunner: legacyRunner, - Watch: watch, - Attach: attach, - Timeout: timeout, - JUnitOutput: junitOutput, - Verbose: verbose, - Color: color, - Stdout: os.Stdout, - Stderr: os.Stderr, + ProjectPath: projectPath, + TestFiles: resolveTestPaths(args, projectPath), + SkipBuild: skipBuild, + Local: local, + LegacyRunner: legacyRunner, + Watch: watch, + Attach: attach, + SkipAppStartup: skipAppStartup, + Timeout: timeout, + JUnitOutput: junitOutput, + Verbose: verbose, + Color: color, + Stdout: os.Stdout, + Stderr: os.Stderr, } result, err := testrunner.Run(opts) diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index be87beadd..c6fc84421 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -373,6 +373,7 @@ func init() { testRunCmd.Flags().Bool("local", false, "Run on mxcli's local runtime instead of Docker (no daemon needed)") testRunCmd.Flags().Bool("legacy-runner", false, "With --local, run tests from the after-startup microflow and parse the log, instead of over the test endpoint") testRunCmd.Flags().BoolP("watch", "w", false, "With --local, keep the runtime warm and re-run the suite on every test or model change (Ctrl-C to stop)") + testRunCmd.Flags().Bool("skip-app-startup", false, "With --local, do not run the project's own after-startup microflow during the test run (it runs by default, so tests see the app as it really boots)") testRunCmd.Flags().Bool("attach", false, "Run against an app already started with 'mxcli run --local --test-endpoint' instead of booting one (tests hit that app's database)") testRunCmd.Flags().BoolP("verbose", "v", false, "Show all runtime log output") testRunCmd.Flags().BoolP("color", "", false, "Use colored output") diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 85560be1e..5bbab9c68 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -283,6 +283,9 @@ Flags: every test or model change (Ctrl-C to stop) --attach Run against an app already started with 'mxcli run --local --test-endpoint' — no boot at all + --skip-app-startup + With --local, do not run the project's own + after-startup microflow (it runs by default) --legacy-runner With --local: use the old after-startup runner -v, --verbose Show runtime log lines -t, --timeout DUR Runtime startup timeout (default: 5m) @@ -292,13 +295,23 @@ Annotations: @expect $var = value Assert variable equals value @expect $obj/Attr = val Assert entity attribute @throws 'message' Expect error - @cleanup rollback|none Cleanup strategy (default: rollback) + @cleanup rollback|none What happens to the test's database writes. + rollback (the default) wraps the test in a + transaction and rolls it back, so nothing it + wrote survives — including when it throws. + none lets the writes commit. --local only: + the Docker path always commits. An unknown + value is a parse error, not a silent commit. How --local runs tests: one microflow per test, invoked by name over a token-guarded HTTP endpoint the app registers at boot. A test that throws fails only itself, and results are returned rather than scraped from the log. Docker still uses the older after-startup runner. +Boot also runs the project's own after-startup microflow, chained after the +endpoint registration, so tests see the app in the state it really boots into +and a suite behaves the same under --local and --attach. + Cost of a run: cold (--local) ~30s boots a runtime on its own ports + DB warm (--local --watch) ~2s runtime stays up between runs diff --git a/cmd/mxcli/testrunner/cleanup_strategy.go b/cmd/mxcli/testrunner/cleanup_strategy.go new file mode 100644 index 000000000..66c855ba4 --- /dev/null +++ b/cmd/mxcli/testrunner/cleanup_strategy.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "fmt" + "io" + "sort" + "strings" +) + +// Cleanup strategies for the @cleanup annotation. +// +// Rollback is the default and always was — the annotation has documented it +// since the runner shipped. It only became real with the test endpoint: the +// endpoint owns the context each test runs in, so it can open a transaction +// around the call and roll it back. The after-startup runner has no such seam, +// which is why the annotation sat parsed-but-unused. +const ( + // CleanupRollback wraps the test in a transaction and rolls it back, so its + // database writes do not survive. The default. + CleanupRollback = "rollback" + // CleanupNone lets the test's writes commit and persist. + CleanupNone = "none" +) + +// cleanupStrategies is the set of accepted @cleanup values. +var cleanupStrategies = map[string]string{ + CleanupRollback: "wrap the test in a transaction and roll it back (default)", + CleanupNone: "let the test's writes commit and persist", +} + +// validateCleanup rejects an unrecognised @cleanup value. +// +// Silently treating a typo as "not rollback" is the worst outcome available: +// `@cleanup rollbak` would leave the test's data in the database while the run +// still reported a clean pass, and nothing anywhere would say why. An unknown +// value is a mistake in the test file, so it is an error. +func validateCleanup(value string) error { + if value == "" || cleanupStrategies[value] != "" { + return nil + } + valid := make([]string, 0, len(cleanupStrategies)) + for k := range cleanupStrategies { + valid = append(valid, k) + } + sort.Strings(valid) + return fmt.Errorf("unknown @cleanup strategy %q (expected one of: %s)", value, strings.Join(valid, ", ")) +} + +// rollsBack reports whether a test's writes should be rolled back. +// +// An empty strategy means the annotation was absent, which is the default — +// rollback. Anything unrecognised has already been rejected by validateCleanup, +// so this never has to guess. +func rollsBack(tc TestCase) bool { + return tc.Cleanup == "" || tc.Cleanup == CleanupRollback +} + +// reportRollbackFailure explains why a requested rollback did not happen. +// +// Two causes are worth telling apart. The endpoint may not support rollback at +// all — with --attach the app is hosted by whatever mxcli started it, which can +// predate this feature — and that is a different fix from a transaction the +// runtime refused to roll back. +func reportRollbackFailure(w io.Writer, tc TestCase, rr *runResponse) { + switch { + case !rr.RollbackRequested: + fmt.Fprintf(w, " WARNING: %s ran without rollback — the app is hosting an older test endpoint\n"+ + " that ignores it. Restart the hosting 'mxcli run --local --test-endpoint'.\n", tc.Name) + case rr.RollbackError != "": + fmt.Fprintf(w, " WARNING: %s could not be rolled back: %s\n", tc.Name, rr.RollbackError) + default: + fmt.Fprintf(w, " WARNING: %s could not be rolled back (no reason reported)\n", tc.Name) + } +} + +// rollbackNote annotates a verbose result line with what happened to the +// transaction. +func rollbackNote(requested bool, rr *runResponse) string { + switch { + case !requested: + return " [committed]" + case rr.RolledBack: + return " [rolled back]" + default: + return " [ROLLBACK FAILED]" + } +} + +// describeStartup says what the generated after-startup microflow will do, +// naming the project's own microflow when there is one. +// +// This line exists because its absence was a reported trap (mxcli-formula1 +// findings #19). The runner printed only that after-startup had been pointed at +// its own microflow; a reader had no way to tell that their app's startup logic +// — a cache load, in that report — was therefore not going to run. The suite +// passed under --attach, where the app boots normally, and failed under --local +// for reasons that had nothing to do with the code under test. +func describeStartup(appAfterStartup string, skipped bool) string { + base := "After-startup set to " + endpointStartupFlow + " (registers the endpoint; runs no tests" + switch { + case appAfterStartup == "": + return base + "; this project has no after-startup microflow of its own)" + case skipped: + return base + "; --skip-app-startup, so " + appAfterStartup + " will NOT run)" + default: + return base + ", then runs your " + appAfterStartup + ")" + } +} diff --git a/cmd/mxcli/testrunner/cleanup_strategy_test.go b/cmd/mxcli/testrunner/cleanup_strategy_test.go new file mode 100644 index 000000000..ea4e4c35e --- /dev/null +++ b/cmd/mxcli/testrunner/cleanup_strategy_test.go @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRollsBack(t *testing.T) { + tests := []struct { + name string + cleanup string + want bool + }{ + {"absent annotation defaults to rollback", "", true}, + {"explicit rollback", CleanupRollback, true}, + {"explicit none", CleanupNone, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := rollsBack(TestCase{Cleanup: tt.cleanup}); got != tt.want { + t.Errorf("rollsBack(%q) = %v, want %v", tt.cleanup, got, tt.want) + } + }) + } +} + +// TestRollbackIsTheDefault pins the contract the annotation has always +// documented: a test with no @cleanup rolls back. It went unimplemented until +// the endpoint gave the runner a context of its own to open a transaction on. +func TestRollbackIsTheDefault(t *testing.T) { + if !rollsBack(TestCase{}) { + t.Error("a test with no @cleanup annotation does not roll back") + } +} + +func TestValidateCleanup(t *testing.T) { + for _, ok := range []string{"", CleanupRollback, CleanupNone} { + if err := validateCleanup(ok); err != nil { + t.Errorf("validateCleanup(%q) rejected a valid strategy: %v", ok, err) + } + } +} + +// TestValidateCleanupRejectsATypo pins the reason this validation exists at all. +// Treating an unrecognised value as "not rollback" would leave the test's data +// in the database while the run still reported a clean pass — the worst +// available outcome, because nothing anywhere would say why. +func TestValidateCleanupRejectsATypo(t *testing.T) { + err := validateCleanup("rollbak") + if err == nil { + t.Fatal("a misspelled strategy was accepted; it would silently skip the rollback") + } + if !strings.Contains(err.Error(), "rollbak") { + t.Errorf("error %q does not quote the offending value", err) + } + // The message has to say what IS allowed, or the user is left guessing. + for _, want := range []string{CleanupRollback, CleanupNone} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not list the valid strategy %q", err, want) + } + } +} + +// TestParserRejectsABadCleanup pins that the rejection happens at parse time, so +// --list catches it too and no runtime is booted for a test file that cannot be +// run correctly. +func TestParserRejectsABadCleanup(t *testing.T) { + body := `/** + * @test something + * @cleanup rollbak + */ +$r = CALL MICROFLOW Mod.A(); +/ +` + dir := t.TempDir() + path := filepath.Join(dir, "bad.test.mdl") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + if _, err := ParseTestFile(path); err == nil { + t.Fatal("a .test.mdl with a misspelled @cleanup parsed without error") + } +} + +// TestMarkdownParserRejectsABadCleanup covers the other file format — the two +// parsers are separate code paths and the first version of this validation only +// reached one of them. +func TestMarkdownParserRejectsABadCleanup(t *testing.T) { + body := "```mdl-test\n/**\n * @test something\n * @cleanup rollbak\n */\n$r = CALL MICROFLOW Mod.A();\n```\n" + dir := t.TempDir() + path := filepath.Join(dir, "bad.test.md") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + if _, err := ParseTestFile(path); err == nil { + t.Fatal("a .test.md with a misspelled @cleanup parsed without error") + } +} + +func TestEndpointJavaSupportsRollback(t *testing.T) { + for _, want := range []string{ + `"1".equals(request.getParameter("rollback"))`, + "ctx.startTransaction();", + "ctx.rollbackTransaction();", + } { + if !strings.Contains(endpointJava, want) { + t.Errorf("the handler is missing %q", want) + } + } +} + +// TestEndpointRollbackIsInAFinallyBlock pins that a test which throws still gets +// its transaction rolled back. Without the finally, a failing test would be +// exactly the one that leaves its half-written data behind. +func TestEndpointRollbackIsInAFinallyBlock(t *testing.T) { + execute := strings.Index(endpointJava, "Core.microflowCall(mf).execute") + finallyIdx := strings.Index(endpointJava, "} finally {") + rollbackIdx := strings.Index(endpointJava, "ctx.rollbackTransaction();") + + if finallyIdx < 0 { + t.Fatal("the execution is not wrapped in try/finally") + } + if !(execute < finallyIdx && finallyIdx < rollbackIdx) { + t.Errorf("the rollback is not in the finally block after execution (execute=%d finally=%d rollback=%d)", + execute, finallyIdx, rollbackIdx) + } +} + +// TestEndpointStartsTheTransactionBeforeExecuting pins the ordering: a +// transaction opened after the microflow ran would roll back nothing. +func TestEndpointStartsTheTransactionBeforeExecuting(t *testing.T) { + start := strings.Index(endpointJava, "ctx.startTransaction();") + execute := strings.Index(endpointJava, "Core.microflowCall(mf).execute") + if start < 0 || execute < 0 { + t.Fatal("a landmark is missing") + } + if start > execute { + t.Error("the transaction is started after the microflow runs, so it would roll back nothing") + } +} + +// TestEndpointReportsRollbackOutcome pins that a rollback which fails is +// reported rather than swallowed — otherwise the data stays and the run still +// says PASS. +func TestEndpointReportsRollbackOutcome(t *testing.T) { + for _, want := range []string{`\"rolledBack\":`, `\"rollbackRequested\":`, `\"rollbackError\":`} { + if !strings.Contains(endpointJava, want) { + t.Errorf("the response does not carry %s", want) + } + } +} + +func TestReportRollbackFailureDistinguishesCauses(t *testing.T) { + tc := TestCase{Name: "some test"} + + tests := []struct { + name string + resp runResponse + want string + }{ + { + name: "an endpoint that ignores the parameter", + resp: runResponse{RollbackRequested: false}, + want: "older test endpoint", + }, + { + name: "a runtime that refused", + resp: runResponse{RollbackRequested: true, RollbackError: "transaction already ended"}, + want: "transaction already ended", + }, + { + name: "no reason given", + resp: runResponse{RollbackRequested: true}, + want: "no reason reported", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + reportRollbackFailure(&buf, tc, &tt.resp) + if !strings.Contains(buf.String(), tt.want) { + t.Errorf("warning %q does not mention %q", buf.String(), tt.want) + } + if !strings.Contains(buf.String(), tc.Name) { + t.Errorf("warning %q does not name the test", buf.String()) + } + }) + } +} + +func TestRollbackNote(t *testing.T) { + tests := []struct { + name string + requested bool + resp runResponse + want string + }{ + {"committed", false, runResponse{}, "committed"}, + {"rolled back", true, runResponse{RolledBack: true}, "rolled back"}, + {"failed", true, runResponse{}, "ROLLBACK FAILED"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := rollbackNote(tt.requested, &tt.resp); !strings.Contains(got, tt.want) { + t.Errorf("rollbackNote = %q, want it to contain %q", got, tt.want) + } + }) + } +} + +// TestDescribeStartup pins the line that mxcli-formula1 findings #19 asked for. +// The runner used to say only that after-startup had been repointed, leaving no +// way to tell that the app's own startup logic would not run — which produced a +// suite that passed under --attach and failed under --local for reasons +// unrelated to the code. +func TestDescribeStartup(t *testing.T) { + tests := []struct { + name string + app string + skipped bool + want []string + absent []string + }{ + { + name: "chains the project's own microflow by default", + app: "MyModule.ASU_Startup", + want: []string{"then runs your MyModule.ASU_Startup"}, + // It must not read as though the app's startup is being skipped. + absent: []string{"NOT run"}, + }, + { + name: "says plainly when it is skipped", + app: "MyModule.ASU_Startup", + skipped: true, + want: []string{"MyModule.ASU_Startup", "NOT run", "--skip-app-startup"}, + }, + { + name: "says when there is nothing to chain", + app: "", + want: []string{"no after-startup microflow of its own"}, + absent: []string{"NOT run"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := describeStartup(tt.app, tt.skipped) + for _, w := range tt.want { + if !strings.Contains(got, w) { + t.Errorf("message %q does not contain %q", got, w) + } + } + for _, a := range tt.absent { + if strings.Contains(got, a) { + t.Errorf("message %q should not contain %q", got, a) + } + } + }) + } +} + +// TestSkippedStartupNamesTheFlagThatCausedIt keeps the skipped message +// actionable: a reader who did not pass the flag themselves (a script did) can +// still tell why their startup logic is missing. +func TestSkippedStartupNamesTheFlagThatCausedIt(t *testing.T) { + got := describeStartup("Mod.Flow", true) + if !strings.Contains(got, "--skip-app-startup") { + t.Errorf("message %q does not name the flag responsible", got) + } +} diff --git a/cmd/mxcli/testrunner/client.go b/cmd/mxcli/testrunner/client.go index 27253f415..63880f461 100644 --- a/cmd/mxcli/testrunner/client.go +++ b/cmd/mxcli/testrunner/client.go @@ -47,6 +47,13 @@ type runResponse struct { DurationMicros int64 `json:"durationMicros"` Result string `json:"result"` Error string `json:"error"` + // RollbackRequested echoes back whether the runner asked for a rollback, so a + // runner talking to an older endpoint that ignores the parameter can tell. + RollbackRequested bool `json:"rollbackRequested"` + // RolledBack reports that the transaction was actually rolled back. + RolledBack bool `json:"rolledBack"` + // RollbackError is why it was not. + RollbackError string `json:"rollbackError"` } // listResponse is the endpoint's reply to a list request. @@ -111,10 +118,16 @@ func (c *endpointClient) list() ([]string, error) { return lr.Microflows, nil } -// run executes one test microflow and returns the endpoint's reply. -func (c *endpointClient) run(mf string) (*runResponse, error) { +// run executes one test microflow and returns the endpoint's reply. With +// rollback set, the endpoint wraps the call in a transaction it rolls back, so +// the test's database writes do not survive. +func (c *endpointClient) run(mf string, rollback bool) (*runResponse, error) { + params := url.Values{"mf": {mf}} + if rollback { + params.Set("rollback", "1") + } var rr runResponse - if err := c.get("run", url.Values{"mf": {mf}}, &rr); err != nil { + if err := c.get("run", params, &rr); err != nil { return nil, err } return &rr, nil diff --git a/cmd/mxcli/testrunner/client_test.go b/cmd/mxcli/testrunner/client_test.go index a95b660f1..582ecc544 100644 --- a/cmd/mxcli/testrunner/client_test.go +++ b/cmd/mxcli/testrunner/client_test.go @@ -21,6 +21,8 @@ type fakeEndpoint struct { // seenTokens records what each request presented, so a test can assert the // client actually sends the token rather than the server merely allowing it. seenTokens []string + // rollbackParams records the rollback query parameter of each run request. + rollbackParams []string } func (f *fakeEndpoint) handler() http.Handler { @@ -45,6 +47,7 @@ func (f *fakeEndpoint) handler() http.Handler { } json.NewEncoder(w).Encode(listResponse{Microflows: names}) case strings.HasSuffix(r.URL.Path, "/run"): + f.rollbackParams = append(f.rollbackParams, r.URL.Query().Get("rollback")) mf := r.URL.Query().Get("mf") resp, ok := f.flows[mf] if !ok { @@ -215,3 +218,29 @@ func TestWaitReadyGivesUp(t *testing.T) { t.Errorf("error %q does not explain the endpoint never came up", err) } } + +// TestClientSendsTheRollbackParameter pins that the runner's per-test decision +// actually reaches the endpoint. Without the parameter the endpoint commits, and +// a test annotated for rollback would silently leave its data behind. +func TestClientSendsTheRollbackParameter(t *testing.T) { + fake, c := newFakeEndpoint(t, "tok", map[string]runResponse{ + testFlowPrefix + "test_1": {OK: true, Result: verdictPass}, + }) + + if _, err := c.run(testFlowPrefix+"test_1", true); err != nil { + t.Fatalf("run with rollback: %v", err) + } + if _, err := c.run(testFlowPrefix+"test_1", false); err != nil { + t.Fatalf("run without rollback: %v", err) + } + + if len(fake.rollbackParams) != 2 { + t.Fatalf("server saw %d run requests, want 2", len(fake.rollbackParams)) + } + if fake.rollbackParams[0] != "1" { + t.Errorf("rollback run sent rollback=%q, want \"1\"", fake.rollbackParams[0]) + } + if fake.rollbackParams[1] != "" { + t.Errorf("non-rollback run sent rollback=%q, want it absent", fake.rollbackParams[1]) + } +} diff --git a/cmd/mxcli/testrunner/endpoint.go b/cmd/mxcli/testrunner/endpoint.go index 86a2fea14..3f5cb4e30 100644 --- a/cmd/mxcli/testrunner/endpoint.go +++ b/cmd/mxcli/testrunner/endpoint.go @@ -235,10 +235,23 @@ com.mendix.core.Core.addRequestHandler("` + endpointPath + `", new com.mendix.ex return; } + // rollback=1 wraps the call in a transaction this handler owns and rolls + // it back afterwards, so the test's database writes do not survive it. + // The microflow joins that transaction rather than committing its own — + // Mendix contexts carry one transaction, and a nested start/end only + // adjusts its depth, so the outer rollback undoes everything inside. + boolean rollback = "1".equals(request.getParameter("rollback")); + long t0 = System.nanoTime(); com.mendix.systemwideinterfaces.core.IContext ctx = com.mendix.core.Core.createSystemContext(); Object result = null; String error = null; + String rollbackError = null; + boolean rolledBack = false; + + if (rollback) { + ctx.startTransaction(); + } try { result = com.mendix.core.Core.microflowCall(mf).execute(ctx); } catch (Throwable t) { @@ -246,6 +259,21 @@ com.mendix.core.Core.addRequestHandler("` + endpointPath + `", new com.mendix.ex while (root.getCause() != null && root.getCause() != root) root = root.getCause(); String msg = root.getMessage(); error = (msg == null || msg.isEmpty()) ? root.getClass().getName() : msg; + } finally { + if (rollback) { + // A rollback that silently fails leaves the data behind while the + // run still reports a clean pass, so its outcome is reported + // rather than swallowed. A microflow that already threw may have + // ended the transaction itself; that is not an error worth + // failing the test over, but it is worth saying. + try { + ctx.rollbackTransaction(); + rolledBack = true; + } catch (Throwable t) { + String msg = t.getMessage(); + rollbackError = (msg == null || msg.isEmpty()) ? t.getClass().getName() : msg; + } + } } long micros = (System.nanoTime() - t0) / 1000L; @@ -255,6 +283,9 @@ com.mendix.core.Core.addRequestHandler("` + endpointPath + `", new com.mendix.ex b.append(",\"durationMicros\":").append(micros); b.append(",\"result\":").append(result == null ? "null" : esc(String.valueOf(result))); if (error != null) b.append(",\"error\":").append(esc(error)); + b.append(",\"rollbackRequested\":").append(rollback); + b.append(",\"rolledBack\":").append(rolledBack); + if (rollbackError != null) b.append(",\"rollbackError\":").append(esc(rollbackError)); b.append('}'); out.write(b.toString()); out.flush(); diff --git a/cmd/mxcli/testrunner/parser.go b/cmd/mxcli/testrunner/parser.go index ae276ac81..aff53633b 100644 --- a/cmd/mxcli/testrunner/parser.go +++ b/cmd/mxcli/testrunner/parser.go @@ -134,6 +134,10 @@ func parseMDLTests(content string, sourcePath string) ([]TestCase, error) { continue } + if err := validateCleanup(annotations.Cleanup); err != nil { + return nil, fmt.Errorf("%s: test %q: %w", sourcePath, annotations.Test, err) + } + testID := fmt.Sprintf("test_%d", i+1) tests = append(tests, TestCase{ ID: testID, @@ -185,6 +189,10 @@ func parseMarkdownTests(content string, sourcePath string) ([]TestCase, error) { doc, body, _ := extractDocAndBody(blockContent, blockContent) annotations := parseAnnotations(doc) + if err := validateCleanup(annotations.Cleanup); err != nil { + return nil, fmt.Errorf("%s: test at line %d: %w", sourcePath, blockStart, err) + } + testNum++ testID := fmt.Sprintf("test_%d", testNum) diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index ffc8bf459..33e7e903e 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -58,6 +58,16 @@ type RunOptions struct { // then run against that app's database rather than a scratch one. Attach bool + // SkipAppStartup stops the project's own after-startup microflow from running + // during a --local test run. + // + // It normally does run: the generated startup flow registers the endpoint and + // then chains it, so tests see the app in the state it actually boots into. + // Set this when the suite wants an empty, deterministic baseline instead — + // e.g. the app seeds demo data at startup and the tests are asserting on + // counts. + SkipAppStartup bool + // Timeout for runtime startup and test execution. Timeout time.Duration @@ -178,10 +188,21 @@ func runEndpoint(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. return nil, err } + // Capture what cleanup will need to restore, before touching anything. This + // must succeed: without it cleanup cannot tell an existing MxTest module from + // the one it is about to create, nor restore the original after-startup — and + // the generated startup flow needs to know what to chain. + state, err := captureProjectState(opts.ProjectPath) + if err != nil { + return nil, fmt.Errorf("capturing project state: %w", err) + } + fmt.Fprintln(w, "Generating test endpoint and test microflows...") - // "" : a test run wants a known starting state, so the project's own - // after-startup is not chained here (a hosted endpoint does chain it). - endpointMDL := GenerateEndpointMDL("") + chain := state.afterStartup + if opts.SkipAppStartup { + chain = "" + } + endpointMDL := GenerateEndpointMDL(chain) flowsMDL := GenerateTestFlows(suite) if opts.Verbose { @@ -191,14 +212,7 @@ func runEndpoint(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. fmt.Fprintln(w, "--- End MDL ---") } - // Capture what cleanup will need to restore, before touching anything. This - // must succeed: without it cleanup cannot tell an existing MxTest module from - // the one it is about to create, nor restore the original after-startup. fmt.Fprintln(w, "Injecting test endpoint into project...") - state, err := captureProjectState(opts.ProjectPath) - if err != nil { - return nil, fmt.Errorf("capturing project state: %w", err) - } // From here on the project is modified, so every exit runs cleanup. // @@ -235,7 +249,7 @@ func runEndpoint(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. return finish(nil, fmt.Errorf("preparing project for the test run (%s): %w", cmd, err)) } } - fmt.Fprintf(w, " After-startup set to %s (registers the endpoint; runs no tests)\n", endpointStartupFlow) + fmt.Fprintln(w, " "+describeStartup(state.afterStartup, opts.SkipAppStartup)) // --watch keeps the runtime and the build server up and re-runs on every // change, so it owns the loop — including printing each run's results, which diff --git a/cmd/mxcli/testrunner/runner_endpoint.go b/cmd/mxcli/testrunner/runner_endpoint.go index c3d67c0de..969c33d56 100644 --- a/cmd/mxcli/testrunner/runner_endpoint.go +++ b/cmd/mxcli/testrunner/runner_endpoint.go @@ -129,6 +129,9 @@ func runSuite(client *endpointClient, suite *TestSuite, opts RunOptions, w io.Wr result := &SuiteResult{Name: suite.Name, Started: time.Now()} fmt.Fprintf(w, "Running %d test(s) over the test endpoint...\n", len(suite.Tests)) + // leaked counts tests whose requested rollback did not happen. + leaked := 0 + for _, tc := range suite.Tests { flow := testFlowName(tc) if !present[flow] { @@ -141,7 +144,8 @@ func runSuite(client *endpointClient, suite *TestSuite, opts RunOptions, w io.Wr continue } - rr, err := client.run(flow) + rollback := rollsBack(tc) + rr, err := client.run(flow, rollback) if err != nil { // A transport failure is not a verdict. Report it against this test // and keep going; if the runtime died the rest will say so too. @@ -154,13 +158,27 @@ func runSuite(client *endpointClient, suite *TestSuite, opts RunOptions, w io.Wr continue } + // A rollback that was asked for and did not happen leaves the test's data + // in the database while the verdict still says PASS. The test itself is + // not wrong, so its verdict stands — but this must not pass in silence. + if rollback && !rr.RolledBack { + leaked++ + reportRollbackFailure(w, tc, rr) + } + res := toResult(tc, rr) result.Tests = append(result.Tests, res) if opts.Verbose { - fmt.Fprintf(w, " %s %s (%s)\n", res.Status, res.Name, res.Duration.Round(time.Millisecond)) + fmt.Fprintf(w, " %s %s (%s)%s\n", res.Status, res.Name, + res.Duration.Round(time.Millisecond), rollbackNote(rollback, rr)) } } + if leaked > 0 { + fmt.Fprintf(w, "\nWARNING: %d test(s) asked for @cleanup rollback and did not get it — "+ + "their writes are still in the database.\n", leaked) + } + result.Duration = time.Since(result.Started) return result, nil } diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 4d45072d2..e79a9d837 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -87,6 +87,54 @@ every request must present that token, non-loopback callers are refused, and it will only ever invoke the generated `MxTest.Test_*` microflows. The token is never written into your project. +### The app's own after-startup microflow + +Boot registers the endpoint and then runs the project's own after-startup +microflow, so tests see the app in the state it really boots into. The run +prints which of the two happened, and `--skip-app-startup` opts out when a suite +wants an empty, deterministic baseline. + +This keeps a suite behaving the same under `--local` and `--attach`. Note that +what the startup microflow writes is not covered by `@cleanup rollback` — it +runs at boot, outside any test's transaction. + +### `@cleanup`: what happens to a test's data + +`rollback` is the **default**, so a test's database writes do not survive it. +The endpoint opens a transaction around the call and rolls it back afterwards, +including when the test throws. + +```mdl +/** + * @test creating an order does not leak + * @expect $result = 'ok' + */ +$result = CALL MICROFLOW Sales.CreateOrder(Amount = 100); +/ + +/** + * @test seed a fixture the app should keep + * @cleanup none + */ +$result = CALL MICROFLOW Sales.SeedCatalogue(); +/ +``` + +| Strategy | Effect | +|---|---| +| `rollback` (default) | The test's writes are rolled back, even if it throws | +| `none` | The writes commit and persist | + +Rollback needs the test endpoint, so it applies to `--local` and `--attach`. +The Docker / `--legacy-runner` path runs tests inside the after-startup action +and has no context of its own to roll back, so it always commits. + +A rollback that fails is reported per test and summarised at the end — data +left behind while the suite still says PASS is exactly what this is for. +`--verbose` tags every test `[rolled back]`, `[committed]` or +`[ROLLBACK FAILED]`. A misspelled strategy is a parse error, not a silent +commit. + **Docker — the after-startup runner.** The whole suite is compiled into the project's after-startup microflow, the container is restarted, and results are parsed out of its log. `--legacy-runner` selects this on a local run too. diff --git a/mdl-examples/doctype-tests/cleanup-rollback.test.mdl b/mdl-examples/doctype-tests/cleanup-rollback.test.mdl new file mode 100644 index 000000000..204177837 --- /dev/null +++ b/mdl-examples/doctype-tests/cleanup-rollback.test.mdl @@ -0,0 +1,56 @@ +-- ============================================================================ +-- @cleanup rollback — worked example +-- ============================================================================ +-- Demonstrates what happens to a test's database writes. +-- +-- @cleanup rollback (the default) the writes are rolled back +-- @cleanup none the writes commit and persist +-- +-- Rollback needs the test endpoint, which owns the context each test runs in, +-- so it applies to `--local` and `--attach`. The Docker / --legacy-runner path +-- runs tests inside the after-startup action and always commits. +-- +-- Setup — these are the microflows under test: +-- +-- create persistent entity App.Person (FirstName: string(100)); +-- +-- create microflow App.CreatePerson (FirstName: string) +-- returns string as $Stored +-- begin +-- declare $Stored String = ''; +-- $P = create App.Person (FirstName = $FirstName); +-- commit $P; +-- set $Stored = $P/FirstName; +-- return $Stored; +-- end; +-- / +-- +-- Run: mxcli test cleanup-rollback.test.mdl -p app.mpr --local --verbose +-- +-- --verbose tags each result [rolled back] / [committed], and afterwards only +-- the PersistedProbe row is in the database. +-- ============================================================================ + +/** + * @test the default is rollback — this Person does not survive the run + * @expect $result = 'RollbackProbe' + */ +$result = CALL MICROFLOW App.CreatePerson(FirstName = 'RollbackProbe'); +/ + +/** + * @test stating rollback explicitly does the same thing + * @expect $result = 'ExplicitRollbackProbe' + * @cleanup rollback + */ +$result = CALL MICROFLOW App.CreatePerson(FirstName = 'ExplicitRollbackProbe'); +/ + +/** + * @test @cleanup none commits — the control that proves rollback is doing the + * work above, and the way to seed a fixture you want to keep + * @expect $result = 'PersistedProbe' + * @cleanup none + */ +$result = CALL MICROFLOW App.CreatePerson(FirstName = 'PersistedProbe'); +/