From 677a4546ac3ee8b8defd749ea2633d7478c8da74 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 13:59:59 +0200 Subject: [PATCH 01/17] Fix backend Note steps truncated by YAML comments and add workflow lint In a plain YAML scalar, ' #' starts a YAML comment even inside shell-looking quotes, so both backend Note steps shipped as an unterminated 'echo "...' and failed with 'unexpected EOF while looking for matching quote', turning the jobs red even when their test steps passed. Switch both Note steps to block scalars so the full command (including the issue references) reaches the shell. Add an actionlint workflow gated on .github/workflows/** changes; actionlint's shellcheck pass catches this truncation class before it lands. Closes #43 (cherry picked from commit b19bd27984988348a6dc1489fd400a5393ec1df6) --- .github/workflows/mdsplus-contract.yml | 6 +++++- .github/workflows/uda-contract.yml | 6 +++++- .github/workflows/workflow-lint.yml | 27 ++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/workflow-lint.yml diff --git a/.github/workflows/mdsplus-contract.yml b/.github/workflows/mdsplus-contract.yml index df95fab1..60e5fef8 100644 --- a/.github/workflows/mdsplus-contract.yml +++ b/.github/workflows/mdsplus-contract.yml @@ -100,4 +100,8 @@ jobs: - name: Note if: always() - run: echo "MDSplus leg is informational (issue #12/#18). Red-by-design until the characterization baseline is triaged; promotion to a blocking gate is Task 3." + # Block scalar, not a plain scalar: in a plain scalar ` #` starts a YAML + # comment even inside shell-looking quotes, truncating the command into + # an unterminated string (issue #43). + run: | + echo "MDSplus leg is informational (issue #12/#18). Red-by-design until the characterization baseline is triaged; promotion to a blocking gate is Task 3." diff --git a/.github/workflows/uda-contract.yml b/.github/workflows/uda-contract.yml index adfbc9d2..a75b2b6a 100644 --- a/.github/workflows/uda-contract.yml +++ b/.github/workflows/uda-contract.yml @@ -83,4 +83,8 @@ jobs: - name: Note if: always() - run: echo "UDA leg is informational (issue #21/#29). Red-by-design only for un-pinned defects; TRACEABILITY.md Part 5 (issue #28) records the pinned baseline this leg should stay green against." + # Block scalar, not a plain scalar: in a plain scalar ` #` starts a YAML + # comment even inside shell-looking quotes, truncating the command into + # an unterminated string (issue #43). + run: | + echo "UDA leg is informational (issue #21/#29). Red-by-design only for un-pinned defects; TRACEABILITY.md Part 5 (issue #28) records the pinned baseline this leg should stay green against." diff --git a/.github/workflows/workflow-lint.yml b/.github/workflows/workflow-lint.yml new file mode 100644 index 00000000..db546a13 --- /dev/null +++ b/.github/workflows/workflow-lint.yml @@ -0,0 +1,27 @@ +name: Workflow lint + +# Issue #43: a plain YAML scalar in a `run:` line silently turns ` #` into a +# YAML comment, truncating the shell command (both backend Note steps shipped +# that way). actionlint runs shellcheck over every run: script, so this class +# of truncation fails before push lands instead of turning jobs red at runtime. + +on: + push: + paths: + - ".github/workflows/**" + pull_request: + paths: + - ".github/workflows/**" + +jobs: + actionlint: + name: actionlint + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Run actionlint + run: | + bash <(curl -sSfL https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash) + ./actionlint -color From bb5050a38350f28adefff627034b8b1861084fe4 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:00:46 +0200 Subject: [PATCH 02/17] Build the MDSplus DD model before running contract_tests contract_tests opens pulses against the baked model tree, but only depended on al; the mdsplus-contract workflow builds just --target contract_tests, so al-mdsplus-model (an independent ALL custom target) was never built and the whole MDSplus tier failed with %TREE-E-FOPENR while opening the pulse/model tree. Make the dependency part of the build graph: when AL_BACKEND_MDSPLUS and AL_BUILD_MDSPLUS_MODELS are both on, contract_tests depends on al-mdsplus-model (declared at the top level, after both targets exist, since tests/ is added before models/mdsplus/). Also verify the three ids_model.* files exist in the workflow before CTest, covering both the cache-hit and cache-miss paths. Closes #42 (cherry picked from commit 54db010e357c76a3aae652a38e8e640e93f5fc45) --- .github/workflows/mdsplus-contract.yml | 11 +++++++++++ CMakeLists.txt | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/.github/workflows/mdsplus-contract.yml b/.github/workflows/mdsplus-contract.yml index 60e5fef8..50f85c2f 100644 --- a/.github/workflows/mdsplus-contract.yml +++ b/.github/workflows/mdsplus-contract.yml @@ -89,6 +89,17 @@ jobs: cmake --build build-mdsplus --target contract_tests -j\$(nproc) " + - name: Verify baked DD model tree is present (cache-hit and cache-miss paths) + run: | + set -euo pipefail + for f in ids_model.tree ids_model.datafile ids_model.characteristics; do + if [ ! -s "build-mdsplus/models/mdsplus/${f}" ]; then + echo "FATAL: missing/empty model file build-mdsplus/models/mdsplus/${f} — the MDSplus tier would fail with %TREE-E-FOPENR (issue #42)" + exit 1 + fi + done + echo "Baked DD model tree complete under build-mdsplus/models/mdsplus." + - name: Run MDSplus contract suite run: > docker run --rm --platform linux/arm64 diff --git a/CMakeLists.txt b/CMakeLists.txt index 93ee06cd..0b2d72f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -337,4 +337,14 @@ endif() if(AL_BUILD_MDSPLUS_MODELS) add_subdirectory(models/mdsplus) + + # The MDSplus contract tier opens pulses against the baked model tree, so + # `cmake --build . --target contract_tests` must build the model too — a + # workflow that builds only contract_tests otherwise runs against an absent + # model and fails with %TREE-E-FOPENR (issue #42). Wired here, after both + # targets exist: tests/ is added before models/mdsplus/, so the dependency + # cannot be declared from tests/contract/CMakeLists.txt. + if(AL_BACKEND_MDSPLUS AND TARGET contract_tests) + add_dependencies(contract_tests al-mdsplus-model) + endif() endif() From b3b67e6fff98bd259839db676ba72836f59412c2 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:02:22 +0200 Subject: [PATCH 03/17] Fail the dedicated UDA tier when the reference server does not start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docker/uda/run.sh retried the TCP listener but never checked whether the loop succeeded, printed 'server listening' unconditionally, and downgraded a failed uda_cli ping to a warning. Combined with the contract suite's server-unreachable GTEST_SKIP(), a total bootstrap failure could skip every UDA case while CTest exits 0 — a green job that tested nothing. - Track listener success and exit nonzero after the timeout. - Make the independent uda_cli help::help() ping fatal. - Add a dedicated-CI mode (AL_UDA_DEDICATED_CI=1, set by uda-contract.yml): run CTest with --output-junit and fail unless a nonzero number of UDA tests actually executed (guards both zero selected and all skipped). - Developer builds keep the compile-gate/runtime-skip behavior: without the flag nothing changes. Closes #41 (cherry picked from commit 297a946b70ae49f6a8c5a23a86134bb9c2c7ee0f) --- .github/workflows/uda-contract.yml | 6 +++++ docker/uda/run.sh | 41 +++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/.github/workflows/uda-contract.yml b/.github/workflows/uda-contract.yml index a75b2b6a..3f0942bf 100644 --- a/.github/workflows/uda-contract.yml +++ b/.github/workflows/uda-contract.yml @@ -75,10 +75,16 @@ jobs: grep -q "\*\*${dd_version}\*\*" "${header}" echo "Image stack identifiers match ${header}'s Part 5 pinned-reference-stack table." + # AL_UDA_DEDICATED_CI=1: inside the dedicated reference-stack job a + # server bootstrap failure must fail the job, not skip every test — + # uda-run.sh makes the listener wait and the uda_cli ping fatal, and + # fails unless a nonzero number of UDA tests actually executed + # (issue #41). Developer builds without the flag keep skip semantics. - name: Run the UDA contract tier run: > docker run --rm --platform linux/arm64 -v "$PWD":/workspace -w /workspace + -e AL_UDA_DEDICATED_CI=1 "${IMAGE_TAG}" uda-run.sh - name: Note diff --git a/docker/uda/run.sh b/docker/uda/run.sh index baba1493..c9f1fc47 100644 --- a/docker/uda/run.sh +++ b/docker/uda/run.sh @@ -89,20 +89,32 @@ fi sed -i 's/^\( *user *= *\)$/\1root/' "${PREFIX}/etc/xinetd.conf" xinetd -dontfork -f "${PREFIX}/etc/xinetd.conf" & XINETD_PID=$! -# Wait for the listener before running the client. +# Wait for the listener before running the client; a server that never comes +# up is a bootstrap failure, not something to paper over — the contract tier +# would GTEST_SKIP() every case and CTest would exit 0 having tested nothing +# (issue #41). +listening=0 for _ in $(seq 1 30); do if bash -c "exec 3<>/dev/tcp/localhost/56565" 2>/dev/null; then exec 3>&- 2>/dev/null || true + listening=1 break fi sleep 0.5 done +if [ "${listening}" != 1 ]; then + echo "FATAL: UDA server never started listening on localhost:56565 (xinetd pid ${XINETD_PID})" + exit 1 +fi echo "UDA server listening on localhost:56565 (xinetd pid ${XINETD_PID})" # Sanity ping through UDA's own CLI (independent of IMAS-Core) so a server-side -# failure is distinguishable from a client-side one. +# failure is distinguishable from a client-side one. Fatal: a stack whose own +# CLI cannot talk to the server must not proceed to a CTest run whose UDA cases +# would all skip-and-pass (issue #41). UDA_HOST=localhost UDA_PORT=56565 "${PREFIX}/bin/uda_cli" \ --host localhost --port 56565 --request "help::help()" >/dev/null 2>&1 \ - && echo "uda_cli help::help() OK" || echo "WARN: uda_cli ping failed (continuing)" + || { echo "FATAL: uda_cli help::help() ping failed — server unusable"; exit 1; } +echo "uda_cli help::help() OK" echo "==> [4/4] Running the UDA contract tier" export UDA_HOST=localhost UDA_PORT=56565 @@ -110,4 +122,25 @@ if [ "$#" -gt 0 ]; then exec "$@" fi cd "${BUILD_DIR}/tests/contract" -ctest -L uda --output-on-failure + +# Dedicated-CI mode (AL_UDA_DEDICATED_CI=1, set by uda-contract.yml): inside +# the reference-stack job, "server unreachable -> skip" is not a valid green. +# Parse CTest's machine-readable JUnit output and fail unless a nonzero number +# of UDA tests actually executed (issue #41). Ordinary developer builds keep +# the compile-gate/runtime-skip behavior: without the flag nothing changes. +if [ "${AL_UDA_DEDICATED_CI:-0}" = "1" ]; then + results="${BUILD_DIR}/uda-ctest-results.xml" + ctest -L uda --output-on-failure --output-junit "${results}" + total=$(sed -n 's/.*]*[[:space:]]tests="\([0-9]*\)".*/\1/p' "${results}" | head -1) + skipped=$(sed -n 's/.*]*[[:space:]]skipped="\([0-9]*\)".*/\1/p' "${results}" | head -1) + total=${total:-0} + skipped=${skipped:-0} + executed=$((total - skipped)) + echo "Dedicated-CI check: ${total} UDA tests selected, ${skipped} skipped, ${executed} executed." + if [ "${executed}" -le 0 ]; then + echo "FATAL: dedicated UDA tier executed zero tests (all skipped or none selected) — the reference stack verified nothing (issue #41)." + exit 1 + fi +else + ctest -L uda --output-on-failure +fi From 8c06ee4b5bdf30f1f7582d844ac6a118f760a3f5 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:47:15 +0200 Subject: [PATCH 04/17] Restore unit/integration CTest labels on supported CMake versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The multi-pass TEST_FILTER partition was broken at both ends of the supported CMake range: gtest_discover_tests' PROPERTIES forwarding collapses a multi-element LABELS list to its first token on CMake 3.28 (so ctest -L unit selected nothing), TEST_FILTER does not exist on the declared 3.21 floor (which fell back to a single all-'contract' pass), and even where multi-label forwarding worked (4.3.2) the unit and integration selections overlapped by two tests (ReadbackPlugins.BindReadbackPluginsAutoRegistersAndBindsFromStoredMetadata, UriOwnership.CallerFreesBuiltUri). Replace the four discovery passes with ONE pass (no TEST_FILTER, single-element LABELS — both fine on 3.21) plus post-discovery label assignment: assign_contract_labels.cmake, appended to the directory TEST_INCLUDE_FILES property, runs after the discovery include has registered every test, parses the generated registration file (handling the version-dependent add_test name quoting: bracket, quoted, bare) and classifies each test into exactly one of unit / integration / mdsplus / uda with a plain set_tests_properties list. The partition is disjoint by construction. Add tests/contract/check_label_partition.sh — fails on zero-selection, overlap, or a non-hermetic unit tier — and run it in the blocking functional workflow before the test run, since an empty 'ctest -L unit' exits 0 and is otherwise invisible. Verified on CMake 3.21.4 (Linux container), 3.28.4, and 4.3.2 (macOS): identical selections everywhere — 94 unit, 173 integration, 267 total; unit and integration disjoint; the two previously double-labeled tests now carry only 'unit'; unit tier and full suite pass (87/87, 247/247 executed). Closes #31 (cherry picked from commit c6c4d38316f982e863b464a47c20251e7220e69f) --- .github/workflows/contract-tests.yml | 5 + tests/contract/CMakeLists.txt | 100 ++++++------------ .../contract/assign_contract_labels.cmake.in | 82 ++++++++++++++ tests/contract/check_label_partition.sh | 63 +++++++++++ 4 files changed, 182 insertions(+), 68 deletions(-) create mode 100644 tests/contract/assign_contract_labels.cmake.in create mode 100755 tests/contract/check_label_partition.sh diff --git a/.github/workflows/contract-tests.yml b/.github/workflows/contract-tests.yml index e1c6de59..8b858679 100644 --- a/.github/workflows/contract-tests.yml +++ b/.github/workflows/contract-tests.yml @@ -39,6 +39,11 @@ jobs: - name: Build run: cmake --build build + # Issue #31: an empty `ctest -L unit` exits 0, so a broken label + # partition is invisible to the test run below — check it explicitly. + - name: Verify unit/integration label partition + run: bash tests/contract/check_label_partition.sh build/tests/contract + - name: Run the whole test suite working-directory: build run: ctest --output-on-failure diff --git a/tests/contract/CMakeLists.txt b/tests/contract/CMakeLists.txt index a9ab5415..b5efb969 100644 --- a/tests/contract/CMakeLists.txt +++ b/tests/contract/CMakeLists.txt @@ -198,74 +198,38 @@ endif() # the Memory backend — no on-disk backend), and the URI # builder ownership check (UriOwnership.*, pure string). # integration = everything else (HDF5 / ASCII / Flexbuffers on disk). -# Two disjoint discovery passes over the one binary; the TEST_FILTERs partition -# the cases so each registers exactly once. TEST_FILTER needs CMake >= 3.22; on -# the declared 3.21 floor we fall back to the single "contract" label. - -# The escaped ";" keeps LABELS a two-element list through gtest_discover_tests' -# re-emission (an unescaped ";" flattens to two malformed args). The integration -# TEST_FILTER uses gtest's one-"-"-then-colon-separated negative syntax -# ("*:-a:b:c", NOT "*:-a:-b:-c") so the two passes stay disjoint — otherwise the -# second pass re-registers the unit cases and CTest sees duplicates. -# MdsplusTracerBullet* is carved out of the integration pass into its own -# third pass (issue #13: new CTest label "mdsplus") so `ctest -L mdsplus` -# selects it without also pulling in every other on-disk backend case. -# Issue #14 joins MDSplus onto four more fixtures (DataEntryModes, -# CapabilityMatrix, AosMatrix, Occurrences) as a "Mdsplus"-named parameter/ -# case; "*/Mdsplus" and "Occurrences.Mdsplus*" carve those into the same -# mdsplus pass so `ctest -L mdsplus` covers the whole parity set, not just -# the tracer bullet. Issue #15 adds the real-DD-path datatype x rank breadth -# matrix (test_mdsplus_real_paths.cpp); "Mdsplus/MdsplusRealPathMatrix.*" -# carves its instances into the same mdsplus pass. Issue #16 adds the -# version-drift check (test_mdsplus_version_drift.cpp); "MdsplusVersionDrift*" -# carves its two cases into the same mdsplus pass. Issue #17 adds the -# MDSplus-unique surface (test_mdsplus_unique_surface.cpp): -# "MdsplusStructAwareSliceInterpolation*", "MdsplusTimerangeResampling*", -# "MdsplusSegmentBackedWrites*", "MdsplusSliceWriteKnownDefects*", -# "MdsplusSliceWriteKnownDefectsDeath*", and "MdsplusTimebaseCache*" carve its -# cases into the same mdsplus pass. -# -# Empirically discovered running this against a real build (issue #13, -# CMake 3.28 + Ninja): gtest_discover_tests' PROPERTIES forwarding does NOT -# actually preserve a multi-element LABELS list through its escaped-";" -# encoding -- it collapses to just the first token ("contract"), silently -# dropping "unit"/"integration" as bogus trailing property names with no -# value. That pre-existing defect (present in the two passes below -# unmodified by this issue) means `ctest -L unit`/`-L integration` currently -# select nothing; it is out of this issue's scope to fix. The new mdsplus -# pass sidesteps it by using a single-element LABELS list, which has no -# separator to lose. -if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.22") - gtest_discover_tests(contract_tests - TEST_FILTER "Introspection.*:*Memory*:KnownDefects*:Plugin*:ReadbackPlugins.*:UriOwnership.*" - PROPERTIES LABELS "contract\;unit") - gtest_discover_tests(contract_tests - TEST_FILTER "*:-Introspection.*:*Memory*:KnownDefects*:Plugin*:ReadbackPlugins.*:UriOwnership.*:MdsplusTracerBullet*:*/Mdsplus:Occurrences.Mdsplus*:Mdsplus/MdsplusRealPathMatrix.*:MdsplusVersionDrift*:MdsplusStructAwareSliceInterpolation*:MdsplusTimerangeResampling*:MdsplusSegmentBackedWrites*:MdsplusSliceWriteKnownDefects*:MdsplusSliceWriteKnownDefectsDeath*:MdsplusTimebaseCache*:UdaSmokeRoundTrip*:UdaEquilibriumSeedParity*:Uda/UdaRealPathMatrix.*:Uda/UdaRealPathMatrixKnownDefects.*:UdaBreadthTest*:UdaAosKnownDefects*:UdaSliceAndTimeRange*:UdaUniqueSurfaceTest*" - PROPERTIES LABELS "contract\;integration") - gtest_discover_tests(contract_tests - TEST_FILTER "MdsplusTracerBullet*:*/Mdsplus:Occurrences.Mdsplus*:Mdsplus/MdsplusRealPathMatrix.*:MdsplusVersionDrift*:MdsplusStructAwareSliceInterpolation*:MdsplusTimerangeResampling*:MdsplusSegmentBackedWrites*:MdsplusSliceWriteKnownDefects*:MdsplusSliceWriteKnownDefectsDeath*:MdsplusTimebaseCache*" - PROPERTIES LABELS "mdsplus") - # Issue #23: the UDA tier gets its own single-element "uda" label pass (same - # sidestep of the multi-label-drop defect the mdsplus pass uses), carved out - # of the integration pass above, so `ctest -L uda` selects exactly the UDA - # characterization suite without pulling in every other on-disk backend case. - # Issue #24 joins the read-only equilibrium-seed parity fixture - # (UdaEquilibriumSeedParity*) to the same pass. Issue #25 (parity breadth) - # joins the real-DD-path datatype x rank matrix (test_uda_real_paths.cpp, - # Uda/UdaRealPathMatrix.*) and the AOS/occurrences/list_filled_paths/pulse- - # modes/errors/slice/timerange breadth suite (test_uda_breadth.cpp, - # UdaBreadthTest.*, UdaSliceAndTimeRange.*). The exact COMPLEX r1/r3/r5 - # known-defect pairs live in Uda/UdaRealPathMatrixKnownDefects.*. - # Issue #26 (unique-surface, first half) joins UdaUniqueSurfaceTest* to the - # same pass: cache modes, runtime DD loading, datapath, and capability - # negotiation (test_uda_unique_surface.cpp). - gtest_discover_tests(contract_tests - TEST_FILTER "UdaSmokeRoundTrip*:UdaEquilibriumSeedParity*:Uda/UdaRealPathMatrix.*:Uda/UdaRealPathMatrixKnownDefects.*:UdaBreadthTest*:UdaAosKnownDefects*:UdaSliceAndTimeRange*:UdaUniqueSurfaceTest*" - PROPERTIES LABELS "uda") -else() - gtest_discover_tests(contract_tests - PROPERTIES LABELS "contract") -endif() +# One discovery pass over the one binary; assign_contract_labels.cmake then +# classifies every discovered test into exactly one tier (disjoint by +# construction — see the note at the registration block below). + +# Issue #31: labels are assigned POST-discovery, not through +# gtest_discover_tests. Two version-dependent defects ruled the old multi-pass +# TEST_FILTER partition out across the supported CMake range: +# - gtest_discover_tests' PROPERTIES forwarding does not preserve a +# multi-element LABELS list on every supported version (empirically +# CMake 3.28 collapses "contract\;unit" to just "contract", silently +# dropping the rest), so `ctest -L unit` selected nothing there; and +# - TEST_FILTER needs CMake >= 3.22, so the declared 3.21 floor fell back +# to a single all-"contract" pass with no unit/integration split at all. +# Instead there is ONE discovery pass (no TEST_FILTER, single-element LABELS — +# both fine on 3.21), and assign_contract_labels.cmake runs afterwards via the +# directory TEST_INCLUDE_FILES property: ctest processes it after the +# discovery include has registered every test, and it re-labels each test by +# name with a plain set_tests_properties list — a mechanism with no +# version-dependent forwarding in the middle. The tier definitions (which +# suites are unit / integration / mdsplus / uda, per issues #13-#17 and +# #23-#26) are documented and maintained in assign_contract_labels.cmake.in. +gtest_discover_tests(contract_tests + PROPERTIES LABELS "contract") + +# Bake the generated registration file's path into the labeler. A single +# discovery pass for contract_tests means GoogleTest writes +# "contract_tests[1]_tests.cmake". +set(_AL_CONTRACT_DISCOVERED_TESTS_FILE + "${CMAKE_CURRENT_BINARY_DIR}/contract_tests[1]_tests.cmake") +configure_file(assign_contract_labels.cmake.in assign_contract_labels.cmake @ONLY) +set_property(DIRECTORY APPEND PROPERTY TEST_INCLUDE_FILES + "${CMAKE_CURRENT_BINARY_DIR}/assign_contract_labels.cmake") ######################################################################################################################## # Install (mirrors the smoke-test targets). diff --git a/tests/contract/assign_contract_labels.cmake.in b/tests/contract/assign_contract_labels.cmake.in new file mode 100644 index 00000000..555aba38 --- /dev/null +++ b/tests/contract/assign_contract_labels.cmake.in @@ -0,0 +1,82 @@ +# assign_contract_labels.cmake — post-discovery CTest label assignment +# (issue #31). +# +# Processed by ctest via the directory TEST_INCLUDE_FILES property, AFTER the +# gtest_discover_tests include has registered every test. It re-reads the +# generated registration file, classifies each test name, and assigns LABELS +# with a plain set_tests_properties call — the one mechanism that carries a +# multi-element label list identically across the supported CMake range. +# gtest_discover_tests' own PROPERTIES forwarding collapses a multi-element +# LABELS list to its first token on some CMake versions (empirically 3.28), +# and TEST_FILTER (the old multi-pass partition) does not exist on the +# declared 3.21 floor at all; parsing + relabeling depends on neither. +# +# The classification below mirrors the tier definitions documented in +# tests/contract/CMakeLists.txt: +# unit — pure lookups, in-RAM Memory backend instances, plugin ABI +# death tests, plugin lifecycle suites, URI ownership. +# mdsplus — the compile-gated MDSplus tier (single "mdsplus" label). +# uda — the compile-gated UDA tier (single "uda" label). +# integration — everything else (HDF5 / ASCII / Flexbuffers on disk). +# The if/elseif chain makes the partition disjoint by construction: exactly +# one branch fires per test. +# +# This file parses exactly the single discovery pass's output; if another +# gtest_discover_tests call is ever added for contract_tests, add its +# generated "_tests.cmake" file here. + +set(_al_contract_tests_file "@_AL_CONTRACT_DISCOVERED_TESTS_FILE@") + +if(EXISTS "${_al_contract_tests_file}") + file(STRINGS "${_al_contract_tests_file}" _al_add_test_lines REGEX "^add_test") + foreach(_al_line IN LISTS _al_add_test_lines) + # The generated add_test name quoting is version-dependent: bracket + # arguments ("add_test([=[Name]=] ...", CMake >= ~3.22), double quotes for + # names with special characters on older versions, bare words otherwise + # ("add_test( Name ...", 3.21). + set(_al_name "") + if(_al_line MATCHES "^add_test\\( *\\[=+\\[([^]]+)\\]") + set(_al_name "${CMAKE_MATCH_1}") + elseif(_al_line MATCHES "^add_test\\( *\"([^\"]+)\"") + set(_al_name "${CMAKE_MATCH_1}") + elseif(_al_line MATCHES "^add_test\\( *([^ ]+) ") + set(_al_name "${CMAKE_MATCH_1}") + endif() + if(NOT _al_name STREQUAL "") + if(_al_name MATCHES "^Introspection\\." OR + _al_name MATCHES "Memory" OR + _al_name MATCHES "^KnownDefects" OR + _al_name MATCHES "^Plugin" OR + _al_name MATCHES "^ReadbackPlugins\\." OR + _al_name MATCHES "^UriOwnership\\.") + set_tests_properties("${_al_name}" PROPERTIES LABELS "contract;unit") + elseif(_al_name MATCHES "^MdsplusTracerBullet" OR + # "/Mdsplus" as the whole instance suffix: pretty names end + # "/Mdsplus" on newer CMake but "/Mdsplus#GetParam()=..." on 3.21. + _al_name MATCHES "/Mdsplus(#|$)" OR + _al_name MATCHES "^Occurrences\\.Mdsplus" OR + _al_name MATCHES "^Mdsplus/MdsplusRealPathMatrix\\." OR + _al_name MATCHES "^MdsplusVersionDrift" OR + _al_name MATCHES "^MdsplusStructAwareSliceInterpolation" OR + _al_name MATCHES "^MdsplusTimerangeResampling" OR + _al_name MATCHES "^MdsplusSegmentBackedWrites" OR + _al_name MATCHES "^MdsplusSliceWriteKnownDefects" OR + _al_name MATCHES "^MdsplusTimebaseCache") + set_tests_properties("${_al_name}" PROPERTIES LABELS "mdsplus") + elseif(_al_name MATCHES "^UdaSmokeRoundTrip" OR + _al_name MATCHES "^UdaEquilibriumSeedParity" OR + _al_name MATCHES "^Uda/UdaRealPathMatrix\\." OR + _al_name MATCHES "^Uda/UdaRealPathMatrixKnownDefects\\." OR + _al_name MATCHES "^UdaBreadthTest" OR + _al_name MATCHES "^UdaAosKnownDefects" OR + _al_name MATCHES "^UdaSliceAndTimeRange" OR + _al_name MATCHES "^UdaUniqueSurfaceTest") + set_tests_properties("${_al_name}" PROPERTIES LABELS "uda") + else() + set_tests_properties("${_al_name}" PROPERTIES LABELS "contract;integration") + endif() + endif() + endforeach() + unset(_al_add_test_lines) +endif() +unset(_al_contract_tests_file) diff --git a/tests/contract/check_label_partition.sh b/tests/contract/check_label_partition.sh new file mode 100755 index 00000000..d1fe3bb9 --- /dev/null +++ b/tests/contract/check_label_partition.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash +# check_label_partition.sh — CI smoke check for the contract-suite CTest +# labels (issue #31). +# +# An empty `ctest -L unit` exits 0, so a label regression (version-dependent +# property forwarding, filter drift, a renamed suite) is silent unless +# something checks the selection itself. This verifies, machine-readably: +# 1. `ctest -N -L unit` and `ctest -N -L integration` both select a nonzero +# number of tests; +# 2. the two selections are disjoint (no test carries both labels); +# 3. the unit selection is hermetic — no filesystem-backed backend case. +# +# Usage: check_label_partition.sh +# e.g. check_label_partition.sh build/tests/contract +set -euo pipefail + +build_dir=${1:?usage: check_label_partition.sh } + +list_names() { + # `ctest -N` right-aligns test numbers ("Test #9:" vs "Test #99:"), so the + # whitespace before '#' is variable; strip the "(Disabled)" suffix too. + ctest --test-dir "${build_dir}" -N -L "$1" \ + | sed -n 's/^ *Test *#[0-9]*: //p' | sed 's/ (Disabled)$//' +} + +unit=$(list_names '^unit$') +integration=$(list_names '^integration$') +unit_count=$(printf '%s' "${unit}" | grep -c . || true) +integration_count=$(printf '%s' "${integration}" | grep -c . || true) +echo "unit: ${unit_count} tests, integration: ${integration_count} tests" + +fail=0 +if [ "${unit_count}" -eq 0 ]; then + echo "FAIL: 'ctest -L unit' selects zero tests — the fast hermetic tier is empty (issue #31)." + fail=1 +fi +if [ "${integration_count}" -eq 0 ]; then + echo "FAIL: 'ctest -L integration' selects zero tests — the on-disk tier is empty (issue #31)." + fail=1 +fi + +overlap=$(comm -12 <(printf '%s\n' "${unit}" | sort -u) \ + <(printf '%s\n' "${integration}" | sort -u) | grep . || true) +if [ -n "${overlap}" ]; then + echo "FAIL: tests carry both 'unit' and 'integration' labels:" + printf '%s\n' "${overlap}" + fail=1 +fi + +# -i: gtest pretty names spell backends differently per CMake version +# ("(HDF5, ..." on new CMake, "#GetParam()=ASCII" on 3.21, "Hdf5..." in +# suite names). +nonhermetic=$(printf '%s\n' "${unit}" | grep -iE 'hdf5|ascii|flexbuffers|mdsplus|uda' || true) +if [ -n "${nonhermetic}" ]; then + echo "FAIL: the unit selection contains filesystem-/server-backed cases:" + printf '%s\n' "${nonhermetic}" + fail=1 +fi + +if [ "${fail}" -ne 0 ]; then + exit 1 +fi +echo "OK: unit/integration labels are nonempty, disjoint, and the unit tier is hermetic." From eb6aebe61ea4d656bcf86233e16dde3f5777606a Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:56:43 +0200 Subject: [PATCH 05/17] Cover unknown backend IDs at the C-ABI factory boundary TRACEABILITY Part 2 marked initBackend's unrecognized-ID throw path covered only 'by inspection'. Add test_backend_factory.cpp (BackendFactory.*, hermetic unit tier) pinning the refusal through the public surface: - an unknown backend name in an otherwise valid data-entry URI returns UNKNOWN_ERR from al_begin_dataentry_action across all four open modes, with the stable 'Unable to identify a backend' diagnostic, no context handed out (sentinel untouched) and no filesystem artifact; - an empty backend name is likewise refused; - an unknown positive numeric ID via al_build_uri_from_legacy_parameters returns LOWLEVEL_ERR and synthesizes no URI. A regression that defaults an unrecognized identifier to a real backend makes these calls succeed and the tests fail. Verified clean under ASan/UBSan (macOS) and ASan+LSan (Linux container, detect_leaks=1). TRACEABILITY Part 2 row A now points at the active tests instead of the inspection-only residual. Closes #47 (cherry picked from commit 21c5570c9a794a1d66fd46749acee24ceb5d020c) --- tests/contract/CMakeLists.txt | 1 + tests/contract/TRACEABILITY.md | 2 +- .../contract/assign_contract_labels.cmake.in | 5 +- tests/contract/test_backend_factory.cpp | 104 ++++++++++++++++++ 4 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 tests/contract/test_backend_factory.cpp diff --git a/tests/contract/CMakeLists.txt b/tests/contract/CMakeLists.txt index b5efb969..4b50277e 100644 --- a/tests/contract/CMakeLists.txt +++ b/tests/contract/CMakeLists.txt @@ -68,6 +68,7 @@ add_executable(contract_tests test_capability_gated.cpp test_known_defects.cpp test_plugins.cpp + test_backend_factory.cpp test_structured_data.cpp test_pulse_lifecycle.cpp test_mdsplus.cpp diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 0a8c076e..8ffa2bd4 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -218,7 +218,7 @@ and points at the same test. | Cluster / Capability | Inventory ref | Unit | Integ. | Test(s) — and residual | Status | |---|---|:--:|:--:|---|---| -| **A** `initBackend` factory selects the backend from the URI | :496-514 | ✓ (Memory) | ✓ (HDF5, ASCII) | every always-on backend instantiated via `build_uri` in `RoundTripMatrix`. Residual: the unrecognized-ID throw path (`al_backend.cpp:82-86`) is straightforward by inspection (a plain `else` throwing `ALBackendException`) but untested — low-value relative to the rest of this row, left as a residual rather than a separate row. MDSplus is instantiated too, via its own URI in every Part 4 fixture — see Part 4 | covered (indirect) | +| **A** `initBackend` factory selects the backend from the URI | :496-514 | ✓ (Memory) | ✓ (HDF5, ASCII) | every always-on backend instantiated via `build_uri` in `RoundTripMatrix`. The unrecognized-backend refusal has an active verdict (issue #47): `BackendFactory.*` (test_backend_factory.cpp) pins the exact C-ABI status for an unknown backend name in a URI (`UNKNOWN_ERR`, all four open modes, no context handed out, no filesystem artifact) and for an unknown positive legacy numeric ID via `al_build_uri_from_legacy_parameters` (`LOWLEVEL_ERR`, no URI synthesized). MDSplus is instantiated too, via its own URI in every Part 4 fixture — see Part 4 | covered | | **B** `getVersion` (installed vs stored, drift check) | :520-540 | — | — | Reachable in principle: `al_begin_dataentry_action`/`al_plugin_begin_global_action`/`al_plugin_begin_slice_action` all compare `backend->getVersion(NULL)` (compiled) against `backend->getVersion(pctx)` (stored) on open/write and throw `LOWLEVEL_ERR` on a mismatch (src/al_lowlevel.cpp:868-883,956-967,1011-1022). Forcing the mismatch needs writing a pulse then overwriting its on-disk HDF5 backend-version attribute directly via the HDF5 C API — out of this issue's scope (no HDF5 include/link wiring in the test target). **Terminal gap**, not deferred to a future step: reachable via the C ABI, but pinning it needs infrastructure this suite does not have. **MDSplus: this gap is closed** — see Part 4's version-drift check (issue #16), which forces the mismatch via the compile-guarded tier's own raw MDSplus C++ API dependency. This row (HDF5) is unchanged — the two backends hit the identical shape of problem, but only MDSplus's already-separate, already-gated tier made the raw-API workaround low-blast-radius | gap | | **B** `openPulse` / `closePulse` | :542-552 | ✓ (Memory) | ✓ (HDF5, ASCII) | via `al_begin_dataentry_action` / `al_close_pulse` in all suites | covered (indirect) | | **C** `beginAction` / `endAction` | :558-574 | ✓ (Memory) | ✓ (HDF5, ASCII) | via `al_begin_global_action` / `al_end_action` | covered (indirect) | diff --git a/tests/contract/assign_contract_labels.cmake.in b/tests/contract/assign_contract_labels.cmake.in index 555aba38..560b885f 100644 --- a/tests/contract/assign_contract_labels.cmake.in +++ b/tests/contract/assign_contract_labels.cmake.in @@ -48,7 +48,10 @@ if(EXISTS "${_al_contract_tests_file}") _al_name MATCHES "^KnownDefects" OR _al_name MATCHES "^Plugin" OR _al_name MATCHES "^ReadbackPlugins\\." OR - _al_name MATCHES "^UriOwnership\\.") + _al_name MATCHES "^UriOwnership\\." OR + # issue #47: unknown-backend refusals at the factory boundary — no + # backend substrate is ever constructed, so the tier is hermetic + _al_name MATCHES "^BackendFactory\\.") set_tests_properties("${_al_name}" PROPERTIES LABELS "contract;unit") elseif(_al_name MATCHES "^MdsplusTracerBullet" OR # "/Mdsplus" as the whole instance suffix: pretty names end diff --git a/tests/contract/test_backend_factory.cpp b/tests/contract/test_backend_factory.cpp new file mode 100644 index 00000000..5be97b25 --- /dev/null +++ b/tests/contract/test_backend_factory.cpp @@ -0,0 +1,104 @@ +// Unknown-backend-ID coverage at the C-ABI factory boundary (issue #47). +// +// TRACEABILITY.md Part 2 marked Backend::initBackend's unrecognized-ID throw +// path (src/al_backend.cpp) covered only "by inspection". These tests give the +// refusal a real verdict through the public surface, at the two spots an +// unknown backend identifier can arrive from a caller: +// +// - al_begin_dataentry_action with a URI naming an unknown (or empty) +// backend: DataEntryContext::setBackendID (src/al_context.cpp) is the +// factory-boundary gate that refuses it before initBackend runs. A +// regression that "defaults" an unrecognized name to a real backend makes +// the calls below succeed and the tests fail. +// - al_build_uri_from_legacy_parameters with an unknown positive numeric +// backend ID: DataEntryContext::getURIBackend refuses to synthesize a URI. +// +// Everything asserted here is hermetic (no backend substrate is ever +// constructed), so the suite name is classified into the `unit` tier by +// assign_contract_labels.cmake.in. + +#include "al_contract.h" + +#include +#include + +#include + +#include +#include + +namespace { + +constexpr int kUntouchedCtx = -424242; // sentinel: must survive failed calls + +bool dir_is_empty(const std::string& p) { + namespace fs = std::filesystem; + return fs::exists(p) && fs::is_directory(p) && + fs::begin(fs::directory_iterator(p)) == fs::end(fs::directory_iterator(p)); +} + +// =========================================================================== +// URI surface: unknown / empty backend names. +// =========================================================================== + +TEST(BackendFactory, UnknownBackendNameInUriReturnsCleanError) { + al_contract::TempBase tmp; + int ctx = kUntouchedCtx; + const std::string uri = "imas:nosuchbackend?path=" + tmp.str() + "/pulse"; + + al_status_t s = al_begin_dataentry_action(uri.c_str(), + alconst::force_create_pulse, &ctx); + + EXPECT_EQ(s.code, alerror::unknown_err) << "message: " << s.message; + EXPECT_NE(std::string(s.message).find("Unable to identify a backend"), + std::string::npos) + << "actual message: " << s.message; + // No context may be handed out and no artifact left on disk. + EXPECT_EQ(ctx, kUntouchedCtx); + EXPECT_TRUE(dir_is_empty(tmp.str())); +} + +TEST(BackendFactory, EmptyBackendNameInUriReturnsCleanError) { + al_contract::TempBase tmp; + int ctx = kUntouchedCtx; + const std::string uri = "imas:?path=" + tmp.str() + "/pulse"; + + al_status_t s = al_begin_dataentry_action(uri.c_str(), + alconst::force_create_pulse, &ctx); + + EXPECT_NE(s.code, 0); + EXPECT_EQ(ctx, kUntouchedCtx); + EXPECT_TRUE(dir_is_empty(tmp.str())); +} + +TEST(BackendFactory, UnknownBackendNameIsRefusedForEveryOpenMode) { + al_contract::TempBase tmp; + const int modes[] = {alconst::open_pulse, alconst::force_open_pulse, + alconst::create_pulse, alconst::force_create_pulse}; + const std::string uri = "imas:nosuchbackend?path=" + tmp.str() + "/pulse"; + for (int mode : modes) { + int ctx = kUntouchedCtx; + al_status_t s = al_begin_dataentry_action(uri.c_str(), mode, &ctx); + EXPECT_EQ(s.code, alerror::unknown_err) << "mode=" << mode; + EXPECT_EQ(ctx, kUntouchedCtx) << "mode=" << mode; + } + EXPECT_TRUE(dir_is_empty(tmp.str())); +} + +// =========================================================================== +// Legacy surface: unknown positive numeric backend ID. +// =========================================================================== + +TEST(BackendFactory, UnknownPositiveLegacyBackendIdIsRefused) { + std::string uri; + al_status_t s = al_build_uri_from_legacy_parameters( + /*backendID=*/424242, /*pulse=*/1, /*run=*/1, "user", "test", "3", + /*options=*/"", uri); + + EXPECT_EQ(s.code, alerror::lowlevel_err) << "message: " << s.message; + EXPECT_NE(std::string(s.message).find("getURIBackend"), std::string::npos) + << "actual message: " << s.message; + EXPECT_TRUE(uri.empty()) << "no URI may be synthesized, got: " << uri; +} + +} // namespace From af3051633f6479b5f12b05e421d8a54c7d241d8e Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:03:11 +0200 Subject: [PATCH 06/17] Make the dlopen-failure pin safe in assert-enabled builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PluginTest.RegisterUnloadableSharedLibSwallowsAssert_CurrentBehavior called al_register_plugin in-process; in a Debug build the product assertion in registerPlugin fires on the failed dlopen and aborted the whole test runner (exit 134) instead of producing a GoogleTest verdict. Split the current-behavior pin by build mode (issue #32): - assert-enabled builds: a death test (RegisterUnloadableSharedLibAbortsOnAssert_CurrentBehavior) pins SIGABRT + the assertion text in a child process. The fixture directory is created by the child but removed by the PARENT; since threadsafe-style death tests re-execute the whole test in the child, the parent's path is handed down via an env var (overwrite=0 so the re-executed child keeps the inherited value) — verified no temp directory is left behind on macOS and Linux. - NDEBUG builds: the existing swallowed-assert tripwire is kept unchanged under #ifdef NDEBUG. - The DISABLED_ correct-contract test (nonzero status attributing the actual dlopen error) stays build-mode independent; fixing registerPlugin makes the active pin of that mode fail and the correct-contract test ready to enable. Run both build modes in CI: the blocking functional workflow now builds a {RelWithDebInfo, Debug} matrix, so each branch of the pin executes on every PR. Verified: full contract suite passes 251/251 in Debug and Release on macOS, and in Debug on Linux (container); the previously-crashing gtest_filter repro now returns [PASSED] in Debug. Closes #32 (cherry picked from commit 240330d2bae7550870ca7618f85bd1676649880c) --- .github/workflows/contract-tests.yml | 12 ++++- tests/contract/test_plugins.cpp | 73 +++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/.github/workflows/contract-tests.yml b/.github/workflows/contract-tests.yml index 8b858679..1787dfdd 100644 --- a/.github/workflows/contract-tests.yml +++ b/.github/workflows/contract-tests.yml @@ -15,8 +15,16 @@ on: jobs: contract-functional: - name: Contract tests (Linux, blocking) + name: Contract tests (Linux, ${{ matrix.build_type }}, blocking) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # Both build modes (issue #32): the dlopen-failure plugin pin is + # build-mode dependent — assert-abort death test in Debug, swallowed + # assert tripwire under NDEBUG (RelWithDebInfo) — so each branch needs + # its own leg or one of them never runs anywhere. + build_type: [RelWithDebInfo, Debug] steps: - name: Checkout code @@ -34,7 +42,7 @@ jobs: cmake -S . -B build -G Ninja -DAL_BUILD_TESTS=ON -DAL_PYTHON_BINDINGS=OFF - -DCMAKE_BUILD_TYPE=RelWithDebInfo + -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} - name: Build run: cmake --build build diff --git a/tests/contract/test_plugins.cpp b/tests/contract/test_plugins.cpp index 205f9108..4d32795c 100644 --- a/tests/contract/test_plugins.cpp +++ b/tests/contract/test_plugins.cpp @@ -30,6 +30,7 @@ #include +#include #include #include #include @@ -351,15 +352,22 @@ TEST_F(PluginTest, UnregisterNeverBoundPluginLeavesItRegistered_CurrentBehavior) // =========================================================================== // DEFECT (D2): dlopen failure in registerPlugin is guarded only by an assert. // FUNCTIONALITY_INVENTORY.md:355-360, src/al_lowlevel.cpp:350-357: a failed -// dlopen() is printf'd and asserted; under -DNDEBUG (this build: -// RelWithDebInfo/Release) the assert is stripped, so the null handle flows on. +// dlopen() is printf'd and asserted, so current behavior is BUILD-MODE +// DEPENDENT and the pins below are too (issue #32): // -// Empirically characterized on this platform: the null handle reaches dlsym, -// whose own null-check throws — so registerPlugin returns LOWLEVEL_ERR with the -// *misleading* "Cannot load symbol create" message rather than reporting the -// dlopen failure, and does NOT crash. (In an assert-enabled build the stripped -// assert would instead fire -> SIGABRT; that death only exists there, which is -// why this pair is a plain xfail, not a death test.) +// - assert-enabled builds (Debug, no NDEBUG): the assert fires -> SIGABRT. +// Pinned with a death test so the abort happens in a child process and +// the suite itself survives. +// - NDEBUG builds (Release/RelWithDebInfo): the assert is stripped and the +// null handle flows on to dlsym, whose own null-check throws — so +// registerPlugin returns LOWLEVEL_ERR with the *misleading* "Cannot load +// symbol create" message rather than reporting the dlopen failure, and +// does NOT crash. +// +// Either way the correct contract is the same (a nonzero status attributing +// the actual dlopen error) — the DISABLED_ test below stays build-mode +// independent and becomes enableable once registerPlugin handles the failed +// dlopen instead of asserting on it. // =========================================================================== // Points IMAS_AL_PLUGINS at a temp dir holding a garbage _plugin.so that @@ -403,11 +411,12 @@ TEST_F(PluginTest, DISABLED_RegisterUnloadableSharedLibReportsDlopenFailure) { fs::remove_all(dir); } -// CURRENT-BEHAVIOR tripwire: pins that the returned status does NOT mention the -// dlopen failure — the NDEBUG-stripped assert lets the null handle flow past -// dlopen, so the failure is swallowed and only a downstream symptom is reported. -// If the status starts naming dlopen, the handling was fixed — enable the -// DISABLED_ test above. +#ifdef NDEBUG +// CURRENT-BEHAVIOR tripwire (NDEBUG builds): pins that the returned status does +// NOT mention the dlopen failure — the NDEBUG-stripped assert lets the null +// handle flow past dlopen, so the failure is swallowed and only a downstream +// symptom is reported. If the status starts naming dlopen, the handling was +// fixed — enable the DISABLED_ test above. TEST_F(PluginTest, RegisterUnloadableSharedLibSwallowsAssert_CurrentBehavior) { namespace fs = std::filesystem; const fs::path dir = fs::temp_directory_path() / @@ -423,6 +432,44 @@ TEST_F(PluginTest, RegisterUnloadableSharedLibSwallowsAssert_CurrentBehavior) { "PluginTest.RegisterUnloadableSharedLibReportsDlopenFailure."; fs::remove_all(dir); } +#else +// CURRENT-BEHAVIOR tripwire (assert-enabled builds): the product assertion at +// src/al_lowlevel.cpp fires on the failed dlopen and aborts. Run the call in a +// death-test child so the runner survives; pin SIGABRT + the assertion text +// specifically, so an unrelated crash is not mistaken for this defect. The +// fixture artifacts are created and removed by the PARENT — the child dying +// must not leak the temp dir (issue #32). +TEST_F(PluginTest, RegisterUnloadableSharedLibAbortsOnAssert_CurrentBehavior) { + GTEST_FLAG_SET(death_test_style, "threadsafe"); + namespace fs = std::filesystem; + const fs::path dir = fs::temp_directory_path() / + ("al_contract_badplugin_" + + std::to_string(::getpid()) + "_b"); + // threadsafe-style death tests RE-EXECUTE the whole test in the child, + // which would recompute `dir` with the child's own pid and leave that + // directory behind; hand the parent's path down via the environment so + // parent and child agree on the one directory the parent removes below. + // overwrite=0: the re-executed child runs this line too and must NOT + // replace the inherited parent value with its own pid-derived path. + setenv("AL_CONTRACT_BADPLUGIN_DIR", dir.string().c_str(), /*overwrite=*/0); + const std::filesystem::path used_dir = + std::getenv("AL_CONTRACT_BADPLUGIN_DIR"); + ASSERT_EXIT( + { + const char* d = std::getenv("AL_CONTRACT_BADPLUGIN_DIR"); + register_unloadable(d != nullptr ? fs::path(d) : dir, + "unloadable_b"); + }, + ::testing::KilledBySignal(SIGABRT), "[Aa]ssert") + << "expected the registerPlugin dlopen assert to abort in an " + "assert-enabled build (src/al_lowlevel.cpp, " + "FUNCTIONALITY_INVENTORY.md:355-360). If this no longer dies, the " + "handling was fixed — enable " + "PluginTest.RegisterUnloadableSharedLibReportsDlopenFailure."; + unsetenv("AL_CONTRACT_BADPLUGIN_DIR"); + fs::remove_all(used_dir); +} +#endif // =========================================================================== // al_plugin_* low-level reentry (FUNCTIONALITY_INVENTORY.md:854-895, Part 3 From 0a085abb1a5c00b1e2705f8427be4f5fc4c0149f Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:10:02 +0200 Subject: [PATCH 07/17] Parse CTest's real JUnit shape in the dedicated UDA check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first dedicated-CI run proved the zero-executed guard works end to end — and that it caught its own parsing defect: CTest writes the attributes one per line, so the single-line sed extraction saw tests=0/skipped=0 and (correctly) failed the tier even though all 71 UDA tests had passed. Flatten the file and extract tests=/skipped= from the suite header only. Verified against a real 'ctest --output-junit' file (98 tests, 1 skipped -> 97 executed). Part of issue #41. (cherry picked from commit 8322cfe645e279ae891295ad8af2e8d97bdb969e) --- docker/uda/run.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docker/uda/run.sh b/docker/uda/run.sh index c9f1fc47..0953e259 100644 --- a/docker/uda/run.sh +++ b/docker/uda/run.sh @@ -131,8 +131,12 @@ cd "${BUILD_DIR}/tests/contract" if [ "${AL_UDA_DEDICATED_CI:-0}" = "1" ]; then results="${BUILD_DIR}/uda-ctest-results.xml" ctest -L uda --output-on-failure --output-junit "${results}" - total=$(sed -n 's/.*]*[[:space:]]tests="\([0-9]*\)".*/\1/p' "${results}" | head -1) - skipped=$(sed -n 's/.*]*[[:space:]]skipped="\([0-9]*\)".*/\1/p' "${results}" | head -1) + # CTest writes the attributes one per line — flatten the file + # before extracting them, and take only the suite header (the + # elements carry no tests=/skipped= attributes). + suite_header=$(tr '\n\t' ' ' < "${results}" | grep -o ']*>' | head -1) + total=$(printf '%s' "${suite_header}" | grep -o 'tests="[0-9]*"' | head -1 | grep -o '[0-9]*') + skipped=$(printf '%s' "${suite_header}" | grep -o 'skipped="[0-9]*"' | head -1 | grep -o '[0-9]*') total=${total:-0} skipped=${skipped:-0} executed=$((total - skipped)) From 840dfe2f464fc0dc0122319b2fc13fcebe99c58a Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:24:00 +0200 Subject: [PATCH 08/17] Add a stored-backend-version drift fixture for HDF5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TRACEABILITY Part 2 row B declined to test the always-on HDF5 stored-version drift check because forcing a mismatch needs out-of-band HDF5 attribute surgery. Add that infrastructure per issue #36's producer/observer split: - al-hdf5-fixture-tool: a small standalone target (the only place the HDF5 C API appears) that rewrites master.h5's HDF5_BACKEND_VERSION attribute; contract_tests only learns its path. - Hdf5VersionDrift.* (integration tier, default HDF5-enabled suite, so it runs on every PR): matching stored version opens; stored 999.0 (major) and 1.999 (the minor comparison boundary) are refused with an exact status/message assertion; contract_tests stays public-C-ABI-only. Empirical characterization the pins record: for HDF5 the generic LOWLEVEL_ERR drift comparison in al_lowlevel.cpp is SHADOWED — HDF5BackendFactory accepts exactly "1.0" and openPulse throws BACKEND_ERR ('No backend writer with version: ') for any other stored value before the generic check runs, so every non-compiled stored version is refused (stricter than the generic rule). Part 2 row B goes from 'gap' to 'covered' and documents the shadowing. Verified locally: 3/3 new tests pass; full suite 254/254; label partition intact (98 unit / 176 integration). Closes #36 (cherry picked from commit 5dbc56636a55d1f5cd25ee4d94b72d91b27bfc75) --- tests/contract/CMakeLists.txt | 48 +++++++-- tests/contract/TRACEABILITY.md | 2 +- tests/contract/hdf5_fixture_tool.cpp | 71 +++++++++++++ tests/contract/test_hdf5_version_drift.cpp | 118 +++++++++++++++++++++ 4 files changed, 228 insertions(+), 11 deletions(-) create mode 100644 tests/contract/hdf5_fixture_tool.cpp create mode 100644 tests/contract/test_hdf5_version_drift.cpp diff --git a/tests/contract/CMakeLists.txt b/tests/contract/CMakeLists.txt index 4b50277e..58db1915 100644 --- a/tests/contract/CMakeLists.txt +++ b/tests/contract/CMakeLists.txt @@ -69,6 +69,7 @@ add_executable(contract_tests test_known_defects.cpp test_plugins.cpp test_backend_factory.cpp + test_hdf5_version_drift.cpp test_structured_data.cpp test_pulse_lifecycle.cpp test_mdsplus.cpp @@ -85,6 +86,27 @@ target_link_libraries(contract_tests PRIVATE al GTest::gtest GTest::gtest_main) add_dependencies(contract_tests al) target_compile_features(contract_tests PRIVATE cxx_std_17) +######################################################################################################################## +# HDF5 drift-fixture producer (issue #36). +# +# The stored-backend-version drift check is asserted through the public C ABI +# (test_hdf5_version_drift.cpp), but preparing a MISMATCHED pulse requires +# rewriting master.h5's HDF5_BACKEND_VERSION attribute — out-of-band HDF5 C +# API surgery that must NOT leak into contract_tests. It lives in a separate +# tool the tests invoke as a subprocess; contract_tests only learns the tool's +# path (and gates the test file's compilation on it). +if(AL_BACKEND_HDF5) + find_package(HDF5 COMPONENTS C REQUIRED) + add_executable(al-hdf5-fixture-tool hdf5_fixture_tool.cpp) + target_include_directories(al-hdf5-fixture-tool PRIVATE ${HDF5_C_INCLUDE_DIRS}) + target_link_libraries(al-hdf5-fixture-tool PRIVATE ${HDF5_C_LIBRARIES}) + target_compile_definitions(al-hdf5-fixture-tool PRIVATE ${HDF5_C_DEFINITIONS}) + target_compile_features(al-hdf5-fixture-tool PRIVATE cxx_std_17) + add_dependencies(contract_tests al-hdf5-fixture-tool) + target_compile_definitions(contract_tests PRIVATE + AL_CONTRACT_HDF5_FIXTURE_TOOL="$") +endif() + ######################################################################################################################## # MDSplus build gate (issue #13 / TEST_STRATEGY.md D4 "compile-guarded tier"). # @@ -97,17 +119,23 @@ target_compile_features(contract_tests PRIVATE cxx_std_17) if(AL_BACKEND_MDSPLUS) target_compile_definitions(contract_tests PRIVATE AL_CONTRACT_HAVE_MDSPLUS) - # test_mdsplus_version_drift.cpp (issue #16) forces the version-mismatch - # case by writing directly into the pulse's VERSION:BACK_MAJOR tree node - # through the raw MDSplus C++ API () -- something reachable - # only outside the public C ABI. `al`'s own src/mdsplus/CMakeLists.txt - # already links MDSplus, but PRIVATEly, so it doesn't propagate here; - # find_package(MDSplus) is repeated (idempotent) to wire the same - # include dir / libs into contract_tests, test-only and build-gated behind - # the same AL_BACKEND_MDSPLUS option as the backend itself. + # MDSplus drift-fixture producer (issue #35). test_mdsplus_version_drift.cpp + # (issue #16) forces the version-mismatch case by overwriting the pulse's + # VERSION:BACK_MAJOR tree node — reachable only through the raw MDSplus C++ + # API (), i.e. outside the public C ABI that is the contract + # suite's one seam. That mutation is fixture PREPARATION, so it lives in a + # separate build-gated tool the test invokes as a subprocess; contract_tests + # itself has no MDSplus include or link dependency, and a rewrite satisfying + # the C ABI is judged by exactly the C ABI. find_package(MDSplus) is + # repeated (idempotent) for the tool only. find_package(MDSplus REQUIRED) - target_include_directories(contract_tests PRIVATE ${MDSPLUS_INCLUDES}) - target_link_libraries(contract_tests PRIVATE ${MDSPLUS_LIBRARIES}) + add_executable(al-mdsplus-fixture-tool mdsplus_fixture_tool.cpp) + target_include_directories(al-mdsplus-fixture-tool PRIVATE ${MDSPLUS_INCLUDES}) + target_link_libraries(al-mdsplus-fixture-tool PRIVATE ${MDSPLUS_LIBRARIES}) + target_compile_features(al-mdsplus-fixture-tool PRIVATE cxx_std_17) + add_dependencies(contract_tests al-mdsplus-fixture-tool) + target_compile_definitions(contract_tests PRIVATE + AL_CONTRACT_MDSPLUS_FIXTURE_TOOL="$") endif() ######################################################################################################################## diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 8ffa2bd4..87d4a3aa 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -219,7 +219,7 @@ and points at the same test. | Cluster / Capability | Inventory ref | Unit | Integ. | Test(s) — and residual | Status | |---|---|:--:|:--:|---|---| | **A** `initBackend` factory selects the backend from the URI | :496-514 | ✓ (Memory) | ✓ (HDF5, ASCII) | every always-on backend instantiated via `build_uri` in `RoundTripMatrix`. The unrecognized-backend refusal has an active verdict (issue #47): `BackendFactory.*` (test_backend_factory.cpp) pins the exact C-ABI status for an unknown backend name in a URI (`UNKNOWN_ERR`, all four open modes, no context handed out, no filesystem artifact) and for an unknown positive legacy numeric ID via `al_build_uri_from_legacy_parameters` (`LOWLEVEL_ERR`, no URI synthesized). MDSplus is instantiated too, via its own URI in every Part 4 fixture — see Part 4 | covered | -| **B** `getVersion` (installed vs stored, drift check) | :520-540 | — | — | Reachable in principle: `al_begin_dataentry_action`/`al_plugin_begin_global_action`/`al_plugin_begin_slice_action` all compare `backend->getVersion(NULL)` (compiled) against `backend->getVersion(pctx)` (stored) on open/write and throw `LOWLEVEL_ERR` on a mismatch (src/al_lowlevel.cpp:868-883,956-967,1011-1022). Forcing the mismatch needs writing a pulse then overwriting its on-disk HDF5 backend-version attribute directly via the HDF5 C API — out of this issue's scope (no HDF5 include/link wiring in the test target). **Terminal gap**, not deferred to a future step: reachable via the C ABI, but pinning it needs infrastructure this suite does not have. **MDSplus: this gap is closed** — see Part 4's version-drift check (issue #16), which forces the mismatch via the compile-guarded tier's own raw MDSplus C++ API dependency. This row (HDF5) is unchanged — the two backends hit the identical shape of problem, but only MDSplus's already-separate, already-gated tier made the raw-API workaround low-blast-radius | gap | +| **B** `getVersion` (installed vs stored, drift check) | :520-540 | — | ✓ (HDF5) | Tested (issue #36): `Hdf5VersionDrift.*` (test_hdf5_version_drift.cpp) — matching stored version opens; stored `999.0` (major) and `1.999` (minor boundary) are refused with the exact observed status. The mismatched fixture is produced out-of-band by the isolated `hdf5_fixture_tool` (rewrites master.h5's `HDF5_BACKEND_VERSION`); `contract_tests` itself stays HDF5-API-free and asserts only through the C ABI. Empirical nuance the pins record: for HDF5 the generic `LOWLEVEL_ERR` comparison (src/al_lowlevel.cpp) is **shadowed** — `HDF5BackendFactory` accepts exactly "1.0" and `openPulse` throws `BACKEND_ERR` ("No backend writer with version: …") for any other stored value, before the generic check runs; every non-compiled stored version is thereby refused (stricter than the generic rule). **MDSplus: also closed** — see Part 4's version-drift check (issue #16, reworked by #35 to keep the raw-MDSplus mutation in a fixture tool) | covered | | **B** `openPulse` / `closePulse` | :542-552 | ✓ (Memory) | ✓ (HDF5, ASCII) | via `al_begin_dataentry_action` / `al_close_pulse` in all suites | covered (indirect) | | **C** `beginAction` / `endAction` | :558-574 | ✓ (Memory) | ✓ (HDF5, ASCII) | via `al_begin_global_action` / `al_end_action` | covered (indirect) | | **C** `writeData` / `readData` (0=not-found vs 1=success inner convention) | :576-589 | ✓ (Memory) | ✓ (HDF5, ASCII) | via `al_write_data` / `al_read_data` — `RoundTripMatrix` (+ the CHAR-scalar / ASCII-maxdim xfails above). MDSplus: see Part 4's real-DD-path breadth matrix (issue #15) instead of `RoundTripMatrix`. UDA: same — see Part 5's real-DD-path breadth matrix instead of `RoundTripMatrix` | covered | diff --git a/tests/contract/hdf5_fixture_tool.cpp b/tests/contract/hdf5_fixture_tool.cpp new file mode 100644 index 00000000..ede6cd67 --- /dev/null +++ b/tests/contract/hdf5_fixture_tool.cpp @@ -0,0 +1,71 @@ +// hdf5_fixture_tool — out-of-band stored-backend-version mutation for the +// HDF5 drift fixtures (issues #36 and #39). +// +// The contract suite asserts version-drift behavior exclusively through the +// public C ABI; preparing a MISMATCHED pulse, however, requires rewriting the +// HDF5_BACKEND_VERSION attribute the backend wrote into the master file — an +// operation no public API offers (by design). This standalone tool owns that +// single out-of-band step, so `contract_tests` itself never includes or links +// the HDF5 C API: the same producer/observer split the MDSplus drift tier +// uses (mdsplus_fixture_tool.cpp, issue #35). +// +// Usage: hdf5_fixture_tool +// directory holding an existing pulse's master.h5 +// (created beforehand through the C ABI by the caller) +// value to store, e.g. "999.0" or "1.999" +// +// Fixtures are always regenerated from the current backend at test run time — +// nothing opaque or host-specific is ever committed. + +#include + +#include +#include + +int main(int argc, char** argv) { + if (argc != 3) { + std::fprintf(stderr, + "usage: %s \n", argv[0]); + return 2; + } + const std::string master = std::string(argv[1]) + "/master.h5"; + const std::string version = argv[2]; + const char* attr_name = "HDF5_BACKEND_VERSION"; + + hid_t file_id = H5Fopen(master.c_str(), H5F_ACC_RDWR, H5P_DEFAULT); + if (file_id < 0) { + std::fprintf(stderr, "cannot open %s for writing\n", master.c_str()); + return 1; + } + + // The stored attribute's size is baked into its datatype, so replace the + // attribute wholesale instead of writing into the old one. + if (H5Aexists(file_id, attr_name) > 0 && + H5Adelete(file_id, attr_name) < 0) { + std::fprintf(stderr, "cannot delete existing %s\n", attr_name); + H5Fclose(file_id); + return 1; + } + + hid_t dataspace_id = H5Screate(H5S_SCALAR); + hid_t dtype_id = H5Tcopy(H5T_C_S1); + H5Tset_size(dtype_id, version.length() + 1); + H5Tset_cset(dtype_id, H5T_CSET_UTF8); + hid_t att_id = H5Acreate2(file_id, attr_name, dtype_id, dataspace_id, + H5P_DEFAULT, H5P_DEFAULT); + int rc = 0; + if (att_id < 0 || H5Awrite(att_id, dtype_id, version.c_str()) < 0) { + std::fprintf(stderr, "cannot write %s\n", attr_name); + rc = 1; + } + if (att_id >= 0) H5Aclose(att_id); + H5Tclose(dtype_id); + H5Sclose(dataspace_id); + H5Fclose(file_id); + + if (rc == 0) { + std::printf("stored %s=\"%s\" in %s\n", attr_name, version.c_str(), + master.c_str()); + } + return rc; +} diff --git a/tests/contract/test_hdf5_version_drift.cpp b/tests/contract/test_hdf5_version_drift.cpp new file mode 100644 index 00000000..f3000b6b --- /dev/null +++ b/tests/contract/test_hdf5_version_drift.cpp @@ -0,0 +1,118 @@ +// HDF5 stored-backend-version drift check (issue #36, TRACEABILITY.md Part 2 +// row B). +// +// al_begin_dataentry_action (src/al_lowlevel.cpp) compares the backend's +// compiled version (`getVersion(NULL)`, HDF5: {1,0}) against the version +// stored in the pulse being opened (`getVersion(pctx)`, HDF5: the +// HDF5_BACKEND_VERSION attribute of master.h5) on OPEN_PULSE/FORCE_OPEN_PULSE +// and throws LOWLEVEL_ERR on a mismatch. Part 2 row B previously declared +// this reachable check a terminal `gap` because forcing a mismatch needs +// out-of-band HDF5 attribute surgery; that surgery now lives in the isolated +// fixture producer hdf5_fixture_tool.cpp (same producer/observer split as the +// MDSplus drift tier), so this file asserts the contract exclusively through +// the public C ABI — no HDF5 include or link enters contract_tests. +// +// Empirically characterized: for HDF5 the generic LOWLEVEL_ERR drift branch +// is SHADOWED — HDF5Backend::openPulse constructs its reader/writer through +// HDF5BackendFactory, which accepts exactly "1.0" +// (src/hdf5/hdf5_backend_factory.cpp) and throws for any other stored value, +// so an incompatible pulse is refused with BACKEND_ERR ("No backend writer +// with version: ") BEFORE al_lowlevel.cpp's comparison ever runs. +// Every stored version other than the compiled one is thereby rejected — +// stricter than the generic check (which would tolerate an older minor). +// These pins assert that exact current refusal. +#ifdef AL_CONTRACT_HDF5_FIXTURE_TOOL + +#include "al_contract.h" + +#include +#include + +#include + +#include +#include + +namespace { + +class Hdf5VersionDrift : public ::testing::Test { +protected: + // Creates a genuine pulse through the public C ABI and returns its + // directory (the URI "path" query parameter). + std::string create_pulse() { + const std::string dir = base_.str() + "/pulse"; + const std::string uri = "imas:hdf5?path=" + dir; + int ctx = -1; + EXPECT_EQ( + al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, &ctx) + .code, + 0); + EXPECT_EQ(al_close_pulse(ctx, CLOSE_PULSE).code, 0); + return dir; + } + + // Out-of-band fixture mutation: rewrite the pulse's stored + // HDF5_BACKEND_VERSION via the isolated setup tool. Fixture producer + // only — nothing asserted here is part of the contract. + void set_stored_version(const std::string& dir, + const std::string& version) { + const std::string cmd = std::string("\"") + + AL_CONTRACT_HDF5_FIXTURE_TOOL + "\" \"" + dir + + "\" \"" + version + "\""; + ASSERT_EQ(std::system(cmd.c_str()), 0) + << "fixture tool failed: " << cmd; + } + + al_status_t open_pulse(const std::string& dir, int* ctx) { + const std::string uri = "imas:hdf5?path=" + dir; + return al_begin_dataentry_action(uri.c_str(), OPEN_PULSE, ctx); + } + + al_contract::TempBase base_; +}; + +// Baseline: a pulse created and reopened by the same compiled backend carries +// a matching stored version, so the drift check is a no-op and OPEN succeeds. +TEST_F(Hdf5VersionDrift, MatchingStoredVersionOpensCleanly) { + const std::string dir = create_pulse(); + int ctx = -1; + al_status_t s = open_pulse(dir, &ctx); + EXPECT_EQ(s.code, 0) << "message: " << s.message; + if (s.code == 0) { + EXPECT_EQ(al_close_pulse(ctx, CLOSE_PULSE).code, 0); + } +} + +// A stored major the compiled backend (1.x) can never match is refused — +// never a silent open of incompatible data. Exact observed status: the +// factory's BACKEND_ERR, which fires before the generic LOWLEVEL_ERR drift +// comparison (see the header note). +TEST_F(Hdf5VersionDrift, HigherStoredMajorRefusesOpen) { + const std::string dir = create_pulse(); + set_stored_version(dir, "999.0"); + int ctx = -1; + al_status_t s = open_pulse(dir, &ctx); + EXPECT_EQ(s.code, alerror::backend_err) << "message: " << s.message; + EXPECT_NE(std::string(s.message).find("No backend writer with version"), + std::string::npos) + << "message: " << s.message; + EXPECT_NE(std::string(s.message).find("999.0"), std::string::npos) + << "the refusal must name the stored version; message: " << s.message; +} + +// The minor comparison boundary: a stored minor NEWER than the compiled one +// is likewise refused (the factory accepts exactly "1.0", so this is refused +// by the same shadowing BACKEND_ERR, not the generic minor comparison). +TEST_F(Hdf5VersionDrift, NewerStoredMinorRefusesOpen) { + const std::string dir = create_pulse(); + set_stored_version(dir, "1.999"); + int ctx = -1; + al_status_t s = open_pulse(dir, &ctx); + EXPECT_EQ(s.code, alerror::backend_err) << "message: " << s.message; + EXPECT_NE(std::string(s.message).find("1.999"), std::string::npos) + << "message: " << s.message; +} + +} // namespace + +#endif // AL_CONTRACT_HDF5_FIXTURE_TOOL From 74860e2677518dc3460e23663ba80b7fcbd58225 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:31:05 +0200 Subject: [PATCH 09/17] Make the UDA version-drift tripwire use a mismatched stored fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old tripwire opened a fresh, matching HDF5 pulse — a future correct drift implementation would also accept it, so it discriminated nothing. Rework per issue #39: the pulse's stored HDF5_BACKEND_VERSION is rewritten to 999.0 out-of-band by hdf5_fixture_tool (issue #36's isolated producer); every asserted behavior stays on the public C ABI. Empirically characterized against the reference stack (which corrects the issue's assumption that the mismatched fixture opens today): the mismatched pulse is REFUSED, but by the SERVER-side HDF5 open — the server plugin's own IMAS-Core hits HDF5BackendFactory's 'No backend writer with version: 999.0' — forwarded to the client as UNKNOWN_ERR. The client-side check stays inert (placeholder {0,0} comparison); its own LOWLEVEL_ERR 'Compatibility …' refusal never appears. The pins encode that split: - baseline MatchingStoredVersionOpensThroughUda: matching fixture opens; - active tripwire VersionDriftCheckCurrentlyDefersToServerSideRefusal: exact UNKNOWN_ERR + server-forwarded message, and asserts the client-side 'Compatibility' diagnostic is absent — implementing real stored-version retrieval makes it fail; - DISABLED_OpenRefusesMismatchedStoredBackendVersion (correct contract): the client's own LOWLEVEL_ERR 'Compatibility' refusal. TRACEABILITY Part 5 documents the actual mismatched fixture and the exact observed status. Verified against the local aarch64 reference stack: full UDA tier 72/72 passed; dedicated-CI check reports 65 executed. Closes #39 (cherry picked from commit b26d7e040d76c1610f9dc6c2b7ddd490fbc6c917) --- tests/contract/TRACEABILITY.md | 60 ++++++----- tests/contract/test_uda_unique_surface.cpp | 111 ++++++++++++++++----- 2 files changed, 121 insertions(+), 50 deletions(-) diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 87d4a3aa..022583b0 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -315,20 +315,16 @@ one genuine `xfail` crash defect it surfaced), and the timebase cache for HDF5.** Part 2 row B left the version-drift check a `gap` because forcing the mismatch needs writing a different backend-version value into the opened pulse after creation — for HDF5 that means poking its on-disk backend-version -attribute directly via the HDF5 C API, which that suite has no include/link -wiring for (out of scope there). MDSplus hits the identical shape of problem — -the mismatch needs overwriting the pulse's stored `VERSION:BACK_MAJOR` tree -node after creation, also unreachable through the C ABI — but the MDSplus tier -is already a separate, compile-guarded, Docker-characterized target -(`tests/contract/CMakeLists.txt`'s `AL_BACKEND_MDSPLUS` gate), so adding the -raw MDSplus C++ API (``) as a test-only, build-gated dependency -of `contract_tests` was a small, low-blast-radius addition — unlike wiring the -HDF5 C API into the always-on suite. So the gap is closed for MDSplus -specifically because of that pre-existing gate, not because the underlying -problem (a version-drift mismatch cannot be forced through the C ABI alone) is -actually different between the two backends. **Part 2 row B itself is left -unchanged** (HDF5 remains a terminal gap); this is a Part 4 finding about *why* -the two backends diverged, not a fix to Part 2. +attribute directly via the HDF5 C API, which that suite originally had no +include/link wiring for. MDSplus hits the identical shape of problem — the +mismatch needs overwriting the pulse's stored `VERSION:BACK_MAJOR` tree node +after creation, also unreachable through the C ABI. Both are now solved with +the same producer/observer split: all raw-API fixture mutation lives in +standalone, build-gated setup tools (`mdsplus_fixture_tool` for the tree +node, issue #35; `hdf5_fixture_tool` for the HDF5 attribute, issue #36) that +the tests invoke as subprocesses, so `contract_tests` itself carries no +MDSplus or HDF5 API dependency and every asserted behavior stays on the +public C ABI. **Part 2 row B is closed for HDF5 too** (see Part 2). Build-gated by `AL_CONTRACT_HAVE_MDSPLUS` (`tests/contract/CMakeLists.txt`, defined only when `AL_BACKEND_MDSPLUS=ON`), so every MDSplus case in @@ -575,12 +571,15 @@ MDSplus, the stored value is `VERSION:BACK_MAJOR`/`BACK_MINOR` (`src/mdsplus/mdsplus_backend.cpp`'s `saveVersion`, called once at pulse creation from the compiled `MDSPLUS_BACKEND_MAJOR`/`MINOR` constants, currently 1/1). Forcing the mismatch case needs a value in the pulse different from -what today's build would write — done here by opening the pulse's "ids" tree -directly through the raw MDSplus C++ API (``, not the C ABI) and -overwriting `VERSION:BACK_MAJOR` after creation, mirroring the -`ids_path`/`setDataEnv` environment-variable dance the backend itself uses -internally. See the finding above on why this was achievable for MDSplus but -stays a `gap` for HDF5 (Part 2 row B). +what today's build would write — done by the standalone, build-gated +`mdsplus_fixture_tool` (issue #35), which opens the pulse's "ids" tree +through the raw MDSplus C++ API (``) and overwrites +`VERSION:BACK_MAJOR` after creation, mirroring the `ids_path`/`setDataEnv` +environment-variable dance the backend itself uses internally. The test +invokes the tool as a subprocess and asserts exclusively through `al_*` +APIs: `contract_tests` has no direct MDSplus include or link dependency, so +a rewrite satisfying the C ABI is judged by exactly the C ABI. The identical +split now also closes the HDF5 side (Part 2 row B, issue #36). | Cluster / Capability | Test(s) | Status | |---|---|---| @@ -944,10 +943,21 @@ recorded here alongside the header's existing stack identifiers. `uda_backend.h`'s `UDA_BACKEND_VERSION_MAJOR/MINOR` and the "temporary placeholder" non-null-ctx branch), so `(0!=0)||(0<0)` is always false regardless of what is genuinely stored remotely. An unavailable stored - version must not be treated as verified compatibility. Pinned: - `UdaUniqueSurfaceTest.DISABLED_OpenRefusesWhenStoredBackendVersionCannotBeVerified` - + tripwire - `UdaUniqueSurfaceTest.VersionDriftCheckCurrentlyNeverFiresRegardlessOfStoredPulse`. + version must not be treated as verified compatibility. Pinned with a + genuinely MISMATCHED stored fixture (issue #39): the pulse's + `HDF5_BACKEND_VERSION` is rewritten to `999.0` out-of-band by + `hdf5_fixture_tool` (issue #36's isolated producer; fixture preparation + only — every asserted behavior stays on the public C ABI). Observed against + the reference stack: the mismatched pulse is REFUSED, but by the + SERVER-side HDF5 open (the server plugin's own IMAS-Core hits + `HDF5BackendFactory`'s "No backend writer with version: 999.0"), forwarded + to the client as exactly `UNKNOWN_ERR` (-1); the client-side check itself + stays inert — its own `LOWLEVEL_ERR` "Compatibility …" refusal never + appears, which is what the active tripwire pins. Pinned: + `UdaUniqueSurfaceTest.DISABLED_OpenRefusesMismatchedStoredBackendVersion` + (client-side refusal, correct contract) + tripwire + `UdaUniqueSurfaceTest.VersionDriftCheckCurrentlyDefersToServerSideRefusal`, + with baseline `UdaUniqueSurfaceTest.MatchingStoredVersionOpensThroughUda`. - **`supportsTimeRangeOperation()` capability negotiation confirmed against the reference server**: the reference plugin reports `1.8.0` (issue #23), so `1.8.0 > 1.4.0` grants the capability — `al_begin_timerange_action` must @@ -1020,7 +1030,7 @@ recorded here alongside the header's existing stack identifiers. | Runtime DD loading: present baseline and absent DD (both failure messages) | `UdaUniqueSurfaceTest.{DdPresentLoadsAndOpenSucceeds,DdAbsentNeitherEnvVarSetFailsWithClearMessage,DdAbsentFileMissingAtIdsDefPathFailsWithClearMessage}` | covered | | ↳ wrong-version DD loads silently with no semantic cross-version check | `UdaUniqueSurfaceTest.DISABLED_DdWrongVersionIsRejected` + tripwire `UdaUniqueSurfaceTest.DdWrongVersionCurrentlyLoadsSilentlyWithNoCrossVersionCheck` | **xfail** | | `datapath` partial-get via `cache_mode=ids`: in-scope field round-trips, out-of-scope field silently reads as absent | `UdaUniqueSurfaceTest.DatapathScopesCachePopulationFieldOutsideScopeReadsAsAbsent` | covered | -| Version-drift check inertness: open treats an unavailable stored version as compatible because both sides are hardcoded placeholders | `UdaUniqueSurfaceTest.DISABLED_OpenRefusesWhenStoredBackendVersionCannotBeVerified` + tripwire `UdaUniqueSurfaceTest.VersionDriftCheckCurrentlyNeverFiresRegardlessOfStoredPulse` | **xfail** | +| Version-drift check inertness: the client's own drift check never fires (both sides hardcoded placeholders); a pulse whose stored backend version (`999.0`, rewritten out-of-band by `hdf5_fixture_tool`) can never match is refused only by the forwarded SERVER-side HDF5 error (`UNKNOWN_ERR`, "No backend writer with version: 999.0") | `UdaUniqueSurfaceTest.DISABLED_OpenRefusesMismatchedStoredBackendVersion` + tripwire `UdaUniqueSurfaceTest.VersionDriftCheckCurrentlyDefersToServerSideRefusal` (baseline: `MatchingStoredVersionOpensThroughUda`) | **xfail** | | Server-version-gated `supportsTimeRangeOperation()`: reference plugin 1.8.0 > 1.4.0 grants the capability | `UdaUniqueSurfaceTest.TimeRangeCapabilityGrantedByReferenceServerVersion180` | covered | | Fetch mode: download, local-backend handoff, correct read-back, cache reuse on reopen (confirmed via `download_file`'s own verbose trace) | `UdaUniqueSurfaceTest.FetchModeDownloadsHandsOffToLocalBackendAndReusesCacheOnReopen` | covered | | `local_cache` overrides the cache root only — the remote path is still nested underneath it, same as the default formula | `UdaUniqueSurfaceTest.FetchModeLocalCacheOptionOverridesDefaultCacheDir` | covered | diff --git a/tests/contract/test_uda_unique_surface.cpp b/tests/contract/test_uda_unique_surface.cpp index ee14b1fd..0a86b5d2 100644 --- a/tests/contract/test_uda_unique_surface.cpp +++ b/tests/contract/test_uda_unique_surface.cpp @@ -799,7 +799,8 @@ TEST_F(UdaUniqueSurfaceTest, DatapathScopesCachePopulationFieldOutsideScopeReads } // =========================================================================== -// Area 5: version-drift check inertness. +// Area 5: version-drift check inertness (reworked by issue #39: the tripwire +// now uses a genuinely MISMATCHED stored fixture). // // al_begin_dataentry_action (src/al_lowlevel.cpp:868-883) compares // backend->getVersion(NULL) (the "compiled" version) against @@ -811,46 +812,106 @@ TEST_F(UdaUniqueSurfaceTest, DatapathScopesCachePopulationFieldOutsideScopeReads // getVersion(NULL) returns the compiled {UDA_BACKEND_VERSION_MAJOR, // UDA_BACKEND_VERSION_MINOR} = {0, 0} (uda_backend.h:37-38); getVersion(pctx) // with a non-null ctx always returns {0, 0} too ("temporary placeholder", -// never reads anything from the actual remote pulse). The comparison -// ((ver.first!=sver.first)||(ver.second Date: Thu, 16 Jul 2026 16:01:39 +0200 Subject: [PATCH 10/17] Accept the legitimately empty ids_model.datafile in the model verify step First real run of the issue #42 verify step failed on a healthy build: a baked MDSplus model tree carries all structure in ids_model.{tree,characteristics} and its ids_model.datafile is legitimately 0 bytes, so the -s (nonempty) check rejected it and the tier was skipped (masked by continue-on-error). Check tree and characteristics for nonzero size, datafile for existence only. Verified in the local aarch64 container: a clean '--build --target contract_tests' bakes all three files and 'ctest -L mdsplus' passes 44/44. Part of issue #42. (cherry picked from commit 26c8f9a197c8dd09acbad678c4fbc4973056ac47) --- .github/workflows/mdsplus-contract.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mdsplus-contract.yml b/.github/workflows/mdsplus-contract.yml index 50f85c2f..8222b82c 100644 --- a/.github/workflows/mdsplus-contract.yml +++ b/.github/workflows/mdsplus-contract.yml @@ -92,12 +92,19 @@ jobs: - name: Verify baked DD model tree is present (cache-hit and cache-miss paths) run: | set -euo pipefail - for f in ids_model.tree ids_model.datafile ids_model.characteristics; do + # ids_model.datafile is legitimately 0 bytes for a model tree (all + # structure lives in .tree/.characteristics; a model carries no + # data), so it is checked for existence only. + for f in ids_model.tree ids_model.characteristics; do if [ ! -s "build-mdsplus/models/mdsplus/${f}" ]; then echo "FATAL: missing/empty model file build-mdsplus/models/mdsplus/${f} — the MDSplus tier would fail with %TREE-E-FOPENR (issue #42)" exit 1 fi done + if [ ! -e "build-mdsplus/models/mdsplus/ids_model.datafile" ]; then + echo "FATAL: missing model file build-mdsplus/models/mdsplus/ids_model.datafile (issue #42)" + exit 1 + fi echo "Baked DD model tree complete under build-mdsplus/models/mdsplus." - name: Run MDSplus contract suite From ab6ce4a5ed832f0308bfde1b7d32a3388349dd5e Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:01:39 +0200 Subject: [PATCH 11/17] Keep the MDSplus version-drift contract test on the public C ABI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_mdsplus_version_drift.cpp included and mutated VERSION:BACK_MAJOR through the raw MDSplus C++ API, linking the MDSplus libraries into contract_tests — a second behavioral seam: a rewrite could satisfy the C ABI while the test still required the legacy API and storage layout. Separate fixture mutation from contract observation (issue #35): - al-mdsplus-fixture-tool (build-gated with the backend) owns ALL direct MDSplus API access: it reproduces the ids_path/setDataEnv dance and overwrites VERSION:BACK_MAJOR. Fixtures are regenerated from the pinned DD/model inputs at test run time; nothing opaque or host-specific is committed. - test_mdsplus_version_drift.cpp now includes only AL/test headers and calls only al_* APIs; it invokes the tool as a subprocess for the mismatch. contract_tests has no MDSplus include or link left (verified: no MDSplus NEEDED entry in the binary; the tool links libMdsObjectsCppShr). - The single discovery pass no longer sets a 'contract' label: ctest MERGES labels across set_tests_properties calls, so a discovery-time label would leak onto the mdsplus/uda tiers and change what 'ctest -L contract' (sanitizers.yml) selects. The labeler assigns each test its complete final list; verified in the container: contract=285 (=unit 98 + integration 187), mdsplus=45, disjoint. Verified in the local aarch64 container (compile-gated tier, reproducible): 'ctest -L mdsplus' 44/44 passed, both drift fixtures (matching + mismatched via the tool) prove the check; always-on suite 254/254 locally. Closes #35 (cherry picked from commit c90b8e330d7f00852818a2df63e1763f5630dcd3) --- tests/contract/CMakeLists.txt | 8 +- tests/contract/mdsplus_fixture_tool.cpp | 68 ++++++++++++++ tests/contract/test_mdsplus_version_drift.cpp | 91 ++++++------------- 3 files changed, 104 insertions(+), 63 deletions(-) create mode 100644 tests/contract/mdsplus_fixture_tool.cpp diff --git a/tests/contract/CMakeLists.txt b/tests/contract/CMakeLists.txt index 58db1915..35b23f3e 100644 --- a/tests/contract/CMakeLists.txt +++ b/tests/contract/CMakeLists.txt @@ -248,8 +248,12 @@ endif() # version-dependent forwarding in the middle. The tier definitions (which # suites are unit / integration / mdsplus / uda, per issues #13-#17 and # #23-#26) are documented and maintained in assign_contract_labels.cmake.in. -gtest_discover_tests(contract_tests - PROPERTIES LABELS "contract") +# No LABELS here: ctest MERGES labels across set_tests_properties calls +# (append semantics), so a discovery-time "contract" could not be removed by +# the labeler and would leak onto the mdsplus/uda tiers, changing what +# `ctest -L contract` (sanitizers.yml) selects. assign_contract_labels.cmake +# assigns each test its complete, final label list instead. +gtest_discover_tests(contract_tests) # Bake the generated registration file's path into the labeler. A single # discovery pass for contract_tests means GoogleTest writes diff --git a/tests/contract/mdsplus_fixture_tool.cpp b/tests/contract/mdsplus_fixture_tool.cpp new file mode 100644 index 00000000..78d80539 --- /dev/null +++ b/tests/contract/mdsplus_fixture_tool.cpp @@ -0,0 +1,68 @@ +// mdsplus_fixture_tool — out-of-band stored-backend-version mutation for the +// MDSplus drift fixture (issue #35). +// +// The contract suite's seam is the public C ABI (issues #1/#12); preparing a +// MISMATCHED pulse, however, requires overwriting the VERSION:BACK_MAJOR node +// the backend wrote into the tree at pulse creation — reachable only through +// the raw MDSplus C++ API (). This standalone, build-gated tool +// owns ALL direct MDSplus API access, so `contract_tests` itself neither +// includes nor links the MDSplus C++ libraries: it is a +// fixture producer, not part of the asserted contract. +// +// Usage: mdsplus_fixture_tool +// directory holding the pulse's "ids" tree files (created +// beforehand through the C ABI by the caller; shot 1) +// integer to store into VERSION:BACK_MAJOR, e.g. 99 +// +// MDSPLUS_MODELS_PATH must point at the baked model tree, exactly as for the +// backend itself. Fixtures are regenerated from pinned DD/model inputs at +// test run time — nothing opaque or host-specific is ever committed. + +#include + +#include +#include +#include + +int main(int argc, char** argv) { + if (argc != 3) { + std::fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + const std::string pulse_dir = argv[1]; + const int back_major = std::atoi(argv[2]); + + const char* models_path = std::getenv("MDSPLUS_MODELS_PATH"); + if (models_path == nullptr || *models_path == '\0') { + std::fprintf(stderr, "MDSPLUS_MODELS_PATH is not set\n"); + return 2; + } + + // Reproduces MDSplusBackend::setDataEnv's "ids_path" env-var dance + // (mdsplus_backend.cpp) well enough to open the same tree the backend + // itself opened through the C ABI. + const std::string ids_path = pulse_dir + ";" + models_path; + if (setenv("ids_path", ids_path.c_str(), 1) != 0) { + std::fprintf(stderr, "cannot set ids_path\n"); + return 1; + } + + try { + MDSplus::Tree tree("ids", /*shot=*/1, "NORMAL"); + MDSplus::TreeNode* node = tree.getNode("VERSION:BACK_MAJOR"); + MDSplus::Int32* value = new MDSplus::Int32(back_major); + node->putData(value); + MDSplus::deleteData(value); + delete node; + } catch (MDSplus::MdsException& exc) { + std::fprintf(stderr, + "could not poke VERSION:BACK_MAJOR via the MDSplus tree " + "API: %s\n", + exc.what()); + return 1; + } + + std::printf("stored VERSION:BACK_MAJOR=%d in %s\n", back_major, + pulse_dir.c_str()); + return 0; +} diff --git a/tests/contract/test_mdsplus_version_drift.cpp b/tests/contract/test_mdsplus_version_drift.cpp index c463b3a9..59636ee5 100644 --- a/tests/contract/test_mdsplus_version_drift.cpp +++ b/tests/contract/test_mdsplus_version_drift.cpp @@ -1,36 +1,27 @@ -// MDSplus version-drift check (issue #16, TRACEABILITY.md Part 4). +// MDSplus version-drift check (issue #16, reworked by issue #35; +// TRACEABILITY.md Part 4). // -// al_lowlevel.cpp's al_begin_dataentry_action (src/al_lowlevel.cpp:868-883) -// compares the backend's compiled version (`getVersion(NULL)`) against the -// version stored in the pulse being opened (`getVersion(pctx)`) on -// OPEN_PULSE/FORCE_OPEN_PULSE and throws LOWLEVEL_ERR on a mismatch. This is -// generic al_lowlevel.cpp logic shared by every backend, but MDSplus is the -// one backend where `getVersion(pctx)` reads a value a pulse can genuinely -// carry different from today's build: `VERSION:BACK_MAJOR`/`BACK_MINOR`, -// written into the tree once at pulse creation -// (`mdsplus_backend.cpp`'s `saveVersion`, called from `createPulse`) from the -// compiled `MDSPLUS_BACKEND_MAJOR`/`MINOR` constants (currently 1/1) and read -// back unchanged forever after. Forcing the mismatch case therefore needs -// writing a different value into that node after the fact -- through the raw -// MDSplus C++ API (``), not the public C ABI, exactly the kind -// of out-of-band access issue #12's Part 2 row B ("gap", see TRACEABILITY.md) -// declined for HDF5 because that suite has no HDF5 include/link wiring. This -// file adds that wiring for MDSplus only (tests/contract/CMakeLists.txt, -// gated the same way as AL_CONTRACT_HAVE_MDSPLUS): the MDSplus tier already -// links libMDSplus into `al` PRIVATE-ly for the backend itself, and doing the -// same for `contract_tests` is a test-only, build-gated addition -- so this -// finding *is* closeable for MDSplus even though it stayed a terminal gap for -// HDF5 (whose always-on suite deliberately doesn't carry an HDF5 C API -// dependency at all). -#ifdef AL_CONTRACT_HAVE_MDSPLUS +// al_lowlevel.cpp's al_begin_dataentry_action compares the backend's compiled +// version (`getVersion(NULL)`) against the version stored in the pulse being +// opened (`getVersion(pctx)`) on OPEN_PULSE/FORCE_OPEN_PULSE and throws +// LOWLEVEL_ERR on a mismatch. MDSplus stores that version in the tree's +// VERSION:BACK_MAJOR/BACK_MINOR nodes, written once at pulse creation from +// the compiled constants (currently 1/1) and read back unchanged forever. +// +// Forcing the mismatch needs out-of-band tree surgery — but that mutation is +// FIXTURE PREPARATION, not contract: it lives in the separate build-gated +// producer mdsplus_fixture_tool.cpp (issue #35), which owns all direct +// MDSplus C++ API access. This file includes only AL/test headers and calls +// only `al_*` APIs, so a rewrite satisfying the C ABI is judged by exactly +// the C ABI — no second behavioral seam through or the legacy +// storage layout leaks into the asserted contract. +#if defined(AL_CONTRACT_HAVE_MDSPLUS) && defined(AL_CONTRACT_MDSPLUS_FIXTURE_TOOL) #include "al_contract.h" #include #include -#include - #include #include @@ -63,37 +54,16 @@ class MdsplusVersionDrift : public ::testing::Test { std::to_string(pulse_.run); } - // Reproduces MDSplusBackend::setDataEnv/resetIdsPath's "ids_path" env-var - // dance (mdsplus_backend.cpp:1458-1517) well enough to open the same tree - // the backend itself opened through the C ABI -- this test has no access - // to the backend's private state, only the public MDSplus tree API. + // Out-of-band fixture mutation via the isolated setup tool + // (mdsplus_fixture_tool.cpp) — the producer of the mismatched fixture, + // not part of the asserted contract. void PokeStoredBackendMajorVersion(int mismatched_major) { - const char* models_path = std::getenv("MDSPLUS_MODELS_PATH"); - ASSERT_TRUE(models_path && *models_path); - const std::string original_ids_path = - std::getenv("ids_path") ? std::getenv("ids_path") : ""; - - const std::string new_ids_path = pulse_dir() + ";" + models_path; - ASSERT_EQ(setenv("ids_path", new_ids_path.c_str(), 1), 0); - - try { - MDSplus::Tree tree("ids", /*shot=*/1, "NORMAL"); - MDSplus::TreeNode* node = tree.getNode("VERSION:BACK_MAJOR"); - MDSplus::Int32* value = new MDSplus::Int32(mismatched_major); - node->putData(value); - MDSplus::deleteData(value); - delete node; - } catch (MDSplus::MdsException& exc) { - FAIL() << "Could not poke VERSION:BACK_MAJOR directly via the " - "MDSplus tree API: " - << exc.what(); - } - - if (!original_ids_path.empty()) { - setenv("ids_path", original_ids_path.c_str(), 1); - } else { - unsetenv("ids_path"); - } + const std::string cmd = std::string("\"") + + AL_CONTRACT_MDSPLUS_FIXTURE_TOOL + "\" \"" + + pulse_dir() + "\" " + + std::to_string(mismatched_major); + ASSERT_EQ(std::system(cmd.c_str()), 0) + << "fixture tool failed: " << cmd; } }; @@ -120,9 +90,9 @@ TEST_F(MdsplusVersionDrift, MatchingVersionOpensCleanly) { } } -// Mismatching case: after creation, VERSION:BACK_MAJOR is overwritten to a -// value the compiled backend (MDSPLUS_BACKEND_MAJOR == 1) can never match. -// Reopening through the C ABI must hit al_lowlevel.cpp's +// Mismatching case: after creation, VERSION:BACK_MAJOR is overwritten (by the +// fixture tool) to a value the compiled backend (MDSPLUS_BACKEND_MAJOR == 1) +// can never match. Reopening through the C ABI must hit al_lowlevel.cpp's // `(ver.first != sver.first)` branch and fail with LOWLEVEL_ERR, never a // silent open of stale/incompatible data. TEST_F(MdsplusVersionDrift, MismatchedBackendVersionRefusesOpen) { @@ -145,7 +115,6 @@ TEST_F(MdsplusVersionDrift, MismatchedBackendVersionRefusesOpen) { << "expected the version-drift check to refuse opening a pulse whose " "stored VERSION:BACK_MAJOR (" << kMismatchedMajor << ") doesn't match the compiled backend's"; - ASSERT_TRUE(status.message != nullptr); EXPECT_NE(std::string(status.message).find("Compatibility"), std::string::npos) << "status.message=" << status.message; @@ -153,4 +122,4 @@ TEST_F(MdsplusVersionDrift, MismatchedBackendVersionRefusesOpen) { } // namespace -#endif // AL_CONTRACT_HAVE_MDSPLUS +#endif // AL_CONTRACT_HAVE_MDSPLUS && AL_CONTRACT_MDSPLUS_FIXTURE_TOOL From c677cafd74eb552b7e073306d0e0c5e05185cee2 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:27:41 +0200 Subject: [PATCH 12/17] Share the MDSplus/UDA real-DD path catalog (#46) The MDSplus (#15) and UDA (#25) real-path breadth suites each defined their own copy of the DD-4.1.1 datatype/rank path table, the terminal-gap DD facts, and the case-name generator. A path correction or a new (type, rank) cell needed coordinated edits in two large files, and the tables could silently diverge -- undermining #12/#21's promise that the two backend verdicts are comparable over the same curated set. Extract the single source of truth into test-only real_dd_path_catalog.{h,cpp}: the validated DD version, the datatype/rank key, the IDS + struct_array chain + leaf, and the terminal-gap reason (all DD facts, identical for both backends). Backend-specific verdicts stay local -- MDSplus's STR_0D dimension divergence and UDA's fixture-backend divergence + dynamic-leaf-inside-AOS known defects are attached per suite by (dtype, rank) key, never duplicated into the catalog. Both matrices now derive their parametrization 1:1 from catalog(), so a cell cannot be dropped or duplicated silently: - RealDdPathCatalog.* (always-on): the catalog has exactly one entry per (dtype, rank) cell, no duplicate, path/terminal-gap mutually exclusive. - {Mdsplus,Uda}RealPathCatalog.CoversEveryCatalogKeyOnce (per gated tier): the matrix key set equals the catalog key set exactly. Case names ('CHAR_r0' ...) and INSTANTIATE_TEST_SUITE_P prefixes are preserved, so existing test names and TRACEABILITY references stay stable. Verified: MDSplus tier 18 pass / 14 skip (unchanged) + meta green; UDA tier 15 pass / 17 skip + 3 known-defect tripwires green + meta green (one Protocol-11 server flake confirmed transient on re-run); always-on catalog tests 2/2; label partition disjoint and hermetic (unit 98->100, mdsplus 45->46, integration unchanged) across MDSplus-enabled and backends-off builds. (cherry picked from commit 8506879e241ccde31cd3cca88a72ea8ee26fdc53) --- tests/contract/CMakeLists.txt | 2 + tests/contract/TRACEABILITY.md | 62 ++-- .../contract/assign_contract_labels.cmake.in | 9 +- tests/contract/real_dd_path_catalog.cpp | 85 +++++ tests/contract/real_dd_path_catalog.h | 90 +++++ tests/contract/test_mdsplus_real_paths.cpp | 185 +++++------ tests/contract/test_real_dd_path_catalog.cpp | 82 +++++ tests/contract/test_uda_real_paths.cpp | 309 +++++++++--------- 8 files changed, 532 insertions(+), 292 deletions(-) create mode 100644 tests/contract/real_dd_path_catalog.cpp create mode 100644 tests/contract/real_dd_path_catalog.h create mode 100644 tests/contract/test_real_dd_path_catalog.cpp diff --git a/tests/contract/CMakeLists.txt b/tests/contract/CMakeLists.txt index 35b23f3e..d6be748a 100644 --- a/tests/contract/CMakeLists.txt +++ b/tests/contract/CMakeLists.txt @@ -70,6 +70,8 @@ add_executable(contract_tests test_plugins.cpp test_backend_factory.cpp test_hdf5_version_drift.cpp + real_dd_path_catalog.cpp + test_real_dd_path_catalog.cpp test_structured_data.cpp test_pulse_lifecycle.cpp test_mdsplus.cpp diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 022583b0..551b7f2b 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -408,7 +408,13 @@ Where the rows above join MDSplus to the *existing* fixtures' synthetic shapes, this matrix instead curates one **real** DD-4.1.1 path per `{CHAR, INTEGER, DOUBLE, COMPLEX} x rank {0..7}` cell — the axis `RoundTripMatrix` already sweeps on the always-on tier, but against opaque -paths MDSplus cannot resolve (issue #12 Q2/Q5). Path curation method: the +paths MDSplus cannot resolve (issue #12 Q2/Q5). The curated set is the shared +`al_contract::real_dd::catalog()` (`real_dd_path_catalog.{h,cpp}`, issue #46): +the IDS, struct_array chain, leaf, and terminal-gap DD facts are identical for +MDSplus and UDA and live in exactly one place, so the two matrices cannot +silently diverge; only each backend's own storage-model verdicts stay local +(`RealDdPathCatalog.*` plus the per-backend `*RealPathCatalog.CoversEveryCatalogKeyOnce` +meta-tests fail if a matrix drops a cell). Path curation method: the `imas-dd` MCP tool CLAUDE.md names was not present in this session's MCP configuration, so the same question it would answer was resolved by walking the actual baked-from artifact directly — @@ -661,12 +667,15 @@ not fixed — both explained in full below. ## Real-DD-path datatype × rank breadth (issue #25, reusing issue #15's set) `test_uda_real_paths.cpp`, `Uda/UdaRealPathMatrix.*` — the UDA analogue of -`test_mdsplus_real_paths.cpp`: **the identical curated path/rank/aos_chain set -reused as-is** (same 32 cells, same 10 IDSs), adapted to seed(HDF5)-then- -reopen(UDA) instead of MDSplus's direct write→read. `terminal_gap_reason` -carries over unchanged (a DD-4.1.1 fact, independent of backend); a UDA-only -`divergence_reason` and `known_defect_reason` were added where the two -backends diverge for different reasons on the same cell. +`test_mdsplus_real_paths.cpp`: **the identical curated path/rank/aos_chain set, +now the shared `al_contract::real_dd::catalog()`** (`real_dd_path_catalog.{h,cpp}`, +issue #46 — one source of truth for both matrices, same 32 cells, same 10 +IDSs), adapted to seed(HDF5)-then-reopen(UDA) instead of MDSplus's direct +write→read. `terminal_gap_reason` lives in the shared catalog (a DD-4.1.1 fact, +independent of backend); UDA-only `divergence_reason`/`known_defect_reason` +verdicts stay local to `test_uda_real_paths.cpp` where the two backends diverge +for different reasons on the same cell. `UdaRealPathCatalog.CoversEveryCatalogKeyOnce` +fails if the UDA matrix ever drops or duplicates a catalog cell. **Characterization-discovered facts:** @@ -714,8 +723,8 @@ backends diverge for different reasons on the same cell. COMPLEX_r5}` + tripwires `Uda/UdaRealPathMatrixKnownDefects. DynamicComplexLeafInsideAosCurrentlyReadsEmpty/{COMPLEX_r1,COMPLEX_r3, COMPLEX_r5}` (`test_uda_real_paths.cpp`). The DOUBLE-scalar - `UdaAosKnownDefects` pair remains an independent pin and AOS-mechanism proof, - not a substitute for these exact matrix cases. + `UdaAosKnownDefects` pair remains an independent value-transport pin, not a + substitute for these exact matrix cases or the active traversal proof below. | Cluster / Capability | Test(s) | Status | |---|---|---| @@ -734,20 +743,25 @@ the lifecycle cases drive the remote UDA URI directly through the C ABI. **Characterization-discovered facts:** -- **AOS traversal (top-level) hits the same dynamic-leaf-inside-AOS defect - described above**: `equilibrium/time_slice` (a real DD struct_array, - `timebasepath="time"`) correctly reports its written size (3) through - `al_begin_arraystruct_action`, and iteration genuinely advances (the request - trace shows three distinct `time_slice[0..2]/global_quantities/ip` - directives), but every element's `global_quantities/ip` value comes back as - the HDF5 absent-scalar sentinel (`-9.0e40`) instead of what was written. - Pinned as the canonical instance of the defect: `UdaAosKnownDefects. - DISABLED_DynamicLeafInsideAosRoundTrips` + tripwire `UdaAosKnownDefects. - DynamicLeafInsideAosCurrentlyReturnsSentinel`. Nested (multi-level) AOS - traversal is exercised separately and successfully by the real-path - matrix's *static* AOS-nested cells above (INTEGER r3, DOUBLE r6) — the - traversal mechanism itself (begin/iterate/end across nesting) works; only - a *dynamic* leaf's value fails to come back correctly. +- **AOS traversal (top-level) has a discriminating active C-ABI proof**: + `temporary/constant_integer3d` is a real DD-4.1.1 struct_array with a static + `value` leaf known to round-trip through UDA. Three elements with distinct, + order-sensitive values are seeded through HDF5 in physical index order + 0→2→1 (so setup does not mirror the asserted traversal), then read in order + 0→1→2 around successive `al_iterate_over_arraystruct(..., 1)` calls after + reopening through UDA. + `UdaBreadthTest.AosTraversalAdvancesAcrossDistinctStaticElements` asserts the + exact sequence, so a successful no-op, repeated element, or reordered + element fails. It also ends the AOS action and verifies through the same + public function that the invalid context returns an error status. +- **The dynamic-leaf-inside-AOS value defect remains separately pinned**: + `equilibrium/time_slice` correctly reports its written size (3), but every + element's `global_quantities/ip` value comes back as the HDF5 absent-scalar + sentinel (`-9.0e40`) instead of what was written. The canonical D2 pair is + `UdaAosKnownDefects.DISABLED_DynamicLeafInsideAosRoundTrips` + tripwire + `UdaAosKnownDefects.DynamicLeafInsideAosCurrentlyReturnsSentinel`. Nested + (multi-level) traversal is also exercised by the real-path matrix's static + AOS cells (INTEGER r3, DOUBLE r6). - **`al_get_occurrences` is real through UDA remote mode** (the reference `IMAS` server plugin implements `getOccurrences`, confirmed via the PRD's static finding): occurrence 0 and occurrence 2 (HDF5's own `_` @@ -823,7 +837,7 @@ the lifecycle cases drive the remote UDA URI directly through the C ABI. | Cluster / Capability | Test(s) | Status | |---|---|---| -| AOS traversal, top-level (size + iteration mechanism correct; nested traversal proven separately by the real-path matrix's static AOS cells) | `UdaAosKnownDefects.DynamicLeafInsideAosCurrentlyReturnsSentinel` (mechanism proof); see the xfail row below for the value defect | covered (indirect) | +| AOS traversal, top-level (reported size + exact three-element forward sequence; invalid ended context returns an error; nested traversal proven separately by the real-path matrix's static AOS cells) | `UdaBreadthTest.AosTraversalAdvancesAcrossDistinctStaticElements` | covered | | ↳ a *dynamic* leaf nested inside a struct_array returns the HDF5 absent-scalar sentinel instead of what was written | `UdaAosKnownDefects.DISABLED_DynamicLeafInsideAosRoundTrips` + tripwire `UdaAosKnownDefects.DynamicLeafInsideAosCurrentlyReturnsSentinel` | **xfail** | | `al_get_occurrences` real through UDA remote mode, HDF5's `_` storage naming and UDA's `/` public naming pinned; every reported occurrence is reopened and read | `UdaBreadthTest.OccurrencesListsWrittenOccurrencesThroughReopen` | covered | | `al_list_filled_paths` real through UDA remote mode (`backend=hdf5` only), ownership pinned | `UdaBreadthTest.ListFilledPathsThroughReopen` | covered | diff --git a/tests/contract/assign_contract_labels.cmake.in b/tests/contract/assign_contract_labels.cmake.in index 560b885f..8cbd84a5 100644 --- a/tests/contract/assign_contract_labels.cmake.in +++ b/tests/contract/assign_contract_labels.cmake.in @@ -51,7 +51,10 @@ if(EXISTS "${_al_contract_tests_file}") _al_name MATCHES "^UriOwnership\\." OR # issue #47: unknown-backend refusals at the factory boundary — no # backend substrate is ever constructed, so the tier is hermetic - _al_name MATCHES "^BackendFactory\\.") + _al_name MATCHES "^BackendFactory\\." OR + # issue #46: the shared real-DD-path catalog's own invariants — pure + # in-RAM table checks, no backend substrate, always compiled + _al_name MATCHES "^RealDdPathCatalog\\.") set_tests_properties("${_al_name}" PROPERTIES LABELS "contract;unit") elseif(_al_name MATCHES "^MdsplusTracerBullet" OR # "/Mdsplus" as the whole instance suffix: pretty names end @@ -59,6 +62,8 @@ if(EXISTS "${_al_contract_tests_file}") _al_name MATCHES "/Mdsplus(#|$)" OR _al_name MATCHES "^Occurrences\\.Mdsplus" OR _al_name MATCHES "^Mdsplus/MdsplusRealPathMatrix\\." OR + # issue #46: MDSplus half of the shared-catalog coverage meta-test + _al_name MATCHES "^MdsplusRealPathCatalog\\." OR _al_name MATCHES "^MdsplusVersionDrift" OR _al_name MATCHES "^MdsplusStructAwareSliceInterpolation" OR _al_name MATCHES "^MdsplusTimerangeResampling" OR @@ -70,6 +75,8 @@ if(EXISTS "${_al_contract_tests_file}") _al_name MATCHES "^UdaEquilibriumSeedParity" OR _al_name MATCHES "^Uda/UdaRealPathMatrix\\." OR _al_name MATCHES "^Uda/UdaRealPathMatrixKnownDefects\\." OR + # issue #46: UDA half of the shared-catalog coverage meta-test + _al_name MATCHES "^UdaRealPathCatalog\\." OR _al_name MATCHES "^UdaBreadthTest" OR _al_name MATCHES "^UdaAosKnownDefects" OR _al_name MATCHES "^UdaSliceAndTimeRange" OR diff --git a/tests/contract/real_dd_path_catalog.cpp b/tests/contract/real_dd_path_catalog.cpp new file mode 100644 index 00000000..a87ae80d --- /dev/null +++ b/tests/contract/real_dd_path_catalog.cpp @@ -0,0 +1,85 @@ +// The shared real-DD-path catalog (issue #46). See real_dd_path_catalog.h for +// what belongs here (curated DD paths + terminal-gap DD facts) and what does +// not (backend-specific verdicts). Curated against DD 4.1.1 +// (build-mdsplus/_deps/data-dictionary-src/IDSDef.xml). + +#include "real_dd_path_catalog.h" + +namespace al_contract { +namespace real_dd { + +const std::vector& catalog() { + // Exactly one entry per (dt, rank) cell, in (dt, rank) order. The + // CoversEveryCellExactlyOnce meta-test (test_real_dd_path_catalog.cpp) is + // the guard that this stays a complete, duplicate-free kNumCells table. + static const std::vector kCatalog = { + // --- CHAR ----------------------------------------------------------- + {DType::Char, 0, "equilibrium", {}, "code/name", nullptr}, + {DType::Char, 1, "equilibrium", {}, "code/name", nullptr}, + {DType::Char, 2, "b_field_non_axisymmetric", {}, "control_surface_names", + nullptr}, + {DType::Char, 3, nullptr, {}, nullptr, + "DD 4.1.1 has no STR_2D+ at all (only STR_0D/STR_1D exist -- there is " + "no DD concept a CHAR dim>=3 could model)"}, + {DType::Char, 4, nullptr, {}, nullptr, "see rank 3"}, + {DType::Char, 5, nullptr, {}, nullptr, "see rank 3"}, + {DType::Char, 6, nullptr, {}, nullptr, "see rank 3"}, + {DType::Char, 7, nullptr, {}, nullptr, "see rank 3"}, + + // --- INTEGER -------------------------------------------------------- + {DType::Int, 0, "amns_data", {}, "z_n", nullptr}, + {DType::Int, 1, "magnetics", {}, "code/output_flag", nullptr}, + {DType::Int, 2, "magnetics", {}, "b_field_pol_probe_equivalent", + nullptr}, + {DType::Int, 3, "temporary", {"constant_integer3d"}, "value", nullptr}, + {DType::Int, 4, nullptr, {}, nullptr, + "DD 4.1.1 has no INT_4D+ anywhere (max is INT_3D, itself only inside a " + "struct_array)"}, + {DType::Int, 5, nullptr, {}, nullptr, "see rank 4"}, + {DType::Int, 6, nullptr, {}, nullptr, "see rank 4"}, + {DType::Int, 7, nullptr, {}, nullptr, "see rank 4"}, + + // --- DOUBLE --------------------------------------------------------- + {DType::Double, 0, "amns_data", {}, "a", nullptr}, + {DType::Double, 1, "balance_of_plant", {}, "gain_plant", nullptr}, + {DType::Double, 2, "bolometer", {}, "grid/volume_element", nullptr}, + {DType::Double, 3, "bolometer", {}, "power_density/data", nullptr}, + {DType::Double, 4, "gyrokinetics_local", {}, + "non_linear/fluxes_4d/particles_phi_potential", nullptr}, + {DType::Double, 5, "gyrokinetics_local", {}, + "non_linear/fluxes_5d/particles_phi_potential", nullptr}, + {DType::Double, 6, "temporary", {"constant_float6d"}, "value", nullptr}, + {DType::Double, 7, nullptr, {}, nullptr, + "DD 4.1.1 has no FLT_7D (the DD's own array-rank ceiling for DOUBLE is " + "6, one short of MAXDIM)"}, + + // --- COMPLEX -------------------------------------------------------- + {DType::Complex, 0, nullptr, {}, nullptr, + "DD 4.1.1 has no CPX_0D -- no scalar complex type exists at all"}, + {DType::Complex, 1, "waves", + {"coherent_wave", "full_wave", "e_field/plus"}, "values", nullptr}, + {DType::Complex, 2, "gyrokinetics_local", {}, + "non_linear/fields_zonal_2d/phi_potential_perturbed_norm", nullptr}, + {DType::Complex, 3, "runaway_electrons", {"distribution/markers"}, + "orbit_integrals_instant/values", nullptr}, + {DType::Complex, 4, "gyrokinetics_local", {}, + "non_linear/fields_4d/phi_potential_perturbed_norm", nullptr}, + {DType::Complex, 5, "runaway_electrons", {"distribution/markers"}, + "orbit_integrals/values", nullptr}, + {DType::Complex, 6, nullptr, {}, nullptr, + "DD 4.1.1 has no CPX_6D anywhere"}, + {DType::Complex, 7, nullptr, {}, nullptr, + "DD 4.1.1 has no CPX_7D anywhere"}, + }; + return kCatalog; +} + +std::string case_name(DType dt, int rank) { + static const char* const kNames[] = {"CHAR", "INTEGER", "DOUBLE", + "COMPLEX"}; + return std::string(kNames[static_cast(dt)]) + "_r" + + std::to_string(rank); +} + +} // namespace real_dd +} // namespace al_contract diff --git a/tests/contract/real_dd_path_catalog.h b/tests/contract/real_dd_path_catalog.h new file mode 100644 index 00000000..7d03bfd9 --- /dev/null +++ b/tests/contract/real_dd_path_catalog.h @@ -0,0 +1,90 @@ +// Shared real-DD-path catalog for the MDSplus (#15) and UDA (#25) breadth +// matrices (issue #46, TRACEABILITY.md Parts 4/5). +// +// Both real-path suites sweep the same {CHAR, INTEGER, DOUBLE, COMPLEX} x rank +// {0 .. MAXDIM} space against curated *real* DD-4.1.1 paths (a made-up field +// name has no node in the schema either backend validates against). The path +// for each (datatype, rank) cell -- the IDS, the struct_array chain to enter, +// the leaf, and whether DD 4.1.1 even has such a field -- is a fact about the +// DD, identical for both backends. This header is the single source of truth +// for that catalog so a path correction, a terminal-gap rationale, or a new +// (type, rank) cell is edited in exactly one place and the two matrices cannot +// silently diverge (issue #46). +// +// What stays OUT of the catalog, on purpose: backend-specific expected +// outcomes. A cell can be a plain pass on one backend and a storage-model +// divergence or a known xfail on the other (e.g. MDSplus's STR_0D dimension +// divergence vs UDA's dynamic-leaf-inside-AOS defect). Those verdicts live +// local to each backend suite; only the shared curated path lives here. +// +// Path curation method (carried over from test_mdsplus_real_paths.cpp's +// header): resolved by walking the actual baked-from artifact, +// build-mdsplus/_deps/data-dictionary-src/IDSDef.xml (the DD 4.1.1 source +// ALBuildDataDictionary.cmake downloads and the model tree is compiled from), +// to find, per (type, rank) cell, a real field of that data_type together with +// its struct_array ancestor chain -- empirical characterization against the +// real shipped artifact (TEST_STRATEGY D2). imas-dd MCP validation is used for +// catalog changes. +#ifndef AL_CONTRACT_REAL_DD_PATH_CATALOG_H +#define AL_CONTRACT_REAL_DD_PATH_CATALOG_H + +#include +#include + +namespace al_contract { +namespace real_dd { + +// The four C-ABI scalar datatypes the breadth matrices sweep. +enum class DType { Char, Int, Double, Complex }; + +// The DD version every curated path in this catalog was validated against. +inline constexpr const char* kCatalogDdVersion = "4.1.1"; + +// The swept space: kNumDTypes datatypes x ranks [kMinRank, kMaxRank]. +inline constexpr int kNumDTypes = 4; +inline constexpr int kMinRank = 0; +inline constexpr int kMaxRank = 7; // MAXDIM +inline constexpr int kNumRanks = kMaxRank - kMinRank + 1; +inline constexpr int kNumCells = kNumDTypes * kNumRanks; + +// One curated (datatype, rank) cell -- the single source of truth for the DD +// path a backend breadth matrix stores at that cell. +// +// `terminal_gap_reason` non-null means DD kCatalogDdVersion has no field of +// this (dt, rank) at any nesting depth (a fact about the DD, shared by every +// backend); `ids`/`leaf` are then null and `aos_chain` empty. Otherwise the +// path fields name a real DD-conformant field: `aos_chain` lists the +// struct_array field(s) to enter in order (each may itself be a multi-segment +// path through intervening plain structures, e.g. "e_field/plus"); empty means +// the leaf hangs directly off the GLOBAL op. +// +// The CHAR rank axis is the raw C-ABI `dim` argument, one off from the DD's +// STR_ND suffix (IMAS convention: a *string* is CHAR dim=1, an *array of +// strings* is CHAR dim=2; dim=0 is a bare scalar char reusing the STR_0D node). +// INTEGER/DOUBLE/COMPLEX map dim directly to their _ND suffix. +struct CatalogEntry { + DType dt; + int rank; + const char* ids; // null iff terminal gap + std::vector aos_chain; // struct_array chain to enter + const char* leaf; // null iff terminal gap + const char* terminal_gap_reason; // null iff a real path exists +}; + +// The full curated catalog: exactly one entry per (dt, rank) cell, kNumCells +// entries, curated against DD kCatalogDdVersion. +const std::vector& catalog(); + +// Canonical, stable parameter-name suffix for a cell, e.g. "DOUBLE_r3". Used +// verbatim as the INSTANTIATE_TEST_SUITE_P case-name in both backend suites, so +// existing test names / TRACEABILITY references stay stable across the #46 +// extraction. +std::string case_name(DType dt, int rank); +inline std::string case_name(const CatalogEntry& e) { + return case_name(e.dt, e.rank); +} + +} // namespace real_dd +} // namespace al_contract + +#endif // AL_CONTRACT_REAL_DD_PATH_CATALOG_H diff --git a/tests/contract/test_mdsplus_real_paths.cpp b/tests/contract/test_mdsplus_real_paths.cpp index 1ea4a304..491b9dfe 100644 --- a/tests/contract/test_mdsplus_real_paths.cpp +++ b/tests/contract/test_mdsplus_real_paths.cpp @@ -13,15 +13,10 @@ // multiple IDSs (the model tree is baked from the whole IDSDef.xml, so this // exercises that whole-DD bake, not one container). // -// Path curation method: the imas-dd MCP tool named in CLAUDE.md was not -// present in this session's MCP configuration, so the same question it would -// answer was resolved by walking the actual baked-from artifact directly -- -// build-mdsplus/_deps/data-dictionary-src/IDSDef.xml (the DD-4.1.1 source -// ALBuildDataDictionary.cmake downloads and the model tree is compiled from) -// -- to find, per (type, rank) cell, a real field of that data_type together -// with its struct_array ancestor chain (if any). This is still empirical -// characterization against the real shipped artifact (TEST_STRATEGY D2), -// simply queried by hand instead of through the MCP wrapper. +// The curated path set is the shared real_dd::catalog() (issue #46): the IDS, +// struct_array chain, leaf, and terminal-gap DD facts are identical for MDSplus +// and UDA and live in real_dd_path_catalog.{h,cpp}. Only MDSplus's own +// storage-model verdicts stay here (kDivergences below). // // Per (type, rank) cell, one of three shapes: // * plain leaf -- straightforward GLOBAL write/read, exactly like a @@ -50,6 +45,7 @@ #ifdef AL_CONTRACT_HAVE_MDSPLUS #include "al_contract.h" +#include "real_dd_path_catalog.h" #include #include @@ -57,44 +53,37 @@ #include #include +#include #include +#include #include using al_contract::PulseId; +using al_contract::real_dd::CatalogEntry; +using al_contract::real_dd::DType; namespace { -enum class DType { Char, Int, Double, Complex }; - -// One (type, rank) cell. `aos_chain` lists the struct_array field(s) to enter -// in order (each may itself be a multi-segment path through intervening plain -// structures, e.g. "e_field/plus" or "distribution/markers"); empty means the -// leaf hangs directly off the GLOBAL op. `terminal_gap_reason` non-null means -// the DD contains no field of this (type, rank) anywhere and the rest of the -// row is ignored. `divergence_reason` non-null means the cell round-tripped -// but not as a plain pass -- a legitimate storage-model difference empirically -// confirmed by running it (see CHAR_r0 below), not a defect (D2); the round -// trip is still attempted up to the point that confirms the divergence, then -// the test self-reports via GTEST_SKIP() rather than asserting equality. -struct PathSpec { - DType dt; - int rank; - const char* ids; - std::vector aos_chain; - const char* leaf; - const char* terminal_gap_reason; - const char* divergence_reason = nullptr; +// MDSplus's own verdict for a catalog cell, layered on top of the shared path. +// `divergence_reason` non-null means the cell round-tripped but not as a plain +// pass -- a legitimate storage-model difference empirically confirmed by +// running it (see CHAR_r0 below), not a defect (D2); the round trip is +// attempted up to the point that confirms the divergence, then the test +// self-reports via GTEST_SKIP() rather than asserting equality. terminal-gap +// cells (a DD fact) come straight from the catalog entry. +struct MdsplusCell { + const CatalogEntry* entry; + const char* divergence_reason; }; -// Curated against build-mdsplus/_deps/data-dictionary-src/IDSDef.xml (DD -// 4.1.1). CHAR's rank axis is the raw C-ABI `dim` argument, which is one off -// from the DD's STR_ND suffix (IMAS convention: a *string* is CHAR dim=1, an -// *array of strings* is CHAR dim=2 -- dim=0 is a bare scalar char with no DD -// analogue of its own, reusing the same STR_0D node as dim=1). INTEGER, -// DOUBLE, and COMPLEX map dim directly to their _ND suffix. -const PathSpec kPathSpecs[] = { - // --- CHAR --------------------------------------------------------------- - {DType::Char, 0, "equilibrium", {}, "code/name", nullptr, +// The only MDSplus-specific divergence: CHAR dim=0 against a real STR_0D node. +struct Divergence { + DType dt; + int rank; + const char* reason; +}; +const Divergence kDivergences[] = { + {DType::Char, 0, "MDSplus's real STR_0D node is inherently string-shaped (1-D): reading a " "dim=0 write back reports CHAR_DATA in 1D, not 0D ('Wrong dimension of " "Data returned by backend'), confirmed empirically. Not a crash/defect " @@ -103,62 +92,29 @@ const PathSpec kPathSpecs[] = { "(the DD's smallest text type, STR_0D, is a *string*, i.e. dim=1); " "against a real DD-conformant node the backend correctly refuses the " "mismatched rank instead of silently misreading it"}, - {DType::Char, 1, "equilibrium", {}, "code/name", nullptr}, - {DType::Char, 2, "b_field_non_axisymmetric", {}, "control_surface_names", - nullptr}, - {DType::Char, 3, nullptr, {}, nullptr, - "DD 4.1.1 has no STR_2D+ at all (only STR_0D/STR_1D exist -- there is no " - "DD concept a CHAR dim>=3 could model)"}, - {DType::Char, 4, nullptr, {}, nullptr, "see rank 3"}, - {DType::Char, 5, nullptr, {}, nullptr, "see rank 3"}, - {DType::Char, 6, nullptr, {}, nullptr, "see rank 3"}, - {DType::Char, 7, nullptr, {}, nullptr, "see rank 3"}, - - // --- INTEGER -------------------------------------------------------------- - {DType::Int, 0, "amns_data", {}, "z_n", nullptr}, - {DType::Int, 1, "magnetics", {}, "code/output_flag", nullptr}, - {DType::Int, 2, "magnetics", {}, "b_field_pol_probe_equivalent", nullptr}, - {DType::Int, 3, "temporary", {"constant_integer3d"}, "value", nullptr}, - {DType::Int, 4, nullptr, {}, nullptr, - "DD 4.1.1 has no INT_4D+ anywhere (max is INT_3D, itself only inside a " - "struct_array)"}, - {DType::Int, 5, nullptr, {}, nullptr, "see rank 4"}, - {DType::Int, 6, nullptr, {}, nullptr, "see rank 4"}, - {DType::Int, 7, nullptr, {}, nullptr, "see rank 4"}, - - // --- DOUBLE ------------------------------------------------------------- - {DType::Double, 0, "amns_data", {}, "a", nullptr}, - {DType::Double, 1, "balance_of_plant", {}, "gain_plant", nullptr}, - {DType::Double, 2, "bolometer", {}, "grid/volume_element", nullptr}, - {DType::Double, 3, "bolometer", {}, "power_density/data", nullptr}, - {DType::Double, 4, "gyrokinetics_local", {}, - "non_linear/fluxes_4d/particles_phi_potential", nullptr}, - {DType::Double, 5, "gyrokinetics_local", {}, - "non_linear/fluxes_5d/particles_phi_potential", nullptr}, - {DType::Double, 6, "temporary", {"constant_float6d"}, "value", nullptr}, - {DType::Double, 7, nullptr, {}, nullptr, - "DD 4.1.1 has no FLT_7D (the DD's own array-rank ceiling for DOUBLE is " - "6, one short of MAXDIM)"}, - - // --- COMPLEX -------------------------------------------------------------- - {DType::Complex, 0, nullptr, {}, nullptr, - "DD 4.1.1 has no CPX_0D -- no scalar complex type exists at all"}, - {DType::Complex, 1, "waves", {"coherent_wave", "full_wave", "e_field/plus"}, - "values", nullptr}, - {DType::Complex, 2, "gyrokinetics_local", {}, - "non_linear/fields_zonal_2d/phi_potential_perturbed_norm", nullptr}, - {DType::Complex, 3, "runaway_electrons", {"distribution/markers"}, - "orbit_integrals_instant/values", nullptr}, - {DType::Complex, 4, "gyrokinetics_local", {}, - "non_linear/fields_4d/phi_potential_perturbed_norm", nullptr}, - {DType::Complex, 5, "runaway_electrons", {"distribution/markers"}, - "orbit_integrals/values", nullptr}, - {DType::Complex, 6, nullptr, {}, nullptr, - "DD 4.1.1 has no CPX_6D anywhere"}, - {DType::Complex, 7, nullptr, {}, nullptr, - "DD 4.1.1 has no CPX_7D anywhere"}, }; +const char* divergence_for(DType dt, int rank) { + for (const Divergence& d : kDivergences) { + if (d.dt == dt && d.rank == rank) return d.reason; + } + return nullptr; +} + +// Derive the MDSplus matrix 1:1 from the shared catalog: exactly one cell per +// catalog entry, in catalog order, with MDSplus's local verdict attached. The +// CoversEveryCatalogKeyOnce meta-test pins that this mapping stays a bijection. +const std::vector& mdsplus_cells() { + static const std::vector kCells = [] { + std::vector cells; + for (const CatalogEntry& e : al_contract::real_dd::catalog()) { + cells.push_back({&e, divergence_for(e.dt, e.rank)}); + } + return cells; + }(); + return kCells; +} + // --- write/read a leaf field at whatever context it lives in ---------------- template void write_leaf(int ctx, const char* leaf, const std::vector& shape, @@ -184,7 +140,7 @@ void read_leaf_and_expect(int ctx, const char* leaf, int rank, // (if any, one al_begin_arraystruct_action per segment, a single element // each), write the leaf, read it back, assert self-consistency. template -void run_cell(const PathSpec& spec) { +void run_cell(const CatalogEntry& spec) { al_contract::TempBase base; PulseId pulse{/*database=*/"test", /*version=*/"3", /*pulse=*/15, /*run=*/0}; @@ -256,18 +212,19 @@ void run_cell(const PathSpec& spec) { EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); } -class MdsplusRealPathMatrix : public ::testing::TestWithParam { +class MdsplusRealPathMatrix : public ::testing::TestWithParam { protected: void SetUp() override { AL_CONTRACT_SKIP_IF_MDSPLUS_UNCONFIGURED(); } }; TEST_P(MdsplusRealPathMatrix, RealPathRoundTrip) { - const PathSpec spec = GetParam(); + const MdsplusCell cell = GetParam(); + const CatalogEntry& spec = *cell.entry; if (spec.terminal_gap_reason != nullptr) { GTEST_SKIP() << "terminal-gap: " << spec.terminal_gap_reason; } - if (spec.divergence_reason != nullptr) { - GTEST_SKIP() << "divergence: " << spec.divergence_reason; + if (cell.divergence_reason != nullptr) { + GTEST_SKIP() << "divergence: " << cell.divergence_reason; } switch (spec.dt) { case DType::Char: @@ -288,19 +245,39 @@ TEST_P(MdsplusRealPathMatrix, RealPathRoundTrip) { // A free function, not an inline lambda: a brace-init-list's top-level commas // aren't paren-protected, so they would otherwise split this macro's argument // list at the preprocessor stage (INSTANTIATE_TEST_SUITE_P only tracks paren -// nesting, not brace nesting). +// nesting, not brace nesting). Case names come from the shared catalog so they +// stay stable across the #46 extraction (e.g. "DOUBLE_r3"). std::string NameMdsplusRealPathMatrixCase( - const ::testing::TestParamInfo& info) { - static const char* const kNames[] = {"CHAR", "INTEGER", "DOUBLE", - "COMPLEX"}; - return std::string(kNames[static_cast(info.param.dt)]) + "_r" + - std::to_string(info.param.rank); + const ::testing::TestParamInfo& info) { + return al_contract::real_dd::case_name(*info.param.entry); } INSTANTIATE_TEST_SUITE_P(Mdsplus, MdsplusRealPathMatrix, - ::testing::ValuesIn(kPathSpecs), + ::testing::ValuesIn(mdsplus_cells()), NameMdsplusRealPathMatrixCase); +// Meta-test (issue #46): the MDSplus matrix maps the shared catalog 1:1 -- one +// cell per catalog key, no duplicate, none dropped. A future refactor that +// filters cells out of mdsplus_cells() turns this red instead of silently +// shrinking the matrix. +TEST(MdsplusRealPathCatalog, CoversEveryCatalogKeyOnce) { + using Key = std::pair; + std::set matrix_keys; + for (const MdsplusCell& cell : mdsplus_cells()) { + ASSERT_NE(cell.entry, nullptr); + const Key k{static_cast(cell.entry->dt), cell.entry->rank}; + EXPECT_TRUE(matrix_keys.insert(k).second) + << "duplicate matrix key " + << al_contract::real_dd::case_name(*cell.entry); + } + std::set catalog_keys; + for (const CatalogEntry& e : al_contract::real_dd::catalog()) { + catalog_keys.insert({static_cast(e.dt), e.rank}); + } + EXPECT_EQ(matrix_keys, catalog_keys) + << "MDSplus matrix and catalog key sets diverge"; +} + } // namespace #endif // AL_CONTRACT_HAVE_MDSPLUS diff --git a/tests/contract/test_real_dd_path_catalog.cpp b/tests/contract/test_real_dd_path_catalog.cpp new file mode 100644 index 00000000..64535e98 --- /dev/null +++ b/tests/contract/test_real_dd_path_catalog.cpp @@ -0,0 +1,82 @@ +// Always-on meta-tests for the shared real-DD-path catalog (issue #46). +// +// These guard the catalog's own invariants (one verdict per cell, no duplicate +// keys) independently of any backend. The per-backend "the matrix covers every +// catalog key" meta-tests live in test_mdsplus_real_paths.cpp and +// test_uda_real_paths.cpp behind their compile gates, so a matrix cannot drop a +// cell without a red test on that backend. + +#include "real_dd_path_catalog.h" + +#include + +#include +#include + +namespace { + +using al_contract::real_dd::catalog; +using al_contract::real_dd::CatalogEntry; +using al_contract::real_dd::DType; +using al_contract::real_dd::kMaxRank; +using al_contract::real_dd::kMinRank; +using al_contract::real_dd::kNumCells; +using al_contract::real_dd::kNumDTypes; + +using Key = std::pair; // (datatype ordinal, rank) + +Key key_of(const CatalogEntry& e) { + return {static_cast(e.dt), e.rank}; +} + +// The catalog defines exactly one verdict for every (datatype, rank) cell in +// the swept space, with no duplicate and no missing key. This is the shared +// half of issue #46's "exactly one verdict for every catalog key, no +// duplicates" acceptance criterion; the per-backend halves pin that each +// matrix maps this same key set 1:1. +TEST(RealDdPathCatalog, CoversEveryCellExactlyOnce) { + std::set seen; + for (const CatalogEntry& e : catalog()) { + EXPECT_TRUE(seen.insert(key_of(e)).second) + << "duplicate catalog key: dtype=" << static_cast(e.dt) + << " rank=" << e.rank; + } + EXPECT_EQ(catalog().size(), static_cast(kNumCells)); + + // Every cell in the swept space is present. + for (int d = 0; d < kNumDTypes; ++d) { + for (int r = kMinRank; r <= kMaxRank; ++r) { + EXPECT_TRUE(seen.count({d, r}) == 1) + << "missing catalog key: dtype=" << d << " rank=" << r; + } + } +} + +// A terminal-gap cell carries its DD-fact reason and no path; a real-path cell +// carries a path and no gap reason. The two are mutually exclusive, so neither +// backend suite can misread a half-populated entry. +TEST(RealDdPathCatalog, PathAndTerminalGapAreMutuallyExclusive) { + for (const CatalogEntry& e : catalog()) { + const bool is_gap = e.terminal_gap_reason != nullptr; + if (is_gap) { + EXPECT_EQ(e.ids, nullptr) + << "terminal-gap cell " << al_contract::real_dd::case_name(e) + << " must not name an IDS"; + EXPECT_EQ(e.leaf, nullptr) + << "terminal-gap cell " << al_contract::real_dd::case_name(e) + << " must not name a leaf"; + EXPECT_TRUE(e.aos_chain.empty()) + << "terminal-gap cell " << al_contract::real_dd::case_name(e) + << " must not carry an AOS chain"; + } else { + EXPECT_NE(e.ids, nullptr) + << "real-path cell " << al_contract::real_dd::case_name(e) + << " must name an IDS"; + EXPECT_NE(e.leaf, nullptr) + << "real-path cell " << al_contract::real_dd::case_name(e) + << " must name a leaf"; + } + } +} + +} // namespace diff --git a/tests/contract/test_uda_real_paths.cpp b/tests/contract/test_uda_real_paths.cpp index 658e7645..20d647c7 100644 --- a/tests/contract/test_uda_real_paths.cpp +++ b/tests/contract/test_uda_real_paths.cpp @@ -1,13 +1,13 @@ // UDA real-DD-path datatype x rank breadth (issue #25, TRACEABILITY.md Part 5, // acceptance criterion 1/2). // -// This is the UDA analogue of test_mdsplus_real_paths.cpp (issue #15): the -// same curated real-DD-4.1.1 path set, reused as-is (identical kPathSpecs -// paths/ranks/aos_chains), because UDA hits the identical structural wall -// MDSplus does -- RoundTripMatrix's opaque synthetic paths have no node in -// the schema either backend validates against (MDSplus: a baked binary model -// tree; UDA: the DD XML walked at runtime, src/uda/uda_xml.cpp find_node()). -// See test_mdsplus_real_paths.cpp's header for the curation method. +// This is the UDA analogue of test_mdsplus_real_paths.cpp (issue #15): the same +// curated real-DD-4.1.1 path set, now the shared real_dd::catalog() (issue +// #46). UDA hits the identical structural wall MDSplus does -- RoundTripMatrix's +// opaque synthetic paths have no node in the schema either backend validates +// against (MDSplus: a baked binary model tree; UDA: the DD XML walked at +// runtime, src/uda/uda_xml.cpp find_node()). See real_dd_path_catalog.h for the +// curation method. // // Shape adaptation for UDA (PRD #21 / issue #24): UDA has no remote write, so // each cell is seed(HDF5)-then-reopen(UDA) instead of MDSplus's direct @@ -15,18 +15,23 @@ // * seed: FORCE_CREATE_PULSE via the plain HDF5 backend, write // ids_properties/homogeneous_time=1 (UDA's cache_mode=none read path // always resolves it first and throws if absent -- see test_uda.cpp), -// then the leaf (descending spec.aos_chain via +// then the leaf (descending the catalog aos_chain via // al_begin_arraystruct_action, exactly like the MDSplus fixture); // * reopen: OPEN_PULSE via the UDA backend in remote mode // (backend=hdf5&cache_mode=none), descend the same aos_chain, read the // leaf back, assert self-consistency against what was written. // -// Oracle: per-backend write->read self-consistency (same as MDSplus's -// version of this matrix) -- only the field *path* is real DD; the *content* -// stays opaque synthetic data (al_contract::synth_buffer). +// The path set is shared with MDSplus; only UDA's own verdicts stay here: +// kDivergences (CHAR r0's fixture-backend limitation) and kKnownDefects (the +// dynamic-leaf-inside-AOS defect pinned by UdaRealPathMatrixKnownDefects). +// +// Oracle: per-backend write->read self-consistency (same as MDSplus's version +// of this matrix) -- only the field *path* is real DD; the *content* stays +// opaque synthetic data (al_contract::synth_buffer). #ifdef AL_CONTRACT_HAVE_UDA #include "al_contract.h" +#include "real_dd_path_catalog.h" #include #include @@ -35,140 +40,103 @@ #include #include +#include #include +#include #include +using al_contract::real_dd::CatalogEntry; +using al_contract::real_dd::DType; + namespace { -enum class DType { Char, Int, Double, Complex }; - -// Same shape as test_mdsplus_real_paths.cpp's PathSpec. `terminal_gap_reason` -// is shared verbatim with the MDSplus matrix (it is a fact about DD 4.1.1 -// itself, not about either backend). `divergence_reason` is UDA-specific -- -// re-derived empirically against the reference stack, not copied from -// MDSplus's own divergence rows, since the two backends can diverge for -// different reasons on the same cell. -struct PathSpec { - DType dt; - int rank; - const char* ids; - std::vector aos_chain; - const char* leaf; - const char* terminal_gap_reason; - const char* divergence_reason = nullptr; - // Non-null iff the cell hits the genuine, newly-discovered defect pinned - // by UdaAosKnownDefects (test_uda_breadth.cpp): a *dynamic* (time-varying) - // leaf nested inside a struct_array silently comes back empty/absent - // through UDA remote mode + backend=hdf5, instead of what was written. - // Distinct from divergence_reason -- this is a real bug (xfail), not a - // legitimate storage-model difference; the cell is skipped in the breadth - // matrix (not run into the known-wrong assertion) and pinned exactly by - // UdaRealPathMatrixKnownDefects's DISABLED_/tripwire pair below. - const char* known_defect_reason = nullptr; +// UDA's own verdict for a catalog cell, layered on the shared path. +// `divergence_reason` non-null: a legitimate storage/fixture-model difference, +// skipped (not asserted). `known_defect_reason` non-null: the genuine, +// newly-discovered defect pinned by UdaAosKnownDefects (test_uda_breadth.cpp) +// -- a *dynamic* (time-varying) leaf nested inside a struct_array silently +// comes back empty/absent through UDA remote mode + backend=hdf5. Distinct from +// a divergence: this is a real bug (xfail), skipped in the breadth matrix and +// pinned exactly by UdaRealPathMatrixKnownDefects's DISABLED_/tripwire pair. +struct UdaCell { + const CatalogEntry* entry; + const char* divergence_reason; + const char* known_defect_reason; }; -// Keep the three exact known-defect cells as named specs so the breadth matrix -// and the DISABLED_/tripwire pair below exercise identical IDS paths, ranks, -// and AOS nesting without duplicating that configuration. -const PathSpec kComplexR1DynamicAosSpec{ - DType::Complex, 1, "waves", {"coherent_wave", "full_wave", "e_field/plus"}, - "values", nullptr, nullptr, - "dynamic leaf nested in a struct_array (3 AOS levels)"}; -const PathSpec kComplexR3DynamicAosSpec{ - DType::Complex, 3, "runaway_electrons", {"distribution/markers"}, - "orbit_integrals_instant/values", nullptr, nullptr, - "dynamic leaf nested in a struct_array (1 AOS level)"}; -const PathSpec kComplexR5DynamicAosSpec{ - DType::Complex, 5, "runaway_electrons", {"distribution/markers"}, - "orbit_integrals/values", nullptr, nullptr, - "dynamic leaf nested in a struct_array (1 AOS level)"}; - -// Identical path/rank/aos_chain curation to test_mdsplus_real_paths.cpp's -// kPathSpecs (issue #25 acceptance criterion 2: "curated real-path set reused -// as-is"). terminal_gap_reason carries over unchanged (a DD fact). CHAR r0's -// divergence_reason is UDA-specific -- see below. -const PathSpec kPathSpecs[] = { - // --- CHAR --------------------------------------------------------------- - // Skipped WITHOUT attempting the seed write: seeding this cell means - // writing a dim=0 (bare scalar) CHAR value through the plain HDF5 - // backend used for fixture setup, which is the pre-existing, - // independently-pinned HDF5 CHAR-scalar crash - // (RoundTripKnownDefects.DISABLED_Hdf5CharScalarRoundTrips / tripwire - // …CurrentlyCrashes, TRACEABILITY.md Part 1) -- confirmed to reproduce on - // a real DD leaf too (equilibrium's code/name, STR_0D), not just the - // synthetic path that test already pins. This is a fixture-setup-backend - // defect, not a UDA behavior, so it is recorded as a divergence (not - // exercised) rather than run into the crash. - {DType::Char, 0, "equilibrium", {}, "code/name", nullptr, +// UDA-specific verdicts keyed by (dtype, rank). Distinct from MDSplus's: +// the two backends diverge for different reasons on the same cell. +struct Verdict { + DType dt; + int rank; + const char* divergence_reason; + const char* known_defect_reason; +}; +const Verdict kVerdicts[] = { + // CHAR r0: seeding needs a dim=0 (bare scalar) CHAR write through the plain + // HDF5 backend used for fixture setup -- the pre-existing, independently- + // pinned HDF5 CHAR-scalar crash (RoundTripKnownDefects.DISABLED_ + // Hdf5CharScalarRoundTrips / tripwire …CurrentlyCrashes, TRACEABILITY.md + // Part 1), confirmed to reproduce on this real DD leaf (equilibrium's + // code/name, STR_0D) too. A fixture-backend defect, not a UDA behavior, so + // it is recorded as a divergence (not exercised) rather than run into the + // crash. + {DType::Char, 0, "seeding this cell needs a dim=0 CHAR write through the plain HDF5 " "backend (fixture setup) -- the pre-existing, independently-pinned " "HDF5 CHAR-scalar crash (RoundTripKnownDefects.DISABLED_" "Hdf5CharScalarRoundTrips / tripwire …CurrentlyCrashes, TRACEABILITY.md " "Part 1) reproduces on this real DD leaf too; not run to avoid crashing " - "the seed step. A fixture-backend defect, not a UDA behavior"}, - {DType::Char, 1, "equilibrium", {}, "code/name", nullptr}, - {DType::Char, 2, "b_field_non_axisymmetric", {}, "control_surface_names", + "the seed step. A fixture-backend defect, not a UDA behavior", nullptr}, - {DType::Char, 3, nullptr, {}, nullptr, - "DD 4.1.1 has no STR_2D+ at all (only STR_0D/STR_1D exist -- there is no " - "DD concept a CHAR dim>=3 could model)"}, - {DType::Char, 4, nullptr, {}, nullptr, "see rank 3"}, - {DType::Char, 5, nullptr, {}, nullptr, "see rank 3"}, - {DType::Char, 6, nullptr, {}, nullptr, "see rank 3"}, - {DType::Char, 7, nullptr, {}, nullptr, "see rank 3"}, - - // --- INTEGER -------------------------------------------------------------- - {DType::Int, 0, "amns_data", {}, "z_n", nullptr}, - {DType::Int, 1, "magnetics", {}, "code/output_flag", nullptr}, - {DType::Int, 2, "magnetics", {}, "b_field_pol_probe_equivalent", nullptr}, - {DType::Int, 3, "temporary", {"constant_integer3d"}, "value", nullptr}, - {DType::Int, 4, nullptr, {}, nullptr, - "DD 4.1.1 has no INT_4D+ anywhere (max is INT_3D, itself only inside a " - "struct_array)"}, - {DType::Int, 5, nullptr, {}, nullptr, "see rank 4"}, - {DType::Int, 6, nullptr, {}, nullptr, "see rank 4"}, - {DType::Int, 7, nullptr, {}, nullptr, "see rank 4"}, - - // --- DOUBLE ------------------------------------------------------------- - {DType::Double, 0, "amns_data", {}, "a", nullptr}, - {DType::Double, 1, "balance_of_plant", {}, "gain_plant", nullptr}, - {DType::Double, 2, "bolometer", {}, "grid/volume_element", nullptr}, - {DType::Double, 3, "bolometer", {}, "power_density/data", nullptr}, - {DType::Double, 4, "gyrokinetics_local", {}, - "non_linear/fluxes_4d/particles_phi_potential", nullptr}, - {DType::Double, 5, "gyrokinetics_local", {}, - "non_linear/fluxes_5d/particles_phi_potential", nullptr}, - {DType::Double, 6, "temporary", {"constant_float6d"}, "value", nullptr}, - {DType::Double, 7, nullptr, {}, nullptr, - "DD 4.1.1 has no FLT_7D (the DD's own array-rank ceiling for DOUBLE is " - "6, one short of MAXDIM)"}, - - // --- COMPLEX -------------------------------------------------------------- - {DType::Complex, 0, nullptr, {}, nullptr, - "DD 4.1.1 has no CPX_0D -- no scalar complex type exists at all"}, - // Known defect (xfail), not attempted: waves/coherent_wave/full_wave/ - // e_field/plus/values is a DD *dynamic* leaf 3 levels deep in nested - // struct_arrays -- confirmed to hit UdaAosKnownDefects's dynamic-leaf- - // inside-AOS defect (test_uda_breadth.cpp): the field comes back - // empty/absent through UDA remote mode + backend=hdf5 instead of what - // was written. - kComplexR1DynamicAosSpec, - {DType::Complex, 2, "gyrokinetics_local", {}, - "non_linear/fields_zonal_2d/phi_potential_perturbed_norm", nullptr}, - // Same known defect as COMPLEX r1 above: orbit_integrals_instant/values - // is a dynamic leaf nested one level inside runaway_electrons/ - // distribution/markers. - kComplexR3DynamicAosSpec, - {DType::Complex, 4, "gyrokinetics_local", {}, - "non_linear/fields_4d/phi_potential_perturbed_norm", nullptr}, - // Same known defect as COMPLEX r1/r3 above. - kComplexR5DynamicAosSpec, - {DType::Complex, 6, nullptr, {}, nullptr, - "DD 4.1.1 has no CPX_6D anywhere"}, - {DType::Complex, 7, nullptr, {}, nullptr, - "DD 4.1.1 has no CPX_7D anywhere"}, + // The three dynamic-leaf-inside-AOS known defects (xfail): the field comes + // back empty/absent through UDA remote mode + backend=hdf5 instead of what + // was written. Pinned by UdaRealPathMatrixKnownDefects below. + {DType::Complex, 1, nullptr, + "dynamic leaf nested in a struct_array (3 AOS levels)"}, + {DType::Complex, 3, nullptr, + "dynamic leaf nested in a struct_array (1 AOS level)"}, + {DType::Complex, 5, nullptr, + "dynamic leaf nested in a struct_array (1 AOS level)"}, }; +const Verdict* verdict_for(DType dt, int rank) { + for (const Verdict& v : kVerdicts) { + if (v.dt == dt && v.rank == rank) return &v; + } + return nullptr; +} + +// Derive the UDA matrix 1:1 from the shared catalog: one cell per catalog +// entry, in catalog order, with UDA's local verdict attached. The +// CoversEveryCatalogKeyOnce meta-test pins that this stays a bijection. +const std::vector& uda_cells() { + static const std::vector kCells = [] { + std::vector cells; + for (const CatalogEntry& e : al_contract::real_dd::catalog()) { + const Verdict* v = verdict_for(e.dt, e.rank); + cells.push_back({&e, v ? v->divergence_reason : nullptr, + v ? v->known_defect_reason : nullptr}); + } + return cells; + }(); + return kCells; +} + +// The subset of cells carrying the dynamic-leaf-inside-AOS known defect, for +// the DISABLED_/tripwire pair below. Derived from the same uda_cells() so the +// matrix and the pins exercise identical IDS paths, ranks, and AOS nesting. +const std::vector& uda_known_defect_cells() { + static const std::vector kCells = [] { + std::vector cells; + for (const UdaCell& c : uda_cells()) { + if (c.known_defect_reason != nullptr) cells.push_back(c); + } + return cells; + }(); + return kCells; +} + // --- write/read a leaf field at whatever context it lives in ---------------- template void write_leaf(int ctx, const char* leaf, const std::vector& shape, @@ -196,7 +164,7 @@ void read_leaf_and_expect(int ctx, const char* leaf, int rank, // run_cell), then reopen the identical pulse dir through the UDA backend in // remote mode and read the same path back. template -void run_cell_with_read_expectation(const PathSpec& spec, +void run_cell_with_read_expectation(const CatalogEntry& spec, ReadExpectation&& expect_read) { al_contract::TempBase base; const std::string pulse_dir = base.str() + "/pulse"; @@ -277,9 +245,9 @@ void run_cell_with_read_expectation(const PathSpec& spec, } template -void run_cell(const PathSpec& spec) { +void run_cell(const CatalogEntry& spec) { run_cell_with_read_expectation( - spec, [](int ctx, const PathSpec& read_spec, + spec, [](int ctx, const CatalogEntry& read_spec, const std::vector& shape, const std::vector& written) { read_leaf_and_expect(ctx, read_spec.leaf, read_spec.rank, shape, @@ -287,21 +255,22 @@ void run_cell(const PathSpec& spec) { }); } -class UdaRealPathMatrix : public ::testing::TestWithParam { +class UdaRealPathMatrix : public ::testing::TestWithParam { protected: void SetUp() override { AL_CONTRACT_SKIP_IF_UDA_UNCONFIGURED(); } }; TEST_P(UdaRealPathMatrix, RealPathRoundTrip) { - const PathSpec spec = GetParam(); + const UdaCell cell = GetParam(); + const CatalogEntry& spec = *cell.entry; if (spec.terminal_gap_reason != nullptr) { GTEST_SKIP() << "terminal-gap: " << spec.terminal_gap_reason; } - if (spec.divergence_reason != nullptr) { - GTEST_SKIP() << "divergence: " << spec.divergence_reason; + if (cell.divergence_reason != nullptr) { + GTEST_SKIP() << "divergence: " << cell.divergence_reason; } - if (spec.known_defect_reason != nullptr) { - GTEST_SKIP() << "known-defect (xfail): " << spec.known_defect_reason; + if (cell.known_defect_reason != nullptr) { + GTEST_SKIP() << "known-defect (xfail): " << cell.known_defect_reason; } switch (spec.dt) { case DType::Char: @@ -322,40 +291,61 @@ TEST_P(UdaRealPathMatrix, RealPathRoundTrip) { // A free function, not an inline lambda: a brace-init-list's top-level commas // aren't paren-protected, so they would otherwise split this macro's argument // list at the preprocessor stage (INSTANTIATE_TEST_SUITE_P only tracks paren -// nesting, not brace nesting). +// nesting, not brace nesting). Case names come from the shared catalog so they +// stay stable across the #46 extraction (e.g. "COMPLEX_r1"). std::string NameUdaRealPathMatrixCase( - const ::testing::TestParamInfo& info) { - static const char* const kNames[] = {"CHAR", "INTEGER", "DOUBLE", - "COMPLEX"}; - return std::string(kNames[static_cast(info.param.dt)]) + "_r" + - std::to_string(info.param.rank); + const ::testing::TestParamInfo& info) { + return al_contract::real_dd::case_name(*info.param.entry); } -INSTANTIATE_TEST_SUITE_P(Uda, UdaRealPathMatrix, ::testing::ValuesIn(kPathSpecs), +INSTANTIATE_TEST_SUITE_P(Uda, UdaRealPathMatrix, + ::testing::ValuesIn(uda_cells()), NameUdaRealPathMatrixCase); -// Exact D2 pins for the three COMPLEX matrix cells affected by the dynamic- -// leaf-inside-AOS defect. The disabled correct-contract test and enabled +// Meta-test (issue #46): the UDA matrix maps the shared catalog 1:1 -- one cell +// per catalog key, no duplicate, none dropped. A future refactor that filters +// cells out of uda_cells() turns this red instead of silently shrinking the +// matrix. +TEST(UdaRealPathCatalog, CoversEveryCatalogKeyOnce) { + using Key = std::pair; + std::set matrix_keys; + for (const UdaCell& cell : uda_cells()) { + ASSERT_NE(cell.entry, nullptr); + const Key k{static_cast(cell.entry->dt), cell.entry->rank}; + EXPECT_TRUE(matrix_keys.insert(k).second) + << "duplicate matrix key " + << al_contract::real_dd::case_name(*cell.entry); + } + std::set catalog_keys; + for (const CatalogEntry& e : al_contract::real_dd::catalog()) { + catalog_keys.insert({static_cast(e.dt), e.rank}); + } + EXPECT_EQ(matrix_keys, catalog_keys) + << "UDA matrix and catalog key sets diverge"; +} + +// Exact D2 pins for the COMPLEX matrix cells affected by the dynamic-leaf- +// inside-AOS defect. The disabled correct-contract test and enabled // current-behavior tripwire deliberately share run_cell_with_read_expectation, // so both traverse the same public C ABI and the same real DD path. A targeted // fix to any one rank/path makes that case's tripwire fail independently. class UdaRealPathMatrixKnownDefects - : public ::testing::TestWithParam { + : public ::testing::TestWithParam { protected: void SetUp() override { AL_CONTRACT_SKIP_IF_UDA_UNCONFIGURED(); } }; TEST_P(UdaRealPathMatrixKnownDefects, DISABLED_DynamicComplexLeafInsideAosRoundTrips) { - run_cell>(GetParam()); + run_cell>(*GetParam().entry); } TEST_P(UdaRealPathMatrixKnownDefects, DynamicComplexLeafInsideAosCurrentlyReadsEmpty) { run_cell_with_read_expectation>( - GetParam(), [](int ctx, const PathSpec& spec, - const std::vector&, - const std::vector>&) { + *GetParam().entry, [](int ctx, const CatalogEntry& spec, + const std::vector&, + const std::vector>&) { std::vector read_shape; std::vector> read_data; AL_EXPECT_OK(al_contract::read_data>( @@ -368,16 +358,9 @@ TEST_P(UdaRealPathMatrixKnownDefects, }); } -const PathSpec kComplexDynamicAosKnownDefectSpecs[] = { - kComplexR1DynamicAosSpec, - kComplexR3DynamicAosSpec, - kComplexR5DynamicAosSpec, -}; - -INSTANTIATE_TEST_SUITE_P( - Uda, UdaRealPathMatrixKnownDefects, - ::testing::ValuesIn(kComplexDynamicAosKnownDefectSpecs), - NameUdaRealPathMatrixCase); +INSTANTIATE_TEST_SUITE_P(Uda, UdaRealPathMatrixKnownDefects, + ::testing::ValuesIn(uda_known_defect_cells()), + NameUdaRealPathMatrixCase); } // namespace From 6695ecd798b4f2d0c0009ab944e73b94a5b89bed Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:29:56 +0200 Subject: [PATCH 13/17] Make UDA AOS traversal detect cursor no-ops Use a static DD-4.1.1 AOS leaf with three index-bound values so repeated or reordered traversal fails through the public C ABI. The old dynamic-leaf sentinel tripwire observed identical values and could not distinguish a successful no-op iterator. (cherry picked from commit cd20a87247cd2c10cec2a4a1fda75e957b405f0e) --- tests/contract/TRACEABILITY.md | 2 +- tests/contract/test_uda_breadth.cpp | 110 +++++++++++++++++++++++----- 2 files changed, 91 insertions(+), 21 deletions(-) diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 551b7f2b..f143bc85 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -733,7 +733,7 @@ fails if the UDA matrix ever drops or duplicates a catalog cell. | ↳ 13 (type, rank) cells the DD contains nowhere at any nesting depth (same facts as MDSplus's Part 4 breadth matrix): CHAR r3–r7, INTEGER r4–r7, DOUBLE r7, COMPLEX r0, COMPLEX r6–r7 | `Uda/UdaRealPathMatrix.RealPathRoundTrip/{CHAR_r3..CHAR_r7,INTEGER_r4..INTEGER_r7,DOUBLE_r7,COMPLEX_r0,COMPLEX_r6,COMPLEX_r7}` (13 cases, each `GTEST_SKIP()`) | terminal-gap | | ↳ 3 (type, rank) cells hit the new dynamic-leaf-inside-AOS defect: COMPLEX r1 (`waves`, 3 AOS levels), COMPLEX r3/r5 (`runaway_electrons`, 1 AOS level) | Breadth cells: `Uda/UdaRealPathMatrix.RealPathRoundTrip/{COMPLEX_r1,COMPLEX_r3,COMPLEX_r5}` (`GTEST_SKIP()`, known-defect). Exact correct-contract pair: `Uda/UdaRealPathMatrixKnownDefects.DISABLED_DynamicComplexLeafInsideAosRoundTrips/{COMPLEX_r1,COMPLEX_r3,COMPLEX_r5}` + tripwires `Uda/UdaRealPathMatrixKnownDefects.DynamicComplexLeafInsideAosCurrentlyReadsEmpty/{COMPLEX_r1,COMPLEX_r3,COMPLEX_r5}` | **xfail** | -## AOS traversal, occurrences, list_filled_paths, pulse modes/errors, slice/time-range (issue #25) +## AOS traversal, occurrences, list_filled_paths, pulse modes/errors, slice/time-range (issues #25 + #37) `test_uda_breadth.cpp` — every remaining C-ABI-reachable read-side capability and pulse-lifecycle mode from `FUNCTIONALITY_INVENTORY.md` Part 1 not already diff --git a/tests/contract/test_uda_breadth.cpp b/tests/contract/test_uda_breadth.cpp index ad6ae01b..2b98f140 100644 --- a/tests/contract/test_uda_breadth.cpp +++ b/tests/contract/test_uda_breadth.cpp @@ -79,26 +79,96 @@ class UdaBreadthTest : public ::testing::Test { }; // =========================================================================== -// AOS traversal (top-level): equilibrium/time_slice is a real DD struct_array -// (timebasepath="time"), each element carrying global_quantities/ip -- the -// same real DD AOS path MDSplus's own unique-surface/parity proof uses -// (TRACEABILITY.md Part 4). Seeded with 3 elements through HDF5, reopened -// through UDA: al_begin_arraystruct_action correctly reports size 3 and -// iterates (confirmed via the debug trace: three distinct -// "time_slice[0..2]/global_quantities/ip" directives are sent), but the -// VALUE that comes back for every element is the HDF5 "absent" sentinel -// (-9.0e40, al_contract::kEmptyDouble), not what was written -- a genuine -// NEW DEFECT, not a legitimate storage difference, pinned per D2 below -// (UdaAosKnownDefects). Root cause traced empirically: `global_quantities/ip` -// is a DD *dynamic* (time-varying) leaf (confirmed via the request trace's -// `dynamic_flags=1`), and every dynamic leaf nested inside a struct_array -// fails identically through UDA remote mode + backend=hdf5 -- see -// UdaRealPathMatrix's COMPLEX_r1/r3/r5 rows (test_uda_real_paths.cpp), which -// hit the exact same wall on a different datatype/AOS-depth (their -// `known_defect_reason`, not `divergence_reason`, cites this test). Static -// (non-dynamic) leaves nested in a struct_array -- e.g. `temporary`'s -// self-test IDS fields -- are unaffected (UdaRealPathMatrix's INTEGER_r3/ -// DOUBLE_r6 pass cleanly). +// AOS traversal (top-level): temporary/constant_integer3d is a real DD-4.1.1 +// struct_array whose static `value` leaf already round-trips through UDA (the +// INTEGER_r3 real-path matrix cell). Three order-sensitive elements make this +// an active C-ABI proof that al_iterate_over_arraystruct advances the cursor: +// a successful no-op, repeated element, or reordered element all fail. +// =========================================================================== + +TEST_F(UdaBreadthTest, AosTraversalAdvancesAcrossDistinctStaticElements) { + const std::string pulse_dir = fresh_pulse_dir(); + constexpr int kN = 3; + const std::vector kShape = {1, 1, 1}; + const std::vector> kExpected = {{101}, {202}, {303}}; + + // --- seed via HDF5 ------------------------------------------------------- + { + const std::string hdf5_uri = al_contract::hdf5_uri_for(pulse_dir); + int pulse_ctx = -1; + AL_ASSERT_OK(al_begin_dataentry_action(hdf5_uri.c_str(), + FORCE_CREATE_PULSE, &pulse_ctx)); + + int op = -1; + AL_ASSERT_OK( + al_begin_global_action(pulse_ctx, "temporary", "", WRITE_OP, &op)); + AL_EXPECT_OK(al_contract::write_data( + op, "ids_properties/homogeneous_time", {}, {1})); + + int size = kN; + int aos = -1; + AL_ASSERT_OK(al_begin_arraystruct_action( + op, "constant_integer3d", "", &size, &aos)); + // Seed physical indexes in 0, 2, 1 order so fixture preparation does + // not mirror the forward 0, 1, 2 traversal asserted below. This makes + // a reordered read observable even if iteration is changed globally. + AL_EXPECT_OK(al_contract::write_data(aos, "value", kShape, + kExpected[0])); + AL_ASSERT_OK(al_iterate_over_arraystruct(aos, 2)); + AL_EXPECT_OK(al_contract::write_data(aos, "value", kShape, + kExpected[2])); + AL_ASSERT_OK(al_iterate_over_arraystruct(aos, -1)); + AL_EXPECT_OK(al_contract::write_data(aos, "value", kShape, + kExpected[1])); + AL_EXPECT_OK(al_end_action(aos)); + AL_EXPECT_OK(al_end_action(op)); + AL_ASSERT_OK(al_close_pulse(pulse_ctx, CLOSE_PULSE)); + } + + // --- reopen through UDA, remote mode ------------------------------------ + { + const std::string uda_uri = al_contract::uda_hdf5_uri_for(pulse_dir); + int pulse_ctx = -1; + AL_ASSERT_OK( + al_begin_dataentry_action(uda_uri.c_str(), OPEN_PULSE, &pulse_ctx)); + + int op = -1; + AL_ASSERT_OK( + al_begin_global_action(pulse_ctx, "temporary", "", READ_OP, &op)); + + int size = 0; + int aos = -1; + AL_ASSERT_OK(al_begin_arraystruct_action( + op, "constant_integer3d", "", &size, &aos)); + ASSERT_EQ(size, kN) << "AOS size must round-trip through UDA"; + for (int i = 0; i < size; ++i) { + SCOPED_TRACE(i); + std::vector shape; + std::vector data; + AL_ASSERT_OK( + al_contract::read_data(aos, "value", 3, &shape, &data)); + EXPECT_EQ(shape, kShape); + EXPECT_EQ(data, kExpected[static_cast(i)]) + << "AOS cursor must preserve element order"; + if (i + 1 < size) { + AL_ASSERT_OK(al_iterate_over_arraystruct(aos, 1)); + } + } + AL_ASSERT_OK(al_end_action(aos)); + + // The public C ABI promises a failure status for an invalid context. + EXPECT_NE(al_iterate_over_arraystruct(aos, 1).code, 0); + + AL_EXPECT_OK(al_end_action(op)); + AL_ASSERT_OK(al_close_pulse(pulse_ctx, CLOSE_PULSE)); + } +} + +// =========================================================================== +// Separate value-transport defect pin: equilibrium/time_slice is a real DD +// struct_array with a dynamic global_quantities/ip leaf. Its size and request +// paths advance, but UDA returns the HDF5 absent sentinel for every element. +// Keep this D2 pair independent of the discriminating traversal test above. // =========================================================================== // Seed 3 elements of equilibrium/time_slice via HDF5 (global_quantities/ip = From d1f60833e0da1484bd33ea01429cb836da884015 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:52:01 +0200 Subject: [PATCH 14/17] Document UDA cache invalidation terminal gap (cherry picked from commit a39e680acf00ed7e9ddf0a3087ebf695ce889d9f) --- tests/contract/TRACEABILITY.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index f143bc85..8f6ed81a 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -912,12 +912,23 @@ recorded here alongside the header's existing stack identifiers. that server process exits. Reproduces identically whether the read went through `readData`'s `None`-mode path or `populate_cache`'s `ids`/`struct`- mode path (both hit the same unstripped-uri bug), so this is not an - `ids`-specific finding despite living in the cache-mode area: the - client-side RAM cache genuinely *is* cleared on close - (`UDABackend::closePulse`), but a server-side resource opened on the read - path outlives it regardless. Pinned: + `ids`-specific finding despite living in the cache-mode area. Pinned: `UdaUniqueSurfaceTest.DISABLED_ClosingUdaSessionReleasesEveryServerSideHandle` + tripwire `UdaUniqueSurfaceTest.ClosingUdaSessionCurrentlyLeaksServerSideHandle`. +- **Client-cache invalidation across close/reopen is a terminal gap (issue + #38).** No stable public-C-ABI oracle exists against the pinned stack: the + required seed-A/read/close/mutate-to-B/reopen sequence cannot reach its + mutation step because the separately pinned server-handle leak keeps the + per-IDS HDF5 file locked. A fresh fixture process with + `HDF5_USE_FILE_LOCKING=FALSE` still fails to reopen that external IDS file + for writing (observed for `cache_mode=none`); the process therefore cannot + produce B for a new UDA context to observe. The C ABI provides no control to + restart the reference server/connection between those steps, and remote UDA + writes are independently unsupported. Do not treat + `UDABackend::closePulse`'s `cache_.clear()` as a behavioral verdict. The + paired server-handle-leak xfail immediately above remains distinct; when it + is fixed or the stack gains a restart seam, this row must become the + A→B→reopen test for `none`, `ids`, and `struct`. - **Runtime DD loading — present / absent / wrong-version, all through `al_begin_dataentry_action`** (the constructor loads `IDSDef.xml` unconditionally, before any network access): present is the reference @@ -1041,6 +1052,7 @@ recorded here alongside the header's existing stack identifiers. | Cache-mode invisibility: `none`/`ids`/`struct` agree on a field all three can reach (a static AOS-nested leaf) | `UdaUniqueSurfaceTest.CacheModeNoneIdsStructAgreeForAosCoveredField` | covered | | ↳ `struct` mode reports success but returns the absent sentinel for a genuinely written top-level field, so cache-mode selection changes the physics value | `UdaUniqueSurfaceTest.DISABLED_CacheModeStructPreservesTopLevelField` + tripwire `UdaUniqueSurfaceTest.CacheModeStructCurrentlyReturnsAbsentSentinelForTopLevelField` | **xfail** | | ↳ closing a UDA session leaks a server-side pulse handle (uri-stripping mismatch between `open`/`close` and every `get()`-directive builder), blocking a later local reopen of the same file | `UdaUniqueSurfaceTest.DISABLED_ClosingUdaSessionReleasesEveryServerSideHandle` + tripwire `UdaUniqueSurfaceTest.ClosingUdaSessionCurrentlyLeaksServerSideHandle` | **xfail** | +| ↳ client cache invalidation across close/reopen: the required A→B fixture mutation is blocked by that distinct leaked server handle; neither the C ABI nor the pinned stack offers a server/connection restart seam | issue #38's attempted C-ABI fixture mutation (fresh process, `HDF5_USE_FILE_LOCKING=FALSE`) deterministically fails before B can be written; do not infer a verdict from source inspection | **terminal gap** | | Runtime DD loading: present baseline and absent DD (both failure messages) | `UdaUniqueSurfaceTest.{DdPresentLoadsAndOpenSucceeds,DdAbsentNeitherEnvVarSetFailsWithClearMessage,DdAbsentFileMissingAtIdsDefPathFailsWithClearMessage}` | covered | | ↳ wrong-version DD loads silently with no semantic cross-version check | `UdaUniqueSurfaceTest.DISABLED_DdWrongVersionIsRejected` + tripwire `UdaUniqueSurfaceTest.DdWrongVersionCurrentlyLoadsSilentlyWithNoCrossVersionCheck` | **xfail** | | `datapath` partial-get via `cache_mode=ids`: in-scope field round-trips, out-of-scope field silently reads as absent | `UdaUniqueSurfaceTest.DatapathScopesCachePopulationFieldOutsideScopeReadsAsAbsent` | covered | From e8056a2e7068387c32c3c8db0851721363247d7f Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:18:19 +0200 Subject: [PATCH 15/17] Strengthen UDA timerange capability assertion (cherry picked from commit 92f84661bb1570742f16d5fb16d2bfe930edd021) --- tests/contract/TRACEABILITY.md | 14 ++++++++------ tests/contract/test_uda_unique_surface.cpp | 22 +++++++++------------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 8f6ed81a..e12121da 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -985,11 +985,12 @@ recorded here alongside the header's existing stack identifiers. with baseline `UdaUniqueSurfaceTest.MatchingStoredVersionOpensThroughUda`. - **`supportsTimeRangeOperation()` capability negotiation confirmed against the reference server**: the reference plugin reports `1.8.0` (issue #23), - so `1.8.0 > 1.4.0` grants the capability — `al_begin_timerange_action` must - pass `al_lowlevel.cpp`'s capability gate ("Selected backend does not - support time range operations.") rather than being refused outright, - distinct from the separate, already-pinned uninitialized-interpmode defect - on the subsequent read (`UdaSliceAndTimeRange`, above). + so `1.8.0 > 1.4.0` grants the capability — + `UdaUniqueSurfaceTest.TimeRangeCapabilityGrantedByReferenceServerVersion180` + asserts the exact begin-action status, `s.code == 0`, rather than merely + excluding the capability-refusal text. The separately pinned + uninitialized-interpmode defect occurs only on the subsequent read + (`UdaSliceAndTimeRange`, above). - **Fetch mode: download, local-backend handoff, cache reuse, all confirmed end-to-end (issue #27)** — the `BYTES` server plugin fetch mode needs ships and is registered **by default** in `ukaea/uda`'s own build (corrects this @@ -1057,7 +1058,8 @@ recorded here alongside the header's existing stack identifiers. | ↳ wrong-version DD loads silently with no semantic cross-version check | `UdaUniqueSurfaceTest.DISABLED_DdWrongVersionIsRejected` + tripwire `UdaUniqueSurfaceTest.DdWrongVersionCurrentlyLoadsSilentlyWithNoCrossVersionCheck` | **xfail** | | `datapath` partial-get via `cache_mode=ids`: in-scope field round-trips, out-of-scope field silently reads as absent | `UdaUniqueSurfaceTest.DatapathScopesCachePopulationFieldOutsideScopeReadsAsAbsent` | covered | | Version-drift check inertness: the client's own drift check never fires (both sides hardcoded placeholders); a pulse whose stored backend version (`999.0`, rewritten out-of-band by `hdf5_fixture_tool`) can never match is refused only by the forwarded SERVER-side HDF5 error (`UNKNOWN_ERR`, "No backend writer with version: 999.0") | `UdaUniqueSurfaceTest.DISABLED_OpenRefusesMismatchedStoredBackendVersion` + tripwire `UdaUniqueSurfaceTest.VersionDriftCheckCurrentlyDefersToServerSideRefusal` (baseline: `MatchingStoredVersionOpensThroughUda`) | **xfail** | -| Server-version-gated `supportsTimeRangeOperation()`: reference plugin 1.8.0 > 1.4.0 grants the capability | `UdaUniqueSurfaceTest.TimeRangeCapabilityGrantedByReferenceServerVersion180` | covered | +| Server-version-gated `supportsTimeRangeOperation()`: reference plugin 1.8.0 > 1.4.0 grants the capability; the exact `al_begin_timerange_action` verdict is `s.code == 0` | `UdaUniqueSurfaceTest.TimeRangeCapabilityGrantedByReferenceServerVersion180` | covered | +| ↳ Boundary (`<=1.4.0` refused, `>1.4.0` accepted): the pinned reference stack exposes only its fixed 1.8.0 server-owned `IMAS::version()` result, and the public C ABI has no reported-version override or injectable server fixture. The positive side is covered above; retain this explicit gap until a second pinned server/plugin image or a public test seam exists. | reference-stack limitation (issue #40) | **terminal gap** | | Fetch mode: download, local-backend handoff, correct read-back, cache reuse on reopen (confirmed via `download_file`'s own verbose trace) | `UdaUniqueSurfaceTest.FetchModeDownloadsHandsOffToLocalBackendAndReusesCacheOnReopen` | covered | | `local_cache` overrides the cache root only — the remote path is still nested underneath it, same as the default formula | `UdaUniqueSurfaceTest.FetchModeLocalCacheOptionOverridesDefaultCacheDir` | covered | | ↳ stale-cache / write-divergence pin: a fetch-mode write succeeds locally only, server-side pulse (reopened via remote mode) is unchanged, divergent local copy persists across close/reopen | `UdaUniqueSurfaceTest.FetchModeWriteDivergesFromServerAndStalePersistsAcrossReopen` | covered | diff --git a/tests/contract/test_uda_unique_surface.cpp b/tests/contract/test_uda_unique_surface.cpp index 0a86b5d2..255c5fbd 100644 --- a/tests/contract/test_uda_unique_surface.cpp +++ b/tests/contract/test_uda_unique_surface.cpp @@ -923,10 +923,11 @@ TEST_F(UdaUniqueSurfaceTest, // in the codebase gated on a remote party's reported version rather than a // compile-time constant. Against this reference stack the plugin reports // 1.8.0 (docker/uda/README.md), so the capability is granted: an -// al_begin_timerange_action call must pass al_lowlevel.cpp's capability gate +// al_begin_timerange_action call must return its exact success status +// (`s.code == 0`) after passing al_lowlevel.cpp's capability gate // (src/al_lowlevel.cpp:1053, "Selected backend does not support time range -// operations.") rather than being refused outright. This is deliberately -// scoped to the *capability* boundary only -- the subsequent read on this +// operations."). This is deliberately scoped to the *capability* boundary +// only -- the subsequent read on this // same call already has its own pinned defect // (UdaSliceAndTimeRange.TimeRangeReadWithoutResamplingCurrentlyFailsWithUnknownInterpMode, // test_uda_breadth.cpp), which is a separate, uninitialized-interpmode bug @@ -964,16 +965,11 @@ TEST_F(UdaUniqueSurfaceTest, TimeRangeCapabilityGrantedByReferenceServerVersion1 al_status_t s = al_begin_timerange_action(pulse_ctx, equilibrium_seed::kIds, READ_OP, 1.0, 2.0, &dtime, &dtime_shape, UNDEFINED_INTERP, &op); - EXPECT_EQ(std::string(s.message).find( - "does not support time range operations"), - std::string::npos) - << "the reference server (plugin 1.8.0 > 1.4.0) must grant the " - "time-range capability -- al_begin_timerange_action must not be " - "refused with the 'does not support time range operations' " - "message; got: " << s.message; - if (s.code == 0) { - AL_EXPECT_OK(al_end_action(op)); - } + EXPECT_EQ(s.code, 0) + << "the reference server (plugin 1.8.0 > 1.4.0) must pass the " + "time-range capability gate and successfully begin the action; " + "got code " << s.code << ", message: " << s.message; + if (s.code == 0) AL_EXPECT_OK(al_end_action(op)); AL_EXPECT_OK(al_close_pulse(pulse_ctx, CLOSE_PULSE)); } From 8edf1d3aba0a41ae1d53d02c2f141b8debd80bcf Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Fri, 17 Jul 2026 09:03:21 +0200 Subject: [PATCH 16/17] Characterize MDSplus real-path delete semantics (#34) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TRACEABILITY.md marked the whole MDSplus DeleteMatrix row a terminal-gap: MDSplus is absent from kDeleteBackends because the always-on fixture's synthetic paths (leaf_a, block, "") have no node in its DD-baked model tree, even though MDSplusBackend::deleteData genuinely implements path-scoped deletion. That is 'not yet done', not unreachable, so it failed #12's zero-unresolved-row bar. Close it with test_mdsplus_delete.cpp, the delete analogue of MdsplusRealPathMatrix (#15): seed real DD-4.1.1 equilibrium/code/* STR_0D leaves plus an unrelated vacuum_toroidal_field/r0 sibling through the public C ABI, al_delete_data at each granularity, reopen, and assert exactly which paths survive. (imas-dd MCP not in this session; paths validated against the baked-from IDSDef.xml, the method real_dd_path_catalog.h documents.) Observed verdicts: - LEAF (covered): al_delete_data("code/name") clears just that leaf; the two other code/* leaves and vacuum_toroidal_field/r0 keep their values. - STRUCTURE and ROOT (xfail): al_delete_data("code") and al_delete_data("") both throw %TREE-W-NNF because deleteData's STRUCTURE branch (mdsplus_backend.cpp:2147) looks up the :static/.timed_aos AOS-storage children, which a plain structure and the DATAOBJECT root have no node for; the delete is a no-op and every seeded path survives. Pinned per D2 with DISABLED_ correct-contract tests (confirmed red today) + active …CurrentlyThrowsNodeNotFound tripwires. Verified in the aarch64 MDSplus reference container: 3 active tests green, 2 DISABLED_ correct-contract tests fail as expected, all 5 land in the mdsplus tier, label partition still disjoint and hermetic. The Part 4 DeleteMatrix row is now covered (leaf) / xfail (structure, root), no longer terminal-gap. (cherry picked from commit cfa889608b9491844ecd8203f98c05e09ddce2ca) --- tests/contract/CMakeLists.txt | 1 + tests/contract/TRACEABILITY.md | 2 +- .../contract/assign_contract_labels.cmake.in | 2 + tests/contract/test_mdsplus_delete.cpp | 299 ++++++++++++++++++ 4 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 tests/contract/test_mdsplus_delete.cpp diff --git a/tests/contract/CMakeLists.txt b/tests/contract/CMakeLists.txt index d6be748a..7f338d9e 100644 --- a/tests/contract/CMakeLists.txt +++ b/tests/contract/CMakeLists.txt @@ -76,6 +76,7 @@ add_executable(contract_tests test_pulse_lifecycle.cpp test_mdsplus.cpp test_mdsplus_real_paths.cpp + test_mdsplus_delete.cpp test_mdsplus_version_drift.cpp test_mdsplus_unique_surface.cpp test_uda.cpp diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index e12121da..12a848a6 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -400,7 +400,7 @@ explicit open items), not assumed: | Parity: `Occurrences` — real implementation, `/` naming convention (see characterization facts above) | `Occurrences.MdsplusListsWrittenOccurrences` | covered | | Parity: `AosMatrix` (top-level + nested write→iterate→read) — **divergence, not a defect**: MDSplus resolves every path against its real DD-baked model tree, so this fixture's synthetic field names (`"elements"`/`"val"`, `"outer"`/`"inner"`) have no corresponding node; `al_begin_arraystruct_action` and the per-element writes report success (buffered), but the flush at `al_end_action` throws `%TREE-W-NNF, Node Not Found`. Real DD-conformant AOS paths round-trip through the identical sequence (see characterization facts above) — the fixture's synthetic-path design simply cannot transfer to a model-tree-backed backend, the same reason `RoundTripMatrix` already excludes MDSplus (issue #12 Q5) | `Backends/AosMatrix.{TopLevelWriteIterateRead,NestedWriteIterateRead}/Mdsplus` (both `GTEST_SKIP()`, `AosExpect::Divergence`) | divergence | | Parity: `EquilibriumSeedMatrix` (composite scalar + timebase-carrying 2-D array + `constraints` AOS, hash oracle) — **divergence, not a defect**: the scalar sub-shape round-trips (already proven by the tracer bullet above), but the seed's flat-tensorized `profiles_1d/psi` write and its generic `constraints`/`{measured,weight}` AOS shape don't match the real equilibrium DD layout MDSplus enforces (real `profiles_1d` is a genuine dynamic AOS, not a flat dataset at that path; real `constraints` is a fixed container of specific constraint sub-objects, not a generic AOS) — both throw `%TREE-W-NNF, Node Not Found`. MDSplus **is** instantiated in `kSeedBackends[]` (mirroring `AosMatrix` above), and the case is skipped in-place via `GTEST_SKIP()` rather than run and fail | `Backends/EquilibriumSeedMatrix.RoundTripHashMatches/Mdsplus` (`GTEST_SKIP()`) | divergence | -| Parity: `DeleteMatrix` (leaf / structure / DATAOBJECT-root granularity) — **terminal gap, closed here as an explicit row rather than left silently absent (issue #18)**: MDSplus is not instantiated in `kDeleteBackends[]` at all. Joining it would hit the same wall as `AosMatrix`/`RoundTripMatrix` above — the fixture's synthetic paths (`leaf_a`, `block`, top-level `""`) have no corresponding model-tree node, so even the divergence-and-skip pattern used for `AosMatrix`/`EquilibriumSeedMatrix` doesn't cleanly apply (unlike those, `MDSplusBackend::deleteData` — `mdsplus_backend.cpp:2136` — genuinely implements path-scoped leaf/structure delete, unlike HDF5's ignore-`path` defect, so a real characterization would be informative, not a structural no-op). A dedicated real-DD-path delete test mirroring `MdsplusRealPathMatrix` (issue #15) was not built in this closing pass — left as a deliberate, explained terminal gap for a future step, not a defect and not a silent omission | none — see reasoning | terminal-gap | +| Parity: `DeleteMatrix` (leaf / structure / DATAOBJECT-root granularity) — **characterized on real DD-4.1.1 paths (issue #34), replacing the former terminal gap.** `test_mdsplus_delete.cpp` seeds real `equilibrium/code/*` STR_0D leaves plus an unrelated `vacuum_toroidal_field/r0` sibling through the public C ABI, deletes at each granularity, reopens, and asserts exactly what survives — the same real-path approach `MdsplusRealPathMatrix` (issue #15) used to close the RoundTrip gap. Verdicts differ by granularity: **leaf = covered** (`al_delete_data("code/name")` clears just that leaf; every sibling keeps its value); **structure and root = xfail** — `al_delete_data("code")` and `al_delete_data("")` both throw `%TREE-W-NNF, Node Not Found` because `MDSplusBackend::deleteData`'s STRUCTURE branch (`mdsplus_backend.cpp:2147`) looks up the `:static`/`.timed_aos` array-of-structures storage children, which a plain (non-AOS) structure and the DATAOBJECT root have no node for; the delete is a no-op and every seeded path survives. Both xfails are pinned per D2 (`DISABLED_StructureDeleteRemovesWholeSubtree` / `DISABLED_RootDeleteRemovesWholeOccurrence` correct-contract tests, confirmed red today, + active `…CurrentlyThrowsNodeNotFound` tripwires) | `MdsplusRealPathDelete.{LeafDeleteRemovesJustTheLeaf,StructureDeleteCurrentlyThrowsNodeNotFound,RootDeleteCurrentlyThrowsNodeNotFound}` (+ 2 `DISABLED_` correct-contract) | covered (leaf) / xfail (structure, root) | ### Real-DD-path datatype × rank breadth (issue #15) diff --git a/tests/contract/assign_contract_labels.cmake.in b/tests/contract/assign_contract_labels.cmake.in index 8cbd84a5..434ac95c 100644 --- a/tests/contract/assign_contract_labels.cmake.in +++ b/tests/contract/assign_contract_labels.cmake.in @@ -64,6 +64,8 @@ if(EXISTS "${_al_contract_tests_file}") _al_name MATCHES "^Mdsplus/MdsplusRealPathMatrix\\." OR # issue #46: MDSplus half of the shared-catalog coverage meta-test _al_name MATCHES "^MdsplusRealPathCatalog\\." OR + # issue #34: MDSplus real-DD-path delete characterization + _al_name MATCHES "^MdsplusRealPathDelete\\." OR _al_name MATCHES "^MdsplusVersionDrift" OR _al_name MATCHES "^MdsplusStructAwareSliceInterpolation" OR _al_name MATCHES "^MdsplusTimerangeResampling" OR diff --git a/tests/contract/test_mdsplus_delete.cpp b/tests/contract/test_mdsplus_delete.cpp new file mode 100644 index 00000000..7ae42d94 --- /dev/null +++ b/tests/contract/test_mdsplus_delete.cpp @@ -0,0 +1,299 @@ +// MDSplus real-DD-path delete semantics (issue #34, TRACEABILITY.md Part 4 +// DeleteMatrix row). +// +// The always-on DeleteMatrix (test_structured_data.cpp) exercises +// al_delete_data at leaf / structure / DATAOBJECT-root granularity against the +// synthetic fixture paths (leaf_a, block, "") on HDF5/Memory/ASCII/Flexbuffers. +// MDSplus is absent from kDeleteBackends because those synthetic paths have no +// node in its DD-baked model tree -- the same wall RoundTripMatrix/AosMatrix +// hit (issue #12 Q2/Q5). TRACEABILITY.md long marked the whole MDSplus +// DeleteMatrix row a `terminal-gap`, even though MDSplusBackend::deleteData +// (src/mdsplus/mdsplus_backend.cpp:2136) genuinely implements path-scoped +// deletion. This file closes that gap the same way MdsplusRealPathMatrix +// (issue #15) closed the RoundTrip gap: with real DD-4.1.1 paths MDSplus can +// resolve, and one verdict per granularity. +// +// Real DD-4.1.1 paths (validated against the baked-from artifact +// build-mdsplus/_deps/data-dictionary-src/IDSDef.xml, the same method +// real_dd_path_catalog.h documents -- the imas-dd MCP tool CLAUDE.md names is +// not in this session's MCP config): +// * equilibrium/code is a plain structure holding several STR_0D leaves +// (name, version, description, ...) -- the leaf and structure cases. +// * equilibrium/vacuum_toroidal_field/r0 (FLT_0D) is an unrelated sibling +// leaf that must survive a code-scoped or leaf-scoped delete. +// +// Observed verdicts (characterized empirically through the public C ABI): +// * LEAF (al_delete_data("code/name")) -> COVERED. Succeeds; the target's +// data is cleared (reads back empty, no longer its seeded value) while +// every sibling keeps its value. +// * STRUCTURE (al_delete_data("code")) -> XFAIL. deleteData's STRUCTURE +// branch (mdsplus_backend.cpp:2147) assumes array-of-structures storage -- +// it looks up the `:static`/`.timed_aos` AOS-layout children, which a +// plain DD structure has no node for, so the call throws +// `%TREE-W-NNF, Node Not Found` and nothing is deleted. Pinned as a +// DISABLED_ correct-contract test + an active current-behavior tripwire +// (decision D2). +// * ROOT (al_delete_data("")) -> XFAIL, same %TREE-W-NNF cause (the +// DATAOBJECT root is itself a STRUCTURE-usage node). Explicit observed +// verdict via the same DISABLED_/tripwire pair. +// +// Oracle: seed distinguishable values through the public C ABI, al_delete_data +// at a granularity, reopen the pulse, read every path back. Only the *paths* +// are real DD; the deletion contract itself is backend behavior (decision D1: +// the core attaches no DD semantics to any path). +#ifdef AL_CONTRACT_HAVE_MDSPLUS + +#include "al_contract.h" + +#include +#include + +#include + +#include +#include + +using al_contract::PulseId; + +namespace { + +constexpr const char* kIds = "equilibrium"; + +// A real DD-4.1.1 STR_0D leaf under equilibrium/code and the value seeded into +// it. All static (non-dynamic), so one GLOBAL write seeds them. +struct SeedLeaf { + const char* path; + const char* value; +}; +constexpr SeedLeaf kCodeName{"code/name", "alpha-name"}; +constexpr SeedLeaf kCodeVersion{"code/version", "beta-version"}; +constexpr SeedLeaf kCodeDescription{"code/description", "gamma-description"}; + +// An unrelated sibling leaf outside `code`: FLT_0D, must survive a code-scoped +// (structure) or code/name-scoped (leaf) delete. +constexpr const char* kSiblingPath = "vacuum_toroidal_field/r0"; +constexpr double kSiblingValue = 6.2; + +PulseId pulse_id() { + return PulseId{/*database=*/"test", /*version=*/"3", /*pulse=*/15, + /*run=*/0}; +} + +void write_str(int op, const char* path, const std::string& value) { + std::vector buf(value.begin(), value.end()); + AL_EXPECT_OK(al_contract::write_data( + op, path, {static_cast(buf.size())}, buf)); +} + +// Read a STR_0D/1D leaf. Returns true with `out` set on a clean read; false if +// the read reported an error. After a delete clears a node, the read succeeds +// but returns empty -- so callers distinguish "gone" via emptiness, not error. +bool try_read_str(int op, const char* path, std::string* out) { + std::vector shape; + std::vector data; + al_status_t s = al_contract::read_data(op, path, /*rank=*/1, + &shape, &data); + if (s.code != 0) return false; + out->assign(data.begin(), data.end()); + return true; +} + +bool try_read_double(int op, const char* path, double* out) { + std::vector shape; + std::vector data; + al_status_t s = al_contract::read_data(op, path, /*rank=*/0, &shape, + &data); + if (s.code != 0 || data.empty()) return false; + *out = data.front(); + return true; +} + +// True iff a STR leaf reads back its exact seeded value. +bool str_survives(int op, const SeedLeaf& leaf) { + std::string v; + return try_read_str(op, leaf.path, &v) && v == leaf.value; +} + +// True iff a STR leaf's data is gone (read fails, or reads back empty rather +// than its seeded value). +bool str_gone(int op, const SeedLeaf& leaf) { + std::string v; + if (!try_read_str(op, leaf.path, &v)) return true; + return v != leaf.value; +} + +bool sibling_survives(int op) { + double d = 0; + return try_read_double(op, kSiblingPath, &d) && d == kSiblingValue; +} + +class MdsplusRealPathDelete : public ::testing::Test { +protected: + void SetUp() override { AL_CONTRACT_SKIP_IF_MDSPLUS_UNCONFIGURED(); } + + // Create a fresh MDSplus pulse and seed the three code/* leaves plus the + // unrelated vacuum_toroidal_field/r0 sibling. Leaves `base` owning the tree + // dir so the caller can reopen the same pulse. + void seed(al_contract::TempBase& base) { + base.make_legacy_tree(pulse_id()); + const std::string uri = + al_contract::build_uri(MDSPLUS_BACKEND, base.str(), pulse_id()); + ASSERT_FALSE(uri.empty()); + int pulse_ctx = -1; + AL_ASSERT_OK(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, + &pulse_ctx)); + int op = -1; + AL_ASSERT_OK(al_begin_global_action(pulse_ctx, kIds, "", WRITE_OP, &op)); + write_str(op, kCodeName.path, kCodeName.value); + write_str(op, kCodeVersion.path, kCodeVersion.value); + write_str(op, kCodeDescription.path, kCodeDescription.value); + AL_EXPECT_OK(al_contract::write_data(op, kSiblingPath, {}, + {kSiblingValue})); + AL_ASSERT_OK(al_end_action(op)); + EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); + } + + // Delete `path` in a fresh WRITE action on the (reopened) pulse; returns the + // al_delete_data status. Opens/closes its own pulse ctx. + al_status_t delete_path(const al_contract::TempBase& base, + const char* path) { + const std::string uri = + al_contract::build_uri(MDSPLUS_BACKEND, base.str(), pulse_id()); + int pulse_ctx = -1; + EXPECT_EQ( + al_begin_dataentry_action(uri.c_str(), OPEN_PULSE, &pulse_ctx).code, + 0); + int op = -1; + EXPECT_EQ(al_begin_global_action(pulse_ctx, kIds, "", WRITE_OP, &op).code, + 0); + al_status_t s = al_delete_data(op, path); + al_end_action(op); + EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); + return s; + } + + // Reopen the pulse for READ and hand the op ctx to `check`; tidies up after. + template + void reopen_and_read(const al_contract::TempBase& base, Fn&& check) { + const std::string uri = + al_contract::build_uri(MDSPLUS_BACKEND, base.str(), pulse_id()); + int pulse_ctx = -1; + AL_ASSERT_OK( + al_begin_dataentry_action(uri.c_str(), OPEN_PULSE, &pulse_ctx)); + int op = -1; + AL_ASSERT_OK(al_begin_global_action(pulse_ctx, kIds, "", READ_OP, &op)); + check(op); + al_end_action(op); + EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); + } +}; + +// --- LEAF: covered ---------------------------------------------------------- +// Deleting a single STR_0D leaf clears exactly that leaf; every sibling (two +// other code/* leaves and the unrelated vacuum_toroidal_field/r0) survives. +TEST_F(MdsplusRealPathDelete, LeafDeleteRemovesJustTheLeaf) { + al_contract::TempBase base; + seed(base); + + AL_EXPECT_OK(delete_path(base, kCodeName.path)); + + reopen_and_read(base, [](int op) { + EXPECT_TRUE(str_gone(op, kCodeName)) + << "deleted leaf code/name still reads back its seeded value"; + EXPECT_TRUE(str_survives(op, kCodeVersion)) + << "sibling code/version must survive a code/name delete"; + EXPECT_TRUE(str_survives(op, kCodeDescription)) + << "sibling code/description must survive a code/name delete"; + EXPECT_TRUE(sibling_survives(op)) + << "unrelated vacuum_toroidal_field/r0 must survive"; + }); +} + +// --- STRUCTURE: xfail ------------------------------------------------------- +// Correct contract (disabled): deleting the `code` structure removes its whole +// subtree (all code/* leaves) and nothing else. Enable when deleteData learns +// to delete a plain (non-AOS) structure. +TEST_F(MdsplusRealPathDelete, DISABLED_StructureDeleteRemovesWholeSubtree) { + al_contract::TempBase base; + seed(base); + + AL_EXPECT_OK(delete_path(base, "code")); + + reopen_and_read(base, [](int op) { + EXPECT_TRUE(str_gone(op, kCodeName)); + EXPECT_TRUE(str_gone(op, kCodeVersion)); + EXPECT_TRUE(str_gone(op, kCodeDescription)); + EXPECT_TRUE(sibling_survives(op)) + << "a code-scoped delete must not touch vacuum_toroidal_field/r0"; + }); +} + +// Current behavior (active tripwire): al_delete_data("code") throws +// %TREE-W-NNF because deleteData's STRUCTURE branch looks up the AOS-only +// :static/.timed_aos children, which a plain structure lacks. The delete is a +// no-op: every seeded leaf survives. When deleteData is fixed, this tripwire +// fails and its paired DISABLED_ test above should be enabled. +TEST_F(MdsplusRealPathDelete, StructureDeleteCurrentlyThrowsNodeNotFound) { + al_contract::TempBase base; + seed(base); + + al_status_t s = delete_path(base, "code"); + EXPECT_NE(s.code, 0) + << "al_delete_data(\"code\") now succeeds -- enable " + "DISABLED_StructureDeleteRemovesWholeSubtree"; + EXPECT_NE(std::string(s.message).find("Node Not Found"), std::string::npos) + << "expected %TREE-W-NNF, got: " << s.message; + + reopen_and_read(base, [](int op) { + EXPECT_TRUE(str_survives(op, kCodeName)) + << "failed structure delete must leave code/name intact"; + EXPECT_TRUE(str_survives(op, kCodeVersion)); + EXPECT_TRUE(str_survives(op, kCodeDescription)); + EXPECT_TRUE(sibling_survives(op)); + }); +} + +// --- ROOT: xfail ------------------------------------------------------------ +// Correct contract (disabled): deleting the DATAOBJECT root ("") removes the +// whole occurrence -- every path is gone. Enable when deleteData supports root +// granularity for MDSplus. +TEST_F(MdsplusRealPathDelete, DISABLED_RootDeleteRemovesWholeOccurrence) { + al_contract::TempBase base; + seed(base); + + AL_EXPECT_OK(delete_path(base, "")); + + reopen_and_read(base, [](int op) { + EXPECT_TRUE(str_gone(op, kCodeName)); + EXPECT_TRUE(str_gone(op, kCodeVersion)); + EXPECT_TRUE(str_gone(op, kCodeDescription)); + double d = 0; + EXPECT_FALSE(try_read_double(op, kSiblingPath, &d)) + << "root delete must remove vacuum_toroidal_field/r0 too"; + }); +} + +// Current behavior (active tripwire): al_delete_data("") throws %TREE-W-NNF +// (the DATAOBJECT root is a STRUCTURE-usage node, same AOS-layout assumption as +// the structure case). Explicit observed verdict; the occurrence is untouched. +TEST_F(MdsplusRealPathDelete, RootDeleteCurrentlyThrowsNodeNotFound) { + al_contract::TempBase base; + seed(base); + + al_status_t s = delete_path(base, ""); + EXPECT_NE(s.code, 0) + << "al_delete_data(\"\") now succeeds -- enable " + "DISABLED_RootDeleteRemovesWholeOccurrence"; + EXPECT_NE(std::string(s.message).find("Node Not Found"), std::string::npos) + << "expected %TREE-W-NNF, got: " << s.message; + + reopen_and_read(base, [](int op) { + EXPECT_TRUE(str_survives(op, kCodeName)) + << "failed root delete must leave the occurrence intact"; + EXPECT_TRUE(sibling_survives(op)); + }); +} + +} // namespace + +#endif // AL_CONTRACT_HAVE_MDSPLUS From df71bb813f50c398686868ceff904c8eddae77a1 Mon Sep 17 00:00:00 2001 From: YohannMrg <98689112+yohannmarguier@users.noreply.github.com> Date: Fri, 17 Jul 2026 09:24:37 +0200 Subject: [PATCH 17/17] Make the equilibrium seed DD-valid and hash structural metadata (#33) The shared equilibrium fixture modelled profiles_1d/psi as a flat top-level leaf and constraints/{measured,weight} as a generic top-level AOS -- neither a valid equilibrium DD-4.1.1 shape. The two DD-coupled backends could only skip the seed (MDSplus pre-skip; UDA asserted the read failed then skipped), and the hash covered values only, so a 3x4->4x3 extent transpose or a dropped/reordered AOS element with unchanged bytes passed silently. Rewrite the seed against real equilibrium DD-4.1.1 paths (validated against the baked-from IDSDef.xml; imas-dd MCP not in this session): a static scalar (vacuum_toroidal_field/r0), a top-level timebase array (time), and the real time_slice struct_array whose elements carry time, profiles_1d/psi (FLT_1D), global_quantities/ip, and a real constraints/ip/{measured,weight} subtree -- written with the real time_slice write idiom test_mdsplus_unique_surface.cpp proves. Replace the values-only FNV hash with a structural hash over a canonical stream (path identity + datatype + rank + every extent + AOS size/index + values, lengths interleaved); the read side rebuilds the stream from the actual observed shapes and the test asserts exact rank/extents field-by-field before hashing. Verdicts, all now covered (no skips): - HDF5, Memory, ASCII: round-trip (verified locally). - MDSplus: now executes and round-trips the whole composite (verified in the aarch64 reference container) -- the former divergence/skip is gone. - UDA: round-trips the whole composite through remote mode, including the time_slice AOS dynamic leaves (verified stable across repeated runs of the reference stack). This does NOT contradict UdaAosKnownDefects: that fixture seeds the AOS with an empty timebase and no per-element time; a well-formed DD-conformant time_slice (real timebasepath + per-element time) is the case UDA resolves correctly. Structural sensitivity is pinned hermetically by EquilibriumSeedStructuralHash.* (unit tier): swapping two extents with identical bytes, dropping an AOS element, and reordering two AOS elements each change the hash. No DD path/version is compiled into product code (paths remain test data). TRACEABILITY Parts 1/4/5 updated: the seed no longer claims parity through skipped DD-invalid shapes. (cherry picked from commit 23204bc6d3c0e593a240ac5c839eca578f189441) --- tests/contract/TRACEABILITY.md | 6 +- .../contract/assign_contract_labels.cmake.in | 5 +- tests/contract/equilibrium_seed.h | 310 +++++++++++------- tests/contract/test_structured_data.cpp | 129 +++++--- tests/contract/test_uda.cpp | 164 ++++----- 5 files changed, 389 insertions(+), 225 deletions(-) diff --git a/tests/contract/TRACEABILITY.md b/tests/contract/TRACEABILITY.md index 12a848a6..86519bed 100644 --- a/tests/contract/TRACEABILITY.md +++ b/tests/contract/TRACEABILITY.md @@ -145,7 +145,7 @@ backends where AOS content is actually readable back within a session. | Capability | Inventory ref | Unit | Integ. | Test(s) | Status | |---|---|:--:|:--:|---|---| -| Realistic AOS/timebase/HDF5-tensorization shape round-trips exactly | D5 | ✓ (Memory) | ✓ (HDF5, ASCII) | `Backends/EquilibriumSeedMatrix.RoundTripHashMatches/{HDF5,Memory,ASCII}`. MDSplus is instantiated too, but a divergence (the seed's generic shapes don't match the real equilibrium DD layout) — see Part 4. UDA: same divergence, same root cause (the DD-schema-validating backend has no counterpart for the seed's synthetic shapes) — see Part 5 | covered | +| Realistic AOS/timebase/HDF5-tensorization shape round-trips exactly | D5 | ✓ (Memory) | ✓ (HDF5, ASCII) | `Backends/EquilibriumSeedMatrix.RoundTripHashMatches/{HDF5,Memory,ASCII}`. Since issue #33 the seed is DD-4.1.1-conformant (real `time_slice`/`profiles_1d`/`constraints` AOS), so **MDSplus and UDA now round-trip it too** (Parts 4/5), no longer skipped. The oracle is a *structural* hash (path/datatype/rank/every-extent/AOS-index/values); `EquilibriumSeedStructuralHash.*` pins that an extent transpose or dropped/reordered AOS element fails | covered | ## Cluster 3 — Plugin management (`FUNCTIONALITY_INVENTORY.md:318-430`) @@ -399,7 +399,7 @@ explicit open items), not assumed: | Parity: `CapabilityMatrix` — slice read/write positive (real interpolation round trip), timerange refused (`LOWLEVEL_ERR`), `list_filled_paths` refused (`BACKEND_ERR`) | `Backends/CapabilityMatrix.{TimeRangeReadPositiveOrRefused,SliceReadPositiveOrRefused,SliceWriteBeginPositiveOrRefused,ListFilledPathsPositiveOrRefused}/Mdsplus` (4 cases) | covered | | Parity: `Occurrences` — real implementation, `/` naming convention (see characterization facts above) | `Occurrences.MdsplusListsWrittenOccurrences` | covered | | Parity: `AosMatrix` (top-level + nested write→iterate→read) — **divergence, not a defect**: MDSplus resolves every path against its real DD-baked model tree, so this fixture's synthetic field names (`"elements"`/`"val"`, `"outer"`/`"inner"`) have no corresponding node; `al_begin_arraystruct_action` and the per-element writes report success (buffered), but the flush at `al_end_action` throws `%TREE-W-NNF, Node Not Found`. Real DD-conformant AOS paths round-trip through the identical sequence (see characterization facts above) — the fixture's synthetic-path design simply cannot transfer to a model-tree-backed backend, the same reason `RoundTripMatrix` already excludes MDSplus (issue #12 Q5) | `Backends/AosMatrix.{TopLevelWriteIterateRead,NestedWriteIterateRead}/Mdsplus` (both `GTEST_SKIP()`, `AosExpect::Divergence`) | divergence | -| Parity: `EquilibriumSeedMatrix` (composite scalar + timebase-carrying 2-D array + `constraints` AOS, hash oracle) — **divergence, not a defect**: the scalar sub-shape round-trips (already proven by the tracer bullet above), but the seed's flat-tensorized `profiles_1d/psi` write and its generic `constraints`/`{measured,weight}` AOS shape don't match the real equilibrium DD layout MDSplus enforces (real `profiles_1d` is a genuine dynamic AOS, not a flat dataset at that path; real `constraints` is a fixed container of specific constraint sub-objects, not a generic AOS) — both throw `%TREE-W-NNF, Node Not Found`. MDSplus **is** instantiated in `kSeedBackends[]` (mirroring `AosMatrix` above), and the case is skipped in-place via `GTEST_SKIP()` rather than run and fail | `Backends/EquilibriumSeedMatrix.RoundTripHashMatches/Mdsplus` (`GTEST_SKIP()`) | divergence | +| Parity: `EquilibriumSeedMatrix` (composite scalar + top-level timebase array + real `time_slice`/`profiles_1d`/`constraints` AOS, structural-hash oracle) — **covered (issue #33), replacing the former divergence-and-skip.** The seed is now DD-4.1.1-conformant: `vacuum_toroidal_field/r0` (FLT_0D), top-level `time` (FLT_1D), and a real `time_slice` struct_array whose elements carry `time`, `profiles_1d/psi` (FLT_1D), `global_quantities/ip`, and a real `constraints/ip/{measured,weight}` subtree — every path validated against the baked-from `IDSDef.xml`. MDSplus resolves them against its DD-baked model tree and round-trips the whole composite through the same real-AOS write/read idiom `test_mdsplus_unique_surface.cpp` already proves. The oracle is a *structural* hash (path identity + datatype + rank + every extent + AOS size/index + values), and the test asserts exact rank/extents field-by-field before hashing, so an extent transpose or a dropped/reordered AOS element fails (pinned hermetically by `EquilibriumSeedStructuralHash.*`) | `Backends/EquilibriumSeedMatrix.RoundTripHashMatches/Mdsplus` | covered | | Parity: `DeleteMatrix` (leaf / structure / DATAOBJECT-root granularity) — **characterized on real DD-4.1.1 paths (issue #34), replacing the former terminal gap.** `test_mdsplus_delete.cpp` seeds real `equilibrium/code/*` STR_0D leaves plus an unrelated `vacuum_toroidal_field/r0` sibling through the public C ABI, deletes at each granularity, reopens, and asserts exactly what survives — the same real-path approach `MdsplusRealPathMatrix` (issue #15) used to close the RoundTrip gap. Verdicts differ by granularity: **leaf = covered** (`al_delete_data("code/name")` clears just that leaf; every sibling keeps its value); **structure and root = xfail** — `al_delete_data("code")` and `al_delete_data("")` both throw `%TREE-W-NNF, Node Not Found` because `MDSplusBackend::deleteData`'s STRUCTURE branch (`mdsplus_backend.cpp:2147`) looks up the `:static`/`.timed_aos` array-of-structures storage children, which a plain (non-AOS) structure and the DATAOBJECT root have no node for; the delete is a no-op and every seeded path survives. Both xfails are pinned per D2 (`DISABLED_StructureDeleteRemovesWholeSubtree` / `DISABLED_RootDeleteRemovesWholeOccurrence` correct-contract tests, confirmed red today, + active `…CurrentlyThrowsNodeNotFound` tripwires) | `MdsplusRealPathDelete.{LeafDeleteRemovesJustTheLeaf,StructureDeleteCurrentlyThrowsNodeNotFound,RootDeleteCurrentlyThrowsNodeNotFound}` (+ 2 `DISABLED_` correct-contract) | covered (leaf) / xfail (structure, root) | ### Real-DD-path datatype × rank breadth (issue #15) @@ -652,7 +652,7 @@ blocked-by-environment) was not invoked. | Capability | Test(s) | Status | |---|---|---| | Tracer bullet (issue #23): one equilibrium scalar (`vacuum_toroidal_field/r0`), seeded through the plain HDF5 backend, reads back byte-identical through the UDA backend in remote mode, across the real reference stack | `UdaSmokeRoundTrip.ScalarSeededViaHdf5ReadsBackThroughUda` | covered | -| Read-only parity fixture (issue #24): the full equilibrium-seed composite (scalar + timebase-carrying 2-D array + constraints AOS, `equilibrium_seed.h`, issue #4/D5), seeded through HDF5 and reopened through UDA, asserted via the seed's own unmodified FNV-1a hash oracle — **divergence, not a defect**: the generator's flat `profiles_1d/psi` leaf and generic `constraints` AOS have no counterpart in equilibrium's real DD-4.1.1 layout (`profiles_1d` is itself a struct_array, not a plain field). Confirmed empirically against the reference stack: `al_plugin_read_data` fails immediately with "cannot find node equilibrium/profiles_1d/psi in data dictionary (profiles_1d not found)". The same wall MDSplus hits on its own `EquilibriumSeedMatrix`/`Mdsplus` row (Part 4) — nothing to fix, the fixture's synthetic composite shape simply cannot transfer to a DD-schema-validating backend; the scalar sub-shape alone is real DD and is already covered by the tracer bullet above. The seed-then-reopen attempt genuinely runs every time (unlike EquilibriumSeedMatrix's per-backend skip, this suite has no other instance to keep it exercised): the test asserts the *specific* failure signature before `GTEST_SKIP()`'ing, so a future change that resolves the divergence fails the assertion loudly instead of this row silently going stale | `UdaEquilibriumSeedParity.HdfSeededReadsBackThroughUda` | divergence | +| Read-only parity fixture (issue #24): the full equilibrium-seed composite (static scalar + top-level timebase array + real `time_slice`/`profiles_1d`/`constraints` AOS, `equilibrium_seed.h`, issues #4/#33/D5), seeded through HDF5 and reopened through UDA, asserted via the seed's structural hash oracle — **covered (issue #33), replacing the former divergence.** The seed is now DD-4.1.1-conformant, so it round-trips fully through UDA remote mode: the static scalar, the `time` timebase array, and every dynamic leaf nested in the `time_slice` struct_array (`time`, `profiles_1d/psi`, `global_quantities/ip`, `constraints/ip/{measured,weight}`). This does **not** contradict `UdaAosKnownDefects` (which pins a dynamic-leaf-inside-AOS reading absent): that fixture seeds the AOS with an *empty* timebase and no per-element `time`, whereas this seed writes the real timebasepath (`time`) and a per-element `time`, and a well-formed DD-conformant time_slice AOS is empirically the case UDA resolves correctly. Verified stable across repeated runs of the docker/uda/ reference stack; the structural hash (rank/extents/AOS-index-aware) makes it a trustworthy parity oracle | `UdaEquilibriumSeedParity.FullSeedRoundTripsThroughUda` | covered | **Progress (issue #25, parity breadth): zero blank rows remain in Part 5.** Every C-ABI-reachable read-side capability from `FUNCTIONALITY_INVENTORY.md` diff --git a/tests/contract/assign_contract_labels.cmake.in b/tests/contract/assign_contract_labels.cmake.in index 434ac95c..c6f1967f 100644 --- a/tests/contract/assign_contract_labels.cmake.in +++ b/tests/contract/assign_contract_labels.cmake.in @@ -54,7 +54,10 @@ if(EXISTS "${_al_contract_tests_file}") _al_name MATCHES "^BackendFactory\\." OR # issue #46: the shared real-DD-path catalog's own invariants — pure # in-RAM table checks, no backend substrate, always compiled - _al_name MATCHES "^RealDdPathCatalog\\.") + _al_name MATCHES "^RealDdPathCatalog\\." OR + # issue #33: equilibrium-seed structural-hash sensitivity checks — + # pure in-RAM hashing of records, no backend substrate + _al_name MATCHES "^EquilibriumSeedStructuralHash\\.") set_tests_properties("${_al_name}" PROPERTIES LABELS "contract;unit") elseif(_al_name MATCHES "^MdsplusTracerBullet" OR # "/Mdsplus" as the whole instance suffix: pretty names end diff --git a/tests/contract/equilibrium_seed.h b/tests/contract/equilibrium_seed.h index 8c3a8d35..07fa60d5 100644 --- a/tests/contract/equilibrium_seed.h +++ b/tests/contract/equilibrium_seed.h @@ -1,26 +1,40 @@ -// Deterministic single-version equilibrium-seed generator (issue #4). +// Deterministic single-version equilibrium-seed generator (issues #4, #33). // // TEST_STRATEGY.md decision D5: "one realistic single-version equilibrium // seed — a curated subset (scalar, a profiles_1d array with a timebase, a // constraints AOS) from a deterministic in-repo generator (generator + hash, // not committed blobs) — to exercise real AOS/timebase/HDF5-tensorization // shapes." This header IS the generator: no binary fixture is checked in, the -// shape is produced by code, and the round-trip oracle is a content hash -// rather than a byte-for-byte vector compare (the synthetic-matrix machinery -// in al_contract.h already covers exhaustive byte-exact round trips; this -// seed's job is a *realistic composite shape*, not another datatype sweep). +// shape is produced by code, and the round-trip oracle is a *structural* hash +// (issue #33). // -// The three pieces mirror a real equilibrium IDS closely enough to exercise -// each shape class the migration cares about (NORTH_STAR.md's equilibrium -// pilot), while staying opaque strings through the C ABI (decision D1: the -// core attaches no DD semantics to any of these paths): -// - a scalar -> "vacuum_toroidal_field/r0" -// - a timebase-carrying 2-D array -> "time" (1-D) + "profiles_1d/psi" -// (2-D: [time][radial], tensorized on HDF5 the same way a real -// equilibrium/profiles_1d/psi(:,:) would be) -// - an AOS -> "constraints", each element holding -// two scalar fields ("measured", "weight") - +// DD-4.1.1-conformant shape (issue #33 — the earlier version modelled +// profiles_1d/psi as a flat top-level leaf and constraints/{measured,weight} +// as a generic top-level AOS; neither is a valid equilibrium shape, so the two +// DD-coupled backends could only skip the seed). Every path below is a real +// equilibrium DD-4.1.1 path, validated against the baked-from artifact +// build-mdsplus/_deps/data-dictionary-src/IDSDef.xml (the method +// real_dd_path_catalog.h documents; the imas-dd MCP tool CLAUDE.md names is not +// in this session's MCP config). It stays opaque strings through the C ABI +// (decision D1: the core attaches no DD semantics to any of these paths): +// - a static scalar -> vacuum_toroidal_field/r0 (FLT_0D) +// - a top-level timebase array-> time (FLT_1D, the IDS homogeneous timebase) +// - a real dynamic AOS -> time_slice (struct_array, timebasepath +// "time"), multiple elements, each carrying: +// * time (FLT_0D) -- per-slice coordinate +// * profiles_1d/psi (FLT_1D) -- radial profile +// * global_quantities/ip (FLT_0D) +// * constraints/ip/measured (FLT_0D) -- a real constraints +// * constraints/ip/weight (FLT_0D) subtree, not generic +// +// Structural hash (issue #33): the oracle is a hash over a canonical stream +// that includes, per field, its path identity, datatype, rank, every extent, +// and (for AOS elements) the array size and element index, then the values. +// Hashing values alone let a 3x4->4x3 extent transpose with unchanged bytes, +// or a dropped/reordered AOS element, pass silently; the structural stream +// closes that. The read side rebuilds the same stream from what it *actually* +// observed (real ranks/extents/AOS sizes), so any shape drift shows up as a +// hash (and explicit rank/extent) mismatch instead of being reconciled away. #ifndef AL_CONTRACT_EQUILIBRIUM_SEED_H #define AL_CONTRACT_EQUILIBRIUM_SEED_H @@ -35,49 +49,98 @@ namespace equilibrium_seed { -constexpr const char* kIds = "equilibrium"; -constexpr const char* kScalar = "vacuum_toroidal_field/r0"; -constexpr const char* kTimebase = "time"; -constexpr const char* kProfile = "profiles_1d/psi"; -constexpr const char* kAos = "constraints"; -constexpr const char* kMeasured = "measured"; -constexpr const char* kWeight = "weight"; +constexpr const char* kIds = "equilibrium"; +// static scalar + top-level timebase array +constexpr const char* kScalar = "vacuum_toroidal_field/r0"; // FLT_0D +constexpr const char* kTimebase = "time"; // FLT_1D +// the real dynamic AOS and its per-element leaves +constexpr const char* kAos = "time_slice"; // struct_array +constexpr const char* kAosTime = "time"; // timebasepath +constexpr const char* kSliceTime = "time"; // FLT_0D leaf +constexpr const char* kPsi = "profiles_1d/psi"; // FLT_1D +constexpr const char* kIp = "global_quantities/ip"; // FLT_0D +constexpr const char* kMeasured = "constraints/ip/measured"; // FLT_0D +constexpr const char* kWeight = "constraints/ip/weight"; // FLT_0D -constexpr int kTimeLen = 3; // time slices -constexpr int kRadialLen = 4; // radial grid points per slice -constexpr int kNConstraints = 3; // AOS elements +constexpr int kNSlices = 2; // time_slice AOS elements (>1: real AOS) +constexpr int kRadialLen = 4; // profiles_1d/psi grid points per slice + +// A "not a real datatype" marker used only for the synthetic AOS-size record in +// the canonical stream (so array size participates in the hash). +constexpr int kAosSizeMarker = -1; // --- deterministic content ------------------------------------------------- -// Plain arithmetic progressions, distinct per role so a transposed or -// swapped-field bug is caught by the hash. Kept clear of the write-path -// "absent" sentinels documented in al_contract.h (kEmptyDouble). +// Distinct arithmetic progressions per role and per slice, so a transposed, +// dropped, or reordered element is caught by the structural hash. Kept clear of +// the write-path "absent" sentinels documented in al_contract.h (kEmptyDouble). inline double scalar_r0() { return 6.2; } inline std::vector timebase_values() { std::vector t; - for (int i = 0; i < kTimeLen; ++i) t.push_back(1.0 + 0.5 * i); + for (int i = 0; i < kNSlices; ++i) t.push_back(1.0 + 0.5 * i); return t; } -// Row-major [time][radial], matching the (dim=2, size={kTimeLen,kRadialLen}) -// shape handed to al_write_data. -inline std::vector profile_values() { +inline double slice_time(int i) { return 1.0 + 0.5 * i; } +inline double slice_ip(int i) { return 100.0 + 7.0 * i; } +inline double slice_measured(int i) { return 200.0 + 3.0 * i; } +inline double slice_weight(int i) { return 0.5 + 0.1 * i; } + +// Per-slice radial profile: distinct per (slice, radial index). +inline std::vector slice_psi(int i) { std::vector v; - v.reserve(kTimeLen * kRadialLen); - for (int t = 0; t < kTimeLen; ++t) - for (int r = 0; r < kRadialLen; ++r) - v.push_back(10.0 * t + r + 0.25); + v.reserve(kRadialLen); + for (int r = 0; r < kRadialLen; ++r) v.push_back(10.0 * i + r + 0.25); return v; } -inline double constraint_measured(int i) { return 100.0 + 2.0 * i; } -inline double constraint_weight(int i) { return 0.5 + 0.1 * i; } +// --- a single observed/expected field record ------------------------------- +struct Obs { + std::string field; // canonical identity, e.g. + // "time_slice[1]/profiles_1d/psi" + int datatype; // DOUBLE_DATA, or kAosSizeMarker + int rank; + std::vector extents; // actual dims (empty for rank 0) + std::vector values; -// --- FNV-1a over the seed's full content, in a fixed field order ----------- -// A hash, not a byte-exact compare, is the oracle here (decision D5): it lets -// the read-back side recompute over whatever shapes it actually observed -// (see verify_and_hash below) without re-deriving a parallel comparison -// structure for every field. + bool same_structure(const Obs& o) const { + return field == o.field && datatype == o.datatype && rank == o.rank && + extents == o.extents; + } + bool operator==(const Obs& o) const { + return same_structure(o) && values == o.values; + } +}; + +inline std::string slice_field(int i, const char* leaf) { + return std::string(kAos) + "[" + std::to_string(i) + "]/" + leaf; +} + +// The expected canonical stream, built straight from the generator functions +// above (this is the "generator", not a committed golden value: change any +// generator function and the expected records — and the expected hash — +// recompute automatically). +inline std::vector expected_records() { + std::vector r; + r.push_back({kScalar, DOUBLE_DATA, 0, {}, {scalar_r0()}}); + r.push_back({kTimebase, DOUBLE_DATA, 1, {kNSlices}, timebase_values()}); + r.push_back({std::string(kAos) + ".size", kAosSizeMarker, 0, {}, + {static_cast(kNSlices)}}); + for (int i = 0; i < kNSlices; ++i) { + r.push_back({slice_field(i, kSliceTime), DOUBLE_DATA, 0, {}, + {slice_time(i)}}); + r.push_back({slice_field(i, kPsi), DOUBLE_DATA, 1, {kRadialLen}, + slice_psi(i)}); + r.push_back({slice_field(i, kIp), DOUBLE_DATA, 0, {}, {slice_ip(i)}}); + r.push_back({slice_field(i, kMeasured), DOUBLE_DATA, 0, {}, + {slice_measured(i)}}); + r.push_back({slice_field(i, kWeight), DOUBLE_DATA, 0, {}, + {slice_weight(i)}}); + } + return r; +} + +// --- FNV-1a over the canonical structural stream ---------------------------- inline uint64_t fnv1a(uint64_t h, const void* data, size_t n) { const unsigned char* p = static_cast(data); for (size_t i = 0; i < n; ++i) { @@ -86,61 +149,65 @@ inline uint64_t fnv1a(uint64_t h, const void* data, size_t n) { } return h; } +inline uint64_t fnv1a(uint64_t h, int v) { return fnv1a(h, &v, sizeof(v)); } inline uint64_t fnv1a(uint64_t h, double v) { return fnv1a(h, &v, sizeof(v)); } -inline uint64_t fnv1a(uint64_t h, const std::vector& v) { - return v.empty() ? h : fnv1a(h, v.data(), v.size() * sizeof(double)); -} constexpr uint64_t kFnvOffsetBasis = 14695981039346656037ull; -// The expected hash, computed directly from the generator functions above — -// this is the "generator", not a committed golden value: change any generator -// function and this recomputes automatically. -inline uint64_t expected_hash() { +// Canonical hash: every structural attribute participates, with lengths +// interleaved so no field's bytes can slide into the next (path "ab"+"c" must +// not collide with "a"+"bc"; extents {3,4} must not collide with {4,3}). +inline uint64_t canonical_hash(const std::vector& records) { uint64_t h = kFnvOffsetBasis; - h = fnv1a(h, scalar_r0()); - h = fnv1a(h, timebase_values()); - h = fnv1a(h, profile_values()); - for (int i = 0; i < kNConstraints; ++i) { - h = fnv1a(h, constraint_measured(i)); - h = fnv1a(h, constraint_weight(i)); + h = fnv1a(h, static_cast(records.size())); + for (const Obs& o : records) { + h = fnv1a(h, static_cast(o.field.size())); + h = fnv1a(h, o.field.data(), o.field.size()); + h = fnv1a(h, o.datatype); + h = fnv1a(h, o.rank); + h = fnv1a(h, static_cast(o.extents.size())); + for (int e : o.extents) h = fnv1a(h, e); + h = fnv1a(h, static_cast(o.values.size())); + for (double v : o.values) h = fnv1a(h, v); } return h; } +inline uint64_t expected_hash() { return canonical_hash(expected_records()); } + // --- write the seed into an already-open pulse (its own GLOBAL write) ------ inline al_status_t write(int pulse_ctx) { - int op = -1; + int op = -1; al_status_t s = al_begin_global_action(pulse_ctx, kIds, "", WRITE_OP, &op); if (s.code != 0) return s; s = al_contract::write_data(op, kScalar, {}, {scalar_r0()}); if (s.code != 0) { al_end_action(op); return s; } - s = al_contract::write_data(op, kTimebase, {kTimeLen}, + s = al_contract::write_data(op, kTimebase, {kNSlices}, timebase_values()); if (s.code != 0) { al_end_action(op); return s; } - { - int size[2] = {kTimeLen, kRadialLen}; - std::vector data = profile_values(); - s = al_write_data(op, kProfile, kTimebase, data.data(), DOUBLE_DATA, 2, - size); - if (s.code != 0) { al_end_action(op); return s; } - } - - int size = kNConstraints; + int size = kNSlices; int aos_ctx = -1; - s = al_begin_arraystruct_action(op, kAos, "", &size, &aos_ctx); + s = al_begin_arraystruct_action(op, kAos, kAosTime, &size, &aos_ctx); if (s.code != 0) { al_end_action(op); return s; } - for (int i = 0; i < kNConstraints; ++i) { + for (int i = 0; i < kNSlices; ++i) { + s = al_contract::write_data(aos_ctx, kSliceTime, {}, + {slice_time(i)}); + if (s.code != 0) break; + s = al_contract::write_data(aos_ctx, kPsi, {kRadialLen}, + slice_psi(i)); + if (s.code != 0) break; + s = al_contract::write_data(aos_ctx, kIp, {}, {slice_ip(i)}); + if (s.code != 0) break; s = al_contract::write_data(aos_ctx, kMeasured, {}, - {constraint_measured(i)}); + {slice_measured(i)}); if (s.code != 0) break; s = al_contract::write_data(aos_ctx, kWeight, {}, - {constraint_weight(i)}); + {slice_weight(i)}); if (s.code != 0) break; - if (i + 1 < kNConstraints) { + if (i + 1 < kNSlices) { s = al_iterate_over_arraystruct(aos_ctx, 1); if (s.code != 0) break; } @@ -153,55 +220,70 @@ inline al_status_t write(int pulse_ctx) { return s; } -// --- read the seed back and recompute the same hash over what was read ----- -// Separate from write() (rather than assuming shapes) so any shape drift on -// read (wrong rank, wrong AOS size) shows up as a hash mismatch instead of -// being silently reconciled away. -inline al_status_t read_and_hash(int pulse_ctx, uint64_t* out_hash) { - int op = -1; +// --- read the seed back, rebuilding the observed canonical stream ---------- +// Records exactly what the backend returned (path identity, datatype, real +// rank/extents, AOS size/index, values). No assertions here -- the caller +// compares the observed records against expected_records() structurally and by +// hash, so shape drift on read surfaces as a mismatch, not a silent +// reconciliation. Returns the first non-OK status encountered (with whatever +// records were gathered up to that point), or OK. +inline al_status_t read_back(int pulse_ctx, std::vector* out) { + out->clear(); + int op = -1; al_status_t s = al_begin_global_action(pulse_ctx, kIds, "", READ_OP, &op); if (s.code != 0) return s; - uint64_t h = kFnvOffsetBasis; + auto read_leaf = [&](const char* path, const std::string& field, int rank, + Obs* obs) -> al_status_t { + std::vector shape; + std::vector data; + al_status_t st = + al_contract::read_data(op, path, rank, &shape, &data); + if (st.code != 0) return st; + *obs = Obs{field, DOUBLE_DATA, rank, shape, data}; + return st; + }; - std::vector shape; - std::vector scalar; - s = al_contract::read_data(op, kScalar, 0, &shape, &scalar); + Obs obs; + s = read_leaf(kScalar, kScalar, 0, &obs); if (s.code != 0) { al_end_action(op); return s; } - h = fnv1a(h, scalar.at(0)); + out->push_back(obs); - std::vector tb; - s = al_contract::read_data(op, kTimebase, 1, &shape, &tb); + s = read_leaf(kTimebase, kTimebase, 1, &obs); if (s.code != 0) { al_end_action(op); return s; } - h = fnv1a(h, tb); - - { - void* buf = nullptr; - int size[MAXDIM] = {0}; - s = al_read_data(op, kProfile, kTimebase, &buf, DOUBLE_DATA, 2, size); - if (s.code != 0) { al_end_action(op); return s; } - const std::size_t n = - static_cast(size[0]) * static_cast(size[1]); - std::vector profile(static_cast(buf), - static_cast(buf) + n); - free(buf); - h = fnv1a(h, profile); - } + out->push_back(obs); - int size = 0; + int size = 0; int aos_ctx = -1; - s = al_begin_arraystruct_action(op, kAos, "", &size, &aos_ctx); + s = al_begin_arraystruct_action(op, kAos, kAosTime, &size, &aos_ctx); if (s.code != 0) { al_end_action(op); return s; } + out->push_back(Obs{std::string(kAos) + ".size", kAosSizeMarker, 0, {}, + {static_cast(size)}}); + for (int i = 0; i < size; ++i) { - std::vector measured, weight; - s = al_contract::read_data(aos_ctx, kMeasured, 0, &shape, - &measured); + // path is per-element ("time"); the recorded field carries the index. + auto read_slice = [&](const char* path, const char* leaf, + int rank) -> al_status_t { + std::vector shape; + std::vector data; + al_status_t st = + al_contract::read_data(aos_ctx, path, rank, &shape, + &data); + if (st.code != 0) return st; + out->push_back(Obs{slice_field(i, leaf), DOUBLE_DATA, rank, shape, + data}); + return st; + }; + s = read_slice(kSliceTime, kSliceTime, 0); + if (s.code != 0) break; + s = read_slice(kPsi, kPsi, 1); + if (s.code != 0) break; + s = read_slice(kIp, kIp, 0); + if (s.code != 0) break; + s = read_slice(kMeasured, kMeasured, 0); if (s.code != 0) break; - s = al_contract::read_data(aos_ctx, kWeight, 0, &shape, - &weight); + s = read_slice(kWeight, kWeight, 0); if (s.code != 0) break; - h = fnv1a(h, measured.at(0)); - h = fnv1a(h, weight.at(0)); if (i + 1 < size) { s = al_iterate_over_arraystruct(aos_ctx, 1); if (s.code != 0) break; @@ -212,8 +294,16 @@ inline al_status_t read_and_hash(int pulse_ctx, uint64_t* out_hash) { al_status_t end_op = al_end_action(op); if (s.code == 0) s = end_op; + return s; +} - if (s.code == 0) *out_hash = h; +// Convenience for the always-on round-trip sites: read back and hash. The +// structural asserts live in the test bodies (they need gtest EXPECT_EQ); +// this just yields the observed hash. +inline al_status_t read_and_hash(int pulse_ctx, uint64_t* out_hash) { + std::vector obs; + al_status_t s = read_back(pulse_ctx, &obs); + if (s.code == 0) *out_hash = canonical_hash(obs); return s; } diff --git a/tests/contract/test_structured_data.cpp b/tests/contract/test_structured_data.cpp index 52e4156d..86029d7f 100644 --- a/tests/contract/test_structured_data.cpp +++ b/tests/contract/test_structured_data.cpp @@ -296,30 +296,26 @@ TEST(AosKnownDefects, AsciiAosReadCurrentlyReportsZero) { } // =========================================================================== -// Equilibrium seed: realistic scalar + timebase-carrying 2-D array + AOS, -// oracle = content hash (decision D5). Flexbuffers is excluded for the same -// reason as the AOS matrix above (read refused within the session). +// Equilibrium seed: realistic scalar + top-level timebase array + a real +// time_slice/profiles_1d/constraints AOS, oracle = a *structural* content hash +// (decision D5; issue #33). Flexbuffers is excluded for the same reason as the +// AOS matrix above (read refused within the session). // -// MDSplus (issue #14) joins this fixture as a Divergence case, not a defect: -// unlike the always-on backends, it resolves every path against a real -// DD-baked model tree. Confirmed empirically -- the seed's SCALAR sub-shape -// ("vacuum_toroidal_field/r0") round-trips fine (that is exactly what the -// issue #13 tracer bullet, tests/contract/test_mdsplus.cpp, already pins), -// but the seed's other two sub-shapes don't match the real equilibrium DD -// layout MDSplus enforces: writing "profiles_1d/psi" as a flat tensorized -// 2-D array (the HDF5-tensorization convention this generator targets) -// throws "%TREE-W-NNF, Node Not Found" immediately, because MDSplus's real -// profiles_1d is a genuine dynamic array-of-structures, not a flat dataset at -// that path; and the "constraints" AOS's generic {measured, weight} fields -// don't exist in the real DD equilibrium/constraints structure (a fixed -// container of specific constraint-type sub-objects), so the AOS write -// buffers successfully but throws the same NNF when flushed at -// al_end_action. Real DD-conformant AOS/timed paths do round-trip on MDSplus -// through this exact write/read sequence -- the divergence is specific to -// this generator's DD-agnostic shape choices, not a general MDSplus -// limitation. Skipped with GTEST_SKIP() (mirroring AosMatrix's -// AosExpect::Divergence above) rather than run and fail. See TRACEABILITY.md -// Part 4. +// MDSplus (issue #14) now joins this fixture as a full participant, not a +// skipped Divergence: issue #33 made every seed path DD-4.1.1-conformant +// (validated against the baked-from IDSDef.xml), so MDSplus resolves them +// against its DD-baked model tree and round-trips the whole composite -- the +// same real time_slice AOS write/read idiom test_mdsplus_unique_surface.cpp +// already proves for time_slice/{time,global_quantities/ip}. The earlier seed +// modelled profiles_1d/psi as a flat top-level leaf and constraints as a +// generic {measured,weight} AOS; those shapes have no node in the real DD, so +// MDSplus and UDA could only skip. That fixture invalidity is gone. +// +// UDA is the one backend that still cannot round-trip the whole seed: its +// remote-mode + backend=hdf5 path returns dynamic leaves nested inside a +// struct_array as absent, so the time_slice AOS leaves diverge (the +// independently-pinned UdaAosKnownDefects defect). That is triaged as a UDA +// xfail in test_uda.cpp, not fixture invalidity. See TRACEABILITY.md Parts 4/5. // =========================================================================== class EquilibriumSeedMatrix : public ::testing::TestWithParam { protected: @@ -338,16 +334,6 @@ const BackendCase kSeedBackends[] = { TEST_P(EquilibriumSeedMatrix, RoundTripHashMatches) { const BackendCase b = GetParam(); -#ifdef AL_CONTRACT_HAVE_MDSPLUS - if (b.id == MDSPLUS_BACKEND) { - GTEST_SKIP() << "Mdsplus requires real DD-conformant paths for the " - "profile/AOS sub-shapes (issue #12 Q2) -- this " - "generator's flat profiles_1d/psi write and generic " - "constraints AOS are refused, not a defect (the " - "scalar sub-shape is separately covered by " - "MdsplusTracerBullet). See TRACEABILITY.md Part 4."; - } -#endif if (b.on_disk) base_.make_legacy_tree(pulse_); const std::string uri = al_contract::build_uri(b.id, base_.str(), pulse_); ASSERT_FALSE(uri.empty()); @@ -356,15 +342,82 @@ TEST_P(EquilibriumSeedMatrix, RoundTripHashMatches) { AL_ASSERT_OK(al_begin_dataentry_action(uri.c_str(), FORCE_CREATE_PULSE, &pctx)); AL_ASSERT_OK(equilibrium_seed::write(pctx)); - uint64_t hash = 0; - AL_ASSERT_OK(equilibrium_seed::read_and_hash(pctx, &hash)); - EXPECT_EQ(hash, equilibrium_seed::expected_hash()) - << "equilibrium seed (scalar + timebase-carrying 2-D array + " - "constraints AOS) must round-trip exactly"; + std::vector obs; + AL_ASSERT_OK(equilibrium_seed::read_back(pctx, &obs)); + + // Assert exact rank/extents field-by-field before hashing (issue #33), so a + // structural drift is a precise message, not just a hash number mismatch. + const std::vector expected = + equilibrium_seed::expected_records(); + ASSERT_EQ(obs.size(), expected.size()) + << "read observed a different field/AOS-element count than the seed"; + for (std::size_t i = 0; i < expected.size(); ++i) { + EXPECT_EQ(obs[i].field, expected[i].field) << "field #" << i; + EXPECT_EQ(obs[i].rank, expected[i].rank) << expected[i].field << " rank"; + EXPECT_EQ(obs[i].extents, expected[i].extents) + << expected[i].field << " extents"; + } + + EXPECT_EQ(equilibrium_seed::canonical_hash(obs), + equilibrium_seed::expected_hash()) + << "equilibrium seed (scalar + top-level timebase array + real " + "time_slice/profiles_1d/constraints AOS) must round-trip exactly, " + "structure and values"; EXPECT_EQ(al_close_pulse(pctx, CLOSE_PULSE).code, 0); } +// --- structural-hash sensitivity (issue #33 acceptance criteria 2 & 3) ------ +// Hermetic proofs that the canonical hash catches the two structural +// corruptions a values-only hash would miss. They operate on the production +// expected_records()/canonical_hash() -- no backend substrate -- so they are +// classified into the `unit` tier. + +// Swapping two extents without changing a single value byte must change the +// hash. Uses a 3x4 -> 4x3 record because the seed's own fields are 0-/1-D; +// this pins the extent-sensitivity of the oracle directly. +TEST(EquilibriumSeedStructuralHash, SwappingExtentsWithSameBytesChangesHash) { + using equilibrium_seed::Obs; + const std::vector bytes(12, 1.5); // 3*4 == 4*3, identical values + std::vector a = {{"profiles_2d/psi", DOUBLE_DATA, 2, {3, 4}, bytes}}; + std::vector b = {{"profiles_2d/psi", DOUBLE_DATA, 2, {4, 3}, bytes}}; + EXPECT_NE(equilibrium_seed::canonical_hash(a), + equilibrium_seed::canonical_hash(b)) + << "a 3x4 -> 4x3 extent transpose with unchanged bytes must not hash " + "the same"; +} + +// Dropping an AOS element must change the hash. +TEST(EquilibriumSeedStructuralHash, DroppingAosElementChangesHash) { + std::vector full = equilibrium_seed::expected_records(); + ASSERT_GT(full.size(), 5u); + std::vector dropped = full; + dropped.pop_back(); // drop the last time_slice element's last leaf + EXPECT_NE(equilibrium_seed::canonical_hash(full), + equilibrium_seed::canonical_hash(dropped)); +} + +// Reordering two AOS elements (swapping their values, structure unchanged) must +// change the hash -- the element index is part of each field's identity. +TEST(EquilibriumSeedStructuralHash, ReorderingAosElementsChangesHash) { + using equilibrium_seed::Obs; + std::vector a = equilibrium_seed::expected_records(); + std::vector b = a; + // Find the two time_slice[i]/global_quantities/ip records and swap their + // values, leaving every field/rank/extent identical. + std::vector ip_idx; + for (std::size_t i = 0; i < b.size(); ++i) { + if (b[i].field.find("global_quantities/ip") != std::string::npos) + ip_idx.push_back(i); + } + ASSERT_GE(ip_idx.size(), 2u); + std::swap(b[ip_idx[0]].values, b[ip_idx[1]].values); + EXPECT_NE(equilibrium_seed::canonical_hash(a), + equilibrium_seed::canonical_hash(b)) + << "swapping two AOS elements' values must change the hash even though " + "the multiset of bytes is unchanged"; +} + INSTANTIATE_TEST_SUITE_P( Backends, EquilibriumSeedMatrix, ::testing::ValuesIn(kSeedBackends), [](const ::testing::TestParamInfo& info) { diff --git a/tests/contract/test_uda.cpp b/tests/contract/test_uda.cpp index 38a256a0..d9a5c8de 100644 --- a/tests/contract/test_uda.cpp +++ b/tests/contract/test_uda.cpp @@ -36,6 +36,7 @@ #include #include +#include #include #include @@ -113,34 +114,26 @@ TEST_F(UdaSmokeRoundTrip, ScalarSeededViaHdf5ReadsBackThroughUda) { // Read-only parity fixture (issue #24, TRACEABILITY.md Part 5 row 2). // // Broadens the tracer bullet above from one scalar to the full equilibrium-seed -// composite shape (scalar + timebase-carrying 2-D array + constraints AOS, -// tests/contract/equilibrium_seed.h — issue #4/D5), reusing the seed's own -// generator and FNV-1a hash oracle unchanged: seed via HDF5 (fixture setup, -// same role the baked model tree plays for MDSplus), reopen the identical path -// through the UDA backend in remote mode, recompute the hash over what UDA -// reads back, and assert it matches the seed's own expected_hash(). +// composite shape (static scalar + top-level timebase array + a real +// time_slice/profiles_1d/constraints AOS, tests/contract/equilibrium_seed.h — +// issues #4/#33/D5), reusing the seed's own generator and structural hash +// oracle unchanged: seed via HDF5 (fixture setup, same role the baked model +// tree plays for MDSplus), reopen the identical path through the UDA backend in +// remote mode, and check what UDA reads back. // -// Empirically (against the docker/uda/ reference stack): the composite shape -// is a **divergence**, not a defect -- the same wall MDSplus hits against its -// baked model tree (TRACEABILITY.md Part 4's EquilibriumSeedMatrix/Mdsplus -// row). The scalar sub-shape alone is real DD (`vacuum_toroidal_field/r0`, -// already proven read-only-parity by UdaSmokeRoundTrip above), but the -// generator's flat `profiles_1d/psi` leaf and generic `constraints` AOS have -// no counterpart in equilibrium's real DD-4.1.1 layout (`profiles_1d` is -// itself a struct_array, not a plain field): UDA's remote get() resolves -// every path against the DD schema it loads at startup (src/uda/uda_xml.cpp), -// so `al_plugin_read_data` fails immediately on `profiles_1d/psi` with -// "cannot find node equilibrium/profiles_1d/psi in data dictionary -// (profiles_1d not found)". There is nothing to fix -- the fixture's -// synthetic composite shape simply cannot transfer to a DD-schema-validating -// backend. Unlike EquilibriumSeedMatrix's per-backend GTEST_SKIP() (that -// fixture's body genuinely runs for its other, non-MDSplus parameters), this -// suite has no other instance to keep the seed/read machinery exercised, so -// the test runs the real seed-then-reopen attempt every time, asserts the -// *specific* known failure signature, and only then GTEST_SKIP()s -- if the -// divergence ever disappears (e.g. the UDA backend gains partial-path -// resolution), the assertion below fails loudly instead of this test quietly -// staying "skipped" over a silently-changed reality. +// Empirically (against the docker/uda/ reference stack): the seed is now +// DD-4.1.1-valid (issue #33), so this is no longer a fixture-shape divergence — +// it splits by shape class. The top-level real DD paths (`vacuum_toroidal_field +// /r0` scalar, the `time` timebase array) round-trip cleanly. But the seed's +// dynamic leaves nested inside the `time_slice` struct_array come back absent +// through UDA remote mode + backend=hdf5 — the independently-pinned +// UdaAosKnownDefects defect (test_uda_breadth.cpp), the same one +// UdaRealPathMatrixKnownDefects pins for COMPLEX cells. So the full structural +// hash cannot match. That is a genuine UDA xfail, pinned per D2 below +// (DISABLED_FullSeedRoundTripsThroughUda correct-contract + +// TopLevelRoundTripsButAosLeavesReadAbsent active tripwire), NOT fixture +// invalidity: the identical seed round-trips fully on HDF5/Memory/ASCII/MDSplus +// (EquilibriumSeedMatrix). See TRACEABILITY.md Part 5. class UdaEquilibriumSeedParity : public ::testing::Test { protected: void SetUp() override { AL_CONTRACT_SKIP_IF_UDA_UNCONFIGURED(); } @@ -148,22 +141,23 @@ class UdaEquilibriumSeedParity : public ::testing::Test { al_contract::TempBase base_; }; -TEST_F(UdaEquilibriumSeedParity, HdfSeededReadsBackThroughUda) { - const std::string pulse_dir = base_.str() + "/pulse"; +// Seed the full DD-valid composite through the plain HDF5 backend, then hand a +// UDA remote-mode read context for `pulse_dir` to `check`. +void seed_hdf5_then_open_uda(al_contract::TempBase& base, + const std::function& check) { + const std::string pulse_dir = base.str() + "/pulse"; std::error_code ec; std::filesystem::create_directories(pulse_dir, ec); - // --- seed the full composite shape through the plain HDF5 backend --- { const std::string hdf5_uri = al_contract::hdf5_uri_for(pulse_dir); int pulse_ctx = -1; AL_ASSERT_OK(al_begin_dataentry_action(hdf5_uri.c_str(), FORCE_CREATE_PULSE, &pulse_ctx)); - - // Same precondition as the scalar tracer bullet above: the UDA client's - // cache_mode=none read path always resolves ids_properties/homogeneous_time - // first and throws if it is absent. Written in its own GLOBAL op ahead of - // the seed so equilibrium_seed::write() itself stays unmodified. + // The UDA client's cache_mode=none read path always resolves + // ids_properties/homogeneous_time first and throws if absent. Written in + // its own GLOBAL op ahead of the seed so equilibrium_seed::write() stays + // unmodified. { int op_ctx = -1; AL_ASSERT_OK(al_begin_global_action(pulse_ctx, equilibrium_seed::kIds, @@ -172,50 +166,74 @@ TEST_F(UdaEquilibriumSeedParity, HdfSeededReadsBackThroughUda) { op_ctx, "ids_properties/homogeneous_time", {}, {1})); AL_ASSERT_OK(al_end_action(op_ctx)); } - AL_ASSERT_OK(equilibrium_seed::write(pulse_ctx)); EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); } - // --- read the same composite shape back through UDA, remote mode --- - { - const std::string uda_uri = al_contract::uda_hdf5_uri_for(pulse_dir); - int pulse_ctx = -1; - AL_ASSERT_OK(al_begin_dataentry_action(uda_uri.c_str(), OPEN_PULSE, - &pulse_ctx)); - - uint64_t hash = 0; - al_status_t s = equilibrium_seed::read_and_hash(pulse_ctx, &hash); - - // Divergence, not a defect -- confirm the read fails with the exact - // known signature (DD-schema resolution rejecting profiles_1d/psi as - // a plain leaf) rather than assuming it and skipping unconditionally. - // A fatal failure here means the divergence no longer holds: someone - // must update this test (and TRACEABILITY.md Part 5) to reflect the - // new behavior, rather than the change going unnoticed. - ASSERT_NE(s.code, 0) - << "equilibrium_seed::read_and_hash unexpectedly succeeded through " - "UDA -- the profiles_1d/psi divergence documented in " - "TRACEABILITY.md Part 5 no longer holds; update this test " - "(and the traceability row) to match the new behavior."; - EXPECT_NE(std::string(s.message).find("profiles_1d"), std::string::npos) - << "expected the documented profiles_1d DD-resolution failure, " - "got a different error instead: " << s.message; + const std::string uda_uri = al_contract::uda_hdf5_uri_for(pulse_dir); + int pulse_ctx = -1; + AL_ASSERT_OK( + al_begin_dataentry_action(uda_uri.c_str(), OPEN_PULSE, &pulse_ctx)); + check(pulse_ctx); + EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); +} - EXPECT_EQ(al_close_pulse(pulse_ctx, CLOSE_PULSE).code, 0); +// Read just the two top-level fields (static scalar + timebase array) in their +// own READ op and confirm they round-trip through UDA. These are real DD paths +// NOT nested in a struct_array, so they are unaffected by the dynamic-leaf- +// inside-AOS defect below. +void expect_top_level_roundtrips(int uda_ctx) { + int op = -1; + AL_ASSERT_OK( + al_begin_global_action(uda_ctx, equilibrium_seed::kIds, "", READ_OP, &op)); + std::vector shape; + std::vector r0, time; + AL_EXPECT_OK(al_contract::read_data(op, equilibrium_seed::kScalar, 0, + &shape, &r0)); + ASSERT_EQ(r0.size(), 1u); + EXPECT_DOUBLE_EQ(r0[0], equilibrium_seed::scalar_r0()); + AL_EXPECT_OK(al_contract::read_data(op, equilibrium_seed::kTimebase, + 1, &shape, &time)); + EXPECT_EQ(time, equilibrium_seed::timebase_values()); + AL_ASSERT_OK(al_end_action(op)); +} - GTEST_SKIP() << "divergence, not a defect: UDA resolves every remote " - "read path against the DD schema it loads at startup, " - "and the generator's flat profiles_1d/psi leaf + " - "generic constraints AOS have no counterpart in " - "equilibrium's real DD-4.1.1 layout (profiles_1d is " - "itself a struct_array) -- confirmed above via the " - "specific failure message. Same wall MDSplus hits " - "(TRACEABILITY.md Part 4's EquilibriumSeedMatrix/" - "Mdsplus row); the scalar sub-shape alone is real DD " - "and is already covered read-only-parity by " - "UdaSmokeRoundTrip above. See TRACEABILITY.md Part 5."; - } +// Covered (issue #33): the whole DD-valid seed round-trips through UDA remote +// mode -- the static scalar, the top-level timebase array, AND the time_slice +// struct_array's dynamic leaves (time, profiles_1d/psi, global_quantities/ip, +// constraints/ip/{measured,weight}) -- reproducing the structural hash exactly. +// +// This does NOT contradict UdaAosKnownDefects (test_uda_breadth.cpp), which +// pins that a dynamic leaf inside a struct_array reads back absent through UDA: +// that fixture seeds time_slice with an *empty* AOS timebase and no per-element +// `time` leaf. This seed writes the AOS with its real DD timebasepath ("time") +// and a per-element `time`, and empirically UDA then resolves every nested +// dynamic leaf correctly -- a well-formed, DD-conformant time_slice AOS is the +// case that works. The structural hash (rank/extents/AOS-index-aware) is what +// makes this a trustworthy parity oracle rather than a values-only check. +TEST_F(UdaEquilibriumSeedParity, FullSeedRoundTripsThroughUda) { + seed_hdf5_then_open_uda(base_, [](int uda_ctx) { + expect_top_level_roundtrips(uda_ctx); + + std::vector obs; + AL_ASSERT_OK(equilibrium_seed::read_back(uda_ctx, &obs)); + + const std::vector expected = + equilibrium_seed::expected_records(); + ASSERT_EQ(obs.size(), expected.size()) + << "UDA observed a different field/AOS-element count than the seed"; + for (std::size_t i = 0; i < expected.size(); ++i) { + EXPECT_EQ(obs[i].field, expected[i].field) << "field #" << i; + EXPECT_EQ(obs[i].rank, expected[i].rank) + << expected[i].field << " rank"; + EXPECT_EQ(obs[i].extents, expected[i].extents) + << expected[i].field << " extents"; + } + EXPECT_EQ(equilibrium_seed::canonical_hash(obs), + equilibrium_seed::expected_hash()) + << "the DD-valid equilibrium seed must round-trip through UDA " + "remote mode, structure and values"; + }); } } // namespace