Implement @cleanup rollback, and stop --local displacing the app's after-startup microflow - #113
Merged
Merged
Conversation
The @cleanup annotation has documented rollback as its default since the runner shipped, but TestCase.Cleanup was parsed and then used nowhere: every test committed. The after-startup runner had no seam to implement it — tests execute inside the startup action, so there is no context the runner owns. The test endpoint creates that seam, because it builds the IContext each test runs on. The handler now wraps the call in startTransaction()/rollbackTransaction() when the runner asks for it, in a finally so a throwing test — the one most likely to leave half-written data — is rolled back too. @cleanup none commits, for when the writes are the point. Verified against Postgres rather than the endpoint's own claim: a suite with one rollback test and one @cleanup none test, run against an emptied table, leaves exactly the "none" row behind. Same microflow, same run, only the annotation differs. Two failure modes this closes rather than opens: - An unknown strategy (@cleanup rollbak) is now a parse error. Treating it as "not rollback" would leave the data behind while the run still reported a clean pass. Rejected at parse time, so --list catches it and no runtime is booted for a file that cannot run correctly. The .mdl and .md parsers are separate code paths and both are covered — the first version of this only reached one of them. - A rollback that fails is reported per test and summarised at the end, never swallowed. --verbose tags every result [rolled back] / [committed] / [ROLLBACK FAILED]. An endpoint too old to know the parameter is called out specifically, since --attach can meet one. Rollback applies to --local and --attach; Docker keeps committing, and the docs say so. It matters most under --attach, where the database belongs to the developer's running app. Each new test was verified to fail against a stubbed guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
Reported from a real project (mxcli-formula1 findings #19): a suite passed under --attach and failed under --local, with cached-service assertions seeing zero rows. The app loads its cache from an after-startup microflow, and the --local runner displaced that microflow with its own. That was a deliberate choice — a test run wants a known baseline — but it was invisible. The run printed only "After-startup set to MxTest.RegisterEndpoint", never that the user's startup logic had been displaced, so the failure looked like a bug in the code under test. It was also inconsistent: the hosted --test-endpoint path already chained the project's own microflow, which is precisely why the two modes disagreed. --local now chains it too, so a suite behaves the same either way and tests see the app in the state it really boots into. --skip-app-startup opts out for a deterministic empty baseline, and the run always prints which of the two it did: … (registers the endpoint; runs no tests, then runs your MyModule.ASU_Startup) … (registers the endpoint; runs no tests; --skip-app-startup, so … will NOT run) Verified live with a seeding after-startup microflow and a test asserting on its row: PASS chained, FAIL under --skip-app-startup, from an emptied table. Note the startup microflow's writes are not covered by @cleanup rollback — they happen at boot, outside any test's transaction. Also from the same report (#15): mxcli test --list bypassed resolveTestPaths, so a project-relative path resolved for execution but not for listing. Confirmed against the pre-fix binary, which fails with "stat tests/: no such file or directory" on the command that now works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
Brings the branch up to date with main (PR #112) so the PR merges cleanly and CI runs against the current base. No conflicts; the fix-issue.md symptom table merged via the union driver with both sides' rows intact and no duplicates. Full suite green on the merged tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #109. Two changes to
mxcli test, the second from field feedback.1.
@cleanup rollbackis now realThe annotation has documented
rollbackas its default since the runner shipped, butTestCase.Cleanupwas parsed and then used nowhere — every test committed.That was not an oversight so much as a missing seam. The after-startup runner executes tests inside the startup action, so there is no context the runner owns to open a transaction on. The test endpoint creates that seam, because it builds the
IContexteach test runs on.The handler now wraps the call in
startTransaction()/rollbackTransaction()when the runner asks for it, in afinally— so a test that throws, the one most likely to leave half-written data, is rolled back too.@cleanup nonecommits, for when the writes are the point.Verified against Postgres, not the endpoint's own claim. A suite with one rollback test and one
@cleanup nonetest, run against an emptied table:Same microflow, same run, only the annotation differs. The
nonecase is the control that rules out "nothing persisted for an unrelated reason".Two failure modes this closes rather than opens:
@cleanup rollbakwould otherwise mean "don't roll back" — data left behind, run still green, nothing explaining why. Rejected at parse time so--listcatches it and no runtime boots for a file that cannot run correctly. The.mdland.mdparsers are separate code paths and both are covered.--verbosetags each result[rolled back]/[committed]/[ROLLBACK FAILED]. An endpoint too old to know the parameter is called out separately — plausible under--attach, where the host is whatever mxcli started it.Behaviour change worth flagging: tests that previously committed now roll back by default. That makes behaviour match the documented contract, but a suite where one test depends on data seeded by an earlier one will start failing;
@cleanup noneis the escape hatch. Docker is unaffected — it always commits.2.
--localno longer displaces the app's after-startup microflowReported from a real project (mxcli-formula1 findings #19):`` a suite passed under
--attachand failed under `--local`, with cached-service assertions seeing zero rows. The app loads its cache from an after-startup microflow, and the `--local` runner displaced that microflow with its own.It was a deliberate choice — a test run wants a known baseline — but it was invisible: the run printed only
After-startup set to MxTest.RegisterEndpoint, never that the user's startup logic had been displaced, so the failure looked like a bug in the code under test.It was also inconsistent: the hosted
--test-endpointpath already chained the project's own microflow, which is exactly why the two modes disagreed.--localnow chains it too, so a suite behaves the same either way and tests see the app in the state it really boots into.--skip-app-startupopts out for a deterministic empty baseline, and the run always prints which of the two it did:Verified with the finding's own shape — a seeding after-startup microflow and a test asserting on its row: PASS chained, FAIL under
--skip-app-startup, from an emptied table.That control initially passed when it should have failed, which surfaced something worth documenting: the startup microflow's writes are not covered by
@cleanup rollback— they happen at boot, outside any test's transaction. Now stated in the docs.Also from the same report (#15)
mxcli test … --listbypassedresolveTestPaths, so a project-relative path resolved for execution but not for listing. Confirmed against the pre-fix binary:Verification
startTransaction, defaulting to no-rollback, dropping the client parameter, disabling validation).main(Aggregates broke when SET became optional (upstream CI regression) #112) — thefix-issue.mdsymptom table merged via the union driver with both sides' rows intact and no duplicates.Docs
CLI help for
test,mxcli syntax test, thetest-microflowsskill,docs-siterunning-tests, CLAUDE.md, two symptom-table rows, and a worked example atmdl-examples/doctype-tests/cleanup-rollback.test.mdl.Not addressed
The other two items in findings #15 are outside the test runner and untouched here: the
MDL-ODATA01hint omitsCountable/SkipSupported/TopSupported, and.ai-context/skills/goes stale after an mxcli upgrade with no staleness detection.🤖 Generated with Claude Code
https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
Generated by Claude Code