Fix the mxcli test --local regression, the last CE0463 field, and add the guard both reports asked for - #361
Merged
Merged
Conversation
`mxcli test --local` could not start a runtime for any project:
Error: local runtime: runtime admin API did not come up:
runtime process exited during startup
java.lang.IllegalArgumentException: Path
'<project>/.mxcli/deployment-test/model/bundles' cannot be resolved
in base path '<project>/.mxcli/deployment-test'
The tree had data/ and no model/ at all, because **mxbuild writes the
deployment to `<app dir>/deployment` and has no option to move it**.
Measured rather than inferred: `--target=deploy` on a project whose
deployment/ had just been deleted recreated it there, `mxbuild --help`
lists no deployment-path flag, and BuildRequest carries none.
So giving the test boot a deployment tree of its own moved where the
RUNTIME reads and not where the BUILD writes. StartLocalApp now refuses a
DeployDir the build will not populate, naming mxbuild as the constraint,
so a caller finds out at the point they can still act on it rather than
inside the JVM against a path they never chose.
The blanking that the scratch tree was meant to prevent — a headless test
boot's packaging pass deleting deployment/web/dist, leaving a concurrent
`run --local` serving HTTP 200 over a blank page — is fixed the only way
the constraint allows: the bundle is copied aside before the boot and put
back after. That costs a few MB of copy against the ~30s re-bundle that
made warning the earlier choice, and returns the exact bundle the dev loop
built. The restore writes only when the bundle is actually gone, so a
newer one built during the boot is never clobbered.
The tests that did not catch this are the lesson. Four of them asserted
DeployDir was set, was under .mxcli/, was per-project and was not the dev
loop's — and all four passed against a build that could not start, because
every one was about the option value while the symptom lived in what is on
disk after mxbuild runs.
Controls, both end-to-end on a real 11.13 project rather than in a unit
test: the pre-fix binary reproduces the reported JVM error byte for byte
while the fixed one runs the suite green; and with preserveWebClientBundle
stubbed out, a sentinel bundle is destroyed by a test run that still
reports every test passed — which is the silent failure §62 described.
Reported as mxcli-ledger FINDINGS §150; supersedes the fix for
mxcli-formula1 FINDINGS §62.
A copied Atlas layout still failed mx check with CE0463, and the reporting
project took it apart to one field: a full diff of Atlas' own brand image
against a describe -> rename -> exec copy, GUIDs masked, differs in one
line of 1480.
=== ONLY IN ATLAS === Object/Properties/21/Value/PrimitiveValue = '250'
=== ONLY IN MINE === Object/Properties/21/Value/PrimitiveValue = '0'
Resolved through its TypePointer that is `maxHeight`, whose declared
default in Image 1.6.0 is 250.
Same class as the width/height fix that shipped alongside it, which did
not cover it — and why is the point. The reset was applied in a loop over
the definition's PROPERTY MAPPINGS, and a mapping is what gives a property
an MDL keyword. `width` and `height` have one; `maxHeight` has none, so it
was never visited and the widget template's captured value stood. The set
of properties that must be default-valued is the widget's editorConfig to
decide, not mxcli's; making it a subset of what MDL has words for was the
mistake.
Three things had to be true, and each was independently wrong:
1. The reset must reach properties that have no mapping.
2. A rule whose CONDITION is an unmapped property was always
indeterminable, so it never fired — `maxHeight` is hidden when
`maxHeightUnit` is "none", and nothing knew what maxHeightUnit was.
The declared default is the right fallback precisely because MDL
cannot name the property, so nothing can have moved it off it; a
value the script set still wins.
3. `def.PropertyVisibility` is EMPTY for every widget whose rules are
lifted live from the .mpk, which is most of them. Keying the new
lookup on that field found nothing at all, and only the end-to-end
run showed it — both consumers now share visibilityRules().
Verified end-to-end on a real 11.13 project with the Image package patched
to declare maxHeight 250, since the version that does is not obtainable
here. Pre-fix binary writes 0 and all four mxcli-authored Image widgets
fail CE0463; fixed binary writes 250 and all four are clean. `minHeight`
stays at its own declared 0 throughout, so this is per-property from the
package rather than a blanket value. The project's ~60 Studio Pro-authored
Images stay stale under both, which is the package change itself and not
mxcli — the Step 0 discrimination the CE0463 skill asks for. On the
unpatched package, three Image configurations plus a datagrid still check
at 0 errors.
Reported as mxcli-ledger FINDINGS §142.
A reporting project ran `mxcli widget sync` on a widget that `mx check` was
reporting CE0463 on at that moment, and got:
Every stored widget instance already matches its installed package.
Nothing to do.
Both were right, about different things. The comparison is over the stored
SCHEMA — each property's declared type attributes — and the CE0463 was a
VALUE, in the widget's Object, which this command does not look at. The
message claimed the wider of the two.
It now says which plane it checked and points at `mxcli fix widgets` for
the rest. The command's own help already says it is partial; this stops the
one line a user actually reads from contradicting that.
Noted in mxcli-ledger FINDINGS §142.
…s against The end-to-end guard two projects independently asked for, in the same words. mxcli-sudoku §51 names it exactly: "an end-to-end assertion that one `mxcli test --local` run leaves a model/ directory where the runtime is told to look." Both reports drew the same lesson, and it is about the tests rather than the code. Four unit tests covered the change that broke `mxcli test --local` for every project, and all four passed against a build that could not start, because each asserted that the OPTION was set — under .mxcli/, per-project, not the dev loop's — while the symptom lived in what is on disk after mxbuild runs. So this asserts the artefact: after a real build, the directory the runtime will be booted against holds model/ and model/bundles — the exact path the JVM named when this broke. It stops at the build, since the mismatch is fully visible one step before a JVM and a database get involved. MXCLI_IT_PROJECT points it at an existing project. Without it the test scaffolds one with `mx create-project`, which produces a project at the INSTALLED mxbuild's version — whose JDK may not be present, and the test then skips. A guard that only ever skips proves nothing, which this repo has already paid for once. For the same reason the scaffold does not use t.TempDir(): it names the directory after the test function, and Mendix's toolset rejects the result with PathTooLongException, which arrives as another skip. Control: reinstating the regression (runtime pointed at .mxcli/deployment-test, guard disabled — main's current state) fails the test naming both missing paths and the directory the runtime would boot against; the fixed code passes. Asked for in mxcli-sudoku FINDINGS §51 and mxcli-ledger FINDINGS §150.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three findings from two projects, and the guard both of them asked for.
The first is a regression I shipped in #347 and is the reason to merge this promptly:
mxcli test --localdoes not start a runtime for any project onmaintoday. Two projects hit it independently, four days apart, and reached the same diagnosis.2cff9ab7mxcli test --localcannot boot — regression on mainee295467b9aab17ewidget syncclaimed more than it checked1d5c9f77§150 / §51 —
mxcli test --localcannot start a runtimeThe tree has
data/and nomodel/. Introduced by2c0aa8d2 fix(test): give a local test run its own deployment tree, which gave the test boot a scratch deployment directory so it would stop rebuilding the one a livemxcli run --localserves.mxbuild writes the deployment to
<app dir>/deploymentand has no option to move it. Measured rather than inferred:--target=deployon a project whosedeployment/had just been deleted recreated it there;mxbuild --helplists no deployment-path flag (-o/--outputnames the.mdafortarget=package); and the serve API'sBuildRequestcarries none.So
DeployDirdecided where the runtime reads while nothing decided where the build writes.StartLocalAppnow refuses aDeployDirthe build will not populate, naming mxbuild as the constraint, so a caller finds out where they can act on it rather than inside the JVM against a path they never chose.The hazard the scratch tree was for is fixed the only way the constraint allows
Sudoku §51 is right that the original change did not achieve its own purpose either — the build still wrote to
deployment/, so only the runtime moved. Confirmed here directly: with the new preservation stubbed out, a sentinel bundle is destroyed by a test run that still reports every test passed.The deployment directory is shared because mxbuild shares it, so the bundle is carried across the boot instead: copied aside before, put back after. That is a few MB of copy against the ~30s re-bundle that made warning the earlier choice, and it returns the exact bundle the dev loop built rather than a rebuilt approximation. The restore writes only when the bundle is actually gone, so a newer one built during the boot is never clobbered.
One correction to both reports. Their prescribed fix — thread
DeployDirthrough the build — cannot be implemented, for the reason above. That is why this takes the other branch.--skip-buildmeans what it always meant again; the guard that refused it until a tree existed that nothing ever created is gone.§142 —
maxHeight, the one differing line in 1480The reporting project took the remaining CE0463 apart to a single field. A full field-level diff of Atlas' own brand image against a
describe→ rename →execcopy of it, GUIDs masked:Resolved through its
TypePointerthat ismaxHeight, whose declared default in Image 1.6.0 is 250.Same class as the width/height fix in #347, which did not cover it — and why is the point. The reset ran in a loop over the definition's property mappings, and a mapping is what gives a property an MDL keyword.
widthandheighthave one;maxHeighthas none, so it was never visited and the widget template's captured value stood.Three things had to be true and each was independently wrong:
maxHeightis hidden whenmaxHeightUnitis"none", and nothing knew whatmaxHeightUnitwas. The declared default is the right fallback precisely because MDL cannot name the property, so nothing can have moved it off it; a value the script set still wins.def.PropertyVisibilityis empty for every widget whose rules are lifted live from the.mpk, which is most of them. Keying the new lookup on that field found nothing at all, and only the end-to-end run showed it — both consumers now sharevisibilityRules().The general rule: the set of properties that must be default-valued is the widget's editorConfig to decide, not mxcli's. Making it a subset of what MDL has words for was the mistake.
widget syncsaid more than it had checkedThe finding notes that
mxcli widget syncreported "Every stored widget instance already matches its installed package. Nothing to do" on a widgetmx checkwas reporting CE0463 on at that moment. Both were right, about different things: sync compares the stored schema, and the CE0463 was a value, in the widget'sObject, which the command does not look at. The message now says which plane it checked and points atmxcli fix widgetsfor the rest. Reconciling the Object plane is a separate feature and is not in this PR.The guard, asked for by name
Sudoku §51 names the remedy exactly: "an end-to-end assertion that one
mxcli test --localrun leaves amodel/directory where the runtime is told to look."Both reports drew the same lesson, and it is about the tests rather than the code. Four unit tests covered the change that broke the command, and all four passed against a build that could not start, because each asserted that the option was set — under
.mxcli/, per-project, not the dev loop's — while the symptom lived in what is on disk after mxbuild runs. Sudoku puts it generally: a fix that redirects a consumer without redirecting the producer moves the failure rather than removing it.So the new integration test asserts the artefact: after a real build, the directory the runtime will be booted against holds
model/andmodel/bundles— the exact path the JVM named. It stops at the build, since the mismatch is fully visible one step before a JVM and a database get involved. CI already runsmake test-integrationnightly and on push.Getting it to actually run took two tries, both worth recording, because both failure modes arrive as a skip — and a guard that only ever skips proves nothing, which this repo has already paid for once:
mx create-projectproduces a project at the installed mxbuild's version, whose JDK may not be present.MXCLI_IT_PROJECTpoints the test at a project the machine can build.t.TempDir()names the directory after the test function, and Mendix's toolset rejects the result withPathTooLongException.Verification
Every fix has a control that passed before the change.
Total: 1 Passed: 1. WithpreserveWebClientBundlestubbed, a sentinel bundle is destroyed by a run that still reports all tests passed; with it, the bundle survives byte-identical including a nested chunk..mxcli/deployment-test, refusal disabled:main's current state) fails it, naming both missing paths and the directory the runtime would boot against..mpkwas patched to declaremaxHeight250, the value the finding measured. Pre-fix writes0and all four mxcli-authored Image widgets fail CE0463; fixed writes250and all four are clean.minHeightstays at its own declared0throughout, so this is per-property from the package rather than a blanket value. The project's ~60 Studio Pro-authored Images stay stale under both — that is the package change itself, not mxcli, which is the Step 0 discriminationdiagnose-ce0463.mdasks for. On the unpatched package, three Image configurations plus a datagrid still check at 0 errors.go test ./...green,make vet/make lint/make check-mdlclean (455 MDL examples).Note on branch history
#347 merged at
ac43acecwhile these commits were being written, so they were pushed to a branch whose PR had already closed. They are rebased onto currentmainhere — no merge commits, no duplicates of whatmainalready has.🤖 Generated with Claude Code
https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
Generated by Claude Code