test(catalog): add unit tests for builder pure helper functions

[retran]

Summary

• Add unit tests for all testable pure helper functions across 6 catalog builder files
• Cover microflow type switches, complexity calculators, BSON extraction, GUID formatting, permission derivation, workflow activity counters, navigation menu counting, and attribute name extraction
• 6 new test files, 1129 lines of test code

Why

Preparing to extract the mdl/ runtime into a reusable library — comprehensive test coverage is needed before refactoring.

Test files

File	Functions tested
builder_microflows_test.go	getMicroflowObjectType, getMicroflowActionType, getDataTypeName, countMicroflowActivities, calculateMcCabeComplexity, countNanoflowActivities, calculateNanoflowComplexity
builder_pages_test.go	extractLayoutRef, extractPageWidgets, extractWidgetsRecursive, extractSnippetWidgets, getBsonArrayElements, toBsonArray, extractString, extractBsonID, decodeBase64GUID, extractBinaryID, formatGUID, bytesToHex
builder_permissions_test.go	entityAccessFromMemberRights
builder_workflows_test.go	countWorkflowActivityTypes
builder_navigation_test.go	countMenuItems
builder_modules_test.go	extractAttrName

Stacked on

• test(linter): add unit tests for all 14 untested linter rules #255 (test/linter-rule-coverage) — D1
• test: expand mock tests for 28 executor handler files #254 (test/handler-mock-expand) — C2
• test: add mock tests for 8 executor handler files #253 (test/executor-mock-tests) — C1

[github-actions]

AI Code Review

What Looks Good

• Comprehensive unit test coverage for pure helper functions across 6 catalog builder files
• Well-structured table-driven tests covering normal cases and edge cases (nil inputs, empty collections, etc.)
• Clear, descriptive test names that explain what each test case verifies
• Proper use of Go testing conventions (t.Run, t.Errorf, t.Fatalf as appropriate)
• Tests focus exclusively on pure functions with no external dependencies, making them reliable and fast
• Good coverage of complex logic like McCabe complexity calculation, widget extraction recursion, and permission derivation
• The tests support the stated goal of preparing to extract the mdl/ runtime into a reusable library by ensuring refactoring safety

Recommendation

The PR is ready to be approved. It adds valuable test coverage for existing helper functions without introducing any new MDL syntax or features, making it a safe and necessary improvement for code maintainability. The tests are thorough, well-written, and directly support the project's refactoring preparations. No changes are requested.

---

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

[github-actions]

AI Code Review

What Looks Good

• The PR adds comprehensive unit tests for pure helper functions across six catalog builder files, covering microflow type switches, complexity calculators, BSON extraction, GUID formatting, permission derivation, workflow activity counters, navigation menu counting, and attribute name extraction.
• Each test file is well‑structured, uses table‑driven tests, and includes meaningful test cases that exercise edge conditions (nil inputs, empty collections, nested structures, etc.).
• The tests are self‑contained, import only the necessary packages, and follow Go testing conventions.
• No MDL syntax or runtime behavior is changed; the PR is purely a test‑coverage increase, which aligns with the stated goal of preparing to extract the mdl/ runtime into a reusable library.
• All generated ANTLR parser files (noise) are correctly ignored as they are not touched.
• The PR does not introduce any duplication with existing test files or proposals; it adds new coverage where previously there was little or none.

Recommendation

• Approve the PR. The changes are focused, high‑quality, and improve test coverage without affecting functionality or introducing risk. No further changes are needed.

---

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

[Copilot]

Pull request overview

Adds substantial unit-test coverage for previously lightly/untested helper logic and command paths to support upcoming mdl/ runtime refactoring into a reusable library.

Changes:

• Add new unit tests for multiple mdl/linter/rules helper functions and rule metadata/nil-reader guards.
• Expand mdl/executor mock tests with additional not-connected, not-found, filter-by-module, JSON, and backend-error scenarios across many commands.
• Add new unit tests for several mdl/catalog builder “pure helper” functions (microflows, pages, permissions, workflows, navigation, modules).

Reviewed changes

Copilot reviewed 54 out of 54 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
mdl/linter/rules/validation_feedback_test.go	Unit tests for empty-template detection and microflow object walking for MPR004.
mdl/linter/rules/security_test.go	Tests for SEC001 plus nil-reader guard + metadata tests for SEC002/SEC003.
mdl/linter/rules/page_navigation_security_test.go	Tests for menu-page collection/module parsing + nil-reader guard for MPR007.
mdl/linter/rules/naming_test.go	Unit tests for naming helpers/patterns and MPR001 metadata.
mdl/linter/rules/mpr008_overlapping_activities_test.go	Nil-reader guard/metadata + constant checks for MPR008.
mdl/linter/rules/helpers_test.go	Shared test helpers (synthetic microflow + in-memory SQLite setup).
mdl/linter/rules/empty_test.go	In-memory DB tests for MPR002 empty-microflow rule + metadata.
mdl/linter/rules/empty_container_test.go	Unit tests for empty container detection + MPR006 metadata.
mdl/linter/rules/domain_size_test.go	In-memory DB tests for MPR003 domain-model size rule + metadata.
mdl/linter/rules/conv_split_caption_test.go	Unit tests for CONV012 exclusive-split caption walker + metadata.
mdl/linter/rules/conv_loop_commit_test.go	Unit tests for CONV011 loop commit walker + nil-reader guard/metadata.
mdl/linter/rules/conv_error_handling_test.go	Unit tests for CONV013/CONV014 walker functions + metadata.
mdl/executor/cmd_write_handlers_mock_test.go	Adds mock coverage for already-exists and multiple not-found drop paths.
mdl/executor/cmd_workflows_mock_test.go	Adds not-found + filter-by-module coverage for workflows.
mdl/executor/cmd_styling_mock_test.go	New mock tests for styling commands’ not-connected / missing-path guards.
mdl/executor/cmd_settings_mock_test.go	Adds not-connected/backend-error/JSON tests for settings listing.
mdl/executor/cmd_security_mock_test.go	Adds filter-by-module + various not-found/unsupported/invalid-input tests for security commands.
mdl/executor/cmd_rest_clients_mock_test.go	Adds not-found + filter-by-module coverage for REST clients.
mdl/executor/cmd_rename_mock_test.go	New comprehensive mock tests for rename (connected guards, happy paths, errors, dry-run).
mdl/executor/cmd_published_rest_mock_test.go	Adds not-found + filter-by-module coverage for published REST services.
mdl/executor/cmd_pages_mock_test.go	Adds filter-by-module tests for snippets/layouts and not-found describe tests.
mdl/executor/cmd_odata_mock_test.go	Adds not-found + filter-by-module coverage for consumed/published OData.
mdl/executor/cmd_navigation_mock_test.go	Adds not-found + “all profiles” describe coverage for navigation.
mdl/executor/cmd_move_mock_test.go	New mock tests for move (guards, happy path, cross-module ref updates, errors).
mdl/executor/cmd_modules_mock_test.go	Adds backend-error + JSON coverage for module listing.
mdl/executor/cmd_misc_mock_test.go	Adds not-connected + no-schema-hash coverage for version output; adds help test.
mdl/executor/cmd_microflows_mock_test.go	Adds notes referencing existing error/JSON coverage (comment-only change).
mdl/executor/cmd_mermaid_mock_test.go	Adds microflow happy/not-found + page not-found + unsupported-type coverage.
mdl/executor/cmd_lint_mock_test.go	New tests for execLint not-connected + ShowRules path.
mdl/executor/cmd_jsonstructures_mock_test.go	Adds filter-by-module + describe/not-found coverage for JSON structures.
mdl/executor/cmd_javascript_actions_mock_test.go	Adds not-found + filter-by-module tests for JS actions.
mdl/executor/cmd_javaactions_mock_test.go	Adds backend-error/not-found/filter-by-module/JSON tests for Java actions.
mdl/executor/cmd_import_mock_test.go	New test for execImport not-connected guard (documents limitations).
mdl/executor/cmd_import_mappings_mock_test.go	Adds filter-by-module + describe/not-found coverage for import mappings.
mdl/executor/cmd_imagecollections_mock_test.go	Adds filter-by-module + not-found coverage for image collections.
mdl/executor/cmd_fragments_mock_test.go	Adds describe/not-found/nil-fragments coverage for fragments.
mdl/executor/cmd_folders_mock_test.go	New mock tests for drop/move folder scenarios (guards, success, errors, nested path).
mdl/executor/cmd_features_mock_test.go	New mock tests for features command across connected/disconnected + version parsing cases.
mdl/executor/cmd_export_mappings_mock_test.go	Adds filter-by-module + describe/not-found coverage for export mappings.
mdl/executor/cmd_enumerations_mock_test.go	Adds notes referencing existing error/JSON coverage (comment-only change).
mdl/executor/cmd_entities_mock_test.go	Adds backend-error + JSON coverage for entity listing.
mdl/executor/cmd_dbconnection_mock_test.go	Adds filter-by-module + not-found coverage for DB connections.
mdl/executor/cmd_datatransformer_mock_test.go	Adds filter-by-module + not-found coverage for data transformers.
mdl/executor/cmd_constants_mock_test.go	Adds notes referencing existing error/JSON coverage (comment-only change).
mdl/executor/cmd_businessevents_mock_test.go	Adds not-found + filter-by-module coverage for business event services.
mdl/executor/cmd_associations_mock_test.go	Adds backend-error + JSON coverage for association listing.
mdl/executor/cmd_alter_page_mock_test.go	New mock tests for alter page/snippet operations and error cases.
mdl/executor/cmd_agenteditor_mock_test.go	Adds not-found/drop-not-found + filter-by-module coverage for agent editor types.
mdl/catalog/builder_workflows_test.go	Unit tests for workflow activity counters and recursion.
mdl/catalog/builder_permissions_test.go	Unit tests for permission derivation from member access rights.
mdl/catalog/builder_pages_test.go	Unit tests for page/snippet widget extraction + BSON/GUID utilities.
mdl/catalog/builder_navigation_test.go	Unit tests for navigation menu item counting.
mdl/catalog/builder_modules_test.go	Unit tests for qualified attribute-name extraction.
mdl/catalog/builder_microflows_test.go	Unit tests for microflow/nanoflow type switches, activity counting, and complexity.

