test: replace vol-tests submodule with HDF5's in-tree API test suite#73
Draft
brtnfld wants to merge 31 commits into
Draft
test: replace vol-tests submodule with HDF5's in-tree API test suite#73brtnfld wants to merge 31 commits into
brtnfld wants to merge 31 commits into
Conversation
… test suite) Add a daos-test job to CI that installs DAOS 2.6.5, builds HDF5 from source (matrixed over hdf5_1.14.6 and develop, the latter marked continue-on-error since it tracks a moving upstream target), builds the connector against it, and runs the full ctest suite (h5vl_test plus HDF5's own native/parallel test binaries ported to run against a VOL connector, plus the connector's own h5daos_test_* targets) against a live, ephemeral, single-node DAOS instance. Key pieces: - Rocky Linux 8 container running --privileged (required to disable transparent hugepages, which DAOS's SPDK housekeeping needs even with a pure-RAM storage tier). - test/scripts/daos_server.yml.in: RAM-emulated SCM storage tier sized and configured for small/CI hosts (system_ram_reserved lowered, scm_hugepages_disabled, nr_hugepages fixed). - test/scripts/daos_pool.sh.in: rewritten for the modern dmg CLI syntax, polling for engine join and pool queryability before signaling readiness to the test driver, to fix races where the harness would start the client before the pool was actually usable. - Excludes 3 known, tracked failures (h5_test_testhdf5, h5_partest_t_bigio, h5vl_ext_h5daos_test_metadata_parallel) so CI stays green on everything that's actually been verified so far - see the PR description for root-cause details on each. - .gitmodules: test/vol temporarily points at the brtnfld/vol-tests fork branch pending HDFGroup/vol-tests#67 merging; revert to HDFGroup/vol-tests once that lands.
1 task
brtnfld
changed the title
This was referenced Jul 6, 2026
…ia CI testing Real, confirmed bugs found while exercising the connector against a live DAOS instance for the first time: - daos_vol_type.c / daos_vol_private.h / daos_vol_obj.c: H5_daos_dtype_t cached committed datatypes as a persistently-open hid_t, which looks identical to a user-visible open object to HDF5 core's native H5Fget_obj_count(H5F_OBJ_ALL, ...) accounting. Cache the datatype in its serialized (H5Tencode'd) form instead. - daos_vol.c: stop falsely advertising H5VL_CAP_FLAG_EXTERNAL_LINKS and H5VL_CAP_FLAG_UD_LINKS - H5_daos_link_create() hard-errors on H5VL_LINK_CREATE_UD (how H5Lcreate_external() is implemented internally by HDF5), so external/UD links were never actually supported despite the advertised flags. - daos_vol_link.c: reject H5Lcreate_hard() across two different files, matching the existing check already present in the H5Lmove/H5Lcopy path. - daos_vol_obj.c: handle H5I_ATTR in the H5VL_OBJECT_BY_SELF target-object resolution used by H5Oget_info and H5Ovisit, instead of type-punning the attribute's H5_daos_item_t header as a full H5_daos_obj_t. HDF5 2.0 (tracked by the develop branch) API compatibility: - daos_vol_private.h: add H5_VERSION_GE-gated macros for H5Tdecode (H5Tdecode1(buf) vs H5Tdecode2(buf, buf_size)) and for looking up this connector's own registered ID by class value (H5VLpeek_connector_id_by_value(), removed in 2.0, vs H5VLget_connector_id_by_value()). Update all 5 H5Tdecode call sites (daos_vol_attr.c, daos_vol_dset.c, daos_vol_map.c, daos_vol_obj.c) to use the new macro. Two fixes previously bundled here have been split out to their own branches/PRs: the H5Fget_obj_count/H5Fget_obj_ids container-label-vs- pointer comparison fix (obj-count-fix branch) and the H5_daos_link_ibco_task() creation-order-untracked fallback fix for H5Ovisit (best-effort-index-fix branch).
Points at the merged HDFGroup/vol-tests#67, which fixes test-driver robustness against benign DAOS log noise, gates known DAOS-specific test gaps on connector name rather than skipping unconditionally, and fixes HDF5 2.0 compatibility issues in the vendored test suite; and HDFGroup#68, which fixes an unrelated pre-existing CI workflow typo (ctest --build isn't a valid flag) discovered while investigating that suite's own CI status. .gitmodules is reverted to the upstream vol-tests URL now that both fixes have landed there.
17 is available in the same Ubuntu 24.04 (ubuntu-latest) universe repo
that already provides 14, and produces byte-identical output on every
file in src/*.c and src/*.h (verified against clang-format 14.0.6 and
17.0.6 directly) - only one trivial, cosmetic rule changed anywhere in
the repo (a space after "//" in a single commented-out line in
test/vol/hdf5_testpar/testpar.h). Drops the comment explaining the
13->14 bump, since that reasoning ("14 is the oldest still available")
no longer applies once jumping straight to the current version.
h5vl_test now hits two known bugs: over-counting on repeated file opens, and H5Ovisit hard-erroring on a group without creation-order tracking. The first failure kills the shared DAOS server mid-run, cascading into most of the rest of the suite. Skip it here until those are resolved and reviewed.
…area Expose DAOS_SERVER_TARGETS as a CMake cache variable instead of hard-coding the engine to 4 targets and papering over CI's smaller core count with DAOS_TARGET_OVERSUBSCRIBE; CI now passes -DDAOS_SERVER_TARGETS="$(nproc)" to match the runner it's actually on. Enable HDF5_VOL_TEST_ENABLE_PART so h5vl_test is registered as one ctest entry per functional area (attribute, dataset, file, group, ...) instead of a single binary, each against its own DAOS server instance. CI now excludes only the two known-broken areas (h5vl_test_file, h5vl_test_object) instead of skipping the whole suite, since a crash in one area no longer takes the rest down with it. Also drops two CMake comments that only described now-removed CLI flags and provider names with no remaining bearing on the current config.
The libdaos prerequisite only listed a years-old minimum version with no indication of what's actually exercised now that CI builds and runs the test suite against a live DAOS instance.
H5_DAOS_G_INIT() previously used H5VLpeek_connector_id_by_value() on HDF5 <2.0 (no owned reference) and H5VLget_connector_id_by_value() on >=2.0 (which does take one) via a version-gated macro, but H5_daos_term() unconditionally just discarded H5_DAOS_g without releasing that reference. H5VLget_connector_id_by_value() has been available since the VOL's introduction, so use it unconditionally and drop the peek/get macro, and release its reference in H5_daos_term() before forgetting the ID.
0b50093 switched H5_daos_dtype_t to cache committed datatypes in their H5Tencode'd form, to keep a live hid_t from inflating H5Fget_obj_count(H5F_OBJ_ALL, ...)'s count. Per review, a VOL connector holding an ID for its own object bookkeeping is legitimate and expected to be counted, so revert to keeping the copied hid_t directly (as before 0b50093), while keeping the H5_DAOS_TDECODE macro for HDF5 2.0's H5Tdecode1/H5Tdecode2 split.
…of skipping it h5vl_test doesn't crash or cascade - the prior run (before it was excluded) completed with 807/896 (90%) internal checks passing; the ctest entry was only marked FAILED because the driver treats any "FAILED" substring in output as fatal, with no way to mark individual checks as known-broken from outside the test binary. Gate the two specific failing checks (overlapping-file-opens object count, and H5Ovisit(_by_name) by creation order on an untracked root group) on the connector name in vol-tests itself (HDFGroup/vol-tests#70), matching the existing pattern already used in that file. .gitmodules points at the fork branch until the PR merges upstream. Revert the h5vl_test PART-mode split and CI exclusion from the last two commits - no longer needed since h5vl_test runs clean as a single test now.
… in CI Every single test in the last run failed identically: server starts, storage formats, but the engine never reports "Joined" within the pool script's 30s poll window - reproducible on the very first test, ruling out resource exhaustion from repeated cycles. The only infra-relevant change was replacing the hardcoded targets=4 + DAOS_TARGET_OVERSUBSCRIBE=1 with -DDAOS_SERVER_TARGETS="$(nproc)" and no oversubscribe: unlike a plain too-many-targets mismatch (which makes the engine refuse to start immediately with a clear error), nproc apparently doesn't reflect the actually-usable core count in this container, and without the oversubscribe fallback the engine hangs during startup instead. Restore DAOS_TARGET_OVERSUBSCRIBE=1 as an unconditional safety net and drop CI's nproc override, falling back to the proven-working default of 4 targets. DAOS_SERVER_TARGETS remains CMake-configurable for anyone who wants to tune it explicitly.
Every test in the last CI run crashed identically on process shutdown after otherwise completing cleanly: "h5vl_test: src/common/tse.c:775: tse_sched_progress: Assertion '0' failed" (SIGABRT). Root cause: H5_daos_term() is invoked by the library while closing the last reference to H5_DAOS_g itself, so calling H5Idec_ref() on that same ID from within the callback recurses into still-being-torn- down library state, corrupting the DAOS task scheduler on the way out. Keep H5_DAOS_G_INIT()'s switch to H5VLget_connector_id_by_value() (still correct - it works the same on both HDF5 <2.0 and >=2.0, so the version macro was unnecessary), but revert closing that reference in H5_daos_term(); the original code's "no need to close it here" was right.
jhendersonHDF flagged (PR HDFGroup#71) that H5_daos_term() not releasing the H5VLget_connector_id_by_value() reference is a real leak in the case where term() is reached outside of the ID's own refcount hitting zero - just not one worth chasing right now. Document why closing it unconditionally already caused a confirmed crash, and what a correct fix would actually require (H5is_library_terminating() plus a way to know H5_DAOS_g's provenance), rather than leaving the tradeoff implicit. Tracked in HDFGroup#74.
Runs test/API and testpar/API (built via -DHDF5_TEST_API_INSTALL=ON) through a dependency-free Python test driver instead of the defunct vol-tests submodule and its vendored C++/KWSys driver. Unsupported features are handled via H5VL_CAP_FLAG_* checks in the test suite itself plus a per-interface/-binary -x subtest exclusion mechanism for gaps with no capability flag to gate on. See docs/api-tests-migration.md for how it's wired.
brtnfld
force-pushed
the
feature/api-tests-migration
branch
from
July 8, 2026 15:04
dd2c74d to
1907823
Compare
brtnfld
changed the title
The server and client shared one --mpiexec-max-numprocs value in h5vl_test_driver.py, so configuring the client's numprocs to 2 (for parallel API tests) also launched two daos_server ranks. The second rank found the control-server port already bound by the first and exited immediately with "another daos_server process is already running", which the driver correctly reported as a failed startup - causing every test in the CI run to fail. The server now always gets its own --mpiexec-server-max-numprocs, hardcoded to 1 in test/CMakeLists.txt since a DAOS server engine here is always a single process regardless of how many client ranks a given test uses.
daos_pool.sh formats storage and creates a pool against whatever the currently running server has initialized. Running --init-command (rm -rf on the DAOS SCM mount) after the server already started wiped that storage out from under it instead of giving the next test a clean mount before startup, so pool create failed to produce a UUID and the driver correctly reported the token as missing.
client-init's output was only collected silently for token-regex matching, so a "token not found" failure gave no visibility into what daos_pool.sh / dmg actually printed. Print it live like the server/client-helper streams already are, and flush those prints immediately so ctest's captured output reflects real chronological order instead of buffering stdout until process exit.
subprocess.run(shlex.split(cmd), ...) never expands shell globs -
shlex.split() only tokenizes, it doesn't invoke a shell. --init-command
is "rm -rf \${DAOS_SERVER_SCM_MNT}/* \${...}/*.h5", so rm received the
literal argument "/mnt/daos/*" (a nonexistent filename), -f silently
swallowed the error, and cleanup was a no-op on every test run. Later
tests inherited the first test's already-formatted DAOS storage and
already-created pool, causing dmg storage format/pool create to fail
with "system is running" / "pool label already exists". Running these
developer-authored command strings through a real shell (shell=True)
lets the glob expand as intended.
…river Depending on an HDF5 install configured with -DHDF5_TEST_API_INSTALL=ON (a flag almost nobody sets outside Spack) meant this connector's test suite silently did nothing for most HDF5 installations, and duplicated orchestration logic (a private Python driver, custom ctest wiring) that HDF5 already provides for exactly this purpose. CI now builds HDF5 from source as the top-level project and pulls this connector in as an HDF5_VOL_ALLOW_EXTERNAL=LOCAL_DIR subproject, the same mechanism HDF5's own CI uses to test vol-async/vol-cache/vol-rest. HDF5's own test/API and testpar/API suites register this connector's ctest entries automatically via HDF5_EXTERNAL_VOL_TARGETS. The DAOS server, agent, and pool now start once per CI job via plain shell steps instead of once per test, removing the private test driver entirely. test/daos_vol's native tests are exposed to HDF5's suite the same way, via HDF5_API_EXT_SERIAL_TESTS/HDF5_API_EXT_PARALLEL_TESTS (previously named HDF5_VOL_EXT_*_TESTS, which didn't match the variable names HDF5's test/API and testpar/API CMakeLists.txt actually read, so they were never being registered at all).
…river Depending on an HDF5 install configured with -DHDF5_TEST_API_INSTALL=ON (a flag almost nobody sets outside Spack) meant this connector's test suite silently did nothing for most HDF5 installations, and duplicated orchestration logic (a private Python driver, custom ctest wiring) that HDF5 already provides for exactly this purpose. CI now builds HDF5 from source as the top-level project and pulls this connector in as an HDF5_VOL_ALLOW_EXTERNAL=LOCAL_DIR subproject, the same mechanism HDF5's own CI uses to test vol-async/vol-cache/vol-rest. HDF5's own test/API and testpar/API suites register this connector's ctest entries automatically via HDF5_EXTERNAL_VOL_TARGETS. The DAOS server, agent, and pool now start once per CI job via plain shell steps instead of once per test. test/daos_vol's native tests are exposed to HDF5's suite the same way, via HDF5_API_EXT_SERIAL_TESTS/HDF5_API_EXT_PARALLEL_TESTS (previously named HDF5_VOL_EXT_*_TESTS, which didn't match the variable names HDF5's test/API and testpar/API CMakeLists.txt actually read, so they were never being registered at all).
daos-test runs inside a rockylinux:8 container. \${{ github.workspace }}
resolves to the host runner's path (/home/runner/work/vol-daos/vol-daos),
not the container's actual mount point (/__w/vol-daos/vol-daos) - so
HDF5_VOL_PATH01 pointed at a path that doesn't exist inside the
container, and CMakeVOL.cmake's existence check failed immediately.
Computing these paths via $(pwd) at shell runtime instead gives the
container's own view of the workspace.
CMakeVOL.cmake pre-sets HDF5_FOUND before HDF5's own src/ subdirectory (which creates the hdf5-shared target) has been added, so this eager if(TARGET ...) check fails even though the target is a valid forward reference that will exist by generate time. HDF5_VOL_ALLOW_EXTERNAL is only ever set by an outer HDF5 build doing exactly this, so use it to distinguish this case from a genuinely misconfigured standalone HDF5 install missing shared libraries.
Same underlying issue as the previous hdf5-shared target-existence fix, but at CMake's generate step instead of configure: export(TARGETS hdf5_vol_daos ...) requires every target hdf5_vol_daos links against to be part of a resolvable export set, but hdf5-shared isn't part of any export set until HDF5 itself is installed. This export is only meant for exposing vol-daos's targets to a parent project in the standalone build, so skip it under the same HDF5_VOL_ALLOW_EXTERNAL condition.
daos_agent_sh lives in test/scripts/, but the daos_agent.sock it starts lives one level up in test/ (DAOS_AGENT_DIR in test/CMakeLists.txt). DAOS_AGENT_DRPC_DIR was derived from dirname(daos_agent_sh), landing one directory too deep, so every test failed at H5_daos_init() with DER_AGENT_COMM before ever reaching real API test code.
dmg reporting a created pool doesn't mean the pool is immediately attachable by an application (as opposed to dmg's own admin RPCs, already polled by daos_pool.sh via "dmg pool query"). daos_control.log shows a transient "pool ... is locked" message right as the pool comes online, and every test run immediately afterward failed at H5_daos_init() with DER_AGENT_COMM, while tests running later in the same job (after other tests had already spent 5-25s) succeeded consistently.
…_PATH These were computed via `find build -name ...`, which returns paths relative to the current directory. HDF5's own test/API/CMakeLists.txt runs each HDF5_VOL_vol-daos-h5_api_test_<iface> test with its own WORKING_DIRECTORY (a per-connector subdirectory), not the directory these paths were computed relative to, so DAOS_AGENT_DRPC_DIR and HDF5_PLUGIN_PATH resolved to the wrong location for every test in that group - explaining why they all failed at H5_daos_init() with DER_AGENT_COMM while HDF5_API_TESTS_EXTRA entries (registered the same way but apparently not sensitive to this) passed. The previous 15s "pool settling" delay didn't change this failure at all once tried against a real run, so it's removed - it wasn't the actual cause.
The old -E pattern excluded h5_api_test_attribute (still correct) and h5_api_ext_test_testhdf5 (wrong name - it's actually h5_api_test_testhdf5, and it passes anyway, so no exclusion was needed). Real CI runs against the inline-build pivot now show two more known, non-regression gaps: h5_api_test_parallel_t_bigio (DER_NOSPACE, pool too small) and h5_api_test_parallel_testphdf5 (h5oflusherror, a native-VOL- only expectation DAOS correctly doesn't share). Also documents the h5dumpgentest crash cascade found on the develop leg's broader tool-test matches as a separate, tracked connector-feature gap (missing external link / UD link / H5Oset_comment support), not something this PR fixes.
Removed in 46aa501 because it passed quickly in two prior runs, but a later run showed it hitting the same known by-idx/paginated attribute lookup gap as h5_api_test_attribute (tattr.c iterates hundreds of attributes; each failing H5Aget_info_by_idx/H5Aget_name_by_idx call cascades diagnostic output). It manifests non-deterministically as either a fast per-call failure or, when it cascades across enough attributes, a full ctest timeout - not a new regression, the same pre-existing gap the original pre-migration CI already excluded this binary for.
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
1 participant
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.
Summary
HDFGroup/vol-testssubmodule with HDF5's in-tree API test suite (test/API/testpar/API).HDF5_VOL_ALLOW_EXTERNAL=LOCAL_DIRsubproject - the same mechanism HDF5's own CI uses to testvol-async/vol-cache/vol-rest, rather than depending on a downstream HDF5 install having been built with-DHDF5_TEST_API_INSTALL=ON(a flag almost nobody sets outside Spack). HDF5's owntest/API/testpar/APICMakeLists.txt registers this connector's ctest entries automatically viaHDF5_EXTERNAL_VOL_TARGETS; there's no private test driver or custom ctest wiring in this repo to maintain.H5VL_CAP_FLAG_*checks. Gaps with no capability flag to gate on are excluded at the ctest level; seedocs/api-tests-migration.mdfor the current list and rationale.This PR is scoped to the test framework only. Connector-source fixes (H5T_COMPLEX support, the attribute decreasing-order iteration fix, external-link/UD-link/
H5Oset_commentsupport needed for full tool-test coverage, etc.) are tracked/landed separately againstmaster.Test plan
daos-testjob,hdf5_1.14.6leg: builds and runs against real DAOS, clean known-gaps-only result (65% tests passed, 6/17 failed, all matching documented gaps)daos-testjob,developleg (non-blocking): tool-test suites (h5dump/h5diff/h5copy/h5ls) also get pulled in via the sameHDF5_EXTERNAL_VOL_TARGETSmechanism; triaging those failures is tracked as follow-up work