diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-test-callers-apply-entity-access.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-test-callers-apply-entity-access.json new file mode 100644 index 0000000000..6360f2c3ab --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-test-callers-apply-entity-access.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-11-11", "symptom": "ako/mxcli#2100 (mendixlabs/mxcli#1805): a .test.mdl test calling a microflow with Apply entity access = Yes failed CE0114 at build ('A microflow that does not apply entity access can only call microflows that also do not apply entity access') and MDL-SEC22 at check; the same for an applying @setup. Making the generated test microflow apply entity access then failed CE0150 on the runner's own handlers ('attributes of System.Error … cannot be accessed in microflows that apply entity access').", "cause": "Three generators wrote callers without the setting: writeTestFlow (endpoint), GenerateTestRunner (legacy MxTest.TestRunner) and CheckSource (MxTest.Check_* wrappers). The runner's verdict and @throws handlers read $latestError/Message directly, which an applying flow may not.", "file": "cmd/mxcli/testrunner/caller_entity_access.go (callerAppliesEntityAccess, errorMessageFlow helper), generator_endpoint.go (attachOnError numbers its handlers), generator.go, check_source.go, runner.go (legacy cleanup), build_attribution.go", "insight": "CE0114 has one direction, so the safe constant is 'apply' (the same call RegisterEndpoint made for #1654) — but the setting has a second, opposite constraint (CE0150), and the fix has to be measured against both: mx check on 11.12.1 at Off and Production showed an applying flow reading an ordinary entity's attribute is 0 errors, so CE0150 is the only cost. Mendix's CE0150 text prescribes the helper. The runtime half needs no measurement because the endpoint runs tests under Core.createSystemContext. A test body that reads $latestError/ itself is the one case 'apply' would newly break, so it keeps the old setting; under the legacy runner that decision is per suite because all tests share one microflow. The reported fix named two generators; the legacy runner was a third.", "refs": ["ako/mxcli#2100", "mendixlabs/mxcli#1805", "ako/mxcli#1654"], "test": "cmd/mxcli/testrunner/caller_entity_access_test.go; mdl-examples/bug-tests/test-runner-2100-*"} diff --git a/.claude/skills/mendix/test-microflows/SKILL.md b/.claude/skills/mendix/test-microflows/SKILL.md index 36ae31be66..7214f02fb4 100644 --- a/.claude/skills/mendix/test-microflows/SKILL.md +++ b/.claude/skills/mendix/test-microflows/SKILL.md @@ -16,14 +16,14 @@ description: "Write and run MDL-based microflow tests with `mxcli test` — anno | 190-232 | Test File Formats | | 233-248 | string Operations | | 249-272 | Running Tests | -| 273-470 | How It Works | -| 471-509 | Check a test file before you run it | -| 510-631 | Writing Good Tests | -| 632-646 | Test File Organization | -| 647-659 | Interpreting Failures | -| 660-681 | CI Integration | -| 682-688 | Related Skills | -| 689-693 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | +| 273-471 | How It Works | +| 472-510 | Check a test file before you run it | +| 511-632 | Writing Good Tests | +| 633-647 | Test File Organization | +| 648-660 | Interpreting Failures | +| 661-682 | CI Integration | +| 683-689 | Related Skills | +| 690-694 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | # Test Microflows Skill @@ -325,11 +325,12 @@ 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. -`MxTest.RegisterEndpoint` applies entity access, so it can call a startup -microflow that does or does not (CE0114 only forbids a non-applying microflow -calling an applying one); yours still runs with its own setting. A build error -located in `MxTest.RegisterEndpoint` or `MxTest.RegisterTestEndpoint` is reported -as an mxcli defect, not as an error in your project. +`MxTest.RegisterEndpoint` and the generated test microflows apply entity +access, so they can call a microflow that does or does not (CE0114 only forbids +a non-applying microflow calling an applying one); yours still runs with its own +setting. Exception: a test whose own error handler reads `$latestError/…` gets a +flow that does not apply it, since an applying one may not (CE0150). A build +error in an `MxTest` document mxcli generated is reported as an mxcli defect. One thing rollback does **not** cover: whatever the startup microflow writes happens at boot, outside any test's transaction, so `@cleanup rollback` does not diff --git a/CHANGELOG.md b/CHANGELOG.md index d5af6761be..6243995019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A test can call a microflow that applies entity access** (ako/mxcli#2100, mendixlabs/mxcli#1805) — the generated test microflows, the legacy runner's `MxTest.TestRunner` and the wrappers `mxcli check` sees did not apply entity access, so a test (or its `@setup`) calling a production microflow that does failed with CE0114 at build time and MDL-SEC22 at check time, and the only workaround was weakening the production microflow. They now apply it, as `MxTest.RegisterEndpoint` already did. An applying microflow may not read `$latestError/Message` (CE0150), so the runner's error handlers read the message through a generated helper, `MxTest.Test_ErrorMessage`, that does not apply entity access and is removed with the test microflows; failure messages and `@throws` matching are unchanged. A test whose own error handler reads `$latestError/…` keeps a non-applying microflow, so it builds as before. Tests run in a system context, so nothing changes at run time. - **`run --local --watch` applies a change in about 5 s instead of 20-28 s on Mendix versions that bundle the client themselves** (ako/mxcli#2150) — measured on a blank 11.16.0 app: a microflow edit took 20-22 s and a page edit 25-28 s with a runtime restart that dropped every browser session, because each change cost a fresh `mxbuild --serve` and a cold build. mxbuild deletes the client source a build wrote, so its next build bundled a client without pages. That source is now kept while a build runs and put back before the next one: the incremental build takes about 2 s, a page edit is a reload, and an open browser tab shows it without a restart or a new login. The full build remains the fallback when the bundle still comes out incomplete. A deleted page's module stays in `dist/pages`, unused, until the next full build. On Windows the copies are read without blocking mxbuild's own deletes, and if a build still fails because `deployment/web` was in use, the kept source is given up for that run and builds go back to the full rebuild. - **`check` accepts the data-container names Studio Pro stores in a grid row, a control bar and a combo box option** (ako/mxcli#2109) — `describe page` → `exec` of four FactoryManagement pages was refused on its own output (MDL-BUTTON02, MDL-PAGEARG01): a show page in a selectable data grid's column passing the grid's selection (`$grid1`), a control-bar button passing the data view around its grid, and a combo box's option content reading the data view around the combo box. All three build in Studio Pro's own pages. `check` and `exec` now accept them and write what Studio Pro stores, and the pages report `Unchanged page`. - **A binding to an outer data view keeps that data view** (ako/mxcli#2107) — a markdown viewer or an attribute-based visibility condition inside a nested data view, bound to the outer one's object, was described as a bare qualified name and rewritten against the nearest data view: CE7247, or "has no attribute". `$dvOuter.Attr` and `$dvOuter/Assoc/Attr` now work on a pluggable widget's attribute property and in `Visible: … in (…)`, as on input widgets, and `describe` prints them; the six FactoryManagement pages report `Unchanged page`. diff --git a/cmd/mxcli/testrunner/build_attribution.go b/cmd/mxcli/testrunner/build_attribution.go index e350428285..fc42c5912a 100644 --- a/cmd/mxcli/testrunner/build_attribution.go +++ b/cmd/mxcli/testrunner/build_attribution.go @@ -227,6 +227,7 @@ func buildFailureHint(other []docker.BuildProblem) string { var runnerDocuments = map[string]string{ endpointStartupFlow: "to register the test endpoint at startup and then run your after-startup microflow", endpointRegisterAction: "to register the test endpoint", + errorMessageFlow: "to read a raised error's message for the test microflows", } // runnerDocumentPattern matches the `document` MxBuild reports — the document's diff --git a/cmd/mxcli/testrunner/caller_entity_access.go b/cmd/mxcli/testrunner/caller_entity_access.go new file mode 100644 index 0000000000..e8e6f90d5d --- /dev/null +++ b/cmd/mxcli/testrunner/caller_entity_access.go @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 + +// The generated test callers apply entity access (ako/mxcli#2100, upstream +// mendixlabs/mxcli#1805). +// +// A test calls the project's microflows, and MxBuild refuses a microflow that +// does not apply entity access calling one that does (CE0114). The callers — +// each endpoint test microflow, the legacy runner's MxTest.TestRunner, and the +// MxTest.Check_* wrappers `mxcli check` and the editor see — were written +// without the setting, so a test could not call a production microflow that +// applies it, and changing the production microflow to suit the harness is no +// fix. The rule has one direction (MDL-SEC22's table; RegisterEndpoint applies +// entity access for the same reason, ako/mxcli#1654), so an applying caller +// may call either kind. +// +// Applying entity access costs one thing, measured with mx check on 11.12.1 at +// security Off and Production: the flow may not read an attribute of +// System.Error or System.SoapFault (CE0150). The generated handlers read the +// raised message, so they pass $latestError to errorMessageFlow, a helper that +// does not apply entity access — what CE0150's own text prescribes. An applying +// flow reading an ordinary entity's attribute is 0 errors at both levels, so +// nothing else in a test body is affected. +// +// Nothing changes at run time: every test runs under a system context +// (Core.createSystemContext), which entity access does not restrict. +package testrunner + +import ( + "regexp" + "strconv" + "strings" + + "github.com/mendixlabs/mxcli/mdl/langver" +) + +const applyEntityAccessAnnotation = "@applyentityaccess" + +// callerAppliesEntityAccess reports whether the generated caller of these +// tests applies entity access: yes, unless a test body itself reads an error +// object's attributes. Such a body — a custom handler inspecting $latestError — +// builds today in a non-applying caller and would fail CE0150 in an applying +// one, so it keeps the setting it had. A test that both reads the error and +// calls an applying microflow cannot be served by either setting; it has to +// pass the error to a non-applying microflow of its own, as Mendix says. +func callerAppliesEntityAccess(tests ...TestCase) bool { + for _, tc := range tests { + if readsErrorAttributes(tc.MDL, tc.Version) { + return false + } + } + return true +} + +// errorAttributeAccess is a member read on the variables Mendix binds to a +// raised error inside a handler, matched without regard to case so no spelling +// the model accepts slips past: a miss would cost the test its build. +var errorAttributeAccess = regexp.MustCompile(`(?i)\$latest(error|soapfault)\s*/`) + +// readsErrorAttributes reports whether a test body reads an attribute of the +// error being handled, outside string literals and comments. +func readsErrorAttributes(body string, v langver.Version) bool { + return errorAttributeAccess.MatchString(maskLiteralsAndComments(body, v)) +} + +// maskLiteralsAndComments blanks the text inside string literals and comments, +// with the same rules as scanLiteralState, so a pattern matched against the +// result only sees code. +func maskLiteralsAndComments(body string, v langver.Version) string { + b := []byte(body) + inString, inLineComment, inBlockComment := false, false, false + for i := 0; i < len(b); i++ { + c := b[i] + switch { + case c == '\n': + inLineComment = false + case inLineComment: + b[i] = ' ' + case inBlockComment: + if c == '*' && i+1 < len(b) && b[i+1] == '/' { + inBlockComment = false + b[i], b[i+1] = ' ', ' ' + i++ + continue + } + b[i] = ' ' + case inString: + switch { + case c == '\\' && v < langver.V1 && i+1 < len(b): + b[i] = ' ' + if b[i+1] != '\n' { + b[i+1] = ' ' + } + i++ + case c == '\'': + inString = false + default: + b[i] = ' ' + } + case c == '\'': + inString = true + case c == '-' && i+1 < len(b) && b[i+1] == '-': + inLineComment = true + b[i] = ' ' + case c == '/' && i+1 < len(b) && b[i+1] == '*': + inBlockComment = true + b[i], b[i+1] = ' ', ' ' + i++ + } + } + return string(b) +} + +// suiteHasFlows reports whether a generated script writes any test microflow, +// and so needs errorMessageFlow: a test whose @expect did not compile gets none. +func suiteHasFlows(suite *TestSuite) bool { + for _, tc := range suite.Tests { + if len(tc.AssertionErrors) == 0 { + return true + } + } + return false +} + +// writeErrorMessageFlow defines errorMessageFlow. It does not apply entity +// access — that is its whole purpose. It is written with the test microflows +// rather than with the endpoint, so a run attached to an endpoint an older +// mxcli installed still has it, and it is removed with them. +func writeErrorMessageFlow(b *strings.Builder) { + b.WriteString("/** Returns a raised error's message to a test microflow, which applies entity access and may not read it (CE0150). */\n") + b.WriteString(createFlow + " " + errorMessageFlow + " (RaisedError: System.Error)\n") + b.WriteString("RETURNS String AS $Message\n") + b.WriteString("BEGIN\n") + b.WriteString(" DECLARE $Message String = $RaisedError/Message;\n") + b.WriteString(" RETURN $Message;\n") + writeFlowEnd(b) +} + +// errorMessageVar names the variable the nth handler of a test reads the +// message into. scope keeps the legacy runner's tests, which share one +// microflow, apart; the endpoint's tests each have their own. +func errorMessageVar(scope string, n int) string { + return "$MxTestErrorMessage" + scope + "_" + strconv.Itoa(n) +} + +// readErrorMessage is the handler line that puts the message of the error +// being handled in msgVar. +func readErrorMessage(msgVar string) string { + return " " + msgVar + " = CALL MICROFLOW " + errorMessageFlow + "(RaisedError = $latestError);" +} diff --git a/cmd/mxcli/testrunner/caller_entity_access_test.go b/cmd/mxcli/testrunner/caller_entity_access_test.go new file mode 100644 index 0000000000..e563c60a6f --- /dev/null +++ b/cmd/mxcli/testrunner/caller_entity_access_test.go @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "regexp" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// The generated test callers apply entity access, and read a raised error's +// message through a helper that does not (ako/mxcli#2100, mendixlabs/mxcli#1805). +// +// Measured with mx check on 11.12.1, at security Off and Production alike: +// +// - a microflow that does not apply entity access calling one that does is +// CE0114, so a test could not call a production microflow that applies it; +// - a microflow that applies entity access reading $latestError/Message is +// CE0150 ("attributes of System.Error … cannot be accessed in microflows +// that apply entity access. Pass the object to a microflow that does not +// apply entity access if you want to inspect it."); +// - an applying flow passing $latestError to a non-applying helper that +// returns $Error/Message is 0 errors — Mendix's own prescription. +// +// The tests run under a system context, which entity access does not restrict, +// so the setting changes what compiles and nothing about what a test sees. + +const callerSuiteSrc = `mdl 1; +/** + * @test calls an applying microflow + * @expect $ok + */ +$ok = call microflow MyModule.ACT_Applying(); +$ok2 = call microflow MyModule.ACT_Applying(); +/ + +/** + * @test throws with a message + * @throws 'must be positive' + */ +$r = call microflow MyModule.Apply(Amount = -5.0); +/ +` + +func callerSuite(t *testing.T, src string) *TestSuite { + t.Helper() + tests, err := parseMDLTests(src, "t.test.mdl") + if err != nil { + t.Fatalf("parse: %v", err) + } + return &TestSuite{Name: "t", Tests: tests} +} + +// microflowsIn parses a generated script and returns its microflows by name. +func microflowsIn(t *testing.T, mdl string) map[string]*ast.CreateMicroflowStmt { + t.Helper() + prog, errs := visitor.Build(mdl) + if len(errs) > 0 { + t.Fatalf("generated MDL does not parse: %v\n%s", errs[0], mdl) + } + flows := map[string]*ast.CreateMicroflowStmt{} + for _, s := range prog.Statements { + if mf, ok := s.(*ast.CreateMicroflowStmt); ok { + flows[mf.Name.String()] = mf + } + } + return flows +} + +func applies(mf *ast.CreateMicroflowStmt) bool { + return mf.ApplyEntityAccess != nil && *mf.ApplyEntityAccess +} + +// outsideHelper is the script with the helper's own definition removed: the +// helper is the one place that may read an error's attributes. +func outsideHelper(mdl string) string { + start := strings.Index(mdl, createFlow+" "+errorMessageFlow) + if start < 0 { + return mdl + } + end := strings.Index(mdl[start:], "END;") + return mdl[:start] + mdl[start+end:] +} + +var errorAttributeRead = regexp.MustCompile(`\$latest(Error|SoapFault)/`) + +func TestEndpointTestFlowsApplyEntityAccess(t *testing.T) { + suite := callerSuite(t, callerSuiteSrc) + mdl := GenerateTestFlows(suite) + flows := microflowsIn(t, mdl) + for _, tc := range suite.Tests { + mf := flows[testFlowName(tc)] + if mf == nil { + t.Fatalf("no %s in:\n%s", testFlowName(tc), mdl) + } + if !applies(mf) { + t.Errorf("%s does not apply entity access, so it cannot call a microflow that does (CE0114)", testFlowName(tc)) + } + } + helper := flows[errorMessageFlow] + if helper == nil { + t.Fatalf("no %s in:\n%s", errorMessageFlow, mdl) + } + if applies(helper) { + t.Errorf("%s applies entity access, so it cannot read the error's message either (CE0150)", errorMessageFlow) + } + if n := strings.Count(mdl, createFlow+" "+errorMessageFlow); n != 1 { + t.Errorf("%s is defined %d times, want once", errorMessageFlow, n) + } + if m := errorAttributeRead.FindString(outsideHelper(mdl)); m != "" { + t.Errorf("an applying test flow reads %q directly (CE0150):\n%s", m, mdl) + } + // The verdict and the @throws comparison still carry the message. + for _, want := range []string{ + "CALL MICROFLOW " + errorMessageFlow + "(RaisedError = $latestError)", + "'" + verdictFailPrefix + "exception during execution: ' + $MxTestErrorMessage_1", + "contains($MxTestErrorMessage_1, 'must be positive')", + } { + if !strings.Contains(mdl, want) { + t.Errorf("generated flows do not contain %q:\n%s", want, mdl) + } + } +} + +// Every handler reads the message into its own variable: a body with two +// calls gets two handlers in one microflow. +func TestErrorMessageVariablePerHandler(t *testing.T) { + mdl := GenerateTestFlows(callerSuite(t, callerSuiteSrc)) + for _, v := range []string{"$MxTestErrorMessage_1 = CALL", "$MxTestErrorMessage_2 = CALL"} { + if !strings.Contains(mdl, v) { + t.Errorf("want %q in:\n%s", v, mdl) + } + } +} + +func TestLegacyTestRunnerAppliesEntityAccess(t *testing.T) { + mdl := GenerateTestRunner(callerSuite(t, callerSuiteSrc)) + flows := microflowsIn(t, mdl) + runner := flows[mxTestRunner] + if runner == nil { + t.Fatalf("no %s in:\n%s", mxTestRunner, mdl) + } + if !applies(runner) { + t.Errorf("%s does not apply entity access (CE0114)", mxTestRunner) + } + if helper := flows[errorMessageFlow]; helper == nil || applies(helper) { + t.Errorf("want a non-applying %s, got %+v", errorMessageFlow, helper) + } + if m := errorAttributeRead.FindString(outsideHelper(mdl)); m != "" { + t.Errorf("the applying runner reads %q directly (CE0150):\n%s", m, mdl) + } + // Two tests share the runner, so their message variables must differ. + if !strings.Contains(mdl, "contains($MxTestErrorMessage_2_1, 'must be positive')") { + t.Errorf("the @throws comparison does not read the helper's result:\n%s", mdl) + } +} + +func TestCheckWrapperAppliesEntityAccess(t *testing.T) { + cs, err := CheckSource(callerSuiteSrc, "t.test.mdl") + if err != nil { + t.Fatal(err) + } + flows := microflowsIn(t, cs.MDL) + n := 0 + for name, mf := range flows { + if !strings.HasPrefix(name, mxTestModule+".Check_") { + continue + } + n++ + if !applies(mf) { + t.Errorf("%s does not apply entity access, so check reports MDL-SEC22 for a call the run accepts", name) + } + } + if n != 2 { + t.Errorf("found %d check wrappers, want 2:\n%s", n, cs.MDL) + } +} + +// A test whose own handler reads the error's attributes keeps a caller that +// does not apply entity access: applying it would fail that read (CE0150) in a +// test that builds today. The control is the same text inside a string, which +// reads nothing. +func TestBodyReadingErrorAttributesKeepsCallerNonApplying(t *testing.T) { + const reads = `mdl 1; +/** + * @test inspects the error itself + * @expect $msg != empty + */ +declare $msg String = empty; +$r = call microflow MyModule.Fail() on error without rollback begin + set $msg = $latestError/Message; +end error; +/ +` + control := strings.Replace(reads, "set $msg = $latestError/Message;", "set $msg = '$latestError/Message';", 1) + + for _, c := range []struct { + name, src string + want bool + }{{"reads the error", reads, false}, {"control: only a string", control, true}} { + t.Run(c.name, func(t *testing.T) { + suite := callerSuite(t, c.src) + if got := applies(microflowsIn(t, GenerateTestFlows(suite))[testFlowName(suite.Tests[0])]); got != c.want { + t.Errorf("endpoint test flow applies entity access = %v, want %v", got, c.want) + } + if got := applies(microflowsIn(t, GenerateTestRunner(suite))[mxTestRunner]); got != c.want { + t.Errorf("legacy runner applies entity access = %v, want %v", got, c.want) + } + cs, err := CheckSource(c.src, "t.test.mdl") + if err != nil { + t.Fatal(err) + } + for name, mf := range microflowsIn(t, cs.MDL) { + if got := applies(mf); got != c.want { + t.Errorf("check wrapper %s applies entity access = %v, want %v", name, got, c.want) + } + } + }) + } +} + +// The helper is generated for the run, so cleanup removes it with the test +// flows, and a build error in it is mxcli's. +func TestErrorMessageFlowIsCleanedUpAndAttributed(t *testing.T) { + got := generatedTestFlowNames([]string{"Test_test_1", strings.TrimPrefix(errorMessageFlow, mxTestModule+"."), "MyOwnFlow"}) + if strings.Join(got, ",") != testFlowPrefix+"test_1,"+errorMessageFlow { + t.Errorf("discovered for cleanup: %v", got) + } + if names := suiteTestFlowNames(suiteOf("test_1")); !containsString(names, errorMessageFlow) { + t.Errorf("the fallback drop list misses %s: %v", errorMessageFlow, names) + } + if cmds := cleanupCommands(projectState{}, true); !containsString(cmds, "DROP MICROFLOW "+errorMessageFlow) { + t.Errorf("legacy cleanup in a pre-existing MxTest module does not drop %s: %v", errorMessageFlow, cmds) + } + + ce := problem("CE0109", "Undefined variable 'x'.", "MxTest", "Microflow '"+strings.TrimPrefix(errorMessageFlow, mxTestModule+".")+"'", "End event") + _, err := resultsForBuildFailure(buildFailure(failedBuild(ce)), suiteOf("test_1")) + if err == nil || !strings.Contains(err.Error(), "defect in mxcli") || strings.Contains(err.Error(), "left behind") { + t.Errorf("an error in %s is not reported as mxcli's: %v", errorMessageFlow, err) + } +} + +func containsString(list []string, s string) bool { + for _, x := range list { + if x == s { + return true + } + } + return false +} diff --git a/cmd/mxcli/testrunner/check_source.go b/cmd/mxcli/testrunner/check_source.go index 96f67429f6..60910c2a86 100644 --- a/cmd/mxcli/testrunner/check_source.go +++ b/cmd/mxcli/testrunner/check_source.go @@ -119,7 +119,13 @@ func CheckSource(content, path string) (CheckedSource, error) { // `/` is refused under mdl 1, and under mdl 0 it drew an MDL-V1-SLASH // warning for a separator that is the test format's, not a terminator // the author wrote. - place(out, first-1, fmt.Sprintf("CREATE OR MODIFY MICROFLOW %s.%s () BEGIN", mxTestModule, checkFlowName(tc, i))) + // The wrapper applies entity access as the run's caller does, on the + // same line so no body line moves; see callerAppliesEntityAccess. + wrapper := fmt.Sprintf("CREATE OR MODIFY MICROFLOW %s.%s () BEGIN", mxTestModule, checkFlowName(tc, i)) + if callerAppliesEntityAccess(tc) { + wrapper = applyEntityAccessAnnotation + " " + wrapper + } + place(out, first-1, wrapper) place(out, first+len(body), "END;") } diff --git a/cmd/mxcli/testrunner/cleanup_leftovers.go b/cmd/mxcli/testrunner/cleanup_leftovers.go index 6f68ac1608..8a66b29c46 100644 --- a/cmd/mxcli/testrunner/cleanup_leftovers.go +++ b/cmd/mxcli/testrunner/cleanup_leftovers.go @@ -95,6 +95,9 @@ func suiteTestFlowNames(suite *TestSuite) []string { for _, tc := range suite.Tests { names = append(names, testFlowName(tc)) } + if len(names) > 0 { + names = append(names, errorMessageFlow) + } return names } diff --git a/cmd/mxcli/testrunner/endpoint.go b/cmd/mxcli/testrunner/endpoint.go index bec837e824..63cfdaa61d 100644 --- a/cmd/mxcli/testrunner/endpoint.go +++ b/cmd/mxcli/testrunner/endpoint.go @@ -31,6 +31,12 @@ const ( endpointStartupFlow = mxTestModule + ".RegisterEndpoint" // testFlowPrefix prefixes every generated per-test microflow. testFlowPrefix = mxTestModule + ".Test_" + // errorMessageFlow returns a raised error's message to a generated test + // caller, which applies entity access and so may not read it itself + // (CE0150, ako/mxcli#2100). It carries the generated prefix so every path + // that finds and removes the generated flows removes it too, and no test ID + // (test_N) can produce its name. + errorMessageFlow = testFlowPrefix + "ErrorMessage" ) // newEndpointToken returns a fresh 256-bit token as hex. diff --git a/cmd/mxcli/testrunner/generator.go b/cmd/mxcli/testrunner/generator.go index bae60d8d9a..423775edff 100644 --- a/cmd/mxcli/testrunner/generator.go +++ b/cmd/mxcli/testrunner/generator.go @@ -112,6 +112,15 @@ func GenerateTestRunner(suite *TestSuite) string { // Module creation (idempotent — only if MxTest module doesn't exist) b.WriteString("CREATE MODULE MxTest;\n\n") + if suiteHasFlows(suite) { + writeErrorMessageFlow(&b) + b.WriteString("\n") + } + // One microflow runs every test, so it applies entity access only if all of + // them allow it; see callerAppliesEntityAccess. + if callerAppliesEntityAccess(suite.Tests...) { + b.WriteString(applyEntityAccessAnnotation + "\n") + } b.WriteString(createFlow + " MxTest.TestRunner ()\n") b.WriteString("RETURNS Boolean AS $AllPassed\n") b.WriteString("BEGIN\n") @@ -224,7 +233,7 @@ func writeThrowsTestBlock(b *strings.Builder, tc TestCase, suffix string) { renamedMDL := renameVariables(tc.MDL, varNames, suffix) lines := strings.Split(renamedMDL, "\n") - rewritten := rewriteForThrowsTest(lines, didThrowVar, tc.Throws, tc.Version) + rewritten := rewriteForThrowsTest(lines, didThrowVar, suffix, tc.Throws, tc.Version) writeBodyLines(b, rewritten, tc.Version) // Check that it did throw @@ -373,12 +382,14 @@ func renameExpect(exp Expect, names map[string]bool, suffix string) Expect { // with a return value (required when the microflow returns a type). Which calls // get one is attachOnError's decision, shared with the endpoint runner. func rewriteWithErrorHandling(lines []string, testID string, v langver.Version) []string { - return attachOnError(lines, v, []string{ - fmt.Sprintf(" LOG ERROR NODE 'MXTEST' 'MXTEST:FAIL:%s:Exception during execution';", - escapeMDLString(testID)), - " SET $TestFailed = true;", - " SET $AllPassed = false;", - " RETURN $AllPassed;", + return attachOnError(lines, v, func(int) []string { + return []string{ + fmt.Sprintf(" LOG ERROR NODE 'MXTEST' 'MXTEST:FAIL:%s:Exception during execution';", + escapeMDLString(testID)), + " SET $TestFailed = true;", + " SET $AllPassed = false;", + " RETURN $AllPassed;", + } }) } @@ -388,19 +399,22 @@ func rewriteWithErrorHandling(lines []string, testID string, v langver.Version) // runner reports a wrong expectation the same way the endpoint runner does — as // "expected an exception but none was thrown", which is the only outcome its log // protocol has for a throws test that did not get what it asked for. -func rewriteForThrowsTest(lines []string, didThrowVar string, want string, v langver.Version) []string { - return attachOnError(lines, v, throwsFlagHandler(didThrowVar, want)) +func rewriteForThrowsTest(lines []string, didThrowVar, suffix, want string, v langver.Version) []string { + return attachOnError(lines, v, func(n int) []string { + return throwsFlagHandler(didThrowVar, want, errorMessageVar(suffix, n)) + }) } // throwsFlagHandler sets the did-throw flag, gated on the expected message when // the annotation carried one. See throwsHandler for why the match is a substring -// and where $latestError comes from. -func throwsFlagHandler(didThrowVar, want string) []string { +// and where the message in msgVar comes from. +func throwsFlagHandler(didThrowVar, want, msgVar string) []string { if want == "" { return []string{fmt.Sprintf(" SET %s = true;", didThrowVar)} } return []string{ - fmt.Sprintf(" IF contains(%s, '%s') THEN", latestErrorMessage, escapeMDLString(want)), + readErrorMessage(msgVar), + fmt.Sprintf(" IF contains(%s, '%s') THEN", msgVar, escapeMDLString(want)), fmt.Sprintf(" SET %s = true;", didThrowVar), " END IF;", } diff --git a/cmd/mxcli/testrunner/generator_endpoint.go b/cmd/mxcli/testrunner/generator_endpoint.go index f63f780c7b..cfeabeaa6b 100644 --- a/cmd/mxcli/testrunner/generator_endpoint.go +++ b/cmd/mxcli/testrunner/generator_endpoint.go @@ -50,6 +50,10 @@ func GenerateTestFlows(suite *TestSuite) string { v := suiteVersion(suite) writeScriptHeader(&b, v) b.WriteString("\n") + if suiteHasFlows(suite) { + writeErrorMessageFlow(&b) + b.WriteString("\n") + } for _, tc := range suite.Tests { // A test with an uncompilable @expect gets no microflow. The runner // reports it as an ERROR from the parse message, which is more useful @@ -66,6 +70,9 @@ func GenerateTestFlows(suite *TestSuite) string { // writeTestFlow writes one test's microflow. func writeTestFlow(b *strings.Builder, tc TestCase, v langver.Version) { fmt.Fprintf(b, "/** %s */\n", escapeMDLComment(tc.Name)) + if callerAppliesEntityAccess(tc) { + b.WriteString(applyEntityAccessAnnotation + "\n") + } fmt.Fprintf(b, "%s %s ()\n", createFlow, testFlowName(tc)) b.WriteString("RETURNS String AS $Verdict\n") b.WriteString("BEGIN\n") @@ -222,15 +229,18 @@ func failVerdictExpr(exp Expect) string { // // The verdict carries the raised message: a bare "exception during execution" // left the user guessing, e.g. at an unresolvable [%CurrentUser%] token (tests -// run in a system context with no user). $latestError/Message is the same -// expression throwsHandler uses, measured there. +// run in a system context with no user). The message is read through +// errorMessageFlow, as in throwsHandler: the test flow applies entity access. func rewriteBodyForVerdict(lines []string, tc TestCase) []string { - handler := []string{ - fmt.Sprintf(" SET $Verdict = '%s' + %s;", - escapeMDLString(verdictFailPrefix+"exception during execution: "), latestErrorMessage), - " RETURN $Verdict;", - } - return attachOnError(lines, tc.Version, handler) + return attachOnError(lines, tc.Version, func(n int) []string { + msg := errorMessageVar("", n) + return []string{ + readErrorMessage(msg), + fmt.Sprintf(" SET $Verdict = '%s' + %s;", + escapeMDLString(verdictFailPrefix+"exception during execution: "), msg), + " RETURN $Verdict;", + } + }) } // rewriteBodyForThrows attaches an ON ERROR handler that clears the pre-set @@ -240,7 +250,7 @@ func rewriteBodyForVerdict(lines []string, tc TestCase) []string { // replaces it with one naming both sides, so a failing @throws says what was // raised instead of only what was expected. func rewriteBodyForThrows(lines []string, want string, v langver.Version) []string { - return attachOnError(lines, v, throwsHandler(want)) + return attachOnError(lines, v, func(n int) []string { return throwsHandler(want, errorMessageVar("", n)) }) } // throwsHandler builds the ON ERROR body of an @throws test. @@ -248,31 +258,28 @@ func rewriteBodyForThrows(lines []string, want string, v langver.Version) []stri // $latestError is the error variable Mendix makes available inside a handler; // its Message member is what the annotation is compared against. Measured // against mxbuild 11.6.0 and 11.13.0 at 0 errors, with `$noSuchThing/Message` as -// the control (CE0109, "Undefined variable"). +// the control (CE0109, "Undefined variable"). The handler reads it into msg +// through errorMessageFlow, because the test microflow applies entity access. // // The comparison is `contains`, so an expectation is a substring of the raised // message: a Mendix error message routinely carries an activity name or an // object id the test cannot predict, and demanding equality would make almost // every real expectation unwritable. -func throwsHandler(want string) []string { +func throwsHandler(want, msgVar string) []string { if want == "" { return []string{fmt.Sprintf(" SET $Verdict = '%s';", verdictPass)} } msg := verdictFailPrefix + "expected an error containing " + quoteForMessage(want) + ", actual: " return []string{ - fmt.Sprintf(" IF contains(%s, '%s') THEN", latestErrorMessage, escapeMDLString(want)), + readErrorMessage(msgVar), + fmt.Sprintf(" IF contains(%s, '%s') THEN", msgVar, escapeMDLString(want)), fmt.Sprintf(" SET $Verdict = '%s';", verdictPass), " ELSE", - fmt.Sprintf(" SET $Verdict = '%s' + %s;", escapeMDLString(msg), latestErrorMessage), + fmt.Sprintf(" SET $Verdict = '%s' + %s;", escapeMDLString(msg), msgVar), " END IF;", } } -// latestErrorMessage is the MDL expression for the message of the error being -// handled. Named once because it appears in both branches of every @throws -// handler and in the legacy runner. -const latestErrorMessage = "$latestError/Message" - // quoteForMessage renders the expected message for a human-readable verdict, // before that verdict is itself escaped for MDL. func quoteForMessage(s string) string { @@ -309,7 +316,10 @@ func quoteForMessage(s string) string { // semicolons) of its own, and `on error` may be inside a string or a comment. // A body that does not parse is returned unchanged — no handler can make it // parse, and the error the author then sees is about their own text. -func attachOnError(lines []string, v langver.Version, handler []string) []string { +// +// handler is called once per wrapped call with its number, from 1 in source +// order, so a handler that declares a variable can give each its own name. +func attachOnError(lines []string, v langver.Version, handler func(n int) []string) []string { body := strings.Join(lines, "\n") prefix := bodyParsePrefix(v) stmts, ok := visitor.FlowStatements(prefix + body + bodyParseSuffix) @@ -320,7 +330,9 @@ func attachOnError(lines []string, v langver.Version, handler []string) []string runes := []rune(body) // The handler is laid out at the indentation of the line its call starts // on, so a call nested in an IF or a loop reads as nested in --verbose. + n := 0 block := func(callStart int) string { + n++ lineStart := callStart for lineStart > 0 && runes[lineStart-1] != '\n' { lineStart-- @@ -332,7 +344,7 @@ func attachOnError(lines []string, v langver.Version, handler []string) []string pad := string(runes[lineStart:indent]) var b strings.Builder b.WriteString("ON ERROR BEGIN\n") - for _, l := range handler { + for _, l := range handler(n) { b.WriteString(pad + l + "\n") } b.WriteString(pad + "END ERROR") diff --git a/cmd/mxcli/testrunner/generator_endpoint_test.go b/cmd/mxcli/testrunner/generator_endpoint_test.go index baceffc9da..8f497057f0 100644 --- a/cmd/mxcli/testrunner/generator_endpoint_test.go +++ b/cmd/mxcli/testrunner/generator_endpoint_test.go @@ -25,8 +25,9 @@ func TestGenerateTestFlowsOneMicroflowPerTest(t *testing.T) { t.Errorf("generated MDL is missing %q", want) } } - if n := strings.Count(mdl, "CREATE OR MODIFY MICROFLOW"); n != 2 { - t.Errorf("got %d microflows, want one per test (2)", n) + // Plus the one errorMessageFlow every handler shares (ako/mxcli#2100). + if n := strings.Count(mdl, "CREATE OR MODIFY MICROFLOW "+testFlowPrefix+"test_"); n != 2 { + t.Errorf("got %d test microflows, want one per test (2)", n) } } @@ -113,9 +114,15 @@ func TestGenerateTestFlowsFailVerdictCarriesErrorMessage(t *testing.T) { }}} mdl := GenerateTestFlows(suite) - want := "SET $Verdict = '" + verdictFailPrefix + "exception during execution: ' + " + latestErrorMessage + ";" - if !strings.Contains(mdl, want) { - t.Errorf("the error handler does not carry $latestError/Message into the verdict; want %q in:\n%s", want, mdl) + // The message comes through errorMessageFlow: the test flow applies entity + // access and may not read $latestError/Message itself (ako/mxcli#2100). + for _, want := range []string{ + readErrorMessage("$MxTestErrorMessage_1"), + "SET $Verdict = '" + verdictFailPrefix + "exception during execution: ' + $MxTestErrorMessage_1;", + } { + if !strings.Contains(mdl, want) { + t.Errorf("the error handler does not carry the raised message into the verdict; want %q in:\n%s", want, mdl) + } } } @@ -200,6 +207,7 @@ func TestEndpointCleanupCommands(t *testing.T) { "ALTER SETTINGS RUNTIME (AfterStartupMicroflow: '')", "DROP MICROFLOW MxTest.Test_test_1", "DROP MICROFLOW MxTest.Test_test_2", + "DROP MICROFLOW " + errorMessageFlow, "DROP MICROFLOW " + endpointStartupFlow, "DROP JAVA ACTION " + endpointRegisterAction, }, diff --git a/cmd/mxcli/testrunner/handshake_test.go b/cmd/mxcli/testrunner/handshake_test.go index 0d1a25a9c6..8450ecd273 100644 --- a/cmd/mxcli/testrunner/handshake_test.go +++ b/cmd/mxcli/testrunner/handshake_test.go @@ -149,7 +149,7 @@ func TestGenerateEndpointMDLNoChainWhenNone(t *testing.T) { func TestDropTestFlows(t *testing.T) { got := dropTestFlows("", &TestSuite{Tests: []TestCase{{ID: "test_1"}, {ID: "test_2"}}}) - want := []string{"DROP MICROFLOW MxTest.Test_test_1", "DROP MICROFLOW MxTest.Test_test_2"} + want := []string{"DROP MICROFLOW MxTest.Test_test_1", "DROP MICROFLOW MxTest.Test_test_2", "DROP MICROFLOW " + errorMessageFlow} if len(got) != len(want) { t.Fatalf("got %q, want %q", got, want) } diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index c8052c59a6..8e58612266 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -941,7 +941,7 @@ func cleanupCommands(st projectState, mxTestPresent bool) []string { if st.createdMxTest { cmds = append(cmds, "DROP MODULE "+mxTestModule) } else { - cmds = append(cmds, "DROP MICROFLOW "+mxTestRunner) + cmds = append(cmds, "DROP MICROFLOW "+mxTestRunner, "DROP MICROFLOW "+errorMessageFlow) } return cmds } diff --git a/cmd/mxcli/testrunner/runner_cleanup_test.go b/cmd/mxcli/testrunner/runner_cleanup_test.go index ced296bb91..f8d17fba14 100644 --- a/cmd/mxcli/testrunner/runner_cleanup_test.go +++ b/cmd/mxcli/testrunner/runner_cleanup_test.go @@ -188,6 +188,7 @@ func TestCleanupCommands(t *testing.T) { want: []string{ "ALTER SETTINGS RUNTIME (AfterStartupMicroflow: 'Mod.Flow')", "DROP MICROFLOW MxTest.TestRunner", + "DROP MICROFLOW " + errorMessageFlow, }, }, { diff --git a/cmd/mxcli/testrunner/throws_test.go b/cmd/mxcli/testrunner/throws_test.go index a293f11cdd..721d29f9d4 100644 --- a/cmd/mxcli/testrunner/throws_test.go +++ b/cmd/mxcli/testrunner/throws_test.go @@ -83,13 +83,13 @@ $result = call microflow M.MF_Import(Json = '{}'); suite := &TestSuite{Name: "s", Tests: []TestCase{tc}} mdl := GenerateTestFlows(suite) - want := "IF contains($latestError/Message, 'the object was not found') THEN" + want := "IF contains($MxTestErrorMessage_1, 'the object was not found') THEN" if !strings.Contains(mdl, want) { t.Errorf("generated handler does not compare the message.\nwant a line: %s\ngot:\n%s", want, mdl) } // The failing branch must name what was actually raised — a verdict that // says only what was expected tells you nothing about what came back. - if !strings.Contains(mdl, "actual: ' + $latestError/Message") { + if !strings.Contains(mdl, "actual: ' + $MxTestErrorMessage_1") { t.Errorf("generated handler does not report the raised message:\n%s", mdl) } } @@ -106,7 +106,7 @@ $result = call microflow M.MF_Import(Json = '{}'); `) mdl := GenerateTestRunner(&TestSuite{Name: "s", Tests: []TestCase{tc}}) - if !strings.Contains(mdl, "contains($latestError/Message, 'not found')") { + if !strings.Contains(mdl, "contains($MxTestErrorMessage_1_1, 'not found')") { t.Errorf("legacy runner does not compare the message:\n%s", mdl) } } @@ -122,7 +122,7 @@ $result = call microflow M.MF_Import(Json = '{}'); / `) mdl := GenerateTestFlows(&TestSuite{Name: "s", Tests: []TestCase{tc}}) - if !strings.Contains(mdl, "contains($latestError/Message, 'it''''s missing')") { + if !strings.Contains(mdl, "contains($MxTestErrorMessage_1, 'it''''s missing')") { t.Errorf("apostrophe not doubled for MDL:\n%s", mdl) } } diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 2700263d8f..2d26da2053 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -450,9 +450,29 @@ The generated startup flow, `MxTest.RegisterEndpoint`, **applies entity access**, so it can call your after-startup microflow whichever way that one is set: Mendix refuses a microflow that does not apply entity access calling one that does (CE0114), and allows the other direction. Your microflow still runs -with its own setting. A build error located in `MxTest.RegisterEndpoint` or -`MxTest.RegisterTestEndpoint` is reported as mxcli's, not your project's — those -documents exist only for the run. +with its own setting. + +The generated test microflows apply entity access for the same reason, so a +test, and its `@setup` microflows, can call a production microflow that applies +it without you changing that microflow's setting. `mxcli check` sees the test +the same way, so it does not report MDL-SEC22 for such a call. Tests run in a +system context, which entity access does not restrict, so what a test reads and +writes is the same either way. + +An applying microflow may not read the attributes of `System.Error` +(CE0150), so the runner's own error handlers pass `$latestError` to a generated +helper, `MxTest.Test_ErrorMessage`, which does not apply entity access and +returns the message. A test whose **own** error handler reads `$latestError/…` +keeps a test microflow that does not apply entity access, so it builds as it +did; if that test also calls a microflow that applies entity access, no setting +serves both, and the handler has to pass `$latestError` to a microflow of yours +that does not apply it, as CE0150's own text says. Under `--legacy-runner` the +whole suite shares one microflow, so one such test keeps all of them +non-applying. + +A build error located in `MxTest.RegisterEndpoint`, +`MxTest.RegisterTestEndpoint` or `MxTest.Test_ErrorMessage` is reported as +mxcli's, not your project's — those documents exist only for the run. ### `@cleanup`: what happens to a test's data diff --git a/mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl b/mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl new file mode 100644 index 0000000000..6d9e17ceca --- /dev/null +++ b/mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl @@ -0,0 +1,44 @@ +-- ako/mxcli#2100 (upstream mendixlabs/mxcli#1805): the generated test +-- microflows did not apply entity access, so a test could not call a +-- production microflow that does: MxBuild refused every one with CE0114, and +-- `mxcli check` with MDL-SEC22. Applying it alone then failed CE0150 on the +-- runner's own handlers, which read $latestError/Message. +-- +-- Setup: test-runner-2100-setup.mdl. Then: +-- +-- mxcli test mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl -p app.mpr --local +-- +-- Measured on 11.12.1 (--local --db-type hsqldb). Before the fix: every test +-- calling Acc.* reported "CE0114 … (at Call microflow activity 'ACT_…')" and +-- none ran. After: 01 and 03 PASS, 02 FAILs with "exception during execution: +-- Amount must be positive" and 04 with "expected an error containing +-- 'something else', actual: Amount must be positive" — the messages arrive +-- through the generated MxTest.Test_ErrorMessage helper. +mdl 1; +/** + * @test 01 calls a microflow that applies entity access + * @expect $ok + */ +$ok = call microflow Acc.ACT_IsReady(); +/ + +/** + * @test 02 an unexpected error fails with its message + * @expect $ok + */ +$ok = call microflow Acc.ACT_Refuse(Amount = -1.0); +/ + +/** + * @test 03 @throws matches the message of an applying microflow + * @throws 'must be positive' + */ +$ok = call microflow Acc.ACT_Refuse(Amount = -1.0); +/ + +/** + * @test 04 @throws with the wrong message fails, naming the actual one + * @throws 'something else' + */ +$ok = call microflow Acc.ACT_Refuse(Amount = -1.0); +/ diff --git a/mdl-examples/bug-tests/test-runner-2100-setup.mdl b/mdl-examples/bug-tests/test-runner-2100-setup.mdl new file mode 100644 index 0000000000..9191506b6b --- /dev/null +++ b/mdl-examples/bug-tests/test-runner-2100-setup.mdl @@ -0,0 +1,32 @@ +-- ako/mxcli#2100 (upstream mendixlabs/mxcli#1805): the setup script for +-- test-runner-2100-applying-callee.test.mdl. Apply it before running the tests: +-- +-- mxcli exec mdl-examples/bug-tests/test-runner-2100-setup.mdl -p app.mpr +-- +-- Production microflows that apply entity access, which the generated test +-- microflows (and check's wrappers) could not call. One raises an error, so +-- the runner's handlers have to read its message from an applying flow. +mdl 1; +create module Acc; + +create java action Acc.JA_Fail(Message: String) returns Boolean +as $$ +throw new com.mendix.core.CoreException(Message); +$$; + +/** Applies entity access, as many production actions do. */ +@applyentityaccess +create microflow Acc.ACT_IsReady () returns Boolean +begin + return true; +end; + +/** Applies entity access and refuses a negative amount. */ +@applyentityaccess +create microflow Acc.ACT_Refuse (Amount: Decimal) returns Boolean +begin + if $Amount < 0 then + $Failed = call java action Acc.JA_Fail(Message = 'Amount must be positive'); + end if; + return true; +end;