Repository navigation
Contract test suite (1/5): GoogleTest harness for the core backends - #95
Open
olivhoenen wants to merge 14 commits into
Open
olivhoenen wants to merge 14 commits into
olivhoenen wants to merge 14 commits into
Conversation
Implements Step 1 (the "scaffold slice") of TEST_STRATEGY.md: build the machine on one example so every mechanism is proven before the matrix is broadened. Binds only to the public C ABI (al_lowlevel.h + al_const.h) per decision D1. What lands: - tests/contract/al_contract.h — reusable harness: RAII temp pulse dir, legacy-parameter URI builder, and typed round-trip helpers over al_write_data/al_read_data honoring the buffer-ownership contract (scalar caller-owned; char-array core-allocated + freed). - test_roundtrip.cpp — TEST_P write->read round trip on HDF5 AND Memory; oracle is round-trip self-consistency, needing zero DD artifacts (D5). - test_known_defects.cpp — the al_setvalue_*_parameter_plugin unregistered-name null-deref (FUNCTIONALITY_INVENTORY.md:410) encoded per D2: a DISABLED_ test asserting the CORRECT contract (returns an error), plus a companion death test pinning today's crash to SIGSEGV specifically. The bug is not frozen as spec; the day it is fixed the xfail flips to pass. - CMake wiring (D3): GoogleTest via find_package-or-FetchContent (pinned to the v1.15.2 commit), gtest_discover_tests with a "contract" label, and an AL_CONTRACT_SANITIZE toggle (D7) that instruments both the tests and libal with ASan/UBSan — non-blocking. The pure-C canary (testlowlevel_c.c) is retained unchanged. Deferred (recorded): D7's dedicated sanitizer CI leg + suppressions file, and the unit/integration label split, arrive with the broader matrix in later steps. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit 3ab1010)
Step 3 of TEST_STRATEGY.md §4 (the thin unit tier), building on the #2 scaffold. Binds only to the public C ABI per decision D1. What lands: - test_introspection.cpp — pure-lookup unit tests for Cluster 4 (FUNCTIONALITY_INVENTORY.md:434-459), all intended-contract (D2): const2str/err2str on mapped ids; the silent "" for unmapped ids asserted as the deliberate contract (incl. the documented TIMERANGE_OP / FLEXBUFFERS_BACKEND map-coverage gaps, :75-79); the two maps proven distinct; getALVersion == the compiled AL_VERSION macro and dotted-numeric; getDDVersion == "!!DEPRECATED!!". - test_version_sentinel.cpp — decision D5 tripwire: a fake DD version written to ids_properties/version_put/data_dictionary round-trips verbatim AND leaves getDDVersion() unchanged, proving the core interprets no DD version. Parametrized over Memory (hermetic) and HDF5 (on-disk). - TRACEABILITY.md — decision D8 matrix; Cluster 4 + version-sentinel rows filled, scaffold rows seeded, remaining clusters listed as gaps for later build-order steps. - CMakeLists.txt — wires both files into contract_tests. The finer CTest unit/integration LABEL split (D6) stays deferred as the scaffold recorded; the pure-lookup subset is addressable via `ctest -R Introspection`. Per the issue's adapted TDD loop, each assertion was mutation-verified to fail for the right reason (real value surfaced) before setting the true expectation green. No new expected-fails; the crash-class defects remain in test_known_defects.cpp. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit 7fa83c3)
Implements Step 2 of TEST_STRATEGY.md — the bulk of the storage contract — building on the scaffold (issue #2) and binding only to the public C ABI (D1). Fixture layer (al_contract.h): - Deterministic synthetic-data generator over the datatype × shape space: synth_value/synth_buffer for CHAR/INTEGER/DOUBLE/COMPLEX and shape_for_rank scalar→7-D (MAXDIM), with values kept clear of the EMPTY sentinels so a scalar write is never silently dropped by data_has_non_zero_shape. - Generic typed write_data/read_data honoring the two ABI ownership regimes (scalar caller-buffer; array core-allocated + freed). - build_uri gains a FLEXBUFFERS branch: getURIBackend (al_context.cpp:280) has no flexbuffers case though the parse side accepts the scheme, so the URI is hand-built (documented, and tracked as an expected-fail). Matrix (test_roundtrip_matrix.cpp): - One TEST_P over Combine({HDF5,Memory,ASCII,Flexbuffers} × {CHAR,INTEGER, DOUBLE,COMPLEX} × ranks 0..7); each cell its own ctest entry. Oracle = round-trip self-consistency, zero DD artifacts (D5). - Each cell classified once (D2/D4): RoundTrip (assert equality), Refused (paired-negative — CHAR>2D on HDF5/ASCII; every Flexbuffers cell, which accepts the write but refuses the read-back as a serializer), or KnownDefect. - Known defects asserted as their correct contract and marked expected-fail: HDF5 crash on a CHAR scalar (DISABLED_ + paired death test), ASCII numeric corruption/abort at the MAXDIM boundary, and the flexbuffers URI-builder gap. TRACEABILITY.md: al_read_data/al_write_data rows filled (D8). Full contract suite: 139 passed, 4 known-defect cells skipped, 6 expected-fails disabled; each expected-fail verified to produce a real red when enabled. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit edbdee0)
TEST_STRATEGY.md §4 step 4 (Cluster E): slice / timerange / al_list_filled_paths tested positive where the backend supports the capability and paired-negative (documented refusal) where it does not, so the supports* matrix is itself under test at the C ABI boundary (D1 — the C++ Backend flags are asserted through their sole ABI-observable consequence, accept-vs-refuse, since they cannot be linked directly). Support columns were established empirically against the binary and follow D2 (assert the real contract, not the issue's assumptions): - Memory genuinely supports slice (full SLICE_OP handling in memory_backend.cpp; correct closest-slice values), so slice is positive on HDF5+Memory and paired-negative only on ASCII+Flexbuffers — not the issue's assumed Memory-negative. - Refusal codes pinned specifically: timerange -> LOWLEVEL_ERR (al_lowlevel.cpp:1053), slice / list_filled_paths -> BACKEND_ERR, slice-read without an interp mode -> CONTEXT_ERR (al_context.cpp:347). What lands: - test_capability_gated.cpp — CapabilityMatrix TEST_P over the four always-on backends for timerange, slice read, slice-write-begin, and list_filled_paths; HDF5-only detail tests for the three interpolation modes (CLOSEST/PREVIOUS/LINEAR) and timerange with/without a resampling dtime grid, each with an exact value oracle (seeded via a single global write, so the read machinery is exercised independently of append). - KNOWN DEFECT per D2: repeated WRITE+UNDEFINED_TIME slices do not accumulate in this build (only the last persists). Encoded as a DISABLED_ correct-contract test asserting both slices persist plus a current-behavior tripwire pinning today's single-slice result; write lifecycle ownership stays with issue #3. - TRACEABILITY.md — Cluster E rows filled (covered + the append xfail); stale Cluster-E gap bullet removed. - CMake: test_capability_gated.cpp added to the contract_tests target. Full contract suite: 164 tests green (1 xfail disabled). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit fcc1e24)
Correction of the .md to fit correctly in order of the processed issues. (cherry picked from commit 1e43819)
What changed: tests/contract/test_capability_gated.cpp — open_seeded() refactored from a int-returning helper using non-fatal EXPECT_* into a void open_seeded(int* pctx) using ASSERT_*, with every call site wrapped in ASSERT_NO_FATAL_FAILURE(open_seeded(&pctx)). Why this fixes the defect: previously, if the URI build / data-entry open / seed write failed, the test continued and the paired-negative branch (EXPECT_NE(s.code,0) / EXPECT_EQ(s.code, BACKEND_ERR)) could pass for the wrong reason — a broken seed looking identical to a genuine documented refusal. ASSERT_* inside a subroutine only aborts the subroutine, so ASSERT_NO_FATAL_FAILURE at the call site is what actually aborts the test on setup failure. Now a refusal assertion can only be reached with a genuinely-seeded entry. The comment, which previously claimed fatal handling it didn't provide, now matches the code. (cherry picked from commit e48e222)
(cherry picked from commit 1e80dd6)
Two scaffold (issue #2) acceptance items were never delivered; this closes both. 1. ctest -L unit label split. The suite carried only the contract label, so ctest -L unit selected nothing (PRD user story 29). Now two disjoint gtest_discover_tests passes label each case by substrate (decision D6): unit = pure lookups + in-RAM Memory + plugin ABI tests (Introspection.*:*Memory*:KnownDefects*), integration = everything on-disk. Both also keep contract. Two gotchas handled: the LABELS list needs an escaped ; to survive re-emission, and gtest's negative-filter syntax is one - then colon-separated (*:-a:b:c), not -a:-b:-c. Guarded by CMake ≥ 3.22 (TEST_FILTER), falling back to the single contract label on the 3.21 floor. Verified: ctest -L unit = 49 hermetic tests (0 on-disk), runs in 0.20 s; integration = 126; contract = 175 (49 + 126, disjoint). 2. Non-blocking sanitizer CI leg + suppressions file. Added .github/workflows/sanitizers.yml: a Linux continue-on-error job that configures -DAL_CONTRACT_SANITIZE=ON, builds, and runs the contract suite under ASan/UBSan/LSan, with tests/contract/sanitizer_suppressions.txt wired via LSAN_OPTIONS. Validated the (previously never-exercised) AL_CONTRACT_SANITIZE build path locally: it compiles, links ASan into the binary, and passes 100% (163 tests) under ASan/UBSan. The run excludes Death tests (deliberate crashes) and the CurrentlyCorrupts tripwires — the latter because the ASCII rank-7 DOUBLE/COMPLEX corruption is UB that ASan's memory layout masks (documented in-code and in the workflow). (cherry picked from commit e5ce77c)
…tinel The sentinel test's real property is non-interpretation: writing a DD version to version_put/data_dictionary must not change what getDDVersion() returns. Compare the post-write value to the captured baseline (dd_before) rather than to the "!!DEPRECATED!!" literal — the literal value is already pinned by test_introspection.cpp::GetDDVersionReturnsDeprecatedSentinel, so this removes the cross-test coupling and keeps the invariance check correct even if the compiled sentinel ever changes. The one baseline assertion stays as the anchor. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit 306df1c)
…le (issue #4) Characterizes existing C-ABI behavior for arrays-of-structures (top-level + nested write/iterate/read), al_delete_data at signal/structure/DATAOBJECT-root granularity, al_get_occurrences, and the remaining pulse-lifecycle detail (al_begin_dataentry_action modes, al_context_info, al_get_backendID, al_close_pulse CLOSE/ERASE) across the always-on backend tier. Adds a deterministic equilibrium-seed generator (scalar + timebase-carrying 2-D array + constraints AOS, oracle = content hash, no committed blobs) to exercise realistic AOS/timebase/HDF5-tensorization shapes. Empirically validated against the built library (not just source reading) per TEST_STRATEGY.md's TDD discipline, which surfaced several defects invisible to static analysis: ASCII's AOS read always reports size 0 (its READ_OP setup drains the stream into a lookup map before the AOS-size cursor ever runs), HDF5/Memory/ASCII/Flexbuffers each implement a disjoint, non-overlapping slice of the three delete granularities, ERASE_PULSE is a no-op-equivalent to CLOSE_PULSE on every always-on backend, ASCII's CREATE_PULSE silently overwrites instead of refusing, and al_get_backendID accepts a wrong-type context via an unchecked static_cast. Each genuine defect is pinned per decision D2: a DISABLED_ correct-contract test paired with an always-run current-behavior tripwire, never frozen as if it were the spec. Fills the AOS / delete / occurrences / lifecycle rows in TRACEABILITY.md. Co-Authored-By: Claude Sonnet 5 <[email protected]> (cherry picked from commit 9a015bb)
Cover User Cluster 3 (plugin management) through the public C ABI, per TEST_STRATEGY.md build-order step 5 and decision D2. - test_plugin_fixture.cpp: a real, self-contained access_layer_plugin built as a loadable alcontract_plugin.so (the classic red-green fixture), so the register/bind/unbind/unregister state machine and the al_setvalue_* happy paths run end to end. Its setParameter logs the received value so tests can prove a value crossed the ABI, not just that the call returned success. - test_plugins.cpp: the register/bind/unbind/unregister state machine + the documented quirks (register-twice throws, bind-unregistered throws, double-bind throws, unbind-unbound silent no-op), al_is_plugin_registered, all three al_setvalue_* variants reaching the plugin, and framework gating. Two defects pinned as xfail + paired current-behavior tripwire: the registered-but-never-bound plugin left un-destroyed on unregister, and the dlopen-failure swallowed by the NDEBUG-stripped assert. - test_known_defects.cpp: add the double- and generic-variant setvalue null-deref death tests (SIGSEGV) + DISABLED_ correct-contract partners, joining the scaffold's int variant; share the unregistered-name constant. - al_contract.h: hoist the shared kUnregisteredPluginName constant. - CMakeLists.txt: build the plugin MODULE (POSIX-guarded), wire its dir/name via compile defs, add test_plugins.cpp, extend the unit label filter. - TRACEABILITY.md: fill the Cluster 3 rows, extend the xfail bookkeeping table (now 11 pairs), mark Part 3 setParameter covered (indirect). Empirical finding vs the inventory: the dlopen defect does not crash on this NDEBUG build (the null handle reaches dlsym, whose null-check throws), so it is a plain xfail, not a death test; the setvalue null-derefs genuinely SIGSEGV and remain death tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit 1cbc103)
…sue #8) Pins caller-frees ownership for al_build_uri_from_legacy_parameters's *uri (the three other flagged APIs were already pinned by earlier issues), resolves empirically that plugin-parameter dim/size is not bounded by MAXDIM, and covers the remaining Plugin-author audience reachable through the C ABI: low-level al_plugin_* reentry, write/read data interception via a bound plugin, al_write_plugins_metadata, and al_bind/unbind_readback_plugins (extending the in-repo test plugin fixture with an env-var-gated readback capability). TRACEABILITY.md now has no blank rows; the two remaining gap rows are marked terminal with an explicit rationale. Co-Authored-By: Claude Sonnet 5 <[email protected]> (cherry picked from commit 10cdaf2)
(cherry picked from commit c2c21db)
- TEST_STRATEGY.md D2 now states the actual (and better) discipline: each expected-fail's FUNCTIONALITY_INVENTORY / issue citation lives in tests/contract/TRACEABILITY.md as the single source of truth, and test files carry a pointer to the matrix instead of repeating citations in-line. - The three flagged test files (test_roundtrip_matrix.cpp, test_pulse_lifecycle.cpp, test_structured_data.cpp) each gained that one-line pointer in their header comment, so a reader landing on a DISABLED_ test knows exactly where its citation is. (cherry picked from commit 86e7f38)
olivhoenen
requested review from
SimonPinches and
prasad-sawantdesai
as code owners
October 14, 2026 06:39
This was referenced Oct 14, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the split of #85 (Tests and Benchmarks for regression) into smaller, single-purpose PRs that are easier to review.
Adds the GoogleTest/CTest contract-test harness (
tests/contract/) that characterizes the public C ABI on the always-on backends (HDF5, ASCII, Memory, Flexbuffers):list_filled_paths);Existing defects are not fixed here: each is pinned as a
DISABLED_correct-contract test plus a current-behavior tripwire, and tracked intests/contract/TRACEABILITY.md. Also adds thesanitizersandcontract-testsCI workflows. Tests and CI only, no library change.Commit 3ae109d (a 2-line TRACEABILITY.md count update) is not included: its content was already part of Yohann's merge resolution used for the preceding conflict.
Fork history: issues yohannmarguier#2 – yohannmarguier#8, fork PRs yohannmarguier#10, yohannmarguier#11, yohannmarguier#19.
Stack position: 1/9 of the stack; depends on #93 (base: its branch).
Authorship: these commits are @yohannmarguier's, cherry-picked unchanged from his fork with
git cherry-pick -x, so each one keeps its original author, date and message plus a(cherry picked from commit …)link back to the fork. Only conflicts with currentdevelopand with the new commit order were resolved.🤖 Generated with Claude Code