---

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

[Copilot]

Pull request overview

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

---

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

[ako]

Code Review

Overview

E1 in the stacked series (#253 → #254 → #255 → #256). Six new test files covering pure helper functions in mdl/catalog/: microflow type switches, complexity calculators, BSON extraction, GUID formatting, permission derivation, workflow activity counters, navigation counting, and attribute name extraction. The scope is well-chosen — these are the functions most likely to silently regress during a refactor.

---

Blocking issue (inherited from #254)

TestHelp_Mock will not compile — same inherited issue as #255. execHelp(ctx) takes two arguments in the current codebase (execHelp(ctx *ExecContext, s *ast.HelpStmt)). Needs to be fixed in #254 and then this chain rebased.

---

Non-blocking issues

TestGetMicroflowObjectType — missing default/unknown coverage

getMicroflowActionType has a test case for the unknown fallthrough (&microflows.UnknownAction{} → "MicroflowAction"). getMicroflowObjectType also has a default: return "MicroflowObject" branch, but no corresponding test case exercises it. Minor asymmetry worth adding for completeness.

TestFormatGUID — "short data" test expects non-printable bytes in want

{
    name: "short data — returns as string",
    data: []byte{0x01, 0x02},
    want: "\x01\x02",
},

string([]byte{0x01, 0x02}) is technically correct, but if this test ever fails the output diff will show unprintable characters. The defensive string(data) path in formatGUID only fires when len(data) != 16 which shouldn't happen with valid MPR data — this test is for a fallback that's never triggered in practice. Consider dropping it or replacing with got == string(tt.data) to make a failure legible.

TestExtractWidgetsRecursive — only tests Forms$DivContainer, not Pages$DivContainer

The implementation skips both Forms$DivContainer and Pages$DivContainer. The test only exercises the Forms$ variant. Adding a Pages$DivContainer case would complete the coverage of that skip list.

---

Strengths

• GUID byte-swap tests (TestFormatGUID, TestDecodeBase64GUID, TestExtractBsonID): These directly verify the Mendix-specific little-endian byte ordering of BSON GUIDs with concrete input/output pairs. This is the highest-value coverage in the PR — the byte ordering is exactly the kind of thing that breaks silently during refactoring.

• TestCalculateMcCabeComplexity — nested loop/split case: The "loop adds 1 plus nested decisions" and "complex flow" test cases verify the recursive countDecisionPoints logic, including that LoopedActivity adds 1 regardless of whether its body has further splits. Catches the recursion boundary correctly.

• TestCountWorkflowActivityTypes — parallel split recursion: Verifying that ParallelSplitActivity itself is counted in total but not in any sub-category (it's structural), while its outcome flows are recursed into, is precisely the kind of nuanced counting rule that's hard to reason about without tests.

• TestEntityAccessFromMemberRights — "empty member accesses falls through to default": Correctly captures that an AccessRule with an empty MemberAccesses slice (not nil) still uses DefaultMemberAccessRights. This is the subtle interaction between the empty-list check and the default path.

• TestGetBsonArrayElements — type indicator stripping: Tests both int32(0) and int(0) as array type indicators, and primitive.A vs []any. The BSON array interleaving format is non-obvious and these tests make the parsing contract explicit.

---

Summary

One blocking compile error (inherited from #254). Three minor coverage gaps worth addressing. The core value of this PR — testing GUID byte-swap logic, complexity counting, and BSON extraction helpers — is well-executed and exactly what's needed before extracting the mdl/ library.

[github-actions]

AI Code Review

What Looks Good

• Comprehensive test coverage for pure helper functions across 6 catalog builder modules
• Well-structured table-driven tests following Go testing best practices
• Tests appropriately isolate pure functions with clear input/output expectations
• Covers a wide range of functionality: microflow type switches, complexity calculators, BSON extraction, GUID formatting, permission derivation, workflow activity counters, navigation menu counting, and attribute name extraction
• Test files are logically organized by builder module with descriptive test names
• No time.Sleep usage or synchronization issues in test code
• PR is narrowly scoped to adding unit tests for existing pure helper functions (no production code changes)

Recommendation

Approve - The PR adds valuable test coverage for pure helper functions in preparation for refactoring, following the project's testing practices and checklist requirements. The tests are well-written, comprehensive, and appropriately scoped.

---

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

[retran]

Thanks for the review! Pushed fixes in a separate commit (5bd8d68), rebased on main after D1 merge:

Blocking issue (TestHelp_Mock): Same as D1 — execHelp has a single-arg signature execHelp(ctx *ExecContext) error. Compiles and passes. Likely a stale diff view.

Non-blocking fixes applied:

1. getMicroflowObjectType default coverage — added unknownMicroflowObject test type that satisfies the interface but isn't in the switch, exercises the default: return "MicroflowObject" branch.

2. TestFormatGUID short data — replaced non-printable \x01\x02 with legible ASCII "AB" ([]byte{0x41, 0x42}). Test failure diffs are now readable.

3. Pages$DivContainer test case — added alongside the existing Forms$DivContainer case to cover both skip-list entries in extractWidgetsRecursive.