Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'make check-vfd' target for Autotools #4211

Merged
merged 2 commits into from
Mar 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions config/cmake/HDF5Macros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,17 @@ macro (H5_SET_VFD_LIST)
split
multi
family
splitter
#log - log VFD currently has file space allocation bugs
# Splitter VFD currently can't be tested with the h5_fileaccess()
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The splitter VFD was previously being tested just with the environment variable approach, where it would add "_wo" to the end of filenames to create the name for the W/O file, which worked well in testing. With the changes in this PR though, any tests that use h5_fileaccess() would setup a FAPL with a single W/O path that gets used for any file create or open calls that use that FAPL, leading to failures due to multiple locks on the same file. We should revisit this at some point.

# approach due to it trying to lock the same W/O file when two
# files are created/opened with the same FAPL that has the VFD
# set on it. When tested with the environment variable and a
# default FAPL, the VFD appends "_wo" to the filename when the
# W/O path isn't specified, which works for all the tests.
#splitter
# Log VFD currently has file space allocation bugs
#log
# Onion VFD not currently tested with VFD tests
#onion
)

if (H5_HAVE_DIRECT)
Expand All @@ -82,16 +91,21 @@ macro (H5_SET_VFD_LIST)
# list (APPEND VFD_LIST mpio)
endif ()
if (H5_HAVE_MIRROR_VFD)
list (APPEND VFD_LIST mirror)
# Mirror VFD needs network configuration, etc. and isn't easy to set
# reasonable defaults for that info.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matching what's in Autotools

# list (APPEND VFD_LIST mirror)
endif ()
if (H5_HAVE_ROS3_VFD)
list (APPEND VFD_LIST ros3)
# This would require a custom test suite
# list (APPEND VFD_LIST ros3)
endif ()
if (H5_HAVE_LIBHDFS)
list (APPEND VFD_LIST hdfs)
# This would require a custom test suite
# list (APPEND VFD_LIST hdfs)
endif ()
if (H5_HAVE_SUBFILING_VFD)
list (APPEND VFD_LIST subfiling)
# Subfiling has a few VFD test failures to be resolved
# list (APPEND VFD_LIST subfiling)
endif ()
if (H5_HAVE_WINDOWS)
list (APPEND VFD_LIST windows)
Expand Down
40 changes: 25 additions & 15 deletions config/conclude.am
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,15 @@ $(TEST_PROG_CHKEXE) $(TEST_PROG_PARA_CHKEXE) dummy.chkexe_:
if test -n "$(HDF5_VOL_CONNECTOR)"; then \
echo "VOL connector: $(HDF5_VOL_CONNECTOR)" | tee -a $${log}; \
fi; \
if test -n "$(HDF5_DRIVER)"; then \
echo "Virtual file driver (VFD): $(HDF5_DRIVER)" | tee -a $${log}; \
if test -n "$(HDF5_TEST_DRIVER)"; then \
echo "Virtual file driver (VFD): $(HDF5_TEST_DRIVER)" | tee -a $${log}; \
fi; \
else \
if test -n "$(HDF5_VOL_CONNECTOR)"; then \
echo "VOL connector: $(HDF5_VOL_CONNECTOR)" >> $${log}; \
fi; \
if test -n "$(HDF5_DRIVER)"; then \
echo "Virtual file driver (VFD): $(HDF5_DRIVER)" >> $${log}; \
if test -n "$(HDF5_TEST_DRIVER)"; then \
echo "Virtual file driver (VFD): $(HDF5_TEST_DRIVER)" >> $${log}; \
fi; \
fi; \
if test -n "$(REALTIMEOUTPUT)"; then \
Expand Down Expand Up @@ -276,11 +276,22 @@ build-check-p: $(LIB) $(PROGS) $(chk_TESTS)
echo "===Parallel tests in `echo ${PWD} | sed -e s:.*/::` ended `date`===";\
fi

VFD_LIST = sec2 stdio core core_paged split multi family splitter
VFD_LIST = sec2 stdio core core_paged split multi family

# Splitter VFD currently can't be tested with the h5_fileaccess()
# approach due to it trying to lock the same W/O file when two
# files are created/opened with the same FAPL that has the VFD
# set on it. When tested with the environment variable and a
# default FAPL, the VFD appends "_wo" to the filename when the
# W/O path isn't specified, which works for all the tests.
# VFD_LIST += splitter

# log VFD currently has file space allocation bugs
# VFD_LIST += log

# Not currently tested with VFD tests
# VFD_LIST += onion

if DIRECT_VFD_CONDITIONAL
VFD_LIST += direct
endif
Expand All @@ -302,21 +313,20 @@ if HDFS_VFD_CONDITIONAL
# VFD_LIST += hdfs
endif
if SUBFILING_VFD_CONDITIONAL
# Several VFD tests fail with Subfiling since it
# doesn't currently support collective I/O
# Subfiling has a few VFD test failures to be resolved
# VFD_LIST += subfiling
endif

# Run test with different Virtual File Driver
check-vfd: $(LIB) $(PROGS) $(chk_TESTS)
@for vfd in $(VFD_LIST) dummy; do \
if test $$vfd != dummy; then \
echo "============================"; \
echo "Testing Virtual File Driver $$vfd"; \
echo "============================"; \
$(MAKE) $(AM_MAKEFLAGS) check-clean || exit 1; \
HDF5_DRIVER=$$vfd $(MAKE) $(AM_MAKEFLAGS) check || exit 1; \
fi; \
@for vfd in $(VFD_LIST) dummy; do \
if test $$vfd != dummy; then \
echo "============================"; \
echo "Testing Virtual File Driver $$vfd"; \
echo "============================"; \
$(MAKE) $(AM_MAKEFLAGS) check-clean || exit 1; \
HDF5_TEST_DRIVER=$$vfd $(MAKE) $(AM_MAKEFLAGS) check || exit 1; \
fi; \
done

# Test with just the native connector, with a single pass-through connector
Expand Down
2 changes: 1 addition & 1 deletion doc/getting-started-with-hdf5-development.md
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ are others in `h5test.h` if you want to emit custom text, dump the HDF5 error
stack when it would not normally be triggered, etc.

Most tests will be set up to run with arbitrary VFDs. To do this, you set the
fapl ID using the `h5_fileaccess()` function, which will check the `HDF5_DRIVER`
fapl ID using the `h5_fileaccess()` function, which will check the `HDF5_TEST_DRIVER`
environment variable and set the fapl's VFD accordingly. The `h5_fixname()`
call can then be used to get a VFD-appropriate filename for the `H5Fcreate()`,
etc. call.
Expand Down
14 changes: 9 additions & 5 deletions src/H5FDtest.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,15 @@
* Purpose: Determines if a VFD supports SWMR.
*
* The function determines SWMR support by inspecting the
* HDF5_DRIVER environment variable, not by checking the
* VFD feature flags (which do not exist until the driver
* is instantiated).
* HDF5_DRIVER and HDF5_TEST_DRIVER environment variables, not
* by checking the VFD feature flags (which do not exist until
* the driver is instantiated).
*
* This function is only intended for use in the test code.
*
* Return: true (1) if the VFD supports SWMR I/O or vfd_name is
* NULL or the empty string (which implies the default VFD).
* NULL or the empty string (which implies the default VFD) or
* compares equal to the default VFD's name.
*
* false (0) if it does not
*
Expand All @@ -89,7 +90,10 @@ H5FD__supports_swmr_test(const char *vfd_name)

FUNC_ENTER_NOAPI_NOINIT_NOERR

if (!vfd_name || !strcmp(vfd_name, "") || !strcmp(vfd_name, "nomatch"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the "nomatch" case. It was only checked in a few places where we can just as easily return "sec2" for the same behavior.

if (!vfd_name)
vfd_name = getenv("HDF5_TEST_DRIVER");

if (!vfd_name || !strcmp(vfd_name, "") || !strcmp(vfd_name, H5_DEFAULT_VFD_NAME))
ret_value = true;
else
ret_value = !strcmp(vfd_name, "log") || !strcmp(vfd_name, "sec2");
Expand Down
3 changes: 2 additions & 1 deletion src/H5private.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@
* H5_init_library(); also, make sure that the initializer for default
* VFD does *not* call H5_init_library().
*/
#define H5_DEFAULT_VFD H5FD_SEC2
#define H5_DEFAULT_VFD H5FD_SEC2
#define H5_DEFAULT_VFD_NAME "sec2"

/* Define the default VOL driver */
#define H5_DEFAULT_VOL H5VL_NATIVE
Expand Down
3 changes: 3 additions & 0 deletions test/CMakeVFDTests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ add_custom_target(HDF5_VFDTEST_LIB_files ALL COMMENT "Copying files needed by HD
links_env
external_env
vds_env
mirror_vfd
ros3
hdfs
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exclude VFD-specific tests from CMake's VFD testing, as it often doesn't make sense to run these tests with other VFDs.

)

# Skip several tests with subfiling VFD, mostly due
Expand Down
25 changes: 12 additions & 13 deletions test/accum.c
Original file line number Diff line number Diff line change
Expand Up @@ -2049,17 +2049,16 @@ fprintf(stderr, "Random # seed was: %u\n", seed);
unsigned
test_swmr_write_big(bool newest_format)
{

hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t fapl = H5I_INVALID_HID; /* File access property list */
H5F_t *rf = NULL; /* File pointer */
char filename[1024];
uint8_t *wbuf2 = NULL, *rbuf = NULL; /* Buffers for reading & writing */
uint8_t wbuf[1024]; /* Buffer for reading & writing */
unsigned u; /* Local index variable */
bool process_success = false;
char *driver = NULL; /* VFD string (from env variable) */
bool api_ctx_pushed = false; /* Whether API context pushed */
const char *driver_name = NULL; /* VFD string (from env variable) */
hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t fapl = H5I_INVALID_HID; /* File access property list */
H5F_t *rf = NULL; /* File pointer */
char filename[1024];
uint8_t *wbuf2 = NULL, *rbuf = NULL; /* Buffers for reading & writing */
uint8_t wbuf[1024]; /* Buffer for reading & writing */
unsigned u; /* Local index variable */
bool process_success = false;
bool api_ctx_pushed = false; /* Whether API context pushed */

if (newest_format)
TESTING("SWMR write of large metadata: with latest format");
Expand All @@ -2077,8 +2076,8 @@ test_swmr_write_big(bool newest_format)
/* Skip this test if SWMR I/O is not supported for the VFD specified
* by the environment variable.
*/
driver = getenv(HDF5_DRIVER);
if (!H5FD__supports_swmr_test(driver)) {
driver_name = h5_get_test_driver_name();
if (!H5FD__supports_swmr_test(driver_name)) {
SKIPPED();
puts(" Test skipped due to VFD not supporting SWMR I/O.");
return 0;
Expand Down
22 changes: 11 additions & 11 deletions test/accum_swmr_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,24 @@ static const char *FILENAME[] = {"accum", "accum_swmr_big", NULL};
int
main(void)
{
hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t fapl = H5I_INVALID_HID; /* file access property list ID */
H5F_t *f = NULL; /* File pointer */
char filename[1024];
unsigned u; /* Local index variable */
uint8_t rbuf[1024]; /* Buffer for reading */
uint8_t buf[1024]; /* Buffer for holding the expected data */
char *driver = NULL; /* VFD string (from env variable) */
bool api_ctx_pushed = false; /* Whether API context pushed */
const char *driver_name = NULL; /* VFD string (from env variable) */
hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t fapl = H5I_INVALID_HID; /* file access property list ID */
H5F_t *f = NULL; /* File pointer */
char filename[1024];
unsigned u; /* Local index variable */
uint8_t rbuf[1024]; /* Buffer for reading */
uint8_t buf[1024]; /* Buffer for holding the expected data */
bool api_ctx_pushed = false; /* Whether API context pushed */

/* Testing setup */
h5_reset();

/* Skip this test if SWMR I/O is not supported for the VFD specified
* by the environment variable.
*/
driver = getenv(HDF5_DRIVER);
if (!H5FD__supports_swmr_test(driver))
driver_name = h5_get_test_driver_name();
if (!H5FD__supports_swmr_test(driver_name))
return EXIT_SUCCESS;

/* Initialize buffers */
Expand Down
8 changes: 3 additions & 5 deletions test/app_ref.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Abrt_Handler(int H5_ATTR_UNUSED sig)
int
main(void)
{
const char *env_h5_drvr; /* File Driver value from environment */
const char *driver_name; /* File Driver value from environment */
hid_t ids[T_NUMCLASSES];
hid_t fapl; /* File Access Property List */
int ninc;
Expand All @@ -94,14 +94,12 @@ main(void)
TESTING("library shutdown with reference count > 1");

/* Get the VFD to use */
env_h5_drvr = getenv(HDF5_DRIVER);
if (env_h5_drvr == NULL)
env_h5_drvr = "nomatch";
driver_name = h5_get_test_driver_name();

/* Don't run this test with the multi/split VFD. A bug in library shutdown
* ordering causes problems with the multi VFD when IDs are left dangling.
*/
if (!strcmp(env_h5_drvr, "multi") || !strcmp(env_h5_drvr, "split")) {
if (!strcmp(driver_name, "multi") || !strcmp(driver_name, "split")) {
puts("\n -- SKIPPED for incompatible VFD --");
return 0;
}
Expand Down
14 changes: 6 additions & 8 deletions test/btree2.c
Original file line number Diff line number Diff line change
Expand Up @@ -8598,7 +8598,7 @@ gen_l4_btree2(const char *filename, hid_t fapl, const H5B2_create_t *cparam, had
*-------------------------------------------------------------------------
*/
static unsigned
test_remove_lots(const char *env_h5_drvr, hid_t fapl, const H5B2_create_t *cparam)
test_remove_lots(const char *driver_name, hid_t fapl, const H5B2_create_t *cparam)
{
hid_t file = H5I_INVALID_HID; /* File ID */
char filename[1024]; /* Filename to use */
Expand Down Expand Up @@ -8656,7 +8656,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
TEST_ERROR;

/* Check for VFD which stores data in multiple files */
single_file_vfd = !h5_driver_uses_multiple_files(env_h5_drvr, H5_EXCLUDE_NON_MULTIPART_DRIVERS);
single_file_vfd = !h5_driver_uses_multiple_files(driver_name, H5_EXCLUDE_NON_MULTIPART_DRIVERS);
if (single_file_vfd) {
/* Make a copy of the file in memory, in order to speed up deletion testing */

Expand Down Expand Up @@ -9916,20 +9916,18 @@ main(void)
unsigned nerrors = 0; /* Cumulative error count */
unsigned reopen; /* Whether to reopen B-tree during tests */
int ExpressMode;
const char *envval = NULL;
const char *driver_name;
bool api_ctx_pushed = false; /* Whether API context pushed */

envval = getenv(HDF5_DRIVER);
if (envval == NULL)
envval = "nomatch";
driver_name = h5_get_test_driver_name();

/* Reset library */
h5_reset();
fapl = h5_fileaccess();
ExpressMode = GetTestExpress();

/* For the Direct I/O driver, skip intensive tests due to poor performance */
if (!strcmp(envval, "direct"))
if (!strcmp(driver_name, "direct"))
ExpressMode = 2;

if (ExpressMode > 1)
Expand Down Expand Up @@ -10013,7 +10011,7 @@ main(void)
if (ExpressMode > 1)
printf("***Express test mode on. test_remove_lots skipped\n");
else
nerrors += test_remove_lots(envval, fapl, &cparam);
nerrors += test_remove_lots(driver_name, fapl, &cparam);

/* Test more complex B-tree queries */
nerrors += test_find_neighbor(fapl, &cparam, &tparam);
Expand Down
8 changes: 3 additions & 5 deletions test/cache_image.c
Original file line number Diff line number Diff line change
Expand Up @@ -7752,15 +7752,13 @@ evict_on_close_test(bool H5_ATTR_PARALLEL_UNUSED single_file_vfd)
int
main(void)
{
const char *env_h5_drvr; /* File driver value from environment */
const char *driver_name; /* File driver value from environment */
bool single_file_vfd; /* Whether VFD used stores data in a single file */
unsigned nerrs = 0;
int express_test;

/* Get the VFD to use */
env_h5_drvr = getenv(HDF5_DRIVER);
if (env_h5_drvr == NULL)
env_h5_drvr = "nomatch";
driver_name = h5_get_test_driver_name();

H5open();

Expand All @@ -7772,7 +7770,7 @@ main(void)
printf("=========================================\n");

/* Check for VFD which stores data in multiple files */
single_file_vfd = !h5_driver_uses_multiple_files(env_h5_drvr, H5_EXCLUDE_NON_MULTIPART_DRIVERS);
single_file_vfd = !h5_driver_uses_multiple_files(driver_name, H5_EXCLUDE_NON_MULTIPART_DRIVERS);

nerrs += check_cache_image_ctl_flow_1(single_file_vfd);
nerrs += check_cache_image_ctl_flow_2(single_file_vfd);
Expand Down
5 changes: 2 additions & 3 deletions test/cork.c
Original file line number Diff line number Diff line change
Expand Up @@ -2229,13 +2229,12 @@ main(void)

for (swmr = 0; swmr <= 1; swmr++) {
if (swmr) {
char *driver = NULL;
const char *driver_name = h5_get_test_driver_name();

/* Skip these tests if SWMR I/O is not supported for the VFD specified
* by the environment variable.
*/
driver = getenv(HDF5_DRIVER);
if (!H5FD__supports_swmr_test(driver)) {
if (!H5FD__supports_swmr_test(driver_name)) {
puts("-- SKIPPED SWMR tests for SWMR-incompatible VFD --");
continue;
}
Expand Down
8 changes: 3 additions & 5 deletions test/dangle.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,18 +623,16 @@ test_dangle_force(void)
int
main(void)
{
const char *env_h5_drvr; /* File Driver value from environment */
const char *driver_name; /* File Driver value from environment */
int nerrors = 0;

/* Get the VFD to use */
env_h5_drvr = getenv(HDF5_DRIVER);
if (env_h5_drvr == NULL)
env_h5_drvr = "nomatch";
driver_name = h5_get_test_driver_name();

/* Don't run this test with the multi/split VFD. A bug in library shutdown
* ordering causes problems with the multi VFD when IDs are left dangling.
*/
if (!strcmp(env_h5_drvr, "multi") || !strcmp(env_h5_drvr, "split")) {
if (!strcmp(driver_name, "multi") || !strcmp(driver_name, "split")) {
puts(" -- SKIPPED for incompatible VFD --");
return 0;
}
Expand Down
Loading
Loading