From 32d631e33e2a87645a6189bb791322b231c3ac6f Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Thu, 21 May 2026 10:00:33 +0200 Subject: [PATCH] core-dev mode for mx core roundtrips Introduces a permanent in-repo "core development" mode that lets a codegen agent churn symbols under src/private/mx/core/ without keeping mx/api, mx/impl, or the higher-level test suites compiling. The existing build and test targets are unchanged when MX_CORE_DEV=OFF. Why --- We are reverse-engineering a new codegen system to regenerate mx/core for MusicXML 4.0 (tracked in docs/ai/projects/gen/). During that work mx/api and mx/impl will frequently fail to compile against an in-progress mx/core, which makes the normal build-and-test loop unusable as a feedback signal. core-dev gives the codegen rewrite a narrow harness: build only mx/core + mx/ezxml + mx/utility, then exercise every MusicXML file under data/ through mx::core::Document::fromXDoc -> toXDoc and compare against a normalized in-memory copy of the input. What ---- - CMake: MX_CORE_DEV option gates a trimmed configuration that builds only mx/core + mx/ezxml + mx/utility plus the core roundtrip test binary. No per-file #ifdef guards. - Makefile: core-dev / check-core-dev / test-core-dev targets, plus a help entry. fmt and check-core-dev run in Docker; test-core-dev runs natively. - Test harness: src/private/mxtest/corert/ dynamically registers one Catch2 test case per *.xml / *.musicxml file under data/ (excluding data/expected/, data/testOutput/, data/generalxml/, data/smufl/). The same suite also runs inside the normal mxtest binary during make test-all, gated on MX_BUILD_CORE_TESTS=ON. Distinct from the api import suite under src/private/mxtest/import/. - Agent surface: AGENTS.md section, README pointer, make help block. - Project docs: docs/ai/projects/core-dev/ records goals, design, plan, session log, and the final gate results. CI is not wired up for core-dev; it is a local iteration harness for the codegen rewrite, not a release gate. The 35 core roundtrip failures observed at the final gate are the intended consumer signal for the codegen rewrite and are out of scope for this commit. --- AGENTS.md | 93 +++- CMakeLists.txt | 270 +++++++---- Dockerfile | 6 + Makefile | 69 ++- README.md | 2 + docs/ai/projects/core-dev/.prompt | 0 .../core-dev/design/agents-md-snippet.md | 50 +++ docs/ai/projects/core-dev/design/design.md | 375 ++++++++++++++++ docs/ai/projects/core-dev/index.md | 44 ++ docs/ai/projects/core-dev/log.md | 101 +++++ docs/ai/projects/core-dev/plan.md | 46 ++ docs/ai/projects/core-dev/state.md | 22 + .../mxtest/corert/CoreRoundtripImpl.cpp | 419 ++++++++++++++++++ src/private/mxtest/corert/CoreRoundtripImpl.h | 64 +++ .../mxtest/corert/CoreRoundtripTest.cpp | 118 +++++ 15 files changed, 1566 insertions(+), 113 deletions(-) create mode 100644 docs/ai/projects/core-dev/.prompt create mode 100644 docs/ai/projects/core-dev/design/agents-md-snippet.md create mode 100644 docs/ai/projects/core-dev/design/design.md create mode 100644 docs/ai/projects/core-dev/index.md create mode 100644 docs/ai/projects/core-dev/log.md create mode 100644 docs/ai/projects/core-dev/plan.md create mode 100644 docs/ai/projects/core-dev/state.md create mode 100644 src/private/mxtest/corert/CoreRoundtripImpl.cpp create mode 100644 src/private/mxtest/corert/CoreRoundtripImpl.h create mode 100644 src/private/mxtest/corert/CoreRoundtripTest.cpp diff --git a/AGENTS.md b/AGENTS.md index fc6e7d7eb..17fb8d46e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,25 +6,25 @@ Keep this section as a Markdown table. When updating entries, maintain the table format. -| Path | Description | -|---------------------------------|-----------------------------------------------------------------------------| -| `src/` | Main C++ source tree (public API, core elements, implementation, tests) | -| `src/include/mx/api/` | Public API headers: `*Data` structs and `DocumentManager` entry point | -| `src/private/mx/api/` | Implementation backing the public API headers | -| `src/private/mx/core/` | Strongly-typed MusicXML element classes (mostly code-generated) | -| `src/private/mx/core/elements/` | Generated element classes, one pair per MusicXML element (1182 files) | -| `src/private/mx/impl/` | Conversion layer mapping the core DOM to the public API | -| `src/private/mx/ezxml/` | Embedded lightweight XML reader/writer used by the core | -| `src/private/mx/utility/` | Shared helpers (string, parsing, file system utilities) | -| `src/private/mxtest/` | Test suite (api, core, file, import, impl, control) | -| `src/private/cpul/` | Catch-based unit-test harness and test runner main | -| `gen/version-a/` | Historical Ruby/shell scripts from the original brute-force code gen | -| `gen/version-b/` | Active Rust code generator for reproducing MusicXML 4.0 element classes | -| `data/` | MusicXML test files and expected-output suites for round-trip tests | -| `docs/musicxml.xsd` | MusicXML specification (reference) | -| `docs/ai/project/` | AI-assisted project planning and codegen design documents | -| `Makefile` | Primary build-and-test entry point (wraps CMake; `make help` lists targets) | -| `CMakeLists.txt` | CMake build configuration | +| Path | Description | +|---------------------------------|--------------------------------------------------------------------------------------| +| `src/` | Main C++ source tree (public API, core elements, implementation, tests) | +| `src/include/mx/api/` | Public API headers: `*Data` structs and `DocumentManager` entry point | +| `src/private/mx/api/` | Implementation backing the public API headers | +| `src/private/mx/core/` | Strongly-typed MusicXML element classes (mostly code-generated) | +| `src/private/mx/core/elements/` | Generated element classes, one pair per MusicXML element (1182 files) | +| `src/private/mx/impl/` | Conversion layer mapping the core DOM to the public API | +| `src/private/mx/ezxml/` | Embedded lightweight XML reader/writer used by the core | +| `src/private/mx/utility/` | Shared helpers (string, parsing, file system utilities) | +| `src/private/mxtest/` | Test suite (api, core, file, api import, impl, control, core roundtrip) | +| `src/private/cpul/` | Catch-based unit-test harness and test runner main | +| `gen/version-a/` | Historical Ruby/shell scripts from the original brute-force code gen | +| `gen/version-b/` | Active Rust code generator for reproducing MusicXML 4.0 element classes | +| `data/` | MusicXML test files and expected-output suites for api import / core roundtrip tests | +| `docs/musicxml.xsd` | MusicXML specification (reference) | +| `docs/ai/project/` | AI-assisted project planning and codegen design documents | +| `Makefile` | Primary build-and-test entry point (wraps CMake; `make help` lists targets) | +| `CMakeLists.txt` | CMake build configuration | ## Code Generation - Historical Context @@ -54,6 +54,19 @@ Code generation was never, and should not in the future, be designed to generate specification. Rather, the goal of code generation is bespoke, to produce what is needed for the `mx` library from the MusicXML specification. +## Terminology: Roundtrip Suites + +Two distinct test suites exercise MusicXML round-tripping. Always use the qualified name; never +say "roundtrip" unqualified. + +- **api import** (`API_IMPORT`): the existing suite under `src/private/mxtest/import/`. Imports + a file through `mx::api::DocumentManager`, which exercises `mx/core` + `mx/impl` + `mx/api`, + then compares against a pre-generated expected XML file under `data/expected/`. +- **core roundtrip** (`CORE_ROUNDTRIP`, `CORE_RT`): the suite under + `src/private/mxtest/roundtrip/`. Round-trips a file through `mx::core::Document` only + (`fromXDoc` → `toXDoc`), no api/impl involvement, comparing against the normalized input + computed in-memory. + ## Quality Gates Always run `make fmt` after modifying code under `src/`. @@ -66,6 +79,48 @@ Check for warnings with: `make check`. CI will run all of these plus the `xcode` targets. +## Core Development Mode (Codegen) + +Use core-dev when modifying files under `src/private/mx/core/` and you do not need `mx/api` +or `mx/impl` to compile. Trimmed build for codegen iteration: only `mx/core` + `mx/ezxml` + +`mx/utility` compile. + +### In-mode iteration + +``` +make core-dev # configure + build (fast: skips api/impl) +make test-core-dev # run all core roundtrip cases +make test-core-dev ARGS='[core-roundtrip] lysuite/*' # subset +``` + +In-mode gate: `make fmt && make check-core-dev && make test-core-dev`. `fmt` and +`check-core-dev` run in Docker; `test-core-dev` runs natively. + +### Full pre-merge gate + +Before merging core changes, run the full gate. Changes under `mx/core/` require +`test-all`, which exercises per-element core unit tests, api import, and core +roundtrip together: + +``` +make fmt && make check && make test-all +``` + +### What core-dev tests + +Backed by the **core roundtrip** suite (`CORE_ROUNDTRIP`, `CORE_RT`) under +`src/private/mxtest/corert/`. Each `*.xml` / `*.musicxml` file under `data/` (excluding +`data/expected/`, `data/testOutput/`, `data/generalxml/`, `data/smufl/`) is one Catch2 +test case round-tripping through `mx::core::Document` (`fromXDoc` → `toXDoc`) against a +normalized input computed in-memory. + +The same suite runs inside the normal `mxtest` binary during `make test-all`, gated on +`MX_BUILD_CORE_TESTS=ON`. Distinct from the **api import** (`API_IMPORT`) suite in +`src/private/mxtest/import/`, which exercises the full `mx::api` stack against pre-generated +expected files under `data/expected/`. + +For design details see `docs/ai/projects/core-dev/design/design.md`. + ## Current Project We are working on reverse engineering a new codegen system to regenerate mx/core for MusicXML 4.0. diff --git a/CMakeLists.txt b/CMakeLists.txt index 70a860f80..a7bd00952 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,12 @@ option( OFF ) +option( + MX_CORE_DEV + "Build only mx/core + ezxml + utility plus the core roundtrip test binary." + OFF +) + set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR}) set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}) @@ -35,50 +41,26 @@ file(GLOB_RECURSE SRC_MX_TEST_IMPL ${PRIVATE_DIR}/mxtest/impl/*.cpp ${PRIVATE_DI file(GLOB_RECURSE SRC_MX_TEST_CORE ${PRIVATE_DIR}/mxtest/core/*.cpp ${PRIVATE_DIR}/mxtest/core/*.h) file(GLOB_RECURSE SRC_MX_TEST_IMPORT ${PRIVATE_DIR}/mxtest/import/*.cpp ${PRIVATE_DIR}/mxtest/import/*.h) file(GLOB_RECURSE SRC_MX_TEST_UTILITY ${PRIVATE_DIR}/mxtest/utility/*.cpp ${PRIVATE_DIR}/mxtest/utility/*.h) -file(GLOB_RECURSE SRC_CPUL ${PRIVATE_DIR}/cpul/*.cpp ${SOURCE}/cpul/*.h) - -# mx static library -add_library(${PROJECT_NAME} STATIC ${SRC_MX_API} ${SRC_MX_CORE} ${SRC_MX_IMPL} ${SRC_MX_UTILITY} ${SRC_MX_EZXML}) -source_group( "api-public" FILES ${HEADERS_MX_API}) -source_group( "api" FILES ${SRC_MX_API} ) -source_group( "core" FILES ${SRC_MX_CORE} ) -source_group( "impl" FILES ${SRC_MX_IMPL} ) -source_group( "utility" FILES ${SRC_MX_UTILITY} ) -source_group( "ezxml" FILES ${SRC_MX_EZXML} ) -set_property(TARGET ${PROJECT_NAME} PROPERTY CXX_STANDARD ${CPP_VERSION}) - -if(MSVC) - target_compile_options(${PROJECT_NAME} PRIVATE /W4) -else() - target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra) -endif() - -target_include_directories(${PROJECT_NAME} PRIVATE ${PRIVATE_DIR}) - -# these header paths are a little different to make it easier to copy-and-paste the ezxml code over to -# here http://github.com/webern/ezxml. However, as far as mx is concerned, ezxml is just another -# namespace in the mx library (that has a slightly different #include path convention) -target_include_directories(${PROJECT_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/include") -target_include_directories(${PROJECT_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/private") - -target_include_directories(${PROJECT_NAME} PUBLIC - $ - $ -) - -# Install Package Configuration -install(TARGETS ${PROJECT_NAME} - EXPORT ${PROJECT_NAME}_targets - ARCHIVE DESTINATION lib - LIBRARY DESTINATION lib -) -install(EXPORT ${PROJECT_NAME}_targets - NAMESPACE ${PROJECT_NAME}:: - FILE ${PROJECT_NAME}-config.cmake - DESTINATION "${CMAKE_INSTALL_DATADIR}/cmake/${PROJECT_NAME}" +file(GLOB_RECURSE SRC_MX_CORE_ROUNDTRIP ${PRIVATE_DIR}/mxtest/corert/*.cpp ${PRIVATE_DIR}/mxtest/corert/*.h) + +# Helpers from mxtest/import/ that the core roundtrip suite reuses. Kept as +# an explicit list (not a glob) because we want exactly these two unguarded +# helpers and not the rest of the api import suite. Both translation units +# have no #ifdef guards and depend only on mx/ezxml; they satisfy the +# Dual-Compilation Invariant (design §2). DecimalFields.h is header-only +# and is picked up transitively via include paths. +set(SRC_MX_CORE_ROUNDTRIP_HELPERS + ${PRIVATE_DIR}/mxtest/import/ChangeValues.cpp + ${PRIVATE_DIR}/mxtest/import/ChangeValues.h + ${PRIVATE_DIR}/mxtest/import/SortAttributes.cpp + ${PRIVATE_DIR}/mxtest/import/SortAttributes.h + ${PRIVATE_DIR}/mxtest/import/DecimalFields.h ) -install(DIRECTORY "${PUBLIC_DIR}/mx" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") +file(GLOB_RECURSE SRC_CPUL ${PRIVATE_DIR}/cpul/*.cpp ${SOURCE}/cpul/*.h) +# PathRoot.h is consumed by the api import suite (via MxFileRepository) and by +# the core roundtrip suite (file discovery under data/). Generated in both +# build branches so MX_REPO_ROOT_PATH is always available. file(WRITE ${PRIVATE_DIR}/mxtest/file/PathRoot.h "// This file was auto-generated by CMake #pragma once @@ -90,58 +72,168 @@ file(WRITE ${PRIVATE_DIR}/mxtest/file/PathRoot.h " ) - -# mxtest -if(MX_BUILD_TESTS) - set(TEST_BINARY_NAME mxtest) - message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} tests will be compiled") - if(MX_BUILD_CORE_TESTS) - message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} core tests will be compiled") - add_definitions(-DMX_COMPILE_CORE_TESTS) +if(MX_CORE_DEV) + + # ========================================================================= + # Core-dev branch: trimmed build for codegen iteration on mx/core. + # + # Builds only mx/core + mx/ezxml + mx/utility into a static library + # `mx-core`, plus a `mxtest-core-dev` executable containing the core + # roundtrip suite. mx/api, mx/impl, and all other test suites are not + # compiled here. MX_BUILD_TESTS / MX_BUILD_CORE_TESTS / MX_BUILD_EXAMPLES + # are ignored in this branch. + # ========================================================================= + + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} MX_CORE_DEV=ON: trimmed core-dev build") + + # mx-core static library: only core + ezxml + utility. + add_library(mx-core STATIC ${SRC_MX_CORE} ${SRC_MX_UTILITY} ${SRC_MX_EZXML}) + source_group("core" FILES ${SRC_MX_CORE}) + source_group("utility" FILES ${SRC_MX_UTILITY}) + source_group("ezxml" FILES ${SRC_MX_EZXML}) + set_property(TARGET mx-core PROPERTY CXX_STANDARD ${CPP_VERSION}) + + if(MSVC) + target_compile_options(mx-core PRIVATE /W4) else() - message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} core tests will NOT be compiled") + target_compile_options(mx-core PRIVATE -Wall -Wextra) endif() - find_package( Threads ) - - add_executable( - ${TEST_BINARY_NAME} - ${SRC_MX_TEST_API} - ${SRC_MX_TEST_CONTROL} - ${SRC_MX_TEST_CORE} - ${SRC_MX_TEST_FILE} - ${SRC_MX_TEST_IMPL} - ${SRC_MX_TEST_IMPORT} - ${SRC_MX_TEST_UTILITY} - ${SRC_MX_TEST_XML} - ${SRC_CPUL} - ) - target_include_directories(${TEST_BINARY_NAME} PRIVATE ${PRIVATE_DIR}) + target_include_directories(mx-core PRIVATE ${PRIVATE_DIR}) + target_include_directories(mx-core PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/include") + target_include_directories(mx-core PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/private") + target_include_directories(mx-core PUBLIC + $ + ) - # See above: ezxml has a slightly different path convention, and it is used by certain tests even though it is - # not considered part of the public API of mx. - target_include_directories(${TEST_BINARY_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/include") - target_include_directories(${TEST_BINARY_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/private") + # mxtest-core-dev executable. Phase A: SRC_MX_CORE_ROUNDTRIP is empty + # (no files under mxtest/corert/ yet). cpul provides Catch2's main(), so + # the executable links and exits 0 with no tests registered. Phase B adds + # real sources into mxtest/corert/ and the glob picks them up. + find_package(Threads) + add_executable(mxtest-core-dev + ${SRC_MX_CORE_ROUNDTRIP} + ${SRC_MX_CORE_ROUNDTRIP_HELPERS} + ${SRC_CPUL} + ) + target_include_directories(mxtest-core-dev PRIVATE ${PRIVATE_DIR}) + target_include_directories(mxtest-core-dev PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/include") + target_include_directories(mxtest-core-dev PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/private") + target_link_libraries(mxtest-core-dev mx-core) + target_link_libraries(mxtest-core-dev ${CMAKE_THREAD_LIBS_INIT}) + set_property(TARGET mxtest-core-dev PROPERTY CXX_STANDARD ${CPP_VERSION}) - target_link_libraries(${TEST_BINARY_NAME} ${PROJECT_NAME}) - target_link_libraries(${TEST_BINARY_NAME} ${CMAKE_THREAD_LIBS_INIT}) - set_property(TARGET ${TEST_BINARY_NAME} PROPERTY CXX_STANDARD ${CPP_VERSION}) else() - message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} tests will NOT be compiled") -endif() -# mx examples -if(MX_BUILD_EXAMPLES) - message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} examples will be compiled") - add_executable(mxread ${PRIVATE_DIR}/mx/examples/Read.cpp) - add_executable(mxwrite ${PRIVATE_DIR}/mx/examples/Write.cpp) - add_executable(mxhide ${PRIVATE_DIR}/mx/examples/Hide.cpp) - target_link_libraries(mxread ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT}) - target_link_libraries(mxwrite ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT}) - target_link_libraries(mxhide ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT}) - set_property(TARGET mxread PROPERTY CXX_STANDARD ${CPP_VERSION}) - set_property(TARGET mxwrite PROPERTY CXX_STANDARD ${CPP_VERSION}) - set_property(TARGET mxhide PROPERTY CXX_STANDARD ${CPP_VERSION}) -else() - message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} examples will NOT be compiled") + # ========================================================================= + # Normal branch: full build (mx library + optional tests + examples). + # ========================================================================= + + # mx static library + add_library(${PROJECT_NAME} STATIC ${SRC_MX_API} ${SRC_MX_CORE} ${SRC_MX_IMPL} ${SRC_MX_UTILITY} ${SRC_MX_EZXML}) + source_group( "api-public" FILES ${HEADERS_MX_API}) + source_group( "api" FILES ${SRC_MX_API} ) + source_group( "core" FILES ${SRC_MX_CORE} ) + source_group( "impl" FILES ${SRC_MX_IMPL} ) + source_group( "utility" FILES ${SRC_MX_UTILITY} ) + source_group( "ezxml" FILES ${SRC_MX_EZXML} ) + set_property(TARGET ${PROJECT_NAME} PROPERTY CXX_STANDARD ${CPP_VERSION}) + + if(MSVC) + target_compile_options(${PROJECT_NAME} PRIVATE /W4) + else() + target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra) + endif() + + target_include_directories(${PROJECT_NAME} PRIVATE ${PRIVATE_DIR}) + + # these header paths are a little different to make it easier to copy-and-paste the ezxml code over to + # here http://github.com/webern/ezxml. However, as far as mx is concerned, ezxml is just another + # namespace in the mx library (that has a slightly different #include path convention) + target_include_directories(${PROJECT_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/include") + target_include_directories(${PROJECT_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/private") + + target_include_directories(${PROJECT_NAME} PUBLIC + $ + $ + ) + + # Install Package Configuration + install(TARGETS ${PROJECT_NAME} + EXPORT ${PROJECT_NAME}_targets + ARCHIVE DESTINATION lib + LIBRARY DESTINATION lib + ) + install(EXPORT ${PROJECT_NAME}_targets + NAMESPACE ${PROJECT_NAME}:: + FILE ${PROJECT_NAME}-config.cmake + DESTINATION "${CMAKE_INSTALL_DATADIR}/cmake/${PROJECT_NAME}" + ) + install(DIRECTORY "${PUBLIC_DIR}/mx" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") + + # mxtest + if(MX_BUILD_TESTS) + set(TEST_BINARY_NAME mxtest) + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} tests will be compiled") + if(MX_BUILD_CORE_TESTS) + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} core tests will be compiled") + add_definitions(-DMX_COMPILE_CORE_TESTS) + else() + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} core tests will NOT be compiled") + endif() + find_package( Threads ) + + # Core roundtrip suite is included in mxtest when MX_BUILD_CORE_TESTS=ON, + # so it runs during `make test-all` alongside the api import, api, impl, + # and per-element core suites. Gating on MX_BUILD_CORE_TESTS (not + # MX_BUILD_TESTS) keeps `make test` fast. + set(SRC_MX_CORE_ROUNDTRIP_IN_MXTEST "") + if(MX_BUILD_CORE_TESTS) + set(SRC_MX_CORE_ROUNDTRIP_IN_MXTEST ${SRC_MX_CORE_ROUNDTRIP}) + endif() + + add_executable( + ${TEST_BINARY_NAME} + ${SRC_MX_TEST_API} + ${SRC_MX_TEST_CONTROL} + ${SRC_MX_TEST_CORE} + ${SRC_MX_TEST_FILE} + ${SRC_MX_TEST_IMPL} + ${SRC_MX_TEST_IMPORT} + ${SRC_MX_TEST_UTILITY} + ${SRC_MX_TEST_XML} + ${SRC_MX_CORE_ROUNDTRIP_IN_MXTEST} + ${SRC_CPUL} + ) + + target_include_directories(${TEST_BINARY_NAME} PRIVATE ${PRIVATE_DIR}) + + # See above: ezxml has a slightly different path convention, and it is used by certain tests even though it is + # not considered part of the public API of mx. + target_include_directories(${TEST_BINARY_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/include") + target_include_directories(${TEST_BINARY_NAME} PRIVATE "${PRIVATE_DIR}/mx/ezxml/src/private") + + target_link_libraries(${TEST_BINARY_NAME} ${PROJECT_NAME}) + target_link_libraries(${TEST_BINARY_NAME} ${CMAKE_THREAD_LIBS_INIT}) + set_property(TARGET ${TEST_BINARY_NAME} PROPERTY CXX_STANDARD ${CPP_VERSION}) + else() + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} tests will NOT be compiled") + endif() + + # mx examples + if(MX_BUILD_EXAMPLES) + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} examples will be compiled") + add_executable(mxread ${PRIVATE_DIR}/mx/examples/Read.cpp) + add_executable(mxwrite ${PRIVATE_DIR}/mx/examples/Write.cpp) + add_executable(mxhide ${PRIVATE_DIR}/mx/examples/Hide.cpp) + target_link_libraries(mxread ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT}) + target_link_libraries(mxwrite ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT}) + target_link_libraries(mxhide ${PROJECT_NAME} ${CMAKE_THREAD_LIBS_INIT}) + set_property(TARGET mxread PROPERTY CXX_STANDARD ${CPP_VERSION}) + set_property(TARGET mxwrite PROPERTY CXX_STANDARD ${CPP_VERSION}) + set_property(TARGET mxhide PROPERTY CXX_STANDARD ${CPP_VERSION}) + else() + message(STATUS "${PROJECT_NAME}:${CMAKE_CURRENT_LIST_LINE} examples will NOT be compiled") + endif() + endif() diff --git a/Dockerfile b/Dockerfile index 8375b9e1a..71cfe28e1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -46,6 +46,12 @@ COPY . . FROM base AS run RUN --mount=type=cache,target=/workspace/build make check +# --- run-core-dev stage: run `make check-core-dev` for its exit code -------- +# Parallel to `run`. Drives the core-dev gate (mx/core + ezxml + utility, +# fmt-check + warning-free build) against the same pinned toolchain. +FROM base AS run-core-dev +RUN --mount=type=cache,target=/workspace/build make check-core-dev + # --- fmt stage: format in place, then export only the src tree ------- FROM base AS fmt RUN --mount=type=cache,target=/workspace/build make fmt diff --git a/Makefile b/Makefile index 5f6ab1e9c..917b324a8 100644 --- a/Makefile +++ b/Makefile @@ -96,13 +96,16 @@ endif # build// for the given mode ($1). mode_dir = $(BUILD_ROOT)/$(1)/$(BUILD_TYPE) -# Configure + build a mode. $1 = mode name, then the three MX_BUILD_* values. +# Configure + build a mode. $1 = mode name, then the three MX_BUILD_* values +# followed by MX_CORE_DEV. core-dev passes MX_CORE_DEV=on with the three +# MX_BUILD_* flags off; every other mode passes MX_CORE_DEV=off. define cmake_build $(CMAKE) -S . -B $(call mode_dir,$(1)) \ -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) \ -DMX_BUILD_TESTS=$(2) \ -DMX_BUILD_CORE_TESTS=$(3) \ -DMX_BUILD_EXAMPLES=$(4) \ + -DMX_CORE_DEV=$(5) \ $(GEN_ARG) $(CMAKE) --build $(call mode_dir,$(1)) --parallel $(JOBS) --config $(BUILD_TYPE) endef @@ -131,7 +134,8 @@ endef .DEFAULT_GOAL := help .PHONY: help lib dev core test test-all examples-run all clean clean-docker \ - check-docker fmt check xcode-gen xcode-build xcode-test + check-docker fmt check core-dev check-core-dev test-core-dev \ + xcode-gen xcode-build xcode-test help: @echo 'mx build/test targets (see the comments at the top of the Makefile):' @@ -164,19 +168,33 @@ help: @echo ' make xcode-build Build the Xcode project.' @echo ' make xcode-test Run tests via xcodebuild.' @echo '' + @echo 'Core development (codegen):' + @echo ' make core-dev Build trimmed library (mx/core + ezxml + utility) and' + @echo ' mxtest-core-dev. No mx/api or mx/impl compiled.' + @echo ' make check-core-dev fmt-check + warning-free build for core-dev (Docker).' + @echo ' make test-core-dev Build core-dev then run the core roundtrip suite.' + @echo ' Each file under data/ is a separate Catch2 test case.' + @echo " Filter: make test-core-dev ARGS='[core-roundtrip] lysuite/*'" + @echo '' @echo 'Knobs: JOBS (=$(JOBS)) BUILD_TYPE (=$(BUILD_TYPE)) GENERATOR ARGS DOCKER' @echo 'Layout: $(BUILD_ROOT)//$(BUILD_TYPE)/' # --- Compile-only targets --------------------------------------------------- lib: - $(call cmake_build,lib,off,off,off) + $(call cmake_build,lib,off,off,off,off) dev: - $(call cmake_build,dev,on,off,on) + $(call cmake_build,dev,on,off,on,off) core: - $(call cmake_build,core,on,on,on) + $(call cmake_build,core,on,on,on,off) + +# Core-dev mode: trimmed build (mx/core + ezxml + utility) plus the core +# roundtrip test binary. mx/api and mx/impl are not compiled. Intended for +# codegen iteration on mx/core. See AGENTS.md and docs/ai/projects/core-dev/. +core-dev: + $(call cmake_build,core-dev,off,off,off,on) # --- Run targets ------------------------------------------------------------ @@ -188,6 +206,13 @@ test-all: core $(call run_examples,$(call mode_dir,core)) $(call run_bin,$(call mode_dir,core),mxtest,$(ARGS)) +# Run the core roundtrip suite from the trimmed build. ARGS is forwarded to +# the binary (e.g., make test-core-dev ARGS='[core-roundtrip] lysuite/'). +# --allow-running-no-tests keeps the target green when ARGS filters to an +# empty set, and also during Phase A before any test cases are registered. +test-core-dev: core-dev + $(call run_bin,$(call mode_dir,core-dev),mxtest-core-dev,--allow-running-no-tests $(ARGS)) + examples-run: dev $(call run_examples,$(call mode_dir,dev)) @@ -244,6 +269,7 @@ check: -DMX_BUILD_TESTS=on \ -DMX_BUILD_CORE_TESTS=off \ -DMX_BUILD_EXAMPLES=on \ + -DMX_CORE_DEV=off \ $(GEN_ARG) > $(BUILD_ROOT)/build.log 2>&1 \ || { cat $(BUILD_ROOT)/build.log; \ echo "ERROR: cmake configure failed (see above)"; exit 1; } @@ -258,6 +284,33 @@ check: fi @echo "=== check passed ===" +# fmt-check + warning-free build for core-dev mode. Same shape as `check` +# but configures with MX_CORE_DEV=on and the three MX_BUILD_* flags off. +check-core-dev: + @echo "=== fmt-check ===" + @$(FIND_CPP) | xargs clang-format --dry-run --Werror + @echo "=== build (warning-free, core-dev) ===" + @mkdir -p $(BUILD_ROOT) + @$(CMAKE) -S . -B $(call mode_dir,core-dev) \ + -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) \ + -DMX_BUILD_TESTS=off \ + -DMX_BUILD_CORE_TESTS=off \ + -DMX_BUILD_EXAMPLES=off \ + -DMX_CORE_DEV=on \ + $(GEN_ARG) > $(BUILD_ROOT)/build.log 2>&1 \ + || { cat $(BUILD_ROOT)/build.log; \ + echo "ERROR: cmake configure failed (see above)"; exit 1; } + @$(CMAKE) --build $(call mode_dir,core-dev) --parallel $(JOBS) --config $(BUILD_TYPE) \ + >> $(BUILD_ROOT)/build.log 2>&1; status=$$?; \ + cat $(BUILD_ROOT)/build.log; \ + if [ $$status -ne 0 ]; then \ + echo "ERROR: build failed (see above)"; exit $$status; \ + fi; \ + if grep -q 'warning:' $(BUILD_ROOT)/build.log; then \ + echo "ERROR: build emitted warnings (see above)"; exit 1; \ + fi + @echo "=== check-core-dev passed ===" + else # ===== Outside the container: delegate to Docker =========================== @@ -271,6 +324,12 @@ check: check-docker $(DOCKER) buildx build --target run \ --output type=cacheonly $(DOCKER_CACHE) . +# Core-dev mirror of `check`: delegates to a parallel `run-core-dev` Docker +# stage that hardcodes `make check-core-dev` against the pinned toolchain. +check-core-dev: check-docker + $(DOCKER) buildx build --target run-core-dev \ + --output type=cacheonly $(DOCKER_CACHE) . + endif # --- Xcode targets ---------------------------------------------------------- diff --git a/README.md b/README.md index e3ffbed40..09d1b773c 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,8 @@ switching modes never recompiles another mode's tree. Knobs: `JOBS` (parallelism `BUILD_TYPE` (default `Debug`), `GENERATOR` (default: CMake's platform default), and `ARGS` (forwarded to `mxtest`, e.g. `make test ARGS='[core]'`). +A `core-dev` mode exists for codegen iteration on `mx/core`; see `AGENTS.md` for details. + ### Build Tenets * `mx` should not depend on any outside libraries (no deps). diff --git a/docs/ai/projects/core-dev/.prompt b/docs/ai/projects/core-dev/.prompt new file mode 100644 index 000000000..e69de29bb diff --git a/docs/ai/projects/core-dev/design/agents-md-snippet.md b/docs/ai/projects/core-dev/design/agents-md-snippet.md new file mode 100644 index 000000000..f80f7f68a --- /dev/null +++ b/docs/ai/projects/core-dev/design/agents-md-snippet.md @@ -0,0 +1,50 @@ +# AGENTS.md Snippet — Core Development Mode + +Verbatim Markdown to append to the repo-root `AGENTS.md` at implementation time. Stored +separately because nested code fences break Markdown rendering in the main design doc. + +Insert as a new top-level section after `## Quality Gates`. + +--- + +## Core Development Mode (Codegen) + +Use core-dev when modifying files under `src/private/mx/core/` and you do not need `mx/api` +or `mx/impl` to compile. Trimmed build for codegen iteration: only `mx/core` + `mx/ezxml` + +`mx/utility` compile. + +### In-mode iteration + +``` +make core-dev # configure + build (fast: skips api/impl) +make test-core-dev # run all core roundtrip cases +make test-core-dev ARGS='[core-roundtrip] lysuite/*' # subset +``` + +In-mode gate: `make fmt && make check-core-dev && make test-core-dev`. `fmt` and +`check-core-dev` run in Docker; `test-core-dev` runs natively. + +### Full pre-merge gate + +Before merging core changes, run the full gate. Changes under `mx/core/` require +`test-all`, which exercises per-element core unit tests, api import, and core +roundtrip together: + +``` +make fmt && make check && make test-all +``` + +### What core-dev tests + +Backed by the **core roundtrip** suite (`CORE_ROUNDTRIP`, `CORE_RT`) under +`src/private/mxtest/corert/`. Each `*.xml` / `*.musicxml` file under `data/` (excluding +`data/expected/`, `data/testOutput/`, `data/generalxml/`, `data/smufl/`) is one Catch2 +test case round-tripping through `mx::core::Document` (`fromXDoc` → `toXDoc`) against a +normalized input computed in-memory. + +The same suite runs inside the normal `mxtest` binary during `make test-all`, gated on +`MX_BUILD_CORE_TESTS=ON`. Distinct from the **api import** (`API_IMPORT`) suite in +`src/private/mxtest/import/`, which exercises the full `mx::api` stack against pre-generated +expected files under `data/expected/`. + +For design details see `docs/ai/projects/core-dev/design/design.md`. diff --git a/docs/ai/projects/core-dev/design/design.md b/docs/ai/projects/core-dev/design/design.md new file mode 100644 index 000000000..375604f15 --- /dev/null +++ b/docs/ai/projects/core-dev/design/design.md @@ -0,0 +1,375 @@ +# core-dev Design + +Permanent in-repo "core development" mode. Lets a codegen agent churn `mx/core` symbols +without keeping `mx/api`, `mx/impl`, or higher-level tests compiling. Activated via +`make core-dev` / `make check-core-dev` / `make test-core-dev`, backed by an `MX_CORE_DEV=ON` +CMake option. + +The mode introduces a new test suite, **core roundtrip** (`CORE_ROUNDTRIP`, `CORE_RT`), under +`src/private/mxtest/corert/`. It runs standalone under `make test-core-dev` and inside the +normal `mxtest` binary during `make test-all`. It does not run during `make test` (the fast +path) — gated on `MX_BUILD_CORE_TESTS=ON`, the same flag that gates the 522 per-element core +unit tests. + +Distinct from **api import** (`API_IMPORT`), the `src/private/mxtest/import/` suite +that exercises the full `mx::api` stack against pre-generated expected files. "roundtrip" +is never used unqualified; always "core roundtrip" or "api import." + +Sections: + +1. Build System +2. Compilation Guards +3. Core Roundtrip Test +4. Agent Surface + +--- + +## 1. Build System + +### CMake Option + +``` +option(MX_CORE_DEV "Build only mx/core + ezxml + utility with roundtrip tests" OFF) +``` + +Default OFF; normal build unaffected. + +### Shared Glob + +`SRC_MX_CORE_ROUNDTRIP` is a new glob set, defined unconditionally near the +`SRC_MX_TEST_*` globs, targeting `src/private/mxtest/corert/*.{cpp,h}`. Both build modes +reference it (see section 3 for contents). + +### Effect of `MX_CORE_DEV=ON` (core-dev branch) + +`CMakeLists.txt` enters a distinct branch that builds only two targets: + +| Target | Type | Sources | +|-------------------|----------------|---------------------------------------------------| +| `mx-core` | static library | `SRC_MX_CORE` + `SRC_MX_EZXML` + `SRC_MX_UTILITY` | +| `mxtest-core-dev` | executable | `SRC_MX_CORE_ROUNDTRIP` + `SRC_CPUL` | + +`SRC_MX_API` and `SRC_MX_IMPL` are never passed to any `add_library` / `add_executable` in the +core-dev branch, so they never compile. `MX_BUILD_TESTS`, `MX_BUILD_CORE_TESTS`, and +`MX_BUILD_EXAMPLES` are ignored in core-dev mode — they control only the normal build. + +`PathRoot.h` is still generated (it records `MX_REPO_ROOT_PATH`, used by the core roundtrip +test for file discovery). + +### Effect of `MX_CORE_DEV=OFF` (normal branch) + +The existing build runs unchanged — `mx` library, `mxtest`, examples, install rules — +with one addition: when `MX_BUILD_TESTS=ON` *and* `MX_BUILD_CORE_TESTS=ON`, +`SRC_MX_CORE_ROUNDTRIP` is appended to `mxtest`'s source list, so the core +roundtrip cases run as part of `mxtest` alongside the api import, api, impl, and +per-element core suites. + +Gating rationale: `MX_BUILD_CORE_TESTS` gates the 522 per-element core unit tests — OFF +during `make test` / `make check`, ON during `make test-all`. Reusing it keeps `make test` +fast and groups the core roundtrip suite with "anything related to core." No new +test-suite flag is introduced. + +### Structure + +One top-level `if(MX_CORE_DEV) ... else() ... endif()` in `CMakeLists.txt`. The `else()` +branch holds the existing normal build with the `MX_BUILD_CORE_TESTS`-gated +append described above; the `if()` branch holds only the two core-dev targets. No small +`if(NOT MX_CORE_DEV)` gates sprinkled through the file. + +### Build Directory + +`build/core-dev/$(BUILD_TYPE)/`, following the `mode_dir` convention. Independent +CMake cache and incremental state, so switching between `dev` and `core-dev` never +reconfigures the other. + +### Makefile Targets + +| Target | What it does | +|------------------|----------------------------------------------------------------| +| `core-dev` | Configure + build with `MX_CORE_DEV=ON` | +| `check-core-dev` | fmt-check + warning-free build in Docker with `MX_CORE_DEV=ON` | +| `test-core-dev` | Depends on `core-dev`; runs `mxtest-core-dev` (passes `ARGS`) | + +`test-core-dev` always passes `--allow-running-no-tests` to the binary so the +target stays green when `ARGS` filters down to zero cases (and during early +implementation phases before any cases are registered). Real failures still +fail the target — the flag only changes how Catch2 treats the empty-result +case. + +`core-dev`: + +```makefile +core-dev: + $(CMAKE) -S . -B $(call mode_dir,core-dev) \ + -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) \ + -DMX_CORE_DEV=ON \ + $(GEN_ARG) + $(CMAKE) --build $(call mode_dir,core-dev) --parallel $(JOBS) \ + --config $(BUILD_TYPE) +``` + +The `cmake_build` macro passes the three `MX_BUILD_*` flags positionally. It is +refactored to also accept `MX_CORE_DEV` as a parameter; `core-dev` invokes it with +`MX_CORE_DEV=ON` and the three `MX_BUILD_*` flags OFF. + +`check-core-dev`: mirrors `check` — fmt-check plus warning-free build, run under the pinned +Docker toolchain so the gate is deterministic. Configures with `MX_CORE_DEV=ON` (and the +three `MX_BUILD_*` flags OFF) instead of the `MX_BUILD_TESTS=on` set `check` uses. + +Docker delegation: `make check` outside the container runs `docker buildx build --target +run`; the `run` stage in `Dockerfile` hardcodes `RUN make check` against the pinned +toolchain. `check-core-dev` follows the same pattern: a new `FROM base AS run-core-dev` +stage parallel to `run`, with `RUN make check-core-dev`. Outside the container, +`make check-core-dev` runs `docker buildx build --target run-core-dev`. + +--- + +## 2. Compilation Guards + +No per-file `#ifdef` guards. Exclusion happens at the CMake target level by omitting +api/impl/higher-test glob sets in the core-dev branch (see section 1). + +`src/private/mxtest/control/CompileControl.h` is unchanged. Its macros +(`MX_COMPILE_API_TESTS`, etc.) guard code in files the core-dev binary does not compile, so +they are inert there. Removing dormant macros is out of scope. + +### Dual-Compilation Invariant + +The core roundtrip sources under `mxtest/corert/` compile in two contexts: + +- Linked into `mxtest-core-dev` (core-dev branch), against `mx-core` only. +- Linked into `mxtest` (normal branch with `MX_BUILD_CORE_TESTS=ON`), against the full `mx` + library. + +The sources must therefore have no `#ifdef MX_CORE_DEV` branches and must not depend on +anything that exists in only one context. Concretely: + +- Includes are limited to `mx/core/*`, `mx/ezxml/*`, `mx/utility/*`, `cpul/catch.h`, and the + two helpers below. +- The suite reuses `ChangeValues.h` and `SortAttributes.h` from `mxtest/import/`. Both are + unguarded today; the implementer must confirm this at integration time. + +--- + +## 3. Core Roundtrip Test + +### Source Location + +New files under `src/private/mxtest/corert/`: + +``` +CoreRoundtripTest.cpp Catch2 dynamic registration + test body +CoreRoundtripImpl.h File discovery, normalization, comparison declarations +CoreRoundtripImpl.cpp Implementations +``` + +No dependency on `mxtest/` infrastructure beyond the two unguarded helpers from +`mxtest/import/` (`ChangeValues.h`, `SortAttributes.h`). The suite does not +use `MxFileRepository`, `ImportTestImpl`, or `CompileControl.h`. It includes only +`mx/core/*`, `mx/ezxml/*`, `mx/utility/*`, and `cpul/catch.h`. + +The corresponding implementation files (`ChangeValues.cpp`, `SortAttributes.cpp`) +are linked into `mxtest-core-dev` via an explicit `SRC_MX_CORE_ROUNDTRIP_HELPERS` +list in `CMakeLists.txt` (the path is enumerated rather than globbed to pick +exactly these two helpers and not the rest of the api import suite, which would +drag in `MxFileRepository`, `ImportTestImpl`, etc.). In the normal branch the +same `.cpp` files are already linked transitively via `SRC_MX_TEST_IMPORT`. + +### File Discovery + +Runs once at static-initialization time, before Catch2's runner starts: + +1. Scan root: `MX_REPO_ROOT_PATH "/data"` (injected by CMake into `PathRoot.h`). +2. Recurse all subdirectories. +3. Collect files with extension `.xml` or `.musicxml`. +4. Exclude any path containing a directory segment named `expected`, `testOutput`, + `generalxml`, or `smufl`. + +The `expected/` and `testOutput/` exclusions remove generated outputs from the input set. +The `generalxml/` and `smufl/` exclusions remove non-MusicXML fixtures: `generalxml/` holds +only `fake.xml` (a non-MusicXML XML sample) and `smufl/` holds only `glyphnames.json` (not +XML). If MusicXML test inputs are ever added to those directories, the exclusions +must be revisited. + +Returns absolute paths. Each test name is the path relative to `data/` (e.g., +`lysuite/ly01a_Pitches_Pitches.xml`). + +### Catch2 Dynamic Registration + +Catch2 v3 (amalgamated at `src/private/cpul/catch.h`) exposes `Catch::AutoReg` and +`Catch::ITestInvoker` — the primitives `TEST_CASE` uses internally. + +A custom `ITestInvoker` carries the file path; a static initializer constructs one +`Catch::AutoReg` per discovered file, tagged `[core-roundtrip]`. Each case runs +independently: one failure does not block the rest. Agents filter via +`mxtest-core-dev [core-roundtrip] lysuite/`, or `mxtest [core-roundtrip]` inside the +`test-all` binary. + +### Per-File Test Flow + +The body below runs inside a `try` block; any thrown exception (from `ezxml`, +`fromXDoc`, `toXDoc`, or normalization) is caught and reported via Catch2's `FAIL` with the +file path and `exception::what()`. Catch2 ends the current case on `FAIL` but continues to +the next, so a thrown file never aborts the suite. + +``` +1. Load input via ezxml → inputXDoc +2. Set input root `version` attribute to the supported + MusicXML version (see note below) +3. mx::core::Document::fromXDoc(inputXDoc) → mxDoc +4. mxDoc->toXDoc() → actualXDoc +5. Apply full normalization to actualXDoc + +6. Load input from disk again (fresh) → expectedXDoc +7. Apply full normalization to expectedXDoc + +8. Depth-first compare expectedXDoc vs actualXDoc + On first mismatch: FAIL with path + detail; return +``` + +If `fromXDoc` returns false (non-throwing failure), `FAIL` immediately with file name. + +**Version string.** Step 2 uses a hardcoded `"3.0"` declared as a local +`constexpr const char *const kMusicXmlVersionString` in `CoreRoundtripImpl.cpp`. + +The original intent was to use `mx::core::toString(mx::core::DEFAULT_MUSIC_XML_VERSION)` +as a single source of truth. That is not currently possible: the `mx::core` +namespace declares the version in two conflicting places +(`DocumentSpec.h::DEFAULT_MUSIC_XML_VERSION`, lower-case `threePointZero`, with +stringification; `DocumentHeader.h::kDefaultMusicXmlVersion`, upper-case +`ThreePointZero`, no stringification helper), and including both in one TU is a +hard compile error — the `DocumentChoice` and `MusicXmlVersion` enums are +redeclared with incompatible scopes. The core roundtrip suite needs `Document.h` +for `makeDocument` / `fromXDoc` / `toXDoc`, which transitively conflicts with +`DocumentSpec.h`. The api import suite hardcodes `"3.0"` for the same reason +(see `ImportTestImpl::loadTestFile`); we follow that precedent. The constant +carries a comment pointing at the conflict so a future cleanup — most likely +part of the codegen rewrite that brings MusicXML 4.0 support — can replace it +with the canonical symbol. + +**Normalization symmetry.** Steps 5 and 7 apply the *same* normalization pipeline to +both sides, so the comparison is canonical-against-canonical. + +### Normalization + +Normalization is restricted to representational canonicalization — operations that neutralize +XML's non-semantic degrees of freedom. It excludes the defect-workaround fixups in +`mxtest/import/ExpectedFiles.cpp::generateExpectedFile`, which keep api import green against +known-bad inputs and `mx::core` quirks. Core roundtrip rejects that approach: a +mismatch is a signal to fix the bug (in `mx::core`, the input, or codegen), not to hide it +with normalization. The helpers (`convertValues`, `addChildIfNone`, etc.) live in +`mxtest/import/ChangeValues.h`; the suite imports only the canonicalization helpers below. + +Steps, in order: + +1. `setXmlDeclaration` — standalone=false, XML 1.0, preserve encoding (default UTF-8). +2. `setDoctype` — based on root element name (`score-partwise` or `score-timewise`). +3. `setRootMusicXmlVersion` — set root `version` from + `mx::core::toString(mx::core::DEFAULT_MUSIC_XML_VERSION)`. +4. `stripZerosFromDecimalFields` — canonicalize decimal text (`"1.000"` → `"1"`) on fields + in the decimal-field set. XML compares text strings, not numeric values, so this is + representational normalization, not a defect workaround. +5. `sortAttributes` — **must be last**. Attribute order is not semantic in XML. + +The same pipeline is applied to both `actualXDoc` (step 5 of the per-file flow) and +`expectedXDoc` (step 7), so the comparison sees two canonically-normalized trees. + +`SortAttributes.h` from `mxtest/import/` is the source for `sortAttributes`; the four +remaining helpers come from `ChangeValues.h`. Both headers are unguarded (per Section 2's +Dual-Compilation Invariant). + +### Depth-First Tree Comparison + +At each node-pair: element name, text content (see open question below), attribute count +and each `(name, value)` pair in order, child count. Attributes are already sorted by +normalization step 5, so the comparator walks them in order. Recurse into each child pair in +order. On first mismatch, record and return. + +Comparison stops at the first mismatch and calls `FAIL`. Catch2 ends the current case and +proceeds to the next, so one file's mismatch never blocks the suite. Continuing past a +mismatch would emit cascade noise — one structural difference makes every subsequent sibling +comparison a likely mismatch — so only the first signal is useful. + +Each call carries a node path like `/score-partwise/part[0]/measure[2]/note[1]/pitch`, which +appears in the failure message. Example: + +``` +core roundtrip mismatch in lysuite/ly01a_Pitches_Pitches.xml + at /score-partwise/part[0]/measure[2]/note[1]/pitch/step + expected element name: 'step' + actual element name: 'alter' +``` + +Attribute mismatches use `[@attr]` in the path. + +**Open question — text trimming.** Whether text comparison should trim leading/trailing +whitespace or compare exactly is deferred to implementation. Start with exact comparison; +if real round-trip cases mismatch only on whitespace inside text nodes, switch to trimmed +comparison and document the case that drove the decision. + +### No Pre-Generated Expected Inputs + +The core roundtrip suite does not read `data/expected/`. Expected is computed in-memory from +the input at runtime — no `GenerateExpected` step, no staleness concern. (A key difference +from api import, which compares against pre-generated expected files under +`data/expected/`.) + +### Output Files on Failure + +On any failure (exception caught, `fromXDoc` returning false, or tree-comparison mismatch), +two files are written to `data/testOutput/corert/` for diffing: + +- `.expected.xml` — `expectedXDoc` after normalization +- `.actual.xml` — `actualXDoc` after normalization + +`` is the test name (path relative to `data/`) with directory separators +replaced by `_`, e.g., `lysuite_ly01a_Pitches_Pitches.xml.expected.xml`. Each failure +is a single pair of files under `data/testOutput/corert/` with no nested subdirectories. + +The `corert/` subdirectory namespaces core roundtrip's debug output away from api import's +output (which writes flat into `data/testOutput/`), so the two suites can run in the same +`make test-all` invocation without filename collisions. The test creates +`data/testOutput/corert/` via `mkdir -p` before writing, so a fresh checkout works. + +--- + +## 4. Agent Surface + +Core-dev is an AI/codegen workflow, not a user feature. Substance is documented in +`AGENTS.md` and `make help`. The README gets a short pointer (≤20 words) at implementation +time naming the mode and pointing at `AGENTS.md`; no further README content. + +### `make help` Addition + +Appended after existing sections, before the Knobs/Layout line: + +``` +Core development (codegen): + make core-dev Build trimmed library (mx/core + ezxml + utility) and + mxtest-core-dev. No mx/api or mx/impl compiled. + make check-core-dev fmt-check + warning-free build for core-dev (Docker). + make test-core-dev Build core-dev then run the core roundtrip suite. + Each file under data/ is a separate Catch2 test case. + Filter: make test-core-dev ARGS='[core-roundtrip] lysuite/*' +``` + +The `ARGS` hint is included because agents iterate on subsets. The same `[core-roundtrip]` +tag filters the suite inside `mxtest` during `make test-all`. + +### `AGENTS.md` Addition + +Verbatim Markdown in `agents-md-snippet.md` (kept separate to avoid nested fenced code +blocks). Introduces a top-level `## Core Development Mode (Codegen)` section: when to use +the mode, in-mode commands, in-mode gate, and full pre-merge gate. + +### Build Directories + +| Action | Command | Build dir used | +|---------------------|----------------------|--------------------------| +| Enter (build) | `make core-dev` | `build/core-dev//` | +| Enter (test) | `make test-core-dev` | `build/core-dev//` | +| Leave (verify full) | `make test-all` | `build/core//` | +| Leave (fast verify) | `make test` | `build/dev//` | + +Each mode has its own build directory; switching never reconfigures the other. No persistent +state changes on entering or leaving the mode. diff --git a/docs/ai/projects/core-dev/index.md b/docs/ai/projects/core-dev/index.md new file mode 100644 index 000000000..a3df04fdc --- /dev/null +++ b/docs/ai/projects/core-dev/index.md @@ -0,0 +1,44 @@ +--- +created: 2026-05-21 +--- + +# core-dev + +## Goal + +A permanent in-repo "core development" mode that lets a codegen agent churn +`mx/core` symbols without keeping `mx/api`, `mx/impl`, or higher-level tests +compiling. + +Activated via `make core-dev` / `make check-core-dev` / `make test-core-dev`, +backed by an `MX_CORE_DEV=ON` CMake option. CMake builds only `mx/core` + +`mx/ezxml` + `mx/utility` plus a roundtrip test binary; no per-file `#ifdef` +guards. + +The test suite is one roundtrip harness: each `*.xml` / `*.musicxml` file under +`data/` (excluding `expected/`, `testOutput/`, `generalxml/`, `smufl/`) is a +Catch2 test case asserting that `Document::fromXDoc -> toXDoc` matches the +normalized input. + +CI does not run core-dev. Surfaced in `AGENTS.md` and `make help`. The consumer +is the codegen rewrite tracked in `docs/ai/projects/gen/`; core-dev is its +harness. + +## Index + +- `plan.md` — milestones +- `state.md` — last/next session +- `log.md` — append-only session log (compressed 2026-05-22) +- `.prompt` — user-owned scratchpad (agents do not read) +- `design/design.md` — full design +- `design/agents-md-snippet.md` — Markdown appended to `AGENTS.md` at + implementation time + +## Notes for Agents + +- Milestones 1–3 COMPLETE as of 2026-05-22. Milestones 4–6 remain (see + `plan.md`). +- Deliverable is permanent; do not leave `TEMP:`-style comment-outs in design + or implementation. +- Quality gates: see repo `AGENTS.md`. `make check-core-dev` and + `make test-core-dev` are live. diff --git a/docs/ai/projects/core-dev/log.md b/docs/ai/projects/core-dev/log.md new file mode 100644 index 000000000..770a62fa7 --- /dev/null +++ b/docs/ai/projects/core-dev/log.md @@ -0,0 +1,101 @@ +# core-dev Log + +Compressed 2026-05-22. Earlier session-by-session notes collapsed to the +retrospective and gotchas below; full per-session entries are in git history if +needed. + +## Milestone 1 — Define goals (2026-05-21) + +User-approved scope: `mx/core` + `mx/ezxml` + `mx/utility` only; `mx/api`, +`mx/impl`, their tests, and the 522 core element unit tests excluded. Test +harness is roundtrip-only with dynamic Catch2 registration via `AutoReg` (one +case per file). Makefile: `core-dev`, `check-core-dev`, `test-core-dev`. CMake +exclusion only (no `#ifdef` guards added). CI out of scope. Docs surface: +`AGENTS.md` + `make help`. + +## Milestone 2 — Design (2026-05-21 → 2026-05-22) + +Final design landed in `design/design.md` + `design/agents-md-snippet.md`. +Notable decisions made during review: + +- One top-level `if(MX_CORE_DEV)...else()...endif()` wrap in `CMakeLists.txt`; + `cmake_build` macro takes `MX_CORE_DEV` as a parameter. +- "Option B" reconciliation: same `.cpp` files compile into two binaries + (`mxtest-core-dev` always; `mxtest` only under `MX_BUILD_CORE_TESTS=ON`), so + core roundtrip runs in `make test-all` too. Dual-Compilation Invariant + documented in design §2. +- Terminology fixed: "core roundtrip" / `CORE_ROUNDTRIP` / `CORE_RT` vs "api + import" / `API_IMPORT`. Never say "roundtrip" unqualified. Repo `AGENTS.md` + carries the terminology section. +- Source directory: `src/private/mxtest/corert/`. Discovery excludes + directories named `expected`, `testOutput`, `generalxml`, `smufl`. +- Normalization narrowed to canonicalization only (xml decl, doctype, root + version, decimal zero-stripping, attribute sort). The api import suite's + defect-workaround block was deliberately dropped — a mismatch here is a + signal to fix `mx::core`, the input, or codegen. +- Failure handling: full normalization on both sides; first-mismatch FAIL with + node path; per-file `try`/`catch` so an exception fails one case rather than + aborting the suite. Text comparison exact; trimming deferred unless a real + case drives it (Phase C surfaced no whitespace-only mismatches). +- Debug output to `data/testOutput/corert/`, flattened filenames + (`lysuite_ly01a.xml.expected.xml` / `.actual.xml`), `mkdir -p` at write + time. + +## Milestone 3 — Implement (2026-05-22) + +Four phases plus wrap-up; each ended green for what it added. + +- **Phase A** — CMake option, Makefile targets, Docker `run-core-dev` stage. + `mxtest-core-dev` reuses `cpul/main.cpp` (Catch2 main) instead of a + placeholder `main()` to avoid a link collision. `make test-core-dev` passes + `--allow-running-no-tests` to stay green when ARGS filters to zero matches. +- **Phase B** — `CoreRoundtripImpl.{h,cpp}` + `CoreRoundtripTest.cpp` with one + hardcoded case (`lysuite/ly01a_Pitches_Pitches.xml`). Helpers reused from + `mxtest/import/` (`ChangeValues`, `SortAttributes`); their `.cpp` files + enumerated explicitly via `SRC_MX_CORE_ROUNDTRIP_HELPERS` so core-dev does + not pull in `MxFileRepository` / `ImportTestImpl`. Version string hardcoded + `"3.0"` — see "Known conflicts" below. +- **Phase C** — Dynamic Catch2 registration. Gotchas resolved: + - `Catch::StringRef` does not own storage; test-name `std::string`s live in + a function-local-static `std::vector` that is `reserve()`d to final size + before any `push_back` so addresses are stable. + - `Catch::AutoReg` is non-copyable and non-movable; owned through + `std::vector>`. + - Custom `ITestInvoker` subclass owns its own `std::string` test name copy. +- **Phase D (docs)** — `## Core Development Mode (Codegen)` section appended + to `AGENTS.md` directly after `## Quality Gates`; one-line README pointer; + `make help` block. Working filter form under Catch2 v3 is + `[core-roundtrip] lysuite/*` (not `lysuite/`); both design surfaces updated + to match. +- **Phase D (wrap-up)** — Full gate green. `plan.md` / `index.md` / `state.md` + finalized. + +## Final gate (2026-05-22) + +| Target | Result | +|---------------------|---------------------------------------------| +| `make fmt` | pass | +| `make check` | pass | +| `make check-core-dev` | pass | +| `make test-all` | 3039 cases, 3004 pass, 35 fail (expected) | +| `make test-core-dev` | 361 cases, 326 pass, 35 fail (same set) | + +The 35 core roundtrip failures are the consumer signal for the codegen rewrite +at `docs/ai/projects/gen/` and out of scope for this project. By rough +category from the failure messages: ~14 attribute mismatches, ~5 text +mismatches, a couple of child-count / attribute-count mismatches, one +`fromXDoc` returning false. Files span `foundsuite/`, parts of `lysuite/` +(figured bass, accordion registrations, articulation texts, parenthesized +accidentals), `mjbsuite/krz_v40.xml` (image element attribute set), +`musuite/test_harmony.xml` (harmony kind text), `musuite/testInvalid.xml` +(identification sub-element count). + +## Known conflicts retained out of scope + +- `DocumentSpec.h` vs `DocumentHeader.h` both declare `MusicXmlVersion`; + `DocumentSpec.h` vs `Document.h` both declare `DocumentChoice`. Co-including + is a hard compile error. The core roundtrip suite needs `Document.h`, so it + hardcodes `"3.0"` to match the api import precedent. Fixing this conflict + belongs to the codegen rewrite, not this project. +- Dormant `MX_COMPILE_*` macros in `CompileControl.h` were left as-is; design + §2 marks removal out of scope. diff --git a/docs/ai/projects/core-dev/plan.md b/docs/ai/projects/core-dev/plan.md new file mode 100644 index 000000000..293d40bc5 --- /dev/null +++ b/docs/ai/projects/core-dev/plan.md @@ -0,0 +1,46 @@ +# core-dev Plan + +## Milestone 1: Define goals ✓ COMPLETE + +See `index.md ## Goal`; Q&A in `log.md`. + +## Milestone 2: Design — in progress + +Design lives in `design/`. Until approved, no code outside this project directory changes. + +Design docs describe current state only (per `/project` skill); decisions and history go in +`log.md`. + +Exit: design reviewed with user; open questions resolved. + +## Milestone 3: Implement ✓ COMPLETE + +Landed the design across four implementation phases plus a wrap-up: + +1. Phase A — CMake option + Makefile targets (normal build unchanged when `MX_CORE_DEV=OFF`). +2. Phase B — core roundtrip sources behind one hardcoded `TEST_CASE` under + `src/private/mxtest/corert/`. +3. Phase C — dynamic Catch2 registration over every discovered file. +4. Phase D — agent surface (`AGENTS.md` section + README pointer + `make help` block). +5. Phase D wrap-up — full pre-merge gate run, docs finalized. + +Final gate (2026-05-22, this branch): + +- `make fmt`, `make check`, `make check-core-dev`: pass. +- `make test-all`: 3039 cases, 3004 pass, 35 fail (expected — codegen-rewrite signals). +- `make test-core-dev`: 361 cases, 326 pass, 35 fail (same set). + +The `TEMP: codegen-rewrite harness` edits were already reverted before Phase A +began (working tree clean as of 2026-05-22), so no revert step was needed during +implementation. CI integration for core-dev (item 5 in the original ordering) is +deferred — the design treats it as optional; no further session is allocated. +The 35 core roundtrip failures are out of scope here and are the consumer signal +for the codegen rewrite tracked in `docs/ai/projects/gen/`. + +## Milestone 4: Pass Tests and CI + +## Milestone 5: Code Coverage + +## Milestone 6: Increase Coverage + + diff --git a/docs/ai/projects/core-dev/state.md b/docs/ai/projects/core-dev/state.md new file mode 100644 index 000000000..7f7d442d5 --- /dev/null +++ b/docs/ai/projects/core-dev/state.md @@ -0,0 +1,22 @@ +# core-dev State + +Milestones 1–3 COMPLETE as of 2026-05-22. Milestones 4–6 (pass tests + CI, +code coverage, increase coverage) remain — see `plan.md`. + +`core-dev` mode is live: `make core-dev` / `make check-core-dev` / +`make test-core-dev` work; the core roundtrip suite also runs inside +`make test-all` (gated on `MX_BUILD_CORE_TESTS=ON`). The consumer is the +codegen rewrite at `docs/ai/projects/gen/`. + +Final gate numbers and the 35-failure breakdown are in `log.md`. + +## If a future session reopens this project + +- Read `index.md`, `plan.md`, then `design/design.md` (design describes + current state; history is in `log.md`). +- Likely reopen triggers: starting Milestone 4+; or an item the design + marked out of scope becoming in scope (CI for core-dev, removing dormant + `MX_COMPILE_*` macros, the `DocumentSpec.h` / `DocumentHeader.h` + version-string conflict). +- The 35 expected core roundtrip failures are not a regression of this + project — investigate via `docs/ai/projects/gen/`. diff --git a/src/private/mxtest/corert/CoreRoundtripImpl.cpp b/src/private/mxtest/corert/CoreRoundtripImpl.cpp new file mode 100644 index 000000000..ec2e5bcd2 --- /dev/null +++ b/src/private/mxtest/corert/CoreRoundtripImpl.cpp @@ -0,0 +1,419 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#include "mxtest/corert/CoreRoundtripImpl.h" + +#include "ezxml/XAttribute.h" +#include "ezxml/XAttributeIterator.h" +#include "ezxml/XElementIterator.h" +#include "ezxml/XFactory.h" +#include "mx/core/Document.h" +#include "mxtest/file/PathRoot.h" +#include "mxtest/import/ChangeValues.h" +#include "mxtest/import/SortAttributes.h" + +#include +#include +#include +#include +#include + +namespace mxtest +{ +namespace corert +{ + +namespace +{ + +constexpr const char *const kDataDirName = "data"; +constexpr const char *const kTestOutputSubdir = "testOutput/corert"; + +const std::string &dataRoot() +{ + static const std::string root = std::string(MX_REPO_ROOT_PATH) + "/" + kDataDirName; + return root; +} + +// Exclude any path containing a directory segment named one of these. +bool isExcludedPath(const std::filesystem::path &p) +{ + static const std::vector excludedSegments = {"expected", "testOutput", "generalxml", "smufl"}; + for (const auto &part : p) + { + const std::string seg = part.string(); + for (const auto &ex : excludedSegments) + { + if (seg == ex) + { + return true; + } + } + } + return false; +} + +bool hasXmlExtension(const std::filesystem::path &p) +{ + const std::string ext = p.extension().string(); + return ext == ".xml" || ext == ".musicxml"; +} + +// Set the root `version` attribute to mx::core's supported MusicXML version. +// Equivalent in shape to mxtest/import's setRootMusicXmlVersion but without +// the doctype cross-check (which we apply via setDoctype below based on the +// root element name). +// Pinned MusicXML version string for the supported `mx::core` schema. +// +// Ideally this would come from a single `mx::core` constant. Today the core +// namespace declares the version in two conflicting places +// (`DocumentSpec.h::DEFAULT_MUSIC_XML_VERSION` lower-case and +// `DocumentHeader.h::kDefaultMusicXmlVersion` upper-case `ThreePointZero`), +// and including both in one TU is a hard compile error. The api import suite +// already hardcodes `"3.0"` for the same reason (see +// `ImportTestImpl::loadTestFile`); we follow that precedent here. When the +// core-side conflict is resolved (likely as part of the codegen rewrite that +// brings MusicXML 4.0 support), replace this with the canonical constant. +constexpr const char *const kMusicXmlVersionString = "3.0"; + +void setRootMusicXmlVersion(const ::ezxml::XDocPtr &xdoc) +{ + const auto root = xdoc->getRoot(); + if (!root) + { + return; + } + for (auto it = root->attributesBegin(); it != root->attributesEnd(); ++it) + { + if (it->getName() == "version") + { + it->setValue(kMusicXmlVersionString); + return; + } + } + root->appendAttribute("version")->setValue(kMusicXmlVersionString); +} + +void setXmlDeclaration(const ::ezxml::XDocPtr &xdoc) +{ + xdoc->setHasStandaloneAttribute(true); + xdoc->setIsStandalone(false); + xdoc->setXmlVersion(::ezxml::XmlVersion::onePointZero); + const auto encoding = xdoc->getEncoding(); + if (encoding == ::ezxml::Encoding::unknown) + { + xdoc->setEncoding(::ezxml::DEFAULT_ENCODING); + } +} + +void setDoctypeFromRoot(const ::ezxml::XDocPtr &xdoc) +{ + const auto root = xdoc->getRoot(); + if (!root) + { + return; + } + const std::string name = root->getName(); + xdoc->setHasDoctypeDeclaration(true); + if (name == "score-timewise") + { + xdoc->setDoctypeValue(mx::core::DOCTYPE_VALUE_SCORE_TIMEWISE); + } + else + { + // Default to partwise for anything else (including the common + // score-partwise root). A non-MusicXML root will be caught upstream + // by fromXDoc. + xdoc->setDoctypeValue(mx::core::DOCTYPE_VALUE_SCORE_PARTWISE); + } +} + +// Apply the full normalization pipeline (design §3 Normalization). Must be +// applied to both the expected (input reloaded) and the actual (toXDoc output) +// trees so the comparison sees canonical-against-canonical input. +void normalize(const ::ezxml::XDocPtr &xdoc) +{ + setXmlDeclaration(xdoc); + setDoctypeFromRoot(xdoc); + setRootMusicXmlVersion(xdoc); + mxtest::stripZerosFromDecimalFields(*xdoc); + mxtest::sortAttributes(*xdoc); // must be last +} + +// Format a node path like /score-partwise/part[0]/measure[2]/note[1]/pitch. +std::string nodePath(const std::vector &segments) +{ + std::string s; + for (const auto &seg : segments) + { + s += "/"; + s += seg; + } + return s.empty() ? "/" : s; +} + +struct CompareFailure +{ + bool isFailure = false; + std::string detail; +}; + +CompareFailure compareElements(const ::ezxml::XElement &expected, const ::ezxml::XElement &actual, + std::vector &pathStack) +{ + CompareFailure fail; + + if (expected.getName() != actual.getName()) + { + std::stringstream ss; + ss << "element name mismatch at " << nodePath(pathStack) << ": expected '" << expected.getName() + << "', actual '" << actual.getName() << "'"; + fail.isFailure = true; + fail.detail = ss.str(); + return fail; + } + + // Compare text payload (exact, per design §3 open question — start exact). + if (expected.getValue() != actual.getValue()) + { + std::stringstream ss; + ss << "text mismatch at " << nodePath(pathStack) << ": expected '" << expected.getValue() << "', actual '" + << actual.getValue() << "'"; + fail.isFailure = true; + fail.detail = ss.str(); + return fail; + } + + // Compare attributes. They are already sorted by the normalization step. + auto eAttrIt = expected.attributesBegin(); + const auto eAttrEnd = expected.attributesEnd(); + auto aAttrIt = actual.attributesBegin(); + const auto aAttrEnd = actual.attributesEnd(); + while (eAttrIt != eAttrEnd && aAttrIt != aAttrEnd) + { + if (eAttrIt->getName() != aAttrIt->getName() || eAttrIt->getValue() != aAttrIt->getValue()) + { + std::stringstream ss; + ss << "attribute mismatch at " << nodePath(pathStack) << "[@" << eAttrIt->getName() << "]: expected '" + << eAttrIt->getName() << "=" << eAttrIt->getValue() << "', actual '" << aAttrIt->getName() << "=" + << aAttrIt->getValue() << "'"; + fail.isFailure = true; + fail.detail = ss.str(); + return fail; + } + ++eAttrIt; + ++aAttrIt; + } + if (eAttrIt != eAttrEnd || aAttrIt != aAttrEnd) + { + std::stringstream ss; + ss << "attribute count mismatch at " << nodePath(pathStack); + fail.isFailure = true; + fail.detail = ss.str(); + return fail; + } + + // Compare children, depth-first. + auto eIt = expected.begin(); + const auto eEnd = expected.end(); + auto aIt = actual.begin(); + const auto aEnd = actual.end(); + int childIndex = 0; + while (eIt != eEnd && aIt != aEnd) + { + std::stringstream segSs; + segSs << eIt->getName() << "[" << childIndex << "]"; + pathStack.push_back(segSs.str()); + const auto childFail = compareElements(*eIt, *aIt, pathStack); + if (childFail.isFailure) + { + return childFail; + } + pathStack.pop_back(); + ++eIt; + ++aIt; + ++childIndex; + } + if (eIt != eEnd || aIt != aEnd) + { + std::stringstream ss; + ss << "child count mismatch at " << nodePath(pathStack); + fail.isFailure = true; + fail.detail = ss.str(); + return fail; + } + + return fail; +} + +} // namespace + +std::vector discoverInputFiles() +{ + std::vector result; + namespace fs = std::filesystem; + const fs::path root(dataRoot()); + if (!fs::exists(root)) + { + return result; + } + for (auto it = fs::recursive_directory_iterator(root); it != fs::recursive_directory_iterator(); ++it) + { + const auto &entry = *it; + if (!entry.is_regular_file()) + { + continue; + } + const fs::path &p = entry.path(); + // Excludes are checked against the path relative to the data root so + // an unrelated `expected` segment higher in the filesystem cannot + // affect classification. + const fs::path relative = fs::relative(p, root); + if (isExcludedPath(relative)) + { + continue; + } + if (!hasXmlExtension(p)) + { + continue; + } + result.push_back(p.string()); + } + std::sort(result.begin(), result.end()); + return result; +} + +std::string toTestName(const std::string &absolutePath) +{ + namespace fs = std::filesystem; + const fs::path root(dataRoot()); + const fs::path abs(absolutePath); + std::error_code ec; + const fs::path rel = fs::relative(abs, root, ec); + if (ec || rel.empty() || rel.native().rfind("..", 0) == 0) + { + return absolutePath; + } + // Forward slashes for cross-platform consistency in test names. + std::string s = rel.generic_string(); + return s; +} + +CoreRoundtripResult runCoreRoundtrip(const std::string &absoluteInputPath) +{ + CoreRoundtripResult r; + try + { + // 1. Load the input via ezxml. + auto inputXDoc = ::ezxml::XFactory::makeXDoc(); + inputXDoc->loadFile(absoluteInputPath); + + // 2. Set input root `version` to the supported MusicXML version. The + // declaration and doctype are set later by normalization. + setRootMusicXmlVersion(inputXDoc); + + // 3. fromXDoc into a mx::core::Document. + auto mxDoc = mx::core::makeDocument(); + std::stringstream fromMsg; + const bool ok = mxDoc->fromXDoc(fromMsg, *inputXDoc); + if (!ok) + { + r.ok = false; + r.message = "fromXDoc returned false: " + fromMsg.str(); + return r; + } + + // 4. toXDoc back out. + auto actualXDoc = ::ezxml::XFactory::makeXDoc(); + mxDoc->toXDoc(*actualXDoc); + + // 5. Normalize the actual document. + normalize(actualXDoc); + + // 6. Load a fresh expected from disk. + auto expectedXDoc = ::ezxml::XFactory::makeXDoc(); + expectedXDoc->loadFile(absoluteInputPath); + // Mirror step 2 so the input version is canonical before normalization. + setRootMusicXmlVersion(expectedXDoc); + + // 7. Normalize the expected document with the same pipeline. + normalize(expectedXDoc); + + // 8. Depth-first compare. + std::vector pathStack; + const auto expectedRoot = expectedXDoc->getRoot(); + const auto actualRoot = actualXDoc->getRoot(); + if (!expectedRoot || !actualRoot) + { + r.ok = false; + r.message = "missing root element after normalization"; + return r; + } + pathStack.push_back(expectedRoot->getName()); + const auto fail = compareElements(*expectedRoot, *actualRoot, pathStack); + + if (fail.isFailure) + { + r.ok = false; + r.message = fail.detail; + std::stringstream eSs; + std::stringstream aSs; + expectedXDoc->saveStream(eSs); + actualXDoc->saveStream(aSs); + r.expectedXml = eSs.str(); + r.actualXml = aSs.str(); + return r; + } + + r.ok = true; + return r; + } + catch (const std::exception &e) + { + r.ok = false; + r.message = std::string("exception: ") + e.what(); + return r; + } + catch (...) + { + r.ok = false; + r.message = "unknown exception"; + return r; + } +} + +void writeFailureFiles(const std::string &testName, const std::string &expectedXml, const std::string &actualXml) +{ + namespace fs = std::filesystem; + const fs::path outDir = fs::path(MX_REPO_ROOT_PATH) / kDataDirName / "testOutput" / "corert"; + std::error_code ec; + fs::create_directories(outDir, ec); + // If create_directories fails, the open below will fail too; we let that + // surface to the caller via the test simply not having debug files. No + // throw — the suite is already in a failure state at this point. + + std::string flat = testName; + std::replace(flat.begin(), flat.end(), '/', '_'); + std::replace(flat.begin(), flat.end(), '\\', '_'); + + const fs::path expectedPath = outDir / (flat + ".expected.xml"); + const fs::path actualPath = outDir / (flat + ".actual.xml"); + { + std::ofstream f(expectedPath); + if (f.is_open()) + { + f << expectedXml; + } + } + { + std::ofstream f(actualPath); + if (f.is_open()) + { + f << actualXml; + } + } +} + +} // namespace corert +} // namespace mxtest diff --git a/src/private/mxtest/corert/CoreRoundtripImpl.h b/src/private/mxtest/corert/CoreRoundtripImpl.h new file mode 100644 index 000000000..3d4fed061 --- /dev/null +++ b/src/private/mxtest/corert/CoreRoundtripImpl.h @@ -0,0 +1,64 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#pragma once + +#include "ezxml/XDoc.h" +#include "ezxml/XElement.h" + +#include +#include + +namespace mxtest +{ +namespace corert +{ + +// Result of running the core roundtrip flow on a single file. +struct CoreRoundtripResult +{ + bool ok = false; + std::string message; + // Populated on failure (and only on failure) so the caller can diff. Each + // is the corresponding XDoc serialized to a string after the full + // normalization pipeline has been applied. + std::string expectedXml; + std::string actualXml; +}; + +// Discover all eligible MusicXML inputs under `data/`. +// +// Scans `MX_REPO_ROOT_PATH "/data"` recursively, collects regular files with +// extension `.xml` or `.musicxml`, and excludes any path containing a +// directory segment named `expected`, `testOutput`, `generalxml`, or `smufl`. +// Returns absolute paths. +// +// Not used by the Phase B hardcoded test case; declared here so Phase C can +// pick it up without further header changes. +std::vector discoverInputFiles(); + +// Convert an absolute path under `data/` into the relative test name used as +// the Catch2 case name (e.g., `lysuite/ly01a_Pitches_Pitches.xml`). If the +// path is not under the data root the absolute path is returned unchanged. +std::string toTestName(const std::string &absolutePath); + +// Run the per-file core roundtrip flow described in design §3. +// +// Steps: load via ezxml; set version attribute on root; fromXDoc into a +// mx::core::Document; toXDoc back out; normalize both the actual and a +// fresh-loaded expected; depth-first compare. Result is a structured value; +// the caller is responsible for emitting Catch2 FAIL / passing assertions and +// writing debug files. The function does not throw — any exception from the +// pipeline is caught and reported via `CoreRoundtripResult::message`. +CoreRoundtripResult runCoreRoundtrip(const std::string &absoluteInputPath); + +// Write a pair of `.expected.xml` / `.actual.xml` debug files to +// `data/testOutput/corert/`. Creates the directory if it does not exist. +// `testName` is the path relative to `data/`; directory separators are +// replaced with `_` to flatten into a single directory. Both strings are the +// already-normalized XML payloads. +void writeFailureFiles(const std::string &testName, const std::string &expectedXml, const std::string &actualXml); + +} // namespace corert +} // namespace mxtest diff --git a/src/private/mxtest/corert/CoreRoundtripTest.cpp b/src/private/mxtest/corert/CoreRoundtripTest.cpp new file mode 100644 index 000000000..a0c497e9d --- /dev/null +++ b/src/private/mxtest/corert/CoreRoundtripTest.cpp @@ -0,0 +1,118 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#include "mxtest/corert/CoreRoundtripImpl.h" +#include "mxtest/file/PathRoot.h" + +#include "cpul/cpulTestHarness.h" + +#include +#include +#include + +namespace +{ + +// Run the core roundtrip flow for one input and report through Catch2. +// +// `relPath` is the path relative to `data/`, also the test name. On failure, +// debug files are written to `data/testOutput/corert/` and `FAIL` is invoked +// with a path-qualified message. +void runOne(const std::string &relPath) +{ + const std::string absPath = std::string(MX_REPO_ROOT_PATH) + "/data/" + relPath; + const auto result = mxtest::corert::runCoreRoundtrip(absPath); + if (!result.ok) + { + if (!result.expectedXml.empty() || !result.actualXml.empty()) + { + mxtest::corert::writeFailureFiles(relPath, result.expectedXml, result.actualXml); + } + FAIL("core roundtrip failed for " + relPath + ": " + result.message); + } +} + +// Dynamic Catch2 registration (design §3 Catch2 Dynamic Registration). +// +// `Catch::StringRef` does not own its storage; `TestCaseInfo` stores the test +// name as a `StringRef` (see `TestCaseInfo::TestCaseInfo` in catch.cpp). +// The backing `std::string`s must therefore outlive Catch2's runner. We hold +// them in function-local statics whose lifetime ends with the program. +// +// `Catch::AutoReg` is non-copyable and non-movable (it inherits +// `Catch::Detail::NonCopyable`), so it cannot live in a `std::vector` by +// value. We heap-allocate each one and own them through a vector of +// `std::unique_ptr`. + +// Per-file invoker: holds the relative-path test name and dispatches to +// `runOne` from `invoke()`. The string is owned by the invoker (not aliased +// to the static name table) so the invoker is self-contained. +class CoreRoundtripInvoker : public Catch::ITestInvoker +{ + public: + explicit CoreRoundtripInvoker(std::string relPath) : m_relPath(std::move(relPath)) + { + } + + void invoke() const override + { + runOne(m_relPath); + } + + private: + std::string m_relPath; +}; + +// Names backing storage: one entry per discovered file, in stable order. +// `Catch::StringRef` references into these strings, so they must outlive +// Catch2's test registry — they are program-lifetime statics. +std::vector &nameStorage() +{ + static std::vector storage; + return storage; +} + +// AutoReg owners. Constructing an AutoReg registers the case with Catch2's +// global registry; destroying it before `main()` returns is fine, but we +// keep them alive for program lifetime to mirror the lifetime of test-case +// names they reference. +std::vector> &autoRegStorage() +{ + static std::vector> storage; + return storage; +} + +// Tag string for every registered case. Static so the StringRef stays valid. +constexpr const char *const kCoreRoundtripTag = "[core-roundtrip]"; + +// Static initializer: discover files and register one Catch2 case per file. +// Runs before `main()`, before Catch2's runner starts. File discovery is the +// only filesystem touch at this point — the per-case body runs later when +// Catch2 invokes each case. +struct CoreRoundtripRegistrar +{ + CoreRoundtripRegistrar() + { + const auto absPaths = mxtest::corert::discoverInputFiles(); + auto &names = nameStorage(); + auto ®s = autoRegStorage(); + names.reserve(absPaths.size()); + regs.reserve(absPaths.size()); + for (const auto &abs : absPaths) + { + names.push_back(mxtest::corert::toTestName(abs)); + const std::string &nameRef = names.back(); + auto invoker = Catch::Detail::make_unique(nameRef); + const Catch::NameAndTags nameAndTags{Catch::StringRef(nameRef.c_str(), nameRef.size()), + Catch::StringRef(kCoreRoundtripTag)}; + regs.push_back(std::unique_ptr(new Catch::AutoReg( + std::move(invoker), Catch::SourceLineInfo(__FILE__, static_cast(__LINE__)), + Catch::StringRef(), nameAndTags))); + } + } +}; + +const CoreRoundtripRegistrar g_coreRoundtripRegistrar; + +} // namespace