diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json new file mode 100644 index 0000000000..6bf4749c58 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-11-04","symptom":"`mxcli check --references` printed `✗ 1 reference error(s) found` and then `0 issues: 0 errors, 0 warnings, 0 info` — the reporter asked why a reference error does not count as an error (mendixlabs/mxcli#1726)","cause":"the closing summary sums the `printed` list, which only the `report` helper appends to; the reference tier prints its errors and warnings itself, so they never reached the total","file":"cmd/mxcli/cmd_check.go (reference tier appends to printed); test cmd/mxcli/check_reference_summary_test.go","insight":"When one summary is computed over a list that several tiers feed, a tier with its own print path drops out of the total silently. Only the run in the report had an earlier tier's violation, so the summary line printed at all; a script with nothing but a reference error printed no summary, which is why the test asserts `1 errors` rather than the absence of `0 errors`. Control: without the append, the test fails with no summary in the output.","refs":["ako/mxcli#1928","mendixlabs/mxcli#1726"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json new file mode 100644 index 0000000000..996b4723aa --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-11-04","symptom":"The nightly goes red on the Mendix 11.14.0 leg only, in `TestTestAttach_KeepsTheHostingAppsPageBundles` (added the day before by ako/mxcli#1917): `mxcli test --attach` exits 1 with `this change needs a runtime restart (an entity or association changed), which --attach cannot do`, although the attach only adds a test microflow. 11.12.x, 11.13, 11.15 and 11.16 are green","cause":"On 11.14.0 every later build in an mxbuild --serve process fails, even with the model unchanged (`Cannot find module …/web/rollup.config.mjs`), and the failed /build response carries no restartRequired key at all. #1917 routes the attach's build through serveRecovery, whose legacy-hint path restarts onto a private serve and builds cold; a cold build reports restartRequired=true whatever changed, and runner_attach read that as a structural change. Before #1917 the same attach failed with the build failure itself, so attach never worked on 11.14.0; the test exposed it because its default 11.12.6 falls back to whatever mxbuild the nightly leg caches","file":"`cmd/mxcli/docker/serve_recovery.go` (rebuildFresh marks the cold answer `BuildResult.ColdRestart`; the incomplete-bundle path clears it when it takes the incremental answer), `cmd/mxcli/testrunner/runner_attach.go` (`attachRestartRefusal`), `cmd/mxcli/cmd_test_attach_bundle_integration_test.go` (11.14 branch)","insight":"A recovery that substitutes a fresh process's answer must say so: a cold build's restartRequired is \"unknown\", and only a caller that can restart may treat it as true. Settled by driving /build directly (a throwaway test with StartServe + Build, no watch loop): 11.14.0 build 2 with no change = Failure with no restartRequired; 11.15.0 = Success, false for no change and for a microflow, true for an entity, 0 pages (the 11.12.5 shape #1917 already recovers). That table is what puts the limit on 11.14.0 alone, so the test pins `>= 11.14 && < 11.15` and fails when an 11.14 mxbuild builds incrementally again. Reproduce in about 4 minutes: `mxcli setup mxbuild --version 11.14.0`, JAVA_HOME at a JDK 25, `MXCLI_RUNLIFE_VERSION=11.14.0 go test -tags integration -run TestTestAttach_KeepsTheHostingAppsPageBundles ./cmd/mxcli/`. Controls: the old test fails with the nightly's exact message; the new test passes on 11.14.0 and 11.15.0 and fails on 11.14.0 against a binary without the fix (it names the entity); stubbing the ColdRestart mark fails the two unit tests that pin it","refs":["mendixlabs/mxcli nightly run 41801646309","ako/mxcli nightly run 41801123003","ako/mxcli#1917","mendixlabs/mxcli#1723"],"test":"cmd/mxcli/cmd_test_attach_bundle_integration_test.go; cmd/mxcli/docker/serve_attached_test.go; cmd/mxcli/docker/serve_recovery_test.go; cmd/mxcli/testrunner/runner_attach_restart_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json new file mode 100644 index 0000000000..0a1bb0de61 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"`check --references` passed drop/rename/move of a document that does not exist, and alter enumeration / alter microflow on one, while exec refused each with ` not found` — 9 of the 11 statement forms that name an existing document (only alter entity and alter page were resolved)","cause":"validateAlterTarget was written for ALTER and extended one statement at a time; the drop/rename/move statements resolve against the same listings but were never routed to it","file":"mdl/executor/validate_alter_target.go (validateAlterTarget, targetListing, checkTarget, moveTargetKind), mdl/executor/validate.go (collectSingle: indirectDocs, renamedModules, opaqueModules, opaqueScript); test cmd/mxcli/check_statement_target_test.go","insight":"The risk in widening a not-found check is the false refusal, not the miss: a document can reach a name without a create statement naming it — an earlier rename or move, a rename module, an execute script, a generated SQL connector, a business event service, a bulk external-entities import. Record each in the pre-pass and stay silent for that name or module; the check must stay quieter than exec. Matching is exact-case because exec's lookups are. A PedApp test per form, each with an existing-target control, plus if-exists / created / renamed / moved controls. Control: without the new cases all 13 forms pass check.","refs":["ako/mxcli#1821","ako/mxcli#555"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json new file mode 100644 index 0000000000..dc6c5df30b --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"`textbox (Attribute: PlannedOn)` in a data view, or `ContentParams: ({1} = PlannedOn)` / `CaptionParams`, on an entity without that attribute: check --references passed, exec wrote it, mxbuild CE1613 \"The selected attribute 'M.E.PlannedOn' no longer exists\" — measured 3 of 3 on a fresh 11.12.6 app, control page clean","cause":"the input-widget scope walk (checkInputBinding) looked the attribute up and returned silently when it was not found unless the name was an association; template parameters were never looked up","file":"mdl/executor/validate_widget_attribute_type.go (reportMissingAttribute, checkTemplateParams), mdl/executor/validate_widget_attribute_scope.go (walk); test cmd/mxcli/check_widget_attribute_exists_test.go","insight":"The context walk, the member index with inheritance and the association hops were already there for the type rule (MDL-WIDGET39) — the not-found case was a silent `return` in it. Before writing a new resolver, grep for the lookup that already returns found/known and see what it does on a miss. Stay quiet where the index is not sure: an unknown chain (known=false), an unresolved context, an altered entity, an expression or $var path, and the audit members (createdDate/changedDate/owner/changedBy), which bind by name but are not in the attribute list. Control: without the two calls the textbox and content-parameter subtests pass check.","refs":["ako/mxcli#555"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json new file mode 100644 index 0000000000..36bd2d5e90 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"`alter page M.P { add parameters $X: … }` then a show page passing X, in one script: check --references and exec refused it with `page M.P has no parameter \"X\"`; the same statements as two scripts ran (mendixlabs/mxcli#1736)","cause":"the script pre-pass recorded page parameter names only from create page, so for an altered page the show page argument check fell through to the stored parameter list","file":"mdl/executor/validate.go (collectDefinitions AlterPageStmt case, pageParamEdits), mdl/executor/validate_show_page_params.go (recordPageParamEdits, applyPageParamEdits); test validate_show_page_params_alter_test.go","insight":"A script-context index that only listens to create statements reads every alter as a no-op — the same class as the #1355 script-set fix, one level down. Record the alter's edits rather than a computed list, because the pre-pass has no project to read the stored list from; apply them over whichever list wins at lookup time. End-to-end on PedApp 11.13.0: the base build refuses with the reported message, the fix passes check and exec writes both. Control: without the pre-pass case, the add/drop/create-then-alter subtests fail.","refs":["ako/mxcli#1940","mendixlabs/mxcli#1736"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json new file mode 100644 index 0000000000..4f439af247 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"with a live and an excluded microflow of one name, `check --references` refused `call microflow M.Test(param1 = …)` with `has no parameter \"param1\" (declared parameters: param2)` — the excluded twin's parameters (mendixlabs/mxcli#1726)","cause":"the validators index stored documents by name in maps (`result[name] = …` over ListMicroflows/ListPages), so the twin listed LAST won: buildFlowSignatures, buildMicroflowReturnTypes, buildPageParamNames, buildPageParamEntities, the workflow call-microflow parameter map and microflowsByQualifiedName","file":"mdl/executor/validate_datasource_args.go, helpers.go, validate_show_page_params.go, validate_workflow_task_signature.go, validate_workflow_refs.go, cmd_odata_actions.go; test validate_excluded_twin_signature_test.go","insight":"The #1469 sweep fixed first-match LOOPS (`for … { if name matches { act; return } }`), where the excluded twin wins by being listed FIRST; a name-keyed MAP is the mirror image — the twin wins by being listed LAST — so #1469's excluded-first fixtures could not see it. Grep `GetQualifiedName(.*ContainerID, .*Name)\\] = ` for value maps (boolean name sets are safe), iterate liveFirst and keep the first entry. Test both list orders; the excluded-last case is the reported one. Control: without the change, the excluded-last subtests fail with the reporter's exact message.","refs":["ako/mxcli#1928","mendixlabs/mxcli#1726","mendixlabs/mxcli#1469"]} diff --git a/.claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json b/.claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json new file mode 100644 index 0000000000..0d83784928 --- /dev/null +++ b/.claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json @@ -0,0 +1 @@ +{"area":"other","date":"2026-11-04","symptom":"A unit test fails at random on one nightly leg, in setup, not in what it tests: `TestCreateMicroflow_NotesRefusedOnFullConstructor` (mdl/backend/mcp) with `copy fixture: open .mxcli/cache/unit-contents.db-journal: no such file or directory`. The same commit passes on every other leg and on re-run","cause":"A cross-package race on the checked-in fixtures, not a product defect. Since the unit-contents store (ako/mxcli#1804) any reader that opens a project writes `.mxcli/cache/unit-contents.db` next to it, and tests that open testdata/expr-checker, testdata/pedapp or modelsdk/mpr/testdata/v1-project in place write it into the fixture; its SQLite journal appears and disappears with each write. go test runs packages in parallel, so another package's whole-directory `os.CopyFS` (25 tests) or hand-written walk listed the journal and then failed to open it. Nothing under a fixture's .mxcli is tracked","file":"`internal/testutil/fixture.go` (`CopyFixture`: os.CopyFS over a view that hides every .mxcli), used by every whole-fixture copy in tests, and by cmd/mxcli's copyTree and copyFixtureTree","insight":"A cache that writes next to the project turns every checked-in fixture into shared mutable state between test packages; any whole-directory copy of one is then a race whose odds grow with the suite. Copy what the fixture commits, never its .mxcli, which also makes the copy what a fresh checkout holds whichever tests ran first (exec_atomic's hashTree already ignored .mxcli for the same reason). After a full `make test`, `find . -name .mxcli -path '*testdata*'` lists the fixtures being written in place. The race is reproduced deterministically by an fs.FS whose listing names a journal that Open reports missing: plain os.CopyFS fails with the nightly's error, CopyFixture copies; with its filter stubbed it fails the same way","refs":["ako/mxcli nightly run 41801123003 (test (Mendix 11.12.2), Unit tests)","ako/mxcli#1804"],"test":"internal/testutil/fixture_test.go"} diff --git a/.claude/skills/mendix/test-microflows/SKILL.md b/.claude/skills/mendix/test-microflows/SKILL.md index aec814a13d..5abcff80a3 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 | 185-227 | Test File Formats | | 228-243 | string Operations | | 244-267 | Running Tests | -| 268-461 | How It Works | -| 462-500 | Check a test file before you run it | -| 501-622 | Writing Good Tests | -| 623-637 | Test File Organization | -| 638-650 | Interpreting Failures | -| 651-672 | CI Integration | -| 673-679 | Related Skills | -| 680-682 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | +| 268-465 | How It Works | +| 466-504 | Check a test file before you run it | +| 505-626 | Writing Good Tests | +| 627-641 | Test File Organization | +| 642-654 | Interpreting Failures | +| 655-676 | CI Integration | +| 677-683 | Related Skills | +| 684-686 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | # Test Microflows Skill @@ -389,6 +389,10 @@ Three things to know before reaching for it: - **A change needing a runtime restart is refused** — a new entity or association. That runtime belongs to the other process. Restart it, or drop `--attach`. +- **On Mendix 11.14.0 every attach is refused**, whatever the tests change: its + `mxbuild --serve` fails every build after its first, so the attach falls back + to a full build, and a full build asks for a runtime restart whatever changed. + Use `--local` there. (11.12.x, 11.13 and 11.15+ build incrementally.) | | Boot | Database | Owns the runtime | |---|---|---|---| diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c9ca2456a..99952a4401 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`test --attach` on Mendix 11.14.0 says why it is refused instead of blaming an entity** — that version's `mxbuild --serve` fails every build after its first, so since ako/mxcli#1917 the attach builds on a fresh serve process, whose first build asks for a runtime restart whatever changed. The refusal claimed "an entity or association changed" and suggested restarting the dev loop, which does not help; it now says the build could not be done incrementally and points to `--local`. The hosting app's pages survive either way. 11.15.0 builds incrementally and attaches as before. - **`describe microflow` no longer leaves out a note linked to the start event or to another note** (ako/mxcli#1933, mendixlabs/mxcli#1731) — such a note counted as linked, so it was not printed as a free note, and the element it is linked to prints no `@annotation`, so it was not printed at all; re-executing the description deleted it. It is now described as a free note, after a `-- WARNING` line saying the link itself has no MDL form and a rewrite drops it. - **Rebuilding a described decision no longer adds a merge** (ako/mxcli#1929, mendixlabs/mxcli#1727) — a `case`, `split type` or `if … elsif` one of whose branches returns is described with its own merge as a label (`end case; join shared1; merge shared1;`). Executing that as a fresh create built the decision's merge and a second one for the label, wired merge to merge, and every further describe → drop + create added another (`join junctionN; merge junctionN;`). The label's merge is now the decision's merge when nothing else reaches it, so the round trip keeps one merge. - **`describe microflow` prints a blank `call external action` argument as `= nothing`** (ako/mxcli#1915) — Studio Pro 11.16 stores an optional OData action parameter left blank as an empty argument, and `describe` printed it as `additional = )`, which does not parse, so every `mdl 1;` `create or modify` of such a microflow was refused (MDL-V1-REBUILD). `call external action` now takes `Param = nothing`, the blank-argument spelling `call microflow` and `call nanoflow` already use; `Param = empty` (what 11.14 stored) stays a different value, and each round-trips as itself. diff --git a/cmd/mxcli/brain/list.go b/cmd/mxcli/brain/list.go new file mode 100644 index 0000000000..0c5b93c4fe --- /dev/null +++ b/cmd/mxcli/brain/list.go @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: Apache-2.0 + +// list.go - the committed store as data. +// +// `brief --json` returns each shard's rendered markdown, which is right for a +// prompt and wrong for a UI: rendering a tree from it means parsing a format +// mxcli owns, a second parser of the same thing that drifts the first time the +// format changes (ako/mxcli#1952). List hands out the parsed entries instead. +package brain + +import ( + "fmt" + "slices" +) + +// ListFilter narrows a listing. The zero value lists everything. +type ListFilter struct { + // Slice keeps one slice's entries: its requirements and the questions + // filed against it. + Slice string + // Kind keeps decisions or requirements. An open question is a decision + // unless it was filed against a slice. + Kind Kind + // Open, when set, keeps only open questions (true) or only settled + // entries (false). + Open *bool +} + +// ListedEntry is one committed entry with the shard it is in — the same shape +// `staged --json` gives a queued entry, so a consumer reads both sides alike. +type ListedEntry struct { + Entry + Shard string `json:"shard"` +} + +// ListResult is a listing plus the blocks that could not be read. Those are +// reported rather than dropped: a UI that silently shows fewer entries than +// the file holds is the failure a structured read exists to prevent. +type ListResult struct { + Entries []ListedEntry `json:"entries"` + Malformed []string `json:"malformed"` +} + +// List returns the committed entries matching f, in store order: project.md, +// then modules, then slices, and file order within each. +func (s *Store) List(f ListFilter) (ListResult, error) { + out := ListResult{Entries: []ListedEntry{}, Malformed: []string{}} + switch f.Kind { + case "", KindDecision, KindRequirement: + default: + return out, fmt.Errorf("kind %q: use %q or %q", f.Kind, KindDecision, KindRequirement) + } + if f.Slice != "" && f.Kind == KindDecision { + return out, fmt.Errorf("a decision has no slice, so --slice with --kind %s matches nothing; drop one of them", KindDecision) + } + + shards, err := s.ListShards() + if err != nil { + return out, err + } + if f.Slice != "" { + want := PlanShard(f.Slice) + shards = slices.DeleteFunc(shards, func(sh string) bool { return sh != want }) + } + for _, sh := range shards { + entries, malformed, err := s.LoadShard(sh) + if err != nil { + return out, err + } + for _, m := range malformed { + out.Malformed = append(out.Malformed, sh+": "+m) + } + for _, e := range entries { + // Explicit, so a consumer never has to know that an empty + // kind means a decision. + e.Kind = e.EntryKind() + if f.Kind != "" && e.Kind != f.Kind { + continue + } + if f.Open != nil && e.Open != *f.Open { + continue + } + out.Entries = append(out.Entries, ListedEntry{Entry: e, Shard: sh}) + } + } + return out, nil +} diff --git a/cmd/mxcli/brain/list_test.go b/cmd/mxcli/brain/list_test.go new file mode 100644 index 0000000000..634e840aba --- /dev/null +++ b/cmd/mxcli/brain/list_test.go @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: Apache-2.0 + +package brain + +import ( + "os" + "slices" + "strings" + "testing" +) + +// ako/mxcli#1952: a structured read of the committed store, so a UI renders +// entries instead of parsing markdown whose format mxcli owns. + +func listFixture(t *testing.T) (s *Store, dec, question, reqA, reqB Entry) { + t.Helper() + s = NewStore(t.TempDir()) + if _, err := s.Init(); err != nil { + t.Fatal(err) + } + dec = mustEntry(t, "Orders are committed by Finance", "@Sales.Order") + q, err := NewQuestion("Do approvers see rejected orders?", nil, "", day) + if err != nil { + t.Fatal(err) + } + question = q + reqA = mustRequirement(t, "A planner can create a visit", "01-scheduling", "@Scheduling.Visit") + reqB = mustRequirement(t, "A dispatcher can assign a visit", "02-dispatch", "@Dispatch.ACT_Assign") + if err := s.Promote(dec, "Sales"); err != nil { + t.Fatal(err) + } + if err := s.Promote(question, ProjectShard); err != nil { + t.Fatal(err) + } + promoteAll(t, s, reqA, reqB) + return +} + +func ids(r ListResult) []string { + var out []string + for _, e := range r.Entries { + out = append(out, e.ID) + } + return out +} + +func TestListFiltersTheCommittedStore(t *testing.T) { + s, dec, question, reqA, reqB := listFixture(t) + yes, no := true, false + + for _, tc := range []struct { + name string + f ListFilter + want []string + }{ + // Store order: project, modules, slices; file order within each. + {"everything", ListFilter{}, []string{question.ID, dec.ID, reqA.ID, reqB.ID}}, + {"one slice", ListFilter{Slice: "02-dispatch"}, []string{reqB.ID}}, + {"requirements", ListFilter{Kind: KindRequirement}, []string{reqA.ID, reqB.ID}}, + {"decisions", ListFilter{Kind: KindDecision}, []string{question.ID, dec.ID}}, + {"open only", ListFilter{Open: &yes}, []string{question.ID}}, + {"settled only", ListFilter{Open: &no}, []string{dec.ID, reqA.ID, reqB.ID}}, + {"no such slice", ListFilter{Slice: "99-none"}, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := s.List(tc.f) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(ids(got), tc.want) { + t.Errorf("ids = %v, want %v", ids(got), tc.want) + } + }) + } + + // Each entry carries its shard and an explicit kind — a consumer must not + // have to know that an empty kind means "decision". + got, err := s.List(ListFilter{}) + if err != nil { + t.Fatal(err) + } + for _, e := range got.Entries { + if e.Kind == "" || e.Shard == "" { + t.Errorf("%s: kind %q shard %q", e.ID, e.Kind, e.Shard) + } + } + if got.Entries[1].Shard != "Sales" || got.Entries[2].Shard != PlanShard("01-scheduling") { + t.Errorf("shards: %+v", got.Entries) + } +} + +// A block whose metadata line cannot be read is reported, not dropped — or a +// UI silently shows fewer entries than the file holds. +func TestListReportsMalformedBlocks(t *testing.T) { + s, _, _, reqA, _ := listFixture(t) + path := s.ShardPath(reqA.Shard()) + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, append(b, []byte("\n## Hand-written, no metadata\n\nSome prose.\n")...), 0o644); err != nil { + t.Fatal(err) + } + got, err := s.List(ListFilter{}) + if err != nil { + t.Fatal(err) + } + if len(got.Malformed) != 1 || !strings.Contains(got.Malformed[0], "Hand-written") || !strings.HasPrefix(got.Malformed[0], reqA.Shard()) { + t.Errorf("malformed = %v", got.Malformed) + } + if len(got.Entries) != 4 { + t.Errorf("the well-formed entries must still be listed: %v", ids(got)) + } + + // Malformed blocks are reported only for the shards the filter covers. + got, err = s.List(ListFilter{Slice: "02-dispatch"}) + if err != nil { + t.Fatal(err) + } + if len(got.Malformed) != 0 { + t.Errorf("a malformed block outside the filtered slice was reported: %v", got.Malformed) + } +} + +func TestListRefusesAFilterThatCannotMatch(t *testing.T) { + s := NewStore(t.TempDir()) + if _, err := s.List(ListFilter{Slice: "01-scheduling", Kind: KindDecision}); err == nil { + t.Error("--slice with --kind decision must be refused: a decision has no slice") + } + if _, err := s.List(ListFilter{Kind: "story"}); err == nil { + t.Error("an unknown kind must be refused, not match nothing") + } + // An uninitialised store lists nothing; it is not an error. + if got, err := s.List(ListFilter{}); err != nil || len(got.Entries) != 0 { + t.Errorf("empty store: %+v, %v", got, err) + } +} diff --git a/cmd/mxcli/brain_before_project_test.go b/cmd/mxcli/brain_before_project_test.go index 1aeaed71a1..2024d6d2a2 100644 --- a/cmd/mxcli/brain_before_project_test.go +++ b/cmd/mxcli/brain_before_project_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/cmd/mxcli/brain" + "github.com/mendixlabs/mxcli/internal/testutil" ) // A brain can be born before its project (ako/mxcli#1949). These tests pin @@ -190,7 +191,7 @@ func TestBrainBornBeforeTheProjectIsCountedAfterIt(t *testing.T) { t.Fatalf("checkNewProjectDestination = %v, %v; want adopt", adopt, err) } stage := filepath.Join(t.TempDir(), "stage") - if err := os.CopyFS(stage, os.DirFS(filepath.Join("..", "..", "testdata", "pedapp"))); err != nil { + if err := testutil.CopyFixture(stage, filepath.Join("..", "..", "testdata", "pedapp")); err != nil { t.Fatal(err) } if err := moveProject(stage, dir); err != nil { diff --git a/cmd/mxcli/check_reference_summary_test.go b/cmd/mxcli/check_reference_summary_test.go new file mode 100644 index 0000000000..f81aeb7ee0 --- /dev/null +++ b/cmd/mxcli/check_reference_summary_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#1928 (mendixlabs/mxcli#1726): a run that failed on a reference +// error ended with "0 issues: 0 errors, 0 warnings, 0 info" — the summary +// counted the violations the other tiers report, and the reference tier +// printed its errors itself. The summary is the total, so it counts them. +func TestCheck_SummaryCountsReferenceErrors(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", filepath.Join(dir, "PedApp.mpr")) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + bad := writeScript(t, scripts, "bad.mdl", `mdl 1; +create or modify microflow MyFirstModule.CallsNothing () +begin + call microflow MyFirstModule.NoSuchFlow1928(); +end; +`) + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{bad}) }) + if code == 0 || !strings.Contains(out, "microflow not found: MyFirstModule.NoSuchFlow1928") { + t.Fatalf("the reference error was not reported (exit %d):\n%s", code, out) + } + if strings.Contains(out, " 0 errors") || !strings.Contains(out, "1 errors") { + t.Errorf("the summary does not count the reference error:\n%s", out) + } +} diff --git a/cmd/mxcli/check_statement_target_test.go b/cmd/mxcli/check_statement_target_test.go new file mode 100644 index 0000000000..104d75e03d --- /dev/null +++ b/cmd/mxcli/check_statement_target_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#1821: `check --references` resolved the target document of alter +// entity and alter page only. drop, rename and move of a document that does not +// exist — and alter enumeration / alter microflow — passed check and were then +// refused by exec. Every form is checked against PedApp: a missing target is +// reported, and the same statement on a target the project has is not. +func TestCheck_StatementTargetMustExist(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", filepath.Join(dir, "PedApp.mpr")) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + run := func(name, body string) (int, string) { + path := writeScript(t, scripts, name+".mdl", "mdl 1;\n"+body+"\n") + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{path}) }) + return code, out + } + + cases := []struct { + name, missing, want, existing string + }{ + {"drop microflow", "drop microflow MyFirstModule.NoSuchFlow;", "microflow not found: MyFirstModule.NoSuchFlow", + "drop microflow MyFirstModule.MyFirstLogic;"}, + {"drop nanoflow", "drop nanoflow Atlas_Web_Content.NoSuchNano;", "nanoflow not found: Atlas_Web_Content.NoSuchNano", + "drop nanoflow Atlas_Web_Content.ACT_Login;"}, + {"drop page", "drop page MyFirstModule.NoSuchPage;", "page not found: MyFirstModule.NoSuchPage", + "drop page MyFirstModule.Home_Web;"}, + {"drop snippet", "drop snippet Administration.NoSuchSnippet;", "snippet not found: Administration.NoSuchSnippet", + "drop snippet Administration.ReadMe;"}, + {"drop entity", "drop entity Administration.NoSuchEntity;", "entity not found: Administration.NoSuchEntity", + "drop entity Administration.AccountPasswordData;"}, + {"drop enumeration", "drop enumeration FeedbackModule.NoSuchEnum;", "enumeration not found: FeedbackModule.NoSuchEnum", + "drop enumeration FeedbackModule.LogNodes;"}, + {"drop association", "drop association Administration.NoSuchAssoc;", "association not found: Administration.NoSuchAssoc", + "drop association Administration.AccountPasswordData_Account;"}, + {"drop constant", "drop constant FeedbackModule.NoSuchConst;", "constant not found: FeedbackModule.NoSuchConst", + "drop constant FeedbackModule.LocalStorageKey;"}, + {"rename microflow", "rename microflow MyFirstModule.NoSuchFlow to Other;", "microflow not found: MyFirstModule.NoSuchFlow", + "rename microflow MyFirstModule.MyFirstLogic to MyRenamedLogic;"}, + {"move microflow", "move microflow MyFirstModule.NoSuchFlow to folder 'X';", "microflow not found: MyFirstModule.NoSuchFlow", + "move microflow MyFirstModule.MyFirstLogic to folder 'X';"}, + {"alter enumeration", "alter enumeration FeedbackModule.NoSuchEnum add value V caption 'V';", "enumeration not found: FeedbackModule.NoSuchEnum", + "alter enumeration FeedbackModule.LogNodes add value if not exists V1821 caption 'V';"}, + {"alter microflow", "alter microflow MyFirstModule.NoSuchFlow { drop $X; };", "microflow not found: MyFirstModule.NoSuchFlow", + ""}, + {"alter nanoflow", "alter nanoflow Atlas_Web_Content.NoSuchNano { drop $X; };", "nanoflow not found: Atlas_Web_Content.NoSuchNano", + ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + code, out := run(strings.ReplaceAll(tc.name, " ", "_"), tc.missing) + if code == 0 || !strings.Contains(out, tc.want) { + t.Errorf("a missing target passed check (exit %d), want %q:\n%s", code, tc.want, out) + } + if tc.existing == "" { + return + } + // Control: the same form on a target the project has is not "not found". + _, out = run(strings.ReplaceAll(tc.name, " ", "_")+"_ok", tc.existing) + if strings.Contains(out, "not found") { + t.Errorf("control: an existing target was reported:\n%s", out) + } + }) + } + + // Controls the check must not break: drop … if exists, a document the + // script creates first, and one an earlier rename lands under the name. + for name, body := range map[string]string{ + "if exists": "drop microflow if exists MyFirstModule.NoSuchFlow;", + "created in the script": `create microflow MyFirstModule.Temp1821 () begin end; +drop microflow MyFirstModule.Temp1821;`, + "renamed earlier in the script": `rename microflow MyFirstModule.MyFirstLogic to Renamed1821; +drop microflow MyFirstModule.Renamed1821;`, + "moved earlier in the script": `move page MyFirstModule.Home_Web to Administration; +drop page Administration.Home_Web;`, + } { + t.Run("control: "+name, func(t *testing.T) { + _, out := run("control_"+strings.ReplaceAll(name, " ", "_"), body) + if strings.Contains(out, "not found") { + t.Errorf("reported a target the script provides:\n%s", out) + } + }) + } +} diff --git a/cmd/mxcli/check_widget_attribute_exists_test.go b/cmd/mxcli/check_widget_attribute_exists_test.go new file mode 100644 index 0000000000..41e41c8b93 --- /dev/null +++ b/cmd/mxcli/check_widget_attribute_exists_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#555: an attribute named inside a widget was not resolved by +// `check --references` — a textbox's Attribute, or a template parameter of a +// dynamic text — so a misspelt name passed check, exec wrote it, and mxbuild +// refused it with CE1613. Each binding is now resolved against the entity its +// data container supplies, and the controls bind what Account has: its own +// attribute, an inherited one, an audit member and an expression. +func TestCheck_WidgetAttributeMustExist(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", filepath.Join(dir, "PedApp.mpr")) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + page := func(file, widgets string) (int, string) { + path := writeScript(t, scripts, file+".mdl", `mdl 1; +create page MyFirstModule.P555 ( Title: 'x', Layout: Atlas_Core.PopupLayout, Params: ( $Acc: Administration.Account ) ) +{ + layoutgrid lg { row { column (desktopwidth: autofill) { + dataview dv (DataSource: $Acc) { +`+widgets+` + } + } } } +}; +`) + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{path}) }) + return code, out + } + + for _, tc := range []struct{ name, widgets, want string }{ + {"textbox attribute", `textbox tb (Label: 'x', Attribute: PlannedOn)`, + "widget `tb` (textbox) binds `PlannedOn`, but Administration.Account has no attribute PlannedOn"}, + {"dynamic text content parameter", `dynamictext dt (Content: 'Hi {1}', ContentParams: ({1} = PlannedOn))`, + "widget `dt` (dynamictext) ContentParams {1} binds `PlannedOn`, but Administration.Account has no attribute PlannedOn"}, + } { + t.Run(tc.name, func(t *testing.T) { + code, out := page(strings.ReplaceAll(tc.name, " ", "_"), tc.widgets) + if code == 0 || !strings.Contains(out, tc.want) || !strings.Contains(out, "CE1613") { + t.Errorf("a missing attribute passed check (exit %d), want %q:\n%s", code, tc.want, out) + } + }) + } + + t.Run("control: attributes the entity has", func(t *testing.T) { + code, out := page("control", `textbox tb1 (Label: 'x', Attribute: FullName) + textbox tb2 (Label: 'x', Attribute: Name) + datepicker dp (Label: 'x', Attribute: changedDate) + dynamictext dt1 (Content: '{1} {2} {3}', ContentParams: ({1} = FullName, {2} = 'literal', {3} = $Acc/Email))`) + if code != 0 || strings.Contains(out, "has no attribute") { + t.Errorf("an attribute Account has was refused (exit %d):\n%s", code, out) + } + }) +} diff --git a/cmd/mxcli/cmd_brain.go b/cmd/mxcli/cmd_brain.go index 31f2afb5cd..543d3aa1ff 100644 --- a/cmd/mxcli/cmd_brain.go +++ b/cmd/mxcli/cmd_brain.go @@ -66,6 +66,7 @@ on 'plan' and 'check' with the same -C count against the model.`, mxcli brain brief --slice 02-approvals -p app.mpr mxcli brain plan -p app.mpr mxcli brain check -p app.mpr + mxcli brain list -p app.mpr --json mxcli brain show -p app.mpr`, } @@ -407,6 +408,71 @@ func runBrainChange(cmd *cobra.Command, id, verb string, change func(brain.Entry fmt.Printf("%s %s in %s\n", verb, id, shardLabel(moved.To)) } +var brainListCmd = &cobra.Command{ + Use: "list", + Short: "The committed entries as data, filterable (the structured read of docs/brain/)", + Long: `List the committed entries — the promoted side of what 'staged' shows. + +With --json each entry is the object 'staged --json' returns (id, kind, slice, +open, title, body, anchors, source, date) plus the shard it is in, so a tool can +render the store without parsing its markdown. 'brief --json' is for a prompt; +this is for a program. + + --slice one slice: its requirements and the questions filed on it + --kind decision or requirement + --open only open questions; --open=false only settled entries + +Blocks whose metadata line cannot be read are listed under "malformed" rather +than dropped, so a listing never silently shows fewer entries than the files +hold. No model is needed.`, + Example: ` mxcli brain list -C ./App --json + mxcli brain list --slice 01-scheduling -p app.mpr --json + mxcli brain list --open -p app.mpr`, + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + store := brain.NewStore(brainProjectDir(cmd)) + var f brain.ListFilter + f.Slice, _ = cmd.Flags().GetString("slice") + kind, _ := cmd.Flags().GetString("kind") + f.Kind = brain.Kind(kind) + if cmd.Flags().Changed("open") { + open, _ := cmd.Flags().GetBool("open") + f.Open = &open + } + res, err := store.List(f) + if err != nil { + brainFatal(err) + } + if globalJSONFlag { + brainJSON(map[string]any{ + "entries": res.Entries, + "count": len(res.Entries), + "malformed": res.Malformed, + }) + return + } + if !store.Exists() { + fmt.Println("No store yet. Create one with 'mxcli brain init'.") + return + } + for _, m := range res.Malformed { + fmt.Printf("MALFORMED %s\n", m) + } + if len(res.Entries) == 0 { + fmt.Println("No committed entries match.") + return + } + for _, e := range res.Entries { + open := "" + if e.Open { + open = " OPEN" + } + fmt.Printf("%s %-18s %s%s\n", e.ID, shardLabel(e.Shard), e.Title, open) + } + fmt.Printf("\n%d committed.\n", len(res.Entries)) + }, +} + var brainShowCmd = &cobra.Command{ Use: "show [shard]", Short: "Size and headroom per shard (computed, never written down)", @@ -1057,6 +1123,7 @@ func brainLocatedCommands() []*cobra.Command { brainInitCmd, brainCaptureCmd, brainStagedCmd, brainPromoteCmd, brainDropCmd, brainShowCmd, brainCheckCmd, brainPlanCmd, brainResolveCmd, brainBriefCmd, brainEditCmd, brainMoveCmd, + brainListCmd, } } @@ -1078,6 +1145,9 @@ func init() { brainEditCmd.Flags().StringSliceP("anchor", "a", nil, "Replace the anchors with these (repeatable); without -a the anchors are kept") brainMoveCmd.Flags().StringP("slice", "s", "", "The slice to move the requirement to") + brainListCmd.Flags().String("slice", "", "Only this slice's entries") + brainListCmd.Flags().String("kind", "", "Only decisions or only requirements (decision|requirement)") + brainListCmd.Flags().Bool("open", false, "Only open questions; --open=false for only settled entries") brainPromoteCmd.Flags().String("to", "", "Override the derived shard (use 'project' for a cross-cutting fact)") brainStagedCmd.Flags().String("since", "", @@ -1097,7 +1167,7 @@ func init() { "Modules being worked, for a session with no slice; repeatable") brainCmd.AddCommand(brainInitCmd, brainCaptureCmd, brainStagedCmd, brainPromoteCmd, brainDropCmd, brainShowCmd, brainCheckCmd, brainPlanCmd, - brainResolveCmd, brainBriefCmd, brainEditCmd, brainMoveCmd) + brainResolveCmd, brainBriefCmd, brainEditCmd, brainMoveCmd, brainListCmd) rootCmd.AddCommand(brainCmd) } @@ -1132,7 +1202,11 @@ type stagedEntry struct { func stagedReport(all, shown []brain.Entry, filter brain.StagedFilter) map[string]any { out := make([]stagedEntry, 0, len(shown)) for _, e := range shown { - out = append(out, stagedEntry{Entry: e, Shard: e.Shard()}) + shard := e.Shard() + // Explicit, as 'list' gives it: a consumer must not have to know + // that an empty kind means a decision. + e.Kind = e.EntryKind() + out = append(out, stagedEntry{Entry: e, Shard: shard}) } rep := map[string]any{"staged": out, "count": len(out)} diff --git a/cmd/mxcli/cmd_brain_list_test.go b/cmd/mxcli/cmd_brain_list_test.go new file mode 100644 index 0000000000..b44a52afff --- /dev/null +++ b/cmd/mxcli/cmd_brain_list_test.go @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/brain" +) + +type listedJSON struct { + Count int `json:"count"` + Entries []struct { + ID string `json:"id"` + Kind string `json:"kind"` + Slice string `json:"slice"` + Title string `json:"title"` + Anchors []string `json:"anchors"` + Shard string `json:"shard"` + } `json:"entries"` + Malformed []string `json:"malformed"` +} + +// ako/mxcli#1952 through the CLI: the promoted side as entry objects, the same +// shape as `staged --json`, filterable, with no model needed. +func TestBrainListGivesTheCommittedStoreAsData(t *testing.T) { + dir := t.TempDir() + + // Before init: still JSON, with an empty list rather than prose. + var got listedJSON + out := runBrainForTest(t, []string{"brain", "list", "-C", dir, "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil || got.Count != 0 || got.Entries == nil || got.Malformed == nil { + t.Fatalf("list --json on no store: %v\n%s", err, out) + } + + runBrainForTest(t, []string{"brain", "init", "-C", dir}) + store, queue := brain.NewStore(dir), brain.NewQueue(dir) + dec, err := brain.NewEntry("Orders are committed by Finance", []string{"@Sales.Order"}, brainTestDay()) + if err != nil { + t.Fatal(err) + } + req, err := brain.NewRequirement("A planner can create a visit", []string{"@Scheduling.Visit"}, "01-scheduling", brainTestDay()) + if err != nil { + t.Fatal(err) + } + for _, e := range []brain.Entry{dec, req} { + if err := store.Promote(e, e.Shard()); err != nil { + t.Fatal(err) + } + } + // Staged, not committed: must not appear in list. + pending, err := brain.NewEntry("Invoices are numbered per year", []string{"@Sales.Invoice"}, brainTestDay()) + if err != nil { + t.Fatal(err) + } + if _, err := queue.Append(pending); err != nil { + t.Fatal(err) + } + + out = runBrainForTest(t, []string{"brain", "list", "-C", dir, "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("list --json: %v\n%s", err, out) + } + if got.Count != 2 || len(got.Entries) != 2 { + t.Fatalf("want the two committed entries, not the staged one:\n%s", out) + } + if e := got.Entries[0]; e.ID != dec.ID || e.Kind != "decision" || e.Shard != "Sales" || len(e.Anchors) != 1 { + t.Errorf("decision: %+v", e) + } + if e := got.Entries[1]; e.ID != req.ID || e.Kind != "requirement" || e.Slice != "01-scheduling" || e.Shard != brain.PlanShard("01-scheduling") { + t.Errorf("requirement: %+v", e) + } + + out = runBrainForTest(t, []string{"brain", "list", "-C", dir, "--slice", "01-scheduling", "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil || got.Count != 1 || got.Entries[0].ID != req.ID { + t.Errorf("--slice: %v\n%s", err, out) + } + out = runBrainForTest(t, []string{"brain", "list", "-C", dir, "--kind", "decision", "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil || got.Count != 1 || got.Entries[0].ID != dec.ID { + t.Errorf("--kind decision: %v\n%s", err, out) + } + + // staged --json now names the kind too, so both sides read alike. + var staged struct { + Staged []struct { + Kind string `json:"kind"` + } `json:"staged"` + } + out = runBrainForTest(t, []string{"brain", "staged", "-C", dir, "--json"}) + if err := json.Unmarshal([]byte(out), &staged); err != nil || len(staged.Staged) != 1 || staged.Staged[0].Kind != "decision" { + t.Errorf("staged --json kind: %v\n%s", err, out) + } + + // Control: without --json it prints for a person. + out = runBrainForTest(t, []string{"brain", "list", "-C", dir}) + var anyJSON map[string]any + if json.Unmarshal([]byte(out), &anyJSON) == nil || !strings.Contains(out, "2 committed.") { + t.Errorf("human output:\n%s", out) + } +} diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index d5584d0906..88e3be3213 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -409,6 +409,9 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { for _, w := range refWarnings { fmt.Fprintf(os.Stderr, " %s\n", w) } + // Printed here, not through report, but part of the total the + // closing summary gives (ako/mxcli#1928). + printed = append(printed, warnViolations...) } else if len(warnViolations) > 0 && len(validationErrors) == 0 { structured = append(structured, warnViolations...) } @@ -430,6 +433,16 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { fmt.Fprintf(os.Stderr, " %v\n", err) } fmt.Fprintf(os.Stderr, "\n✗ %d reference error(s) found\n", len(validationErrors)) + // The closing summary is the run's total; it read "0 errors" + // after this line, because these are printed here rather than + // through report (ako/mxcli#1928). + for _, err := range validationErrors { + printed = append(printed, linter.Violation{ + RuleID: "MDL-REF", + Severity: linter.SeverityError, + Message: err.Error(), + }) + } if hint := testFileMissingTargetHint(filePath, validationErrors); hint != "" { fmt.Fprint(os.Stderr, hint) } diff --git a/cmd/mxcli/cmd_fmt_upgrade_test.go b/cmd/mxcli/cmd_fmt_upgrade_test.go index 618137047f..0457ca3147 100644 --- a/cmd/mxcli/cmd_fmt_upgrade_test.go +++ b/cmd/mxcli/cmd_fmt_upgrade_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/executor" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -226,22 +227,10 @@ func TestFmtUpgrade_FindOnACallResultReadsTheProject(t *testing.T) { } } +// copyTree copies a fixture project, leaving out the .mxcli caches other +// packages' tests write into it in place (testutil.CopyFixture). func copyTree(src, dst string) error { - return filepath.Walk(src, func(p string, info os.FileInfo, err error) error { - if err != nil { - return err - } - rel, _ := filepath.Rel(src, p) - target := filepath.Join(dst, rel) - if info.IsDir() { - return os.MkdirAll(target, 0o755) - } - data, err := os.ReadFile(p) - if err != nil { - return err - } - return os.WriteFile(target, data, 0o644) - }) + return testutil.CopyFixture(dst, src) } // ako/mxcli#873: without a project, a bare commit in a `create or modify` diff --git a/cmd/mxcli/cmd_layout_flows_test.go b/cmd/mxcli/cmd_layout_flows_test.go index 0b6c0a170d..713f5e49ad 100644 --- a/cmd/mxcli/cmd_layout_flows_test.go +++ b/cmd/mxcli/cmd_layout_flows_test.go @@ -4,11 +4,11 @@ package main import ( "bytes" - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -18,7 +18,7 @@ import ( func flowLayoutFixture(t *testing.T) (*bytes.Buffer, func(args ...string) error) { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } exec, logger := newLoggedExecutor("test") diff --git a/cmd/mxcli/cmd_test_attach_bundle_integration_test.go b/cmd/mxcli/cmd_test_attach_bundle_integration_test.go index efaa08fe2e..d9f5277db1 100644 --- a/cmd/mxcli/cmd_test_attach_bundle_integration_test.go +++ b/cmd/mxcli/cmd_test_attach_bundle_integration_test.go @@ -29,6 +29,14 @@ import ( // // The control is the same page module before the attach, which the boot's // first (complete) build serves with a 200. +// +// On Mendix 11.14.0 the attach cannot apply at all: every later build in a serve +// process fails, even for an unchanged model, and the failed response carries no +// restartRequired (measured over mxbuild's /build API; 11.15.0 builds them and +// answers false for a microflow). The only answer is a cold build's, which asks +// for a restart whatever changed, so the attach is refused and must say why +// rather than claim an entity changed. Its pages must survive all the same. When +// an 11.14 mxbuild builds incrementally again, that branch fails and should go. func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { version := os.Getenv("MXCLI_RUNLIFE_VERSION") if version == "" { @@ -57,6 +65,7 @@ func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { t.Skipf("mx create-project: %v\n%s", err, out) } mpr := filepath.Join(work, "App.mpr") + coldAnswerOnly := projectAtLeast(t, mpr, 11, 14) && !projectAtLeast(t, mpr, 11, 15) const appPort, adminPort, servePort = 8487, 8497, 6943 for _, p := range []int{appPort, adminPort, servePort} { @@ -64,15 +73,20 @@ func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { t.Skipf("port %d is in use", p) } } - mxcli := func(args ...string) string { + run := func(args ...string) (string, error) { cmd := exec.Command(bin, args...) cmd.Dir = work var out bytes.Buffer cmd.Stdout, cmd.Stderr = &out, &out - if err := cmd.Run(); err != nil { - t.Fatalf("mxcli %v: %v\n%s", args, err, out.String()) + err := cmd.Run() + return out.String(), err + } + mxcli := func(args ...string) string { + out, err := run(args...) + if err != nil { + t.Fatalf("mxcli %v: %v\n%s", args, err, out) } - return out.String() + return out } t.Cleanup(func() { _ = exec.Command(bin, "run", "stop", "-p", mpr).Run() }) @@ -102,9 +116,16 @@ func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { if err := os.WriteFile(test, []byte(src), 0o644); err != nil { t.Fatal(err) } - out := mxcli("test", test, "-p", mpr, "--attach", "--skip-app-startup", "--require-assertions") - if !strings.Contains(out, "Passed: 1") { - t.Fatalf("the attach run did not pass:\n%s", out) + out, err := run("test", test, "-p", mpr, "--attach", "--skip-app-startup", "--require-assertions") + switch { + case coldAnswerOnly: + if err == nil || !strings.Contains(out, "could not build this change incrementally, and the full build") || + strings.Contains(out, "an entity or association changed") { + t.Fatalf("Mendix 11.14: want the attach refused for want of an incremental answer, not for an entity "+ + "change; if this mxbuild builds incrementally again, drop the 11.14 branch (err %v):\n%s", err, out) + } + case err != nil || !strings.Contains(out, "Passed: 1"): + t.Fatalf("the attach run did not pass (err %v):\n%s", err, out) } if got := status(); got != http.StatusOK { diff --git a/cmd/mxcli/docker/mxserve.go b/cmd/mxcli/docker/mxserve.go index 66c4ecacf8..925c225864 100644 --- a/cmd/mxcli/docker/mxserve.go +++ b/cmd/mxcli/docker/mxserve.go @@ -78,6 +78,15 @@ type BuildResult struct { Message string `json:"message"` Problems BuildProblems `json:"problems"` Raw json.RawMessage `json:"-"` + + // ColdRestart marks a RestartRequired that comes from a fresh serve + // process's first build with no incremental build to correct it. A first + // build reports true even for an unchanged model, so here true means "could + // not tell", not "an entity changed". serveRecovery sets it; mxbuild never + // sends it. Measured on 11.14.0: every later build in a serve process + // fails, even for an unchanged model, and the failed response has no + // restartRequired at all, so this is the only answer a build there gets. + ColdRestart bool `json:"-"` } // BuildProblems is the serve response's problems object. The consistency errors diff --git a/cmd/mxcli/docker/serve_attached_test.go b/cmd/mxcli/docker/serve_attached_test.go index 305bbe6690..d355f0bf7a 100644 --- a/cmd/mxcli/docker/serve_attached_test.go +++ b/cmd/mxcli/docker/serve_attached_test.go @@ -107,6 +107,40 @@ func TestAttachedServe_CompletesTheBundleOnItsOwnServe(t *testing.T) { } } +// Mendix 11.14.0: the host's build fails outright, even for an unchanged model, +// and the failure carries no restartRequired (measured over mxbuild's /build +// API). The private serve's cold build is then the only answer, and it asks for +// a restart whatever changed: the result says it is that answer, so the attach +// can refuse without blaming an entity. +func TestAttachedServe_FailedHostBuildLeavesOnlyAColdAnswer(t *testing.T) { + deploy := prebuiltDeployment(t) + config := filepath.Join(deploy, "web", "rollup.config.mjs") + host := &scriptedServe{build: func(_, _ int) (*BuildResult, error) { + return &BuildResult{Status: "Failure", Message: "Compilation of the app bundle failed.", + Raw: []byte(`{"problems":{"errors":[{"details":"Error [ERR_MODULE_NOT_FOUND]: Cannot find module '` + + config + `' imported from rollup-runner.mjs"}]},"status":"Failure"}`)}, nil + }} + var own *privateServe + a := attachedServeFor(host, deploy, func() (ownedServe, error) { + own = newPrivateServe(t, deploy) + return own, nil + }) + res, err := a.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}) + if err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v", res, err) + } + if !res.RestartRequired || !res.ColdRestart { + t.Fatalf("want the cold build's restartRequired, marked ColdRestart; got %+v", res) + } + if host.restarts != 0 || own == nil { + t.Fatalf("host restarts = %d, private serve started = %v; want only a private one", host.restarts, own != nil) + } + if pagesIn(t, deploy) == 0 { + t.Fatal("dist/pages is empty after the attach's build") + } + a.Close() +} + // Control: a host build that leaves the bundle complete is all there is — no // private serve process is started for it. func TestAttachedServe_CompleteHostBuildStartsNothing(t *testing.T) { diff --git a/cmd/mxcli/docker/serve_recovery.go b/cmd/mxcli/docker/serve_recovery.go index bef1a47205..cba890248a 100644 --- a/cmd/mxcli/docker/serve_recovery.go +++ b/cmd/mxcli/docker/serve_recovery.go @@ -77,6 +77,7 @@ type serveRecovery struct { } func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { + freshProcess := r.restartPending if r.restartPending { fmt.Fprintln(r.out, " starting mxbuild --serve (the last restart failed)...") if err := r.restart(); err != nil { @@ -89,6 +90,9 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { if err != nil { return res, err } + if freshProcess && res.OK() { + res.ColdRestart = true // the first build of the process just started + } staleTheme := r.themeFailed && isThemeFailure(res) r.observe(res) @@ -122,7 +126,7 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { // succeeded does, and its model is the one the cold build rewrote // byte-for-byte; taking the cold answer would restart the runtime — and // drop every browser session — on every change. - fresh.RestartRequired = res.RestartRequired + fresh.RestartRequired, fresh.ColdRestart = res.RestartRequired, false return fresh, nil } @@ -130,13 +134,17 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { fmt.Fprintln(r.out, " mxbuild --serve cannot build this change incrementally (it wants client source its "+ "first build does not leave behind); restarting mxbuild --serve for a full build...") // No incremental build succeeded, so the cold build's restartRequired - // is the only answer there is. + // is the only answer there is, and it is marked ColdRestart: a caller + // that cannot restart the runtime must not read it as "an entity + // changed". return r.rebuildFresh(req) } return res, nil } -// rebuildFresh replaces the serve process and builds once on the new one. +// rebuildFresh replaces the serve process and builds once on the new one. Its +// RestartRequired is a first build's, so it is marked ColdRestart; a caller +// that has an incremental answer replaces both. func (r *serveRecovery) rebuildFresh(req BuildRequest) (*BuildResult, error) { if err := r.restart(); err != nil { return nil, err @@ -144,6 +152,9 @@ func (r *serveRecovery) rebuildFresh(req BuildRequest) (*BuildResult, error) { res, err := r.serve.Build(req) if err == nil { r.observe(res) + if res != nil && res.OK() { + res.ColdRestart = true + } } return res, err } diff --git a/cmd/mxcli/docker/serve_recovery_test.go b/cmd/mxcli/docker/serve_recovery_test.go index 3306e4bcc7..b0d28cb81f 100644 --- a/cmd/mxcli/docker/serve_recovery_test.go +++ b/cmd/mxcli/docker/serve_recovery_test.go @@ -132,8 +132,8 @@ func TestServeRecovery_IncompletePrebuiltBundleRestartsServe(t *testing.T) { // The fresh process says restartRequired for an unchanged model; the // incremental build's answer is the one that knows what changed. Taking the // cold one would restart the runtime — and drop every session — per change. - if res.RestartRequired { - t.Fatal("RestartRequired must be the incremental build's (false), not the cold build's") + if res.RestartRequired || res.ColdRestart { + t.Fatalf("RestartRequired must be the incremental build's (false), not the cold build's: %+v", res) } if got := out.String(); !strings.Contains(got, "dist/pages") || !strings.Contains(got, "mxbuild --serve") { t.Fatalf("the recovery must be reported in one line naming what was lost, got:\n%s", got) @@ -195,6 +195,12 @@ func TestServeRecovery_MissingClientSourceFailureRestartsServe(t *testing.T) { if !res.RestartRequired { t.Fatal("with no successful incremental build to say otherwise, the cold build's restartRequired stands") } + // ...but it is a first build's answer, true whatever changed, and a caller + // that cannot restart the runtime (test --attach) must be able to say so + // rather than claim an entity changed. + if !res.ColdRestart { + t.Fatal("the cold build's restartRequired must be marked ColdRestart") + } if f.restarts != 1 { t.Fatalf("restarts = %d, want 1", f.restarts) } @@ -225,6 +231,9 @@ func TestServeRecovery_FailedRestartIsRetriedOnNextBuild(t *testing.T) { if f.restarts != before+1 { t.Fatalf("the next build must start a serve process first, restarts %d -> %d", before, f.restarts) } + if !res.ColdRestart { + t.Fatal("the first build of the process just started is a cold answer and must be marked ColdRestart") + } } // themeCompileFailure is the first Sass failure in a serve process, as diff --git a/cmd/mxcli/docker/update_widgets_test.go b/cmd/mxcli/docker/update_widgets_test.go index ef47c53fef..b15bb15797 100644 --- a/cmd/mxcli/docker/update_widgets_test.go +++ b/cmd/mxcli/docker/update_widgets_test.go @@ -18,6 +18,7 @@ import ( "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/types" ) @@ -37,7 +38,7 @@ func storageVersion(t *testing.T, mprPath string) types.MPRVersion { func v2Fixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy v2 fixture: %v", err) } p := filepath.Join(dst, "minimal.mpr") @@ -51,7 +52,7 @@ func v2Fixture(t *testing.T) string { func v1Fixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + if err := testutil.CopyFixture(dst, "../../../modelsdk/mpr/testdata/v1-project"); err != nil { t.Fatalf("copy v1 fixture: %v", err) } p := filepath.Join(dst, "App.mpr") diff --git a/cmd/mxcli/exec_atomic_test.go b/cmd/mxcli/exec_atomic_test.go index 86b5e89c00..cb7d9a3a20 100644 --- a/cmd/mxcli/exec_atomic_test.go +++ b/cmd/mxcli/exec_atomic_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/executor" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -110,22 +111,9 @@ func runScriptOn(t *testing.T, mprPath, script string) error { func copyFixtureTree(t *testing.T, src string) string { t.Helper() dst := t.TempDir() - err := filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - rel, _ := filepath.Rel(src, p) - target := filepath.Join(dst, rel) - if d.IsDir() { - return os.MkdirAll(target, 0o755) - } - data, err := os.ReadFile(p) - if err != nil { - return err - } - return os.WriteFile(target, data, 0o644) - }) - if err != nil { + // Not .mxcli: other packages' tests write their caches into the fixture in + // place (testutil.CopyFixture), and hashTree ignores it anyway. + if err := testutil.CopyFixture(dst, src); err != nil { t.Fatal(err) } return dst diff --git a/cmd/mxcli/exec_preflight_timeline_1762_test.go b/cmd/mxcli/exec_preflight_timeline_1762_test.go index e948758ed6..e52f7f0ec7 100644 --- a/cmd/mxcli/exec_preflight_timeline_1762_test.go +++ b/cmd/mxcli/exec_preflight_timeline_1762_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" @@ -62,7 +63,7 @@ func TestExecPreflightRefusesHiddenTimelineEnumerations(t *testing.T) { t.Skipf("expr-checker fixture not found: %v", err) } dir := t.TempDir() - if err := os.CopyFS(dir, os.DirFS(src)); err != nil { + if err := testutil.CopyFixture(dir, src); err != nil { t.Fatal(err) } mpr := filepath.Join(dir, "minimal.mpr") diff --git a/cmd/mxcli/marketplace/snapshot_test.go b/cmd/mxcli/marketplace/snapshot_test.go index 4b2639dc13..1098e84ac2 100644 --- a/cmd/mxcli/marketplace/snapshot_test.go +++ b/cmd/mxcli/marketplace/snapshot_test.go @@ -4,10 +4,10 @@ package marketplace import ( "bytes" - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" @@ -26,7 +26,7 @@ const fixtureDir = "../../../testdata/expr-checker" func copyFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS(fixtureDir)); err != nil { + if err := testutil.CopyFixture(dst, fixtureDir); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/cmd/mxcli/mcp_exec_no_file_write_test.go b/cmd/mxcli/mcp_exec_no_file_write_test.go index 06985605a5..a85bb45f70 100644 --- a/cmd/mxcli/mcp_exec_no_file_write_test.go +++ b/cmd/mxcli/mcp_exec_no_file_write_test.go @@ -11,6 +11,8 @@ import ( "sort" "strings" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // mendixlabs/mxcli#1684 (ako/mxcli#1833): "`mxcli exec --mcp ` @@ -35,7 +37,7 @@ const mcpProbeConstant = "create constant MyFirstModule.LiveProbe (Type: String, func mcpLockedProject(t *testing.T, lock bool) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } if lock { diff --git a/cmd/mxcli/mcp_trace_test.go b/cmd/mxcli/mcp_trace_test.go index 9de5c2a0ac..7613e4775f 100644 --- a/cmd/mxcli/mcp_trace_test.go +++ b/cmd/mxcli/mcp_trace_test.go @@ -14,6 +14,7 @@ import ( "sync" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -112,7 +113,7 @@ func TestMCPTrace_ListsThePEDCallsOfAWrite(t *testing.T) { srv, calls := fakeStudioPro(t) withMCPFlags(t, srv.URL+"/mcp", strings.TrimPrefix(srv.URL, "http://"), true) dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } diff --git a/cmd/mxcli/project_reader_test.go b/cmd/mxcli/project_reader_test.go index 3d2fc937b7..eb9116ccca 100644 --- a/cmd/mxcli/project_reader_test.go +++ b/cmd/mxcli/project_reader_test.go @@ -13,15 +13,16 @@ package main // silently depend on. import ( - "os" "path/filepath" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func fixtureProject(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/cmd/mxcli/show_version_format_test.go b/cmd/mxcli/show_version_format_test.go index 2b77f6df6a..2a9e770cf3 100644 --- a/cmd/mxcli/show_version_format_test.go +++ b/cmd/mxcli/show_version_format_test.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // SHOW VERSION printed `MPR Format: v2` for an MPR v1 project — a single .mpr @@ -29,7 +31,7 @@ func TestShowVersionReportsTheStorageFormatOnDisk(t *testing.T) { } v2dir := t.TempDir() - if err := os.CopyFS(v2dir, os.DirFS(filepath.Join("..", "..", "testdata", "testapp-views"))); err != nil { + if err := testutil.CopyFixture(v2dir, filepath.Join("..", "..", "testdata", "testapp-views")); err != nil { t.Fatalf("copy v2 fixture: %v", err) } v2 := filepath.Join(v2dir, "TestApp.mpr") diff --git a/cmd/mxcli/testrunner/runner_attach.go b/cmd/mxcli/testrunner/runner_attach.go index f6e75d6eb2..5c5334536e 100644 --- a/cmd/mxcli/testrunner/runner_attach.go +++ b/cmd/mxcli/testrunner/runner_attach.go @@ -94,19 +94,36 @@ func (a *attachedApp) applyModelChange(projectPath string) (string, error) { return "", buildFailure(build) } // No restart callback: the runtime belongs to the other process. A structural - // change is refused rather than half-applied — see the error below. + // change is refused rather than half-applied — see attachRestartRefusal. action, err := a.ctrl.ApplyBuild(build, nil) if err != nil { return action.String(), err } if action == docker.ActionRestart { - return action.String(), fmt.Errorf("this change needs a runtime restart (an entity or association changed), " + - "which --attach cannot do — the runtime belongs to the 'mxcli run --local' process.\n" + - " Restart that process, or drop --attach to run against a scratch runtime.") + return action.String(), attachRestartRefusal(build) } return action.String(), nil } +// attachRestartRefusal is why an attach cannot apply a build that asks for a +// runtime restart. The runtime belongs to the other process, so a structural +// change is refused rather than half-applied. +func attachRestartRefusal(build *docker.BuildResult) error { + if build.ColdRestart { + // Nothing says an entity changed: the serve process could not build + // incrementally (Mendix 11.14.0 fails every later build), and a first + // build asks for a restart whatever changed. Restarting the dev loop does + // not help either — the attach's build is again a later one in its serve. + return fmt.Errorf("mxbuild --serve could not build this change incrementally, and the full build it " + + "fell back to asks for a runtime restart whatever changed (it has no earlier build to compare with), " + + "which --attach cannot do — the runtime belongs to the 'mxcli run --local' process.\n" + + " Drop --attach to run against a scratch runtime.") + } + return fmt.Errorf("this change needs a runtime restart (an entity or association changed), " + + "which --attach cannot do — the runtime belongs to the 'mxcli run --local' process.\n" + + " Restart that process, or drop --attach to run against a scratch runtime.") +} + // runAttached injects the test microflows into the already-running app, runs the // suite, and removes them again. // diff --git a/cmd/mxcli/testrunner/runner_attach_restart_test.go b/cmd/mxcli/testrunner/runner_attach_restart_test.go new file mode 100644 index 0000000000..316bcff3a2 --- /dev/null +++ b/cmd/mxcli/testrunner/runner_attach_restart_test.go @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// On Mendix 11.14.0 an attach's build only ever gets a cold build's answer, +// which asks for a restart whatever changed. The refusal must say that, not +// claim an entity changed, and must not offer restarting the dev loop, which +// leaves the next attach exactly where this one is. +func TestAttachRestartRefusal_ColdAnswerBlamesNoEntity(t *testing.T) { + msg := attachRestartRefusal(&docker.BuildResult{Status: "Success", RestartRequired: true, ColdRestart: true}).Error() + if strings.Contains(msg, "entity") || strings.Contains(msg, "Restart that process") { + t.Errorf("a cold build's restart answer is not evidence of an entity change:\n%s", msg) + } + if !strings.Contains(msg, "could not build this change incrementally") || !strings.Contains(msg, "Drop --attach") { + t.Errorf("want why the answer is unknown and the way out:\n%s", msg) + } +} + +// Control: an incremental build that asks for a restart did see a structural change. +func TestAttachRestartRefusal_IncrementalAnswerNamesTheChange(t *testing.T) { + msg := attachRestartRefusal(&docker.BuildResult{Status: "Success", RestartRequired: true}).Error() + if !strings.Contains(msg, "an entity or association changed") { + t.Errorf("want the structural-change refusal:\n%s", msg) + } +} diff --git a/docs-site/src/tools/project-brain.md b/docs-site/src/tools/project-brain.md index 6f2b42323f..d13fea2a9d 100644 --- a/docs-site/src/tools/project-brain.md +++ b/docs-site/src/tools/project-brain.md @@ -316,6 +316,41 @@ rewrite the file. The queue is less forgiving: an older build that rewrites `.mxcli/brain/staged.jsonl` (on a capture, drop or promote) drops the source of the entries still queued there. +## Reading the store from a program + +```bash +mxcli brain list -C ./App --json +mxcli brain list --slice 01-scheduling --json -p app.mpr +mxcli brain list --open --json -p app.mpr # --open=false: only settled entries +``` + +`brain list --json` is the committed side of `brain staged --json`: the same +entry objects — `id`, `kind`, `slice`, `open`, `title`, `body`, `anchors`, +`source`, `date` — each with the `shard` it is in, in store order (`project.md`, +modules, slices; file order within each). `kind` is always present. + +```json +{ + "count": 1, + "entries": [ + { + "id": "72dfdd", "kind": "requirement", "slice": "01-scheduling", + "title": "A planner can create a visit", + "anchors": ["@Scheduling.Visit"], "source": "docs/sample-prd.md#L18-21", + "date": "2026-11-04", "shard": "plan/01-scheduling" + } + ], + "malformed": [] +} +``` + +Use it rather than parsing the shard files: their format is mxcli's to change. +`brief --json` is the other read, meant for a prompt — it returns each shard's +rendered markdown. A block whose metadata line cannot be read is listed under +`malformed` instead of being dropped, so a listing never quietly shows fewer +entries than the files hold. Built-ness is not in the listing: that is derived +from the model by `brain plan`, never stored on an entry. + ## Editing an entry ```bash @@ -425,6 +460,7 @@ progress from 65/65 to 63/65 and the number was the only symptom. | `brain plan [--slice ]` | Each slice's requirements counted against the model | | `brain brief --slice \| --module ` | The reading pack: exactly the shards that work needs | | `brain check [--changed]` | Anchors resolve, entries filed correctly, plus slice progress | +| `brain list [--slice ] [--kind ] [--open]` | The committed entries; with `--json`, as data | | `brain show []` | Entries, lines and headroom per shard | Dropping the last entry from a module shard deletes the file, so the directory diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 4a903b24c6..d953d66af9 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -176,6 +176,13 @@ data behind in the app you are looking at. It needs the dev loop to have been started with `--test-endpoint`, because the endpoint's handler is registered by the after-startup microflow and cannot be added to an app that is already up. +On **Mendix 11.14.0** an attach is always refused. That version's +`mxbuild --serve` fails every build after its first, even for an unchanged model, +and says nothing about whether the runtime needs a restart. So the attach builds +on a fresh serve process instead, and a fresh process's first build asks for a +restart whatever changed — which an attach cannot do, the runtime being the other +process's. Use `--local` on 11.14.0; 11.15.0 builds incrementally again. + ## How tests execute **`--local` — the test endpoint.** One microflow is generated per test, plus a diff --git a/internal/testutil/fixture.go b/internal/testutil/fixture.go new file mode 100644 index 0000000000..fa5288083f --- /dev/null +++ b/internal/testutil/fixture.go @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "io/fs" + "os" +) + +// CopyFixture copies the fixture project at src into dst the way os.CopyFS +// does, leaving out every .mxcli directory. +// +// .mxcli holds mxcli's own caches, never anything a fixture commits: nothing +// under it is tracked. But a test that opens a fixture in place writes them +// there (the unit-contents store, ako/mxcli#1804, a SQLite file whose journal +// comes and goes with each write), and go test runs packages in parallel. A +// whole-directory copy in another package then lists a journal that is gone by +// the time it opens it, and fails with +// +// open .mxcli/cache/unit-contents.db-journal: no such file or directory +// +// (seen on the nightly in mdl/backend/mcp). Leaving .mxcli out also makes the +// copy what a fresh checkout holds, whichever tests ran first. +func CopyFixture(dst, src string) error { + return copyFixtureFS(dst, os.DirFS(src)) +} + +func copyFixtureFS(dst string, fsys fs.FS) error { + return os.CopyFS(dst, withoutMxcli{fsys}) +} + +// withoutMxcli hides every .mxcli entry from the directory listings os.CopyFS +// walks. +type withoutMxcli struct{ fs.FS } + +func (f withoutMxcli) ReadDir(name string) ([]fs.DirEntry, error) { + entries, err := fs.ReadDir(f.FS, name) + kept := entries[:0] + for _, e := range entries { + if e.Name() != ".mxcli" { + kept = append(kept, e) + } + } + return kept, err +} diff --git a/internal/testutil/fixture_test.go b/internal/testutil/fixture_test.go new file mode 100644 index 0000000000..67ae843846 --- /dev/null +++ b/internal/testutil/fixture_test.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "errors" + "io/fs" + "os" + "path/filepath" + "testing" + "testing/fstest" +) + +// vanishingJournal is a fixture caught mid-write by another package's test: its +// listing still names the cache's journal, which is gone when the copy opens it. +type vanishingJournal struct{ fstest.MapFS } + +const journal = ".mxcli/cache/unit-contents.db-journal" + +func (v vanishingJournal) Open(name string) (fs.File, error) { + if name == journal { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + return v.MapFS.Open(name) +} + +func fixtureWithJournal() vanishingJournal { + return vanishingJournal{fstest.MapFS{ + "minimal.mpr": {Data: []byte("mpr")}, + "mprcontents/ab/cd/unit.mxunit": {Data: []byte("unit")}, + ".mxcli/cache/unit-contents.db": {Data: []byte("cache")}, + journal: {Data: []byte("journal")}, + "widgets/.mxcli/widgets/x.def.json": {Data: []byte("def")}, + }} +} + +// The nightly's symptom, reproduced deterministically: a plain os.CopyFS of the +// fixture fails on the journal another test's write has already removed. +func TestCopyFixture_Control_PlainCopyFailsOnAVanishedJournal(t *testing.T) { + err := os.CopyFS(t.TempDir(), fixtureWithJournal()) + if !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("os.CopyFS = %v, want the vanished journal's not-exist error", err) + } +} + +func TestCopyFixture_LeavesOutEveryMxcliDirectory(t *testing.T) { + dst := t.TempDir() + if err := copyFixtureFS(dst, fixtureWithJournal()); err != nil { + t.Fatalf("copy: %v", err) + } + for _, rel := range []string{"minimal.mpr", "mprcontents/ab/cd/unit.mxunit"} { + if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); err != nil { + t.Errorf("the project's %s was not copied: %v", rel, err) + } + } + for _, rel := range []string{".mxcli", "widgets/.mxcli"} { + if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); !os.IsNotExist(err) { + t.Errorf("%s was copied (stat err %v); it holds caches, never the fixture", rel, err) + } + } +} diff --git a/mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl b/mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl new file mode 100644 index 0000000000..fd915139aa --- /dev/null +++ b/mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl @@ -0,0 +1,19 @@ +-- ako/mxcli#1821: `check --references` resolved the target document of alter +-- entity and alter page only. Each statement below names a document that does +-- not exist; exec refuses every one ("microflow not found", …), and check now +-- reports each in the alter entity style, with what the module does have: +-- microflow not found: BugTest1821.NoSuchFlow (referenced by drop microflow) — BugTest1821 has … +-- Run against a project with a BugTest1821 module: every statement is reported. +-- `drop … if exists`, a document the script creates first, and one an earlier +-- rename or move lands under the name are not reported. +mdl 1; +drop microflow BugTest1821.NoSuchFlow; +drop page BugTest1821.NoSuchPage; +drop entity BugTest1821.NoSuchEntity; +drop enumeration BugTest1821.NoSuchEnum; +drop association BugTest1821.NoSuchAssoc; +rename microflow BugTest1821.NoSuchFlow to Other; +move microflow BugTest1821.NoSuchFlow to folder 'X'; +alter enumeration BugTest1821.NoSuchEnum add value V caption 'V'; +alter microflow BugTest1821.NoSuchFlow { drop $X; }; +drop microflow if exists BugTest1821.NoSuchFlow; diff --git a/mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl b/mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl new file mode 100644 index 0000000000..5c690ecdba --- /dev/null +++ b/mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl @@ -0,0 +1,15 @@ +-- ako/mxcli#1928 (mendixlabs/mxcli#1726): `check --references` validated a call +-- against an EXCLUDED microflow when a live one has the same name, and refused +-- a correct call: "microflow BugTest1928.Test has no parameter "param1" +-- (declared parameters: param2)". +-- +-- Setup in Studio Pro (or with mxcli on a copy): BugTest1928.Test($param1) live, +-- and a second BugTest1928.Test($param2) marked "Exclude from project", listed +-- after it. Mendix accepts the pair (CE0122 counts active documents only). +-- Then `mxcli check this.mdl -p app.mpr --references` passes; naming param2 +-- instead is refused against the live flow's parameter list. +mdl 1; +create or modify microflow BugTest1928.Test2 () +begin + call microflow BugTest1928.Test(param1 = 'test'); +end; diff --git a/mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl b/mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl new file mode 100644 index 0000000000..614d7f8300 --- /dev/null +++ b/mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl @@ -0,0 +1,15 @@ +-- ako/mxcli#1940 (mendixlabs/mxcli#1736): a parameter added by an earlier +-- `alter page … add parameters` in the same script was not declared for a later +-- show page, so check --references and exec refused the script. Run against a +-- project with a page BugTest1940.Edit taking $Account: check passes, exec +-- alters the page and creates the flow. A `drop parameters` earlier in the +-- script likewise removes the name. +mdl 1; +alter page BugTest1940.Edit { + add parameters $Extra: Administration.Account; +}; +create microflow BugTest1940.OpenEdit ($Acc: Administration.Account) +begin + show page BugTest1940.Edit(Account = $Acc, Extra = $Acc); + return; +end; diff --git a/mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl b/mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl new file mode 100644 index 0000000000..5cebd55ece --- /dev/null +++ b/mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl @@ -0,0 +1,19 @@ +-- ako/mxcli#555: an attribute named inside a widget was not resolved by +-- `check --references`, so a misspelt name passed check, exec wrote it and +-- mxbuild refused it. Measured on a fresh 11.12.6 app: each of the three +-- bindings below is CE1613 "The selected attribute 'BugTest555.WorkOrder.PlannedOn' +-- no longer exists" (3 errors), and the same page binding PlannedDate / Title +-- builds clean. check -p --references now reports all three before the write. +mdl 1; +create module BugTest555; +create persistent entity BugTest555.WorkOrder ( PlannedDate: DateTime, Title: String(100) ); +create page BugTest555.P_WorkOrder ( Title: 'x', Layout: Atlas_Core.Atlas_Default, Params: ( $W: BugTest555.WorkOrder ) ) +{ + layoutgrid lg { row { column (desktopwidth: autofill) { + dataview dv (DataSource: $W) { + textbox tb (Label: 'Planned', Attribute: PlannedOn) + dynamictext dt (Content: 'Planned {1}', ContentParams: ({1} = PlannedOn)) + actionbutton btn (Caption: 'Go {1}', CaptionParams: ({1} = PlannedOn), Action: close page) + } + } } } +}; diff --git a/mdl/backend/mcp/connect_reads_test.go b/mdl/backend/mcp/connect_reads_test.go index 35add397e7..e3d6bb1840 100644 --- a/mdl/backend/mcp/connect_reads_test.go +++ b/mdl/backend/mcp/connect_reads_test.go @@ -12,11 +12,11 @@ package mcp // half goes over MCP to Studio Pro and is covered by the fakePED tests around it. import ( - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" @@ -28,7 +28,7 @@ import ( func localProject(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/mdl/backend/modelsdk/domainmodel_get_cost_test.go b/mdl/backend/modelsdk/domainmodel_get_cost_test.go index 49db6ac554..526c945f40 100644 --- a/mdl/backend/modelsdk/domainmodel_get_cost_test.go +++ b/mdl/backend/modelsdk/domainmodel_get_cost_test.go @@ -3,11 +3,11 @@ package modelsdkbackend import ( - "os" "path/filepath" "reflect" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -104,7 +104,7 @@ func TestGetDomainModel_AgreesWithTheFullListing(t *testing.T) { func copyV1Project(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + if err := testutil.CopyFixture(dst, "../../../modelsdk/mpr/testdata/v1-project"); err != nil { t.Fatalf("copy v1 fixture: %v", err) } return filepath.Join(dst, "App.mpr") diff --git a/mdl/backend/modelsdk/domainmodel_write_test.go b/mdl/backend/modelsdk/domainmodel_write_test.go index b66f13781d..964e424826 100644 --- a/mdl/backend/modelsdk/domainmodel_write_test.go +++ b/mdl/backend/modelsdk/domainmodel_write_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -15,7 +15,7 @@ import ( func copyFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/mdl/backend/modelsdk/flow_version_keys_test.go b/mdl/backend/modelsdk/flow_version_keys_test.go index 1203b2d2d8..07b823c88f 100644 --- a/mdl/backend/modelsdk/flow_version_keys_test.go +++ b/mdl/backend/modelsdk/flow_version_keys_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -40,7 +40,7 @@ import ( func copyV9Fixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + if err := testutil.CopyFixture(dst, "../../../modelsdk/mpr/testdata/v1-project"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "App.mpr") diff --git a/mdl/backend/modelsdk/issue1373_property_sets_test.go b/mdl/backend/modelsdk/issue1373_property_sets_test.go index 875ccee8f9..53df2a8133 100644 --- a/mdl/backend/modelsdk/issue1373_property_sets_test.go +++ b/mdl/backend/modelsdk/issue1373_property_sets_test.go @@ -3,12 +3,12 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" bsonv2 "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -27,7 +27,7 @@ import ( // Documentation — the key whose absence made `mx diff` throw. func TestIssue1373_RewrittenLoopKeepsItsPropertySet(t *testing.T) { dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go b/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go index d39a63e126..d77a3a87e8 100644 --- a/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go +++ b/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go @@ -11,6 +11,7 @@ import ( "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" @@ -166,7 +167,7 @@ func TestIssue1805_PublishedMicroflowWritesItsParametersWhenThereAreNone(t *test // name the elements of a type disagree on. func TestIssue1805_PublishedServiceCensusHasNoMixedNames(t *testing.T) { dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go b/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go index f73ef0c0b8..c6ef6a97b0 100644 --- a/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go +++ b/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -17,7 +17,7 @@ import ( func copyPedApp(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/pedapp")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/pedapp"); err != nil { t.Fatalf("copy PedApp fixture: %v", err) } return filepath.Join(dst, "PedApp.mpr") diff --git a/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go b/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go index 180a4dc9ad..2b1cd8586d 100644 --- a/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go +++ b/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go @@ -3,11 +3,11 @@ package modelsdkbackend import ( - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" genDm "github.com/mendixlabs/mxcli/modelsdk/gen/domainmodels" ) @@ -298,7 +298,7 @@ func assertNoDanglingAssociations(t *testing.T, b *Backend) { func copyPedAppFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/pedapp")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/pedapp"); err != nil { t.Fatalf("copy PedApp fixture: %v", err) } return filepath.Join(dst, "PedApp.mpr") diff --git a/mdl/backend/modelsdk/settings_create_configuration_test.go b/mdl/backend/modelsdk/settings_create_configuration_test.go index 72ad858088..e9a005e632 100644 --- a/mdl/backend/modelsdk/settings_create_configuration_test.go +++ b/mdl/backend/modelsdk/settings_create_configuration_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" ) @@ -17,7 +17,7 @@ import ( // 11.6 and 11.13 fixtures have no such part, which is why it went unnoticed. func TestUpdateProjectSettings_CreateConfigurationOn1114(t *testing.T) { dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/backend/modelsdk/widget_custom_find_test.go b/mdl/backend/modelsdk/widget_custom_find_test.go index 16732ce85a..3b5251e355 100644 --- a/mdl/backend/modelsdk/widget_custom_find_test.go +++ b/mdl/backend/modelsdk/widget_custom_find_test.go @@ -8,11 +8,12 @@ package modelsdkbackend // — the method would quietly go back to erroring. import ( - "os" "path/filepath" "testing" "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // imageWidgetID is in the shared fixture; combobox and datagrid are too. @@ -21,7 +22,7 @@ const imageWidgetID = "com.mendix.widget.web.image.Image" func customWidgetFixture(t *testing.T) *Backend { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } b := New() diff --git a/mdl/backend/modelsdk/workflow_esp_test.go b/mdl/backend/modelsdk/workflow_esp_test.go index 20c8e12789..bf9560b245 100644 --- a/mdl/backend/modelsdk/workflow_esp_test.go +++ b/mdl/backend/modelsdk/workflow_esp_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" genWf "github.com/mendixlabs/mxcli/modelsdk/gen/workflows" "github.com/mendixlabs/mxcli/sdk/workflows" "go.mongodb.org/mongo-driver/v2/bson" @@ -105,7 +106,7 @@ func TestCreateWorkflow_EventSubProcessesRoundTrip(t *testing.T) { // fixture: on the 11.6 one the storage layer refuses the workflow // (TestCreateWorkflow_EventSubProcessesRefusedBeforeTheirVersion). dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/executor/cmd_odata_actions.go b/mdl/executor/cmd_odata_actions.go index f6447ad919..71b843611e 100644 --- a/mdl/executor/cmd_odata_actions.go +++ b/mdl/executor/cmd_odata_actions.go @@ -140,8 +140,11 @@ func microflowsByQualifiedName(ctx *ExecContext) (map[string]*microflows.Microfl return nil, mdlerrors.NewBackend("list microflows", err) } out := make(map[string]*microflows.Microflow, len(mfs)) - for _, mf := range mfs { - out[strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name))] = mf + for _, mf := range liveFirst(mfs, microflowExcluded) { + key := strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name)) + if _, seen := out[key]; !seen { // live before an excluded twin (#1928) + out[key] = mf + } } return out, nil } diff --git a/mdl/executor/helpers.go b/mdl/executor/helpers.go index 9b89b18930..b38e2ec237 100644 --- a/mdl/executor/helpers.go +++ b/mdl/executor/helpers.go @@ -669,8 +669,11 @@ func buildMicroflowReturnTypes(ctx *ExecContext) map[string]string { if err != nil { return result } - for _, mf := range mfs { + for _, mf := range liveFirst(mfs, microflowExcluded) { qn := h.GetQualifiedName(mf.ContainerID, mf.Name) + if _, seen := result[qn]; seen { + continue // an excluded twin of a live flow (#1928) + } if mf.ReturnType == nil { result[qn] = "" continue diff --git a/mdl/executor/typecheck_test.go b/mdl/executor/typecheck_test.go index 7314a59222..2e5b53acc1 100644 --- a/mdl/executor/typecheck_test.go +++ b/mdl/executor/typecheck_test.go @@ -4,11 +4,11 @@ package executor import ( "bytes" - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/mdl/linter" @@ -27,7 +27,7 @@ import ( func typeCheckFixture(t *testing.T) *Executor { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "minimal.mpr") diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index a50eeadd97..415f74e2bb 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -36,15 +36,25 @@ type scriptContext struct { // association endpoint check from refusing what exec will find (#555). indirectEntities map[string]bool externalEntityModules map[string]bool - enumerations map[string]bool // Enumerations created (Module.Enum) - microflows map[string]bool // Microflows created (Module.Microflow) - nanoflows map[string]bool // Nanoflows created (Module.Nanoflow) - pages map[string]bool // Pages created (Module.Page) - snippets map[string]bool // Snippets created (Module.Snippet) - layouts map[string]bool // Layouts created (Module.Layout) - menus map[string]bool // Menu documents created (Module.Menu) - constants map[string]bool // Constants created (Module.Constant) - workflows map[string]bool // Workflows created (Module.Workflow) + // indirectDocs is the same for every document kind ("microflow:M.Name"), + // and renamedModules the names a rename module gives, so the statement + // target check does not refuse what an earlier statement provides + // (ako/mxcli#1821). opaqueModules are modules a statement fills with + // documents it does not name (generated connectors, business event + // services); opaqueScript is an execute script, which can create anything. + indirectDocs map[string]bool + renamedModules map[string]bool + opaqueModules map[string]bool + opaqueScript bool + enumerations map[string]bool // Enumerations created (Module.Enum) + microflows map[string]bool // Microflows created (Module.Microflow) + nanoflows map[string]bool // Nanoflows created (Module.Nanoflow) + pages map[string]bool // Pages created (Module.Page) + snippets map[string]bool // Snippets created (Module.Snippet) + layouts map[string]bool // Layouts created (Module.Layout) + menus map[string]bool // Menu documents created (Module.Menu) + constants map[string]bool // Constants created (Module.Constant) + workflows map[string]bool // Workflows created (Module.Workflow) // Task queues created (Module.Queue, lower-cased — a queue is resolved // case-insensitively, like buildQueueQualifiedNames). Missing, a queue the // script creates was "not found" for every `in queue` call to it @@ -99,9 +109,13 @@ type scriptContext struct { // the workflow task signature checks, for the same reason as flowParams: the // task page, the targeting microflow, the context entity and the workflow are // ordinarily one script. - pageParams map[string][]string // Module.Page (lower-cased) -> parameter entities - pageParamNames map[string][]string // Module.Page (lower-cased) -> parameter names, for show page arguments - entityGeneralizations map[string]string // Module.Entity (lower-cased) -> generalization, "" for none + pageParams map[string][]string // Module.Page (lower-cased) -> parameter entities + pageParamNames map[string][]string // Module.Page (lower-cased) -> parameter names, for show page arguments + // pageParamEdits is each page's add/drop parameter operations from alter + // page statements, in script order, applied over pageParamNames or the + // stored list (ako/mxcli#1940). + pageParamEdits map[string][]ast.AlterPageOperation + entityGeneralizations map[string]string // Module.Entity (lower-cased) -> generalization, "" for none // Associations and entity attributes declared in the script, for // MDL-XPATH01. Same reason as flowParams above: the overwhelmingly common @@ -173,6 +187,10 @@ func newScriptContext() *scriptContext { pageParams: make(map[string][]string), pageParamNames: make(map[string][]string), + pageParamEdits: make(map[string][]ast.AlterPageOperation), + indirectDocs: make(map[string]bool), + renamedModules: make(map[string]bool), + opaqueModules: make(map[string]bool), entityGeneralizations: make(map[string]string), } } @@ -304,10 +322,24 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { if s.ObjectType == "entity" && s.Name.Module != "" && s.NewName != "" { sc.indirectEntities[s.Name.Module+"."+s.NewName] = true } + if s.ObjectType == "module" && s.NewName != "" { + sc.renamedModules[s.NewName] = true + } else if s.Name.Module != "" && s.NewName != "" && !s.DryRun { + sc.indirectDocs[s.ObjectType+":"+s.Name.Module+"."+s.NewName] = true + } case *ast.MoveStmt: if s.DocumentType == ast.DocumentTypeEntity && s.TargetModule != "" { sc.indirectEntities[s.TargetModule+"."+s.Name.Name] = true } + if s.TargetModule != "" { + sc.indirectDocs[moveTargetKind(s.DocumentType)+":"+s.TargetModule+"."+s.Name.Name] = true + } + case *ast.ExecuteScriptStmt: + sc.opaqueScript = true + case *ast.SQLGenerateConnectorStmt: + sc.opaqueModules[s.Module] = true + case *ast.CreateBusinessEventServiceStmt: + sc.opaqueModules[s.Name.Module] = true case *ast.CreateEnumerationStmt: if s.Name.Module != "" { sc.enumerations[s.Name.String()] = true @@ -336,6 +368,10 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { if s.Name.Module != "" { sc.snippets[s.Name.String()] = true } + case *ast.AlterPageStmt: + if s.ContainerType == "PAGE" && s.PageName.Module != "" { + sc.recordPageParamEdits(s.PageName.String(), s.Operations) + } case *ast.CreateLayoutStmt: if s.Name.Module != "" { sc.layouts[s.Name.String()] = true diff --git a/mdl/executor/validate_alter_target.go b/mdl/executor/validate_alter_target.go index a39168b967..f0f9d6ae6e 100644 --- a/mdl/executor/validate_alter_target.go +++ b/mdl/executor/validate_alter_target.go @@ -33,61 +33,161 @@ import ( // a false "not found" blocks a script that would have worked, which is worse // than the silence it replaces. -// validateAlterTarget reports an ALTER whose target document does not exist, -// counting documents the script itself creates earlier. +// validateAlterTarget reports a statement whose target document does not +// exist, counting documents the script itself creates or lands under the name. +// +// It began with ALTER; drop, rename and move name an existing document the same +// way and passed check while exec refused them — 9 of the 11 forms that name +// one (ako/mxcli#1821). They resolve against the same listings, so each kind +// below means the same thing for every verb. func validateAlterTarget(ctx *ExecContext, stmt ast.Statement, sc *scriptContext) error { if !ctx.Connected() { return nil } + // DROP … IF EXISTS names something that may legitimately be absent. + if g, ok := stmt.(ast.IfExistsDrop); ok && g.DropIfExists() { + return nil + } switch s := stmt.(type) { case *ast.AlterPageStmt: // One statement type, three document kinds — the visitor sets // ContainerType from the keyword. switch strings.ToUpper(s.ContainerType) { case "SNIPPET": - return checkAlterTarget(ctx, sc, "snippet", s.PageName, - buildSnippetQualifiedNames, func(qn string) bool { return sc.snippets[qn] }) + return checkTarget(ctx, sc, "alter", "snippet", s.PageName) case "LAYOUT": - return checkAlterTarget(ctx, sc, "layout", s.PageName, - buildLayoutQualifiedNames, func(qn string) bool { return sc.layouts[qn] }) + return checkTarget(ctx, sc, "alter", "layout", s.PageName) default: - return checkAlterTarget(ctx, sc, "page", s.PageName, - buildPageQualifiedNames, func(qn string) bool { return sc.pages[qn] }) + return checkTarget(ctx, sc, "alter", "page", s.PageName) } case *ast.AlterEntityStmt: + return checkTarget(ctx, sc, "alter", "entity", s.Name) + case *ast.AlterEnumerationStmt: + return checkTarget(ctx, sc, "alter", "enumeration", s.Name) + case *ast.AlterFlowStmt: + return checkTarget(ctx, sc, "alter", s.Kind(), s.Name) + case *ast.DropMicroflowStmt: + return checkTarget(ctx, sc, "drop", "microflow", s.Name) + case *ast.DropNanoflowStmt: + return checkTarget(ctx, sc, "drop", "nanoflow", s.Name) + case *ast.DropPageStmt: + return checkTarget(ctx, sc, "drop", "page", s.Name) + case *ast.DropSnippetStmt: + return checkTarget(ctx, sc, "drop", "snippet", s.Name) + case *ast.DropEntityStmt: + return checkTarget(ctx, sc, "drop", "entity", s.Name) + case *ast.DropEnumerationStmt: + return checkTarget(ctx, sc, "drop", "enumeration", s.Name) + case *ast.DropAssociationStmt: + return checkTarget(ctx, sc, "drop", "association", s.Name) + case *ast.DropConstantStmt: + return checkTarget(ctx, sc, "drop", "constant", s.Name) + case *ast.RenameStmt: + if s.DryRun { + return nil + } + return checkTarget(ctx, sc, "rename", s.ObjectType, s.Name) + case *ast.MoveStmt: + return checkTarget(ctx, sc, "move", moveTargetKind(s.DocumentType), s.Name) + } + return nil +} + +// moveTargetKind is a move's document type as the kind name rename and the +// target check use; "" for one they do not resolve. +func moveTargetKind(t ast.DocumentType) string { + switch t { + case ast.DocumentTypePage: + return "page" + case ast.DocumentTypeMicroflow: + return "microflow" + case ast.DocumentTypeNanoflow: + return "nanoflow" + case ast.DocumentTypeSnippet: + return "snippet" + case ast.DocumentTypeEntity: + return "entity" + case ast.DocumentTypeEnumeration: + return "enumeration" + case ast.DocumentTypeConstant: + return "constant" + } + return "" +} + +// targetListing is, per kind, the project's documents and the script's own. +// A kind missing here is not resolved: a false "not found" blocks a script +// that would have run, which is worse than the silence it replaces. +func targetListing(ctx *ExecContext, sc *scriptContext, kind string) (stored func() map[string]bool, inScript func(string) bool) { + switch kind { + case "page": + return func() map[string]bool { return buildPageQualifiedNames(ctx) }, func(qn string) bool { return sc.pages[qn] } + case "snippet": + return func() map[string]bool { return buildSnippetQualifiedNames(ctx) }, func(qn string) bool { return sc.snippets[qn] } + case "layout": + return func() map[string]bool { return buildLayoutQualifiedNames(ctx) }, func(qn string) bool { return sc.layouts[qn] } + case "microflow": + return func() map[string]bool { return buildMicroflowQualifiedNames(ctx) }, func(qn string) bool { return sc.microflows[qn] } + case "nanoflow": + return func() map[string]bool { return buildNanoflowQualifiedNames(ctx) }, func(qn string) bool { return sc.nanoflows[qn] } + case "entity": // A MOVE or RENAME earlier in the script lands an entity under a name // no CREATE declares (mendixlabs/mxcli#1601). - return checkAlterTarget(ctx, sc, "entity", s.Name, buildEntityQualifiedNames, - func(qn string) bool { return sc.entities[qn] || sc.indirectEntities[qn] }) + return func() map[string]bool { return buildEntityQualifiedNames(ctx) }, + func(qn string) bool { return sc.entities[qn] || sc.indirectEntities[qn] } + case "enumeration": + return func() map[string]bool { return buildEnumerationQualifiedNames(ctx) }, func(qn string) bool { return sc.enumerations[qn] } + case "association": + return func() map[string]bool { return buildAssociationQualifiedNames(ctx) }, + func(qn string) bool { + _, name, _ := strings.Cut(qn, ".") + return sc.associations[name] == qn || sc.ambiguousAssc[name] + } + case "constant": + return func() map[string]bool { + names, ok := buildConstantQualifiedNames(ctx) + if !ok { + return nil + } + return names + }, + func(qn string) bool { return sc.constants[qn] } } - return nil + return nil, nil } -// checkAlterTarget resolves one target, and on a miss reports the near names so +// checkTarget resolves one target, and on a miss reports the near names so // the author can see the typo rather than only that they made one. -func checkAlterTarget(ctx *ExecContext, sc *scriptContext, kind string, name ast.QualifiedName, - known func(*ExecContext) map[string]bool, inScript func(string) bool) error { +func checkTarget(ctx *ExecContext, sc *scriptContext, verb, kind string, name ast.QualifiedName) error { + stored, inScript := targetListing(ctx, sc, kind) + if stored == nil { + return nil + } qn := name.String() - // An unqualified name is a different error, already reported elsewhere; and - // a module the script creates has no listing to resolve against yet. - if name.Module == "" || sc.modules[name.Module] { + // An unqualified name is a different error, already reported elsewhere; a + // module the script creates or renames has no listing to resolve against + // yet; and a module something fills without naming its documents cannot + // be judged from the statements. + if name.Module == "" || sc.modules[name.Module] || sc.renamedModules[name.Module] || + sc.opaqueScript || sc.opaqueModules[name.Module] || + (kind == "entity" && sc.externalEntityModules[name.Module]) { return nil } - if inScript(qn) { + if inScript(qn) || sc.indirectDocs[kind+":"+qn] { return nil } - stored := known(ctx) - if stored[qn] { + known := stored() + if known[qn] { return nil } // An empty listing means the backend could not answer, not that the project // has no pages. Reporting "not found" from it would fail every script. - if len(stored) == 0 { + if len(known) == 0 { return nil } return mdlerrors.NewNotFoundMsg(kind, qn, fmt.Sprintf( - "%s not found: %s (referenced by alter %s)%s", kind, qn, kind, - nearNamesIn(stored, name.Module))) + "%s not found: %s (referenced by %s %s)%s", kind, qn, verb, kind, + nearNamesIn(known, name.Module))) } // nearNamesIn lists what the module does have, capped so a large module does not diff --git a/mdl/executor/validate_datasource_args.go b/mdl/executor/validate_datasource_args.go index 38cba040b8..1572215791 100644 --- a/mdl/executor/validate_datasource_args.go +++ b/mdl/executor/validate_datasource_args.go @@ -631,21 +631,25 @@ func buildFlowSignatures(ctx *ExecContext) map[string]*flowSignature { return result } if mfs, err := ctx.Backend.ListMicroflows(); err == nil { - for _, mf := range mfs { + for _, mf := range liveFirst(mfs, microflowExcluded) { if mf == nil { continue } - result[strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name))] = - sdkFlowSignature(mf.Parameters, mf.ReturnType) + key := strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name)) + if _, seen := result[key]; !seen { + result[key] = sdkFlowSignature(mf.Parameters, mf.ReturnType) + } } } if nfs, err := ctx.Backend.ListNanoflows(); err == nil { - for _, nf := range nfs { + for _, nf := range liveFirst(nfs, nanoflowExcluded) { if nf == nil { continue } - result[strings.ToLower(h.GetQualifiedName(nf.ContainerID, nf.Name))] = - sdkFlowSignature(nf.Parameters, nf.ReturnType) + key := strings.ToLower(h.GetQualifiedName(nf.ContainerID, nf.Name)) + if _, seen := result[key]; !seen { + result[key] = sdkFlowSignature(nf.Parameters, nf.ReturnType) + } } } return result diff --git a/mdl/executor/validate_excluded_twin_signature_test.go b/mdl/executor/validate_excluded_twin_signature_test.go new file mode 100644 index 0000000000..33fc022e95 --- /dev/null +++ b/mdl/executor/validate_excluded_twin_signature_test.go @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1928 (mendixlabs/mxcli#1726): two microflows of one qualified name, +// one excluded — which Mendix accepts (CE0122 counts active documents only). +// `check --references` validated a call against the excluded twin's parameters +// and refused a correct call. The validators index stored documents by name in +// a map, so the twin listed LAST won. Each index must hold the live one, in +// either list order. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +type twinOrder struct { + name string + excludedFirst bool +} + +var twinOrders = []twinOrder{{"excluded twin listed last", false}, {"excluded twin listed first", true}} + +func twinPair[T any](live, excluded T, excludedFirst bool) []T { + if excludedFirst { + return []T{excluded, live} + } + return []T{live, excluded} +} + +func newExcludedTwinCtx(t *testing.T, order twinOrder) *ExecContext { + t.Helper() + mod := mkModule("Examples") + live := mkMicroflow(mod.ID, "Test") + live.Parameters = []*microflows.MicroflowParameter{{Name: "param1", Type: µflows.StringType{}}} + live.ReturnType = µflows.BooleanType{} + excl := mkMicroflow(mod.ID, "Test") + excl.Excluded = true + excl.Parameters = []*microflows.MicroflowParameter{{Name: "param2", Type: µflows.StringType{}}} + excl.ReturnType = µflows.StringType{} + + nlive := mkNanoflow(mod.ID, "NTest") + nlive.Parameters = []*microflows.MicroflowParameter{{Name: "param1", Type: µflows.StringType{}}} + nexcl := mkNanoflow(mod.ID, "NTest") + nexcl.Excluded = true + nexcl.Parameters = []*microflows.MicroflowParameter{{Name: "param2", Type: µflows.StringType{}}} + + plive := mkPage(mod.ID, "Edit") + plive.Parameters = []*pages.PageParameter{{Name: "param1", EntityName: "Examples.Live"}} + pexcl := mkPage(mod.ID, "Edit") + pexcl.Excluded = true + pexcl.Parameters = []*pages.PageParameter{{Name: "param2", EntityName: "Examples.Excluded"}} + + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return twinPair(live, excl, order.excludedFirst), nil + }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { + return twinPair(nlive, nexcl, order.excludedFirst), nil + }, + ListPagesFunc: func() ([]*pages.Page, error) { + return twinPair(plive, pexcl, order.excludedFirst), nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx +} + +func TestExcludedTwin_ValidatorIndexesHoldTheLiveDocument(t *testing.T) { + for _, order := range twinOrders { + t.Run(order.name, func(t *testing.T) { + ctx := newExcludedTwinCtx(t, order) + + sigs := buildFlowSignatures(ctx) + for _, key := range []string{"examples.test", "examples.ntest"} { + if got := sigs[key].paramNames(); len(got) != 1 || got[0] != "param1" { + t.Errorf("buildFlowSignatures[%s] params = %v, want the live [param1]", key, got) + } + } + if got := buildMicroflowReturnTypes(ctx)["Examples.Test"]; got != (µflows.BooleanType{}).GetTypeName() { + t.Errorf("buildMicroflowReturnTypes = %q, want the live flow's Boolean", got) + } + if got := buildPageParamNames(ctx)["examples.edit"]; len(got) != 1 || got[0] != "param1" { + t.Errorf("buildPageParamNames = %v, want the live [param1]", got) + } + if got := buildPageParamEntities(ctx)["examples.edit"]; len(got) != 1 || got[0] != "Examples.Live" { + t.Errorf("buildPageParamEntities = %v, want the live [Examples.Live]", got) + } + mfs, err := microflowsByQualifiedName(ctx) + if err != nil || mfs["examples.test"] == nil || mfs["examples.test"].Excluded { + t.Errorf("microflowsByQualifiedName picked the excluded twin (err %v)", err) + } + }) + } +} + +// The reported script: a call written against the live flow's parameter. +func TestExcludedTwin_CallAgainstLiveFlowPasses(t *testing.T) { + arg := func(name string) []ast.CallArgument { + return []ast.CallArgument{{Name: name, Value: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "test"}}} + } + for _, order := range twinOrders { + t.Run(order.name, func(t *testing.T) { + ctx := newExcludedTwinCtx(t, order) + ok := []ast.MicroflowStatement{&ast.CallMicroflowStmt{MicroflowName: qn("Examples", "Test"), Arguments: arg("param1")}} + if errs := validateFlowBodyReferences(ctx, ok, newScriptContext()); len(errs) != 0 { + t.Fatalf("call with the live flow's parameter refused: %v", errs) + } + // Control: the excluded twin's parameter is not one the app has. + bad := []ast.MicroflowStatement{&ast.CallMicroflowStmt{MicroflowName: qn("Examples", "Test"), Arguments: arg("param2")}} + errs := validateFlowBodyReferences(ctx, bad, newScriptContext()) + if len(errs) != 1 || !strings.Contains(errs[0], `has no parameter "param2" (declared parameters: param1)`) { + t.Fatalf("errors = %v, want the excluded twin's parameter refused against the live one", errs) + } + }) + } +} diff --git a/mdl/executor/validate_project_reader_test.go b/mdl/executor/validate_project_reader_test.go index 08490ea56d..a30fb69650 100644 --- a/mdl/executor/validate_project_reader_test.go +++ b/mdl/executor/validate_project_reader_test.go @@ -14,10 +14,10 @@ package executor import ( "bytes" - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/mdl/visitor" @@ -27,7 +27,7 @@ import ( func projectFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/mdl/executor/validate_show_page_params.go b/mdl/executor/validate_show_page_params.go index a7e05bd5e3..9fa2920b8e 100644 --- a/mdl/executor/validate_show_page_params.go +++ b/mdl/executor/validate_show_page_params.go @@ -3,6 +3,7 @@ package executor import ( + "slices" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -30,6 +31,7 @@ func validateShowPageParams(ctx *ExecContext, calls []codeActionCallRef, sc *scr if !ok { continue } + declared = applyPageParamEdits(declared, sc.pageParamEdits[key]) for _, msg := range validateCodeActionParams("page", call, declared) { for _, written := range call.argNames { if strings.EqualFold(written, "NumberOfPagesToClose") && strings.Contains(msg, `"`+written+`"`) { @@ -67,17 +69,21 @@ func buildPageParamNames(ctx *ExecContext) map[string][]string { if err != nil { return out } - for _, p := range all { + for _, p := range liveFirst(all, pageExcluded) { if p == nil { continue } + key := strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name)) + if _, seen := out[key]; seen { + continue // an excluded twin of a live page (#1928) + } names := []string{} for _, param := range p.Parameters { if param != nil { names = append(names, param.Name) } } - out[strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name))] = names + out[key] = names } return out } @@ -92,3 +98,37 @@ func (sc *scriptContext) recordPageParamNames(qualifiedName string, params []ast } sc.pageParamNames[strings.ToLower(qualifiedName)] = names } + +// recordPageParamEdits keeps an alter page's add/drop parameter operations, so +// a show page later in the script is checked against the page as the script +// leaves it, not as it is stored (ako/mxcli#1940): adding a parameter to a page +// and passing it from the flow that opens the page is one change. +func (sc *scriptContext) recordPageParamEdits(qualifiedName string, ops []ast.AlterPageOperation) { + key := strings.ToLower(qualifiedName) + for _, op := range ops { + switch op.(type) { + case *ast.AddParameterOp, *ast.DropParameterOp: + sc.pageParamEdits[key] = append(sc.pageParamEdits[key], op) + } + } +} + +// applyPageParamEdits returns declared with the edits applied in order. It +// never changes declared, which may be the stored page's list. +func applyPageParamEdits(declared []string, edits []ast.AlterPageOperation) []string { + if len(edits) == 0 { + return declared + } + out := append([]string{}, declared...) + for _, op := range edits { + switch o := op.(type) { + case *ast.AddParameterOp: + if !slices.Contains(out, o.Parameter.Name) { + out = append(out, o.Parameter.Name) + } + case *ast.DropParameterOp: + out = slices.DeleteFunc(out, func(n string) bool { return n == o.ParameterName }) + } + } + return out +} diff --git a/mdl/executor/validate_show_page_params_alter_test.go b/mdl/executor/validate_show_page_params_alter_test.go new file mode 100644 index 0000000000..939a0aa084 --- /dev/null +++ b/mdl/executor/validate_show_page_params_alter_test.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1940 (mendixlabs/mxcli#1736): `alter page … { add parameters $X }` +// followed in the same script by a show page passing X was refused — by check +// --references and by exec's pre-flight — because the argument check read the +// page as stored before the script. Split into two scripts, both ran. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func TestValidate_ShowPageSeesParametersAlteredInTheScript(t *testing.T) { + mod := &model.Module{Name: "M"} + mod.ID = "mod1" + stored := &pages.Page{Name: "Edit", Parameters: []*pages.PageParameter{{Name: "District"}, {Name: "Choice"}}} + stored.ContainerID = "mod1" + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{stored}, nil }, + } + show := func(page string, names ...string) ast.MicroflowStatement { + var args []ast.ShowPageArg + for _, n := range names { + args = append(args, ast.ShowPageArg{ParamName: n, Value: &ast.VariableExpr{Name: "V"}}) + } + return &ast.ShowPageStmt{PageName: qn("M", page), Arguments: args} + } + alter := func(page string, ops ...ast.AlterPageOperation) ast.Statement { + return &ast.AlterPageStmt{ContainerType: "PAGE", PageName: qn("M", page), Operations: ops} + } + add := func(name string) ast.AlterPageOperation { + return &ast.AddParameterOp{Parameter: ast.PageParameter{Name: name}} + } + drop := func(name string) ast.AlterPageOperation { return &ast.DropParameterOp{ParameterName: name} } + + cases := []struct { + name string + before []ast.Statement + call ast.MicroflowStatement + wantErr string + }{ + {"the reported script: a parameter added to a stored page", + []ast.Statement{alter("Edit", add("Extra"))}, show("Edit", "District", "Choice", "Extra"), ""}, + {"a parameter dropped from a stored page is no longer declared", + []ast.Statement{alter("Edit", drop("Choice"))}, show("Edit", "District", "Choice"), + `page M.Edit has no parameter "Choice" (declared parameters: District)`}, + {"add then drop across two alters", + []ast.Statement{alter("Edit", add("Extra")), alter("Edit", drop("Extra"))}, show("Edit", "Extra"), + `page M.Edit has no parameter "Extra" (declared parameters: Choice, District)`}, + {"a page created in the script, then altered", + []ast.Statement{&ast.CreatePageStmtV3{Name: qn("M", "New")}, alter("New", add("Obj"))}, show("New", "Obj"), ""}, + {"control: no alter, the stored page still refuses an unknown name", + nil, show("Edit", "Extra"), `page M.Edit has no parameter "Extra" (declared parameters: Choice, District)`}, + {"control: an alter snippet does not change the page of that name", + []ast.Statement{&ast.AlterPageStmt{ContainerType: "SNIPPET", PageName: qn("M", "Edit"), Operations: []ast.AlterPageOperation{add("Extra")}}}, + show("Edit", "Extra"), `page M.Edit has no parameter "Extra"`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(mb)) + body := []ast.MicroflowStatement{tc.call} + stmts := append(append([]ast.Statement{}, tc.before...), &ast.CreateMicroflowStmt{Name: qn("M", "Caller"), Body: body}) + sc := newScriptContext() + sc.collectDefinitions(&ast.Program{Statements: stmts}) + errs := validateFlowBodyReferences(ctx, body, sc) + if tc.wantErr == "" { + if len(errs) != 0 { + t.Fatalf("refused: %v", errs) + } + return + } + if len(errs) != 1 || !strings.Contains(errs[0], tc.wantErr) { + t.Fatalf("errors = %v, want one containing %q", errs, tc.wantErr) + } + }) + } + // The stored page itself is untouched by the simulation. + if len(stored.Parameters) != 2 { + t.Errorf("stored page parameters changed: %d", len(stored.Parameters)) + } +} diff --git a/mdl/executor/validate_widget_attribute_scope.go b/mdl/executor/validate_widget_attribute_scope.go index f1ffe47e6d..83298b0172 100644 --- a/mdl/executor/validate_widget_attribute_scope.go +++ b/mdl/executor/validate_widget_attribute_scope.go @@ -121,6 +121,7 @@ func (v *attributeScopeValidator) walk(w *ast.WidgetV3, enclosing dataContext) { } inner := childContext(enclosing, w.GetDataSource(), v.sigs, v.pageParams) v.checkInputBinding(w, enclosing) + v.checkTemplateParams(w, enclosing) v.checkAssocListSource(w, enclosing) v.checkViewAssocReverse(w, enclosing) v.checkAssocHopStart(w, enclosing) diff --git a/mdl/executor/validate_widget_attribute_type.go b/mdl/executor/validate_widget_attribute_type.go index 2ba843c552..7cfca730d3 100644 --- a/mdl/executor/validate_widget_attribute_type.go +++ b/mdl/executor/validate_widget_attribute_type.go @@ -386,8 +386,12 @@ func (v *attributeScopeValidator) checkInputBinding(w *ast.WidgetV3, enclosing d "anything else). Bind an attribute over it (`%s/`), or select the associated object "+ "with a `combobox` over the association ["+widgetAttributeTypeRule+"]", w.Name, strings.ToLower(w.Type), name, assocQN, entity, name, name)) + return } } + if !isComboBox { + v.reportMissingAttribute(w, "", attr, entity, name) + } return } if a.kind == "" || !measuredAttributeKinds[a.kind] || rule.accepts[a.kind] { @@ -403,6 +407,92 @@ func (v *attributeScopeValidator) checkInputBinding(w *ast.WidgetV3, enclosing d v.errs = append(v.errs, msg+" ["+widgetAttributeTypeRule+"]") } +// auditMembers are the system members an entity's audit settings add — +// bindable by these names, and never in its attribute list. +var auditMembers = kindSet("createddate", "changeddate", "owner", "changedby") + +// reportMissingAttribute is CE1613 for a binding to an attribute the entity +// in context does not have (ako/mxcli#555). Exec wrote the name as given and +// mxbuild refused it; check had resolved neither an input widget's Attribute +// nor a template parameter. where names the template parameter ("" for the +// widget's own Attribute). +func (v *attributeScopeValidator) reportMissingAttribute(w *ast.WidgetV3, where, path, entity, name string) { + if auditMembers[strings.ToLower(name)] { + return + } + var have []string + for _, e := range v.types.chain(entity) { + for _, a := range v.types.attrs[e] { + have = append(have, a.name) + } + } + sort.Strings(have) + const max = 12 + list := strings.Join(have, ", ") + if len(have) > max { + list = fmt.Sprintf("%s and %d more", strings.Join(have[:max], ", "), len(have)-max) + } + if list == "" { + list = "none" + } + subject := fmt.Sprintf("widget `%s` (%s)", w.Name, strings.ToLower(w.Type)) + if where != "" { + subject += " " + where + } + v.errs = append(v.errs, fmt.Sprintf( + "%s binds `%s`, but %s has no attribute %s (attributes: %s) — mxbuild reports CE1613 "+ + "\"The selected attribute '%s.%s' no longer exists\"", + subject, path, entity, name, list, entity, name)) +} + +// checkTemplateParams judges the attribute bindings among a widget's template +// parameters (ContentParams, CaptionParams) the way checkInputBinding judges an +// input's Attribute: against the entity the enclosing data container supplies, +// the context exec qualifies a bare name with (ako/mxcli#555). An expression, +// a $variable path, or a context this walk cannot resolve is not judged. +func (v *attributeScopeValidator) checkTemplateParams(w *ast.WidgetV3, enclosing dataContext) { + if enclosing.unresolved || len(enclosing.entities) == 0 { + return + } + for _, set := range []struct { + key string + params []ast.ParamAssignmentV3 + }{{"ContentParams", w.GetContentParams()}, {"CaptionParams", w.GetCaptionParams()}} { + for _, p := range set.params { + raw, ok := p.Value.(string) + if !ok || isTemplateExpression(raw) { + continue + } + path := strings.TrimPrefix(strings.TrimSpace(raw), "$currentObject/") + if path == "" || strings.HasPrefix(path, "$") { + continue + } + entity := enclosing.entities[len(enclosing.entities)-1] + segs := strings.Split(path, "/") + resolved := true + for _, seg := range segs[:len(segs)-1] { + _, next, ok := v.types.follow(seg, entity) + if !ok { + resolved = false + break + } + entity = next + } + name := segs[len(segs)-1] + if !resolved || name == "" || strings.Contains(name, ".") { + continue + } + if _, _, found, known := v.types.lookup(entity, name); !known || found { + continue + } + if _, _, isAssoc := v.types.follow(name, entity); isAssoc { + continue + } + v.reportMissingAttribute(w, fmt.Sprintf("%s {%d}", set.key, p.Index), raw, entity, name) + } + } +} + func anPrefix(kind string) string { if strings.ContainsAny(kind[:1], "AEIOU") { return "n" diff --git a/mdl/executor/validate_widget_attribute_type_test.go b/mdl/executor/validate_widget_attribute_type_test.go index f9f694ded0..eba69ac0e2 100644 --- a/mdl/executor/validate_widget_attribute_type_test.go +++ b/mdl/executor/validate_widget_attribute_type_test.go @@ -215,15 +215,17 @@ create page Sales.P2 (title: 'P', layout: Atlas_Core.Atlas_Default, params: ($Sh } // What the walk cannot place is not judged: an association data source, a page -// variable, an attribute nobody declares. +// variable. An attribute the entity does not have is not a TYPE error — there is +// no type to judge — but it is the CE1613 of ako/mxcli#555, reported once. func TestCheck_AttributeTypeUnjudgedWhenUnknown(t *testing.T) { ctx := typeCheckCtx(t, "Yes") errs := typeErrs(t, ctx, `dataview dv (datasource: $Thing) { dataview dv2 (datasource: association Thing_Other) { textbox tb1 (label: 'x', attribute: Shade) } textbox tb2 (label: 'x', attribute: NoSuchAttribute) }`, nil) - if len(errs) != 0 { - t.Fatalf("want nothing judged, got %q", errs) + if len(errs) != 1 || !strings.Contains(errs[0], "widget `tb2`") || + !strings.Contains(errs[0], "has no attribute NoSuchAttribute") || strings.Contains(errs[0], widgetAttributeTypeRule) { + t.Fatalf("want only tb2's missing-attribute CE1613, no type error, got %q", errs) } } diff --git a/mdl/executor/validate_widget_object_property_test.go b/mdl/executor/validate_widget_object_property_test.go index 56348e3b68..11d256b603 100644 --- a/mdl/executor/validate_widget_object_property_test.go +++ b/mdl/executor/validate_widget_object_property_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/visitor" @@ -26,7 +27,7 @@ func fixtureProjectWithDefs(t *testing.T) string { t.Helper() src := filepath.Dir(fixtureProject(t)) dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS(src)); err != nil { + if err := testutil.CopyFixture(dst, src); err != nil { t.Fatalf("copy fixture: %v", err) } // Start from no definitions whatever the developer's tree holds, so the diff --git a/mdl/executor/validate_workflow_refs.go b/mdl/executor/validate_workflow_refs.go index 23b4ab68e9..2ecb6b3944 100644 --- a/mdl/executor/validate_workflow_refs.go +++ b/mdl/executor/validate_workflow_refs.go @@ -32,8 +32,11 @@ func validateWorkflowParameterMappings(ctx *ExecContext, s *ast.CreateWorkflowSt return nil } paramsByMF := make(map[string][]*microflows.MicroflowParameter, len(mfs)) - for _, mf := range mfs { - paramsByMF[h.GetQualifiedName(mf.ContainerID, mf.Name)] = mf.Parameters + for _, mf := range liveFirst(mfs, microflowExcluded) { + key := h.GetQualifiedName(mf.ContainerID, mf.Name) + if _, seen := paramsByMF[key]; !seen { // live before an excluded twin (#1928) + paramsByMF[key] = mf.Parameters + } } contextEntity := s.ParameterEntity.String() diff --git a/mdl/executor/validate_workflow_task_signature.go b/mdl/executor/validate_workflow_task_signature.go index ade66b56c5..1f653c397c 100644 --- a/mdl/executor/validate_workflow_task_signature.go +++ b/mdl/executor/validate_workflow_task_signature.go @@ -271,17 +271,21 @@ func buildPageParamEntities(ctx *ExecContext) map[string][]string { if err != nil { return out } - for _, p := range all { + for _, p := range liveFirst(all, pageExcluded) { if p == nil { continue } + key := strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name)) + if _, seen := out[key]; seen { + continue // an excluded twin of a live page (#1928) + } entities := make([]string, 0, len(p.Parameters)) for _, param := range p.Parameters { if param != nil { entities = append(entities, param.EntityName) } } - out[strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name))] = entities + out[key] = entities } return out } diff --git a/mdl/roundtrip/viewentity_identity_test.go b/mdl/roundtrip/viewentity_identity_test.go index fc6fa615f4..5cc6c1980d 100644 --- a/mdl/roundtrip/viewentity_identity_test.go +++ b/mdl/roundtrip/viewentity_identity_test.go @@ -7,12 +7,12 @@ package roundtrip import ( "bytes" "encoding/hex" - "os" "path/filepath" "testing" "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" @@ -153,7 +153,7 @@ func copyViewsFixture(t *testing.T) string { t.Fatal(err) } dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS(src)); err != nil { + if err := testutil.CopyFixture(dst, src); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, viewsFixtureMPR)