Skip to content

Roundtrip and integration-harness stabilization — batch of small fixes #257

@hjotha

Description

@hjotha

Summary

While exercising the describe / create roundtrip on real .mpr files and running the integration test harness end-to-end, I hit ~10 distinct small issues spread across the parser, writer, describer, flow-graph builder, and test harness. Fixes are ready as a stack of focused PRs.

Opening this as an umbrella issue (rather than 10 separate ones) because the fixes are small, already implemented, and reviewing them together gives useful context. Happy to split into individual issues if preferred.

Scope of fixes

Parser / writer (MPR storage layer):

  • Parse legacy NewCaseValue sequence flows emitted by older .mpr files so they roundtrip cleanly.
  • Persist ContentsHash on v1 unit writes — without this, v1 projects open corrupt after a roundtrip.
  • Preserve the original UnitID across DROP + CREATE OR REPLACE so references from other documents don't dangle.
  • Accept qualified function calls (e.g. Module.rule(...)) in the expression parser.
  • Normalise built-in Mendix function case in the expression roundtrip (e.g. toString vs ToString).
  • Gate Mendix 9-only microflow roundtrip keys by project version so Mx 10/11 projects don't get spurious fields.

Describer / flow-graph builder (MDL output + create microflow):

  • Harden the microflow describe roundtrip (emit the subset of fields we understand, don't drop the rest).
  • Support log node expressions in the microflow roundtrip.
  • Preserve rule-based decision subtype (RuleSplitCondition) across microflow roundtrips — previously silently demoted to expression split.
  • Emit the describer side of rule splits, free-floating annotations, and a needMerge guard so if branches that all return don't emit an orphan SequenceFlow with an empty DestinationPointer.
  • Preserve decision/loop captions across nested control flow (@caption was lost on nested if/loop).
  • Treat a terminal nested if (both branches return) as returning in the flow builder, so the parent doesn't try to wire a merge edge that has no source.

Integration / test harness:

  • Prefer the installed mx binary in the integration harness (falls back to downloaded mxbuild) to avoid re-downloading on every run.
  • Stabilize the MDL test runner and docker flow (timeouts, cleanup, error propagation).
  • Stabilize MDL roundtrip and integration flows — specifically: FindOrCreate mapping roundtrip, DROP + CREATE OR REPLACE UnitID preservation, default document access roles.
  • Align role casing and unstick integration roundtrip tests (case-insensitive role lookup + test fixtures updated).

Proposed PR layout

10 PRs, mostly independent. Three of them form a small stack (caption → rule split subtype → describer/free-annotations) because they touch the same structs and duplicating the field additions would cause conflicts.

Each PR:

  • Is scoped to one concern.
  • Ships with a failing-then-passing test (parser test, backend mutation test, or executor handler test using MockBackend).
  • Passes make build, make test, make vet, gofmt on touched files.
  • Preserves Studio Pro roundtrip on the fixtures exercised.

Why now

These all surfaced from the same end-to-end exercise (roundtrip + harness hardening). Shipping them in small focused pieces rather than one large PR makes review and reverting easy; the umbrella issue is just to give a single place to track them and honor the "file an issue first" workflow in CONTRIBUTING.md.

Happy to adjust the layout (single consolidated PR, one issue per fix, whatever works best for the maintainers).

Heads-up: pre-existing main test breakage (not mine)

While validating each PR against the current main tip (96a5d9e), I noticed an unrelated compile break in mdl/executor/cmd_misc_mock_test.go:67:

mdl/executor/cmd_misc_mock_test.go:67:28: not enough arguments in call to execHelp
    have (*ExecContext)
    want (*ExecContext, *ast.HelpStmt)

TestHelp_Mock calls execHelp(ctx) but the current execHelp signature is func execHelp(ctx *ExecContext, s *ast.HelpStmt) error. The mismatch landed from merging #253 (TestHelp_Mock added by 02db813) and #254 alongside #252 (which changed execHelp's signature). This is not introduced by any of the PRs in this stack — same failure is reproducible on a clean origin/main checkout. One-line fix: execHelp(ctx, &ast.HelpStmt{}). Happy to include it as an additional PR if it helps.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions