test: add mock tests for 8 executor handler files

[retran]

Why

The mxcli project is preparing to extract the mdl/ runtime into a reusable library. Before any refactoring can safely begin, comprehensive test coverage is needed to catch behavioral regressions from renaming, splitting files, extracting interfaces, and moving packages. Phase 1 (PRs #222–#239) established the architectural foundation; Phase 2 builds the test safety net.

This PR targets 8 executor handler files that had zero test coverage — any refactoring of these handlers would have no regression protection without these tests.

Part of the stacked PR series: #253 (C1) → #254 (C2) → #255 (D1).

Summary

• Add 49 tests across 8 new *_mock_test.go files covering executor handlers that previously had zero test coverage
• Covers: cmd_folders, cmd_move, cmd_rename, cmd_alter_page, cmd_features, cmd_styling, cmd_lint, cmd_import
• Uses the mock backend infrastructure from PR test: add visitor tests for 20 untested source files #245 (mock_test_helpers_test.go)

Coverage breakdown

Handler	Tests	Coverage
cmd_rename	11	Full: happy-path, error, edge (entity/module/microflow/page/enum/assoc/const/nanoflow)
cmd_alter_page	9	Full: set property, snippets, widgets, variables, layout-on-snippet
cmd_folders	9	Full: DropFolder + MoveFolder, nested paths, cross-module
cmd_features	7	Full: ForVersion, AddedSince, connected, InArea, invalid version
cmd_move	6	Full: page moves, cross-module refs, not-found, unsupported type
cmd_styling	5	Partial: not-connected guards only — happy paths need filesystem/BSON
cmd_lint	2	Partial: not-connected + ShowRules — full lint needs executor/catalog
cmd_import	1	Minimal: not-connected only — handler depends on SQL connections

Notes

• cmd_styling, cmd_lint, and cmd_import have documented limitations — their happy paths depend on filesystem I/O, BSON parsing, or SQL connections that cannot be mocked through the current Backend interface
• 1270 lines added, 0 deleted

[github-actions]

AI Code Review

What Looks Good

• Excellent test coverage expansion: Adds 49 tests across 8 previously untested executor handlers, significantly improving code reliability.
• Proper use of mock infrastructure: Correctly leverages the existing mock_test_helpers_test.go from PR test: add visitor tests for 20 untested source files #245, maintaining consistency with the project's testing approach.
• Honest limitation documentation: Clearly identifies why cmd_styling, cmd_lint, and cmd_import happy paths cannot be fully mocked (filesystem/BSON/SQL dependencies), setting correct expectations.
• Comprehensive error handling tests: Each handler tests not-connected states, not-found scenarios, and backend errors where applicable.
• Happy path validation where possible: For handlers that can be mocked (cmd_rename, cmd_alter_page, cmd_folders, cmd_move, cmd_features), tests verify successful execution paths.
• Deterministic test structure: Uses consistent helper functions (mkModule, mkPage, etc.) and clear assertion patterns.
• No code duplication: Tests are focused solely on executor handler logic without reimplementing backend functionality.
• Atomic scoped changes: PR is exclusively about adding test coverage with no unrelated modifications.

Recommendation

Approve - This PR successfully addresses a critical testing gap by adding meaningful unit tests for executor handlers using the established mock backend infrastructure. The tests are well-written, honestly scoped to what can be mocked, and follow existing project patterns. The limitations for certain handlers are appropriately documented and understood. No changes are needed.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

[Copilot]

Pull request overview

Adds mock-backed unit tests to improve coverage for executor handlers that previously had none/limited coverage, leveraging the existing MockBackend + newMockCtx test infrastructure.

Changes:

• Introduces 8 new *_mock_test.go files covering cmd_folders, cmd_move, cmd_rename, cmd_alter_page, cmd_features, cmd_styling, cmd_lint, and cmd_import.
• Exercises key branches: not-connected guards, happy paths (where feasible), not-found/unsupported cases, and representative backend error propagation.
• Documents known gaps where current Backend mocking can’t reasonably cover filesystem/BSON/SQL-dependent paths.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
mdl/executor/cmd_styling_mock_test.go	Adds guard-rail tests for styling handlers (not connected / missing project path).
mdl/executor/cmd_rename_mock_test.go	Adds broad rename coverage across entity/module/document types, including dry-run and backend error cases.
mdl/executor/cmd_move_mock_test.go	Adds move coverage for pages (folder moves, cross-module reference updates, not-found/unsupported, backend error).
mdl/executor/cmd_lint_mock_test.go	Adds lint tests for not-connected and the ShowRules branch output.
mdl/executor/cmd_import_mock_test.go	Adds minimal import coverage for the not-connected guard.
mdl/executor/cmd_folders_mock_test.go	Adds coverage for drop/move folder flows including nested paths and backend errors.
mdl/executor/cmd_features_mock_test.go	Adds coverage for show-features behavior across connected/disconnected and version parsing paths.
mdl/executor/cmd_alter_page_mock_test.go	Adds alter-page/snippet coverage for several mutator operations plus open/save error propagation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ako]

Code Review

Overview

Adds 49 unit tests across 8 new *_mock_test.go files covering executor handlers that previously had zero test coverage. This is part of a pre-refactoring safety net for extracting the mdl/ runtime into a reusable library. The tests use the MockBackend infrastructure established in PR #245.

---

Code Quality & Style

Strengths:

• Consistent structure across all 8 files — section headers, not-connected guard first, then happy path, then error paths
• Good use of boolean sentinels (setPropCalled, dropCalled, renameCalled) to verify the correct backend method is dispatched — this catches routing bugs that output checks alone wouldn't catch
• TestDropFolder_NestedPath verifies the correct child ID is deleted, not just that deletion was called — good specificity
• TestMove_Page_CrossModule verifies both MovePage AND UpdateQualifiedNameInAllUnits are called — correctly models the two-step cross-module move
• TestRename_Entity_DryRun asserts dryRun=true inside the func, not just that the func was called — precise

Minor issues:

• TestAlterPage_Snippet_Success opens a mutator with zero operations and just calls Save. It verifies Save is called and the output contains "Altered snippet", but doesn't test any actual snippet-specific behavior. This is more a smoke test than a behavioral test — worth noting in a comment or expanding with a SetPropertyOp.
• TestShowFeatures_InArea_ForVersion asserts assertContainsStr(t, out, "domain_model") — this tests a string that appears in the input (InArea: "domain_model"), which could pass trivially if the handler echoes it back without actually filtering. Consider asserting that at least one feature entry is present, or that the output count is smaller than the unfiltered count.
• TestLint_ShowRules hardcodes "MPR001" and "NamingConvention" which makes it brittle if rule IDs change. A comment acknowledging this is acceptable, but a shorter string or t.Skip with a note would be safer.

---

Correctness

No behavioral issues found. The mock wiring is correct — backends are set up with exactly the methods the handler will call, nothing more.

• TestAlterPage_SetLayout_Snippet_Unsupported correctly documents that SetLayout is not allowed on snippets.
• TestRename_UnsupportedType using "workflow" (a real type that isn't handled) is a good choice — avoids false negatives from typos.

---

Coverage Limitations (documented, appropriate)

The PR honestly documents what can't be reached with the current mock infrastructure (cmd_styling happy paths need BSON data, cmd_lint needs ctx.executor/catalog, cmd_import needs SQL mocks). The NOT_CONNECTED guards still add value. No concern here.

One undocumented gap: cmd_move has no test for a microflow/snippet/workflow move — only page and "UNKNOWN" are exercised. Adding TestMove_Microflow_Success would cover the other branch of the type switch.

---

Suggestions

1. TestAlterPage_Snippet_Success — add at least one operation (e.g. a SetPropertyOp) so the test exercises mutator dispatch, not just open+save.

2. TestShowFeatures_InArea_ForVersion — strengthen the assertion: verify the output doesn't contain features from a different area, or assert count < total unfiltered.

3. cmd_move missing microflow path — add TestMove_Microflow_Success to cover the non-page branch of execMove's type switch.

4. TestAlterPage_OpenMutatorError — assertContainsStr(t, err.Error(), "open page") is correct but fragile if the error message changes. Consider a shorter prefix or errors.Is check if the error type supports it.

---

Verdict: Approve with minor suggestions. Solid safety-net addition — consistent patterns, honest documentation of limitations, and correct mock wiring throughout.