Fix VOL-compatibility issues in External Link API test #4039

mattjala · 2024-02-23T15:52:17Z

Fixed some external link tests not prepending the API test prefix to the filename.
Modified link tests that use H5Iget_name to accept any of the valid names for the target object, since the API does not specify which of an object's possible names will be returned.
Fixed link iteration test counting skips as failures due to doing the capability flag checks from within the link callbacks. Now, the capability flags are checked before the link iteration is performed.

fortnern · 2024-02-23T17:58:16Z

test/API/H5_api_link_test.c

    /* Check name */
    if (H5Iget_name(group_id2, objname, (size_t)HARD_LINK_TEST_GROUP_MANY_NAME_BUF_SIZE) < 0) {
        H5_FAILED();
        printf("    couldn't get the name of the object '%s'\n", HARD_LINK_TEST_GROUP_MANY_FINAL_NAME);
        goto error;
    }

-    if (strcmp(objname, "/" LINK_TEST_GROUP_NAME "/" HARD_LINK_TEST_GROUP_MANY_NAME "/hard21")) {
+    for (size_t i = 1; i < HARD_LINK_TEST_GROUP_MANY_NUM_HARD_LINKS + 1; i++) {


Might as well add a short circuit success here. For the test after the loop you could just switch the order to valid_name_matched || !strcmp(...

fortnern · 2024-02-23T17:58:52Z

test/API/H5_api_link_test.c

@@ -1709,9 +1728,24 @@ test_create_soft_link_many(void)
        goto error;
    }

-    if (strcmp(objname, "/" LINK_TEST_GROUP_NAME "/" SOFT_LINK_TEST_GROUP_MANY_NAME "/soft16")) {
+    for (size_t i = 1; i < SOFT_LINK_TEST_GROUP_MANY_NAME_SOFT_LINK_COUNT + 1; i++) {


fortnern · 2024-02-23T18:01:24Z

test/API/H5_api_link_test.c

@@ -1999,7 +2033,8 @@ test_create_external_link(void)
        return 0;
    }

-    snprintf(ext_link_filename, H5_API_TEST_FILENAME_MAX_LENGTH, "%s", EXTERNAL_LINK_TEST_FILE_NAME);
+    snprintf(ext_link_filename, H5_API_TEST_FILENAME_MAX_LENGTH, "%s%s", test_path_prefix,


Seems that testing absolute and relative path resolutions are two different things. What is the motivation for changing it? Should we test both?

This isn't related to path resolution within a file. Some VOLs need to place their test files within a folder (e.g. the REST VOL creates its test files under /home/test_user1). Other places in the API tests that use a filename apply the prefix, but a few of these external link tests didn't get updated. For the native vol, test_path_prefix is empty.

fortnern · 2024-02-27T16:55:10Z

test/API/H5_api_link_test.c

@@ -20843,6 +21011,15 @@ test_link_iterate_mixed_links(void)
        {
            TESTING_2("H5Literate_by_name2 by link name in increasing order");

+            if (!(vol_cap_flags_g & H5VL_CAP_FLAG_CREATION_ORDER) ||


Does this test and the next really need creation order?

No, support for link name ordering shouldn't depend on creation order support. I've removed a few unnecessary checks for this flag.

fortnern · 2024-02-27T16:56:27Z

test/API/H5_api_link_test.c

+                snprintf(name_possibility, H5_API_TEST_FILENAME_MAX_LENGTH, "%s%zu%s", "/link", i,
+                         "/new_group");
+
+                valid_name_matched = !strcmp(objname, name_possibility) || valid_name_matched;


(Minor) maybe add short circuit here

Fix link API tests with incorrect filename

* Remove oneapi return value warning. (#4028) * Replaced last sprintf with snprintf (#4007) * Replaced last sprintf with snprintf To have the size of the buffer, it was required to change a function signature, and change all users of it. In most cases, determining the buffer size wasn't trivial and so SIZE_MAX is passed. But at least this improves the infrastructure. Someone can later figure out the correct sizes. * Test vlen sequence IO in API tests (#4027) * Check argument for CMake REGEX FCMangle.h. (#4029) * Replace deprecated Fortran 'include mpif.h' with 'USE mpi' (#4031) With MPI 4.1 the use of the mpif.h include file has been deprecated. Codes should transition to USE mpi or USE mpi_f08. Signed-off-by: Christoph Niethammer <[email protected]> * Fix H5F_get_access_plist to copy file locking settings (#4030) H5F_get_access_plist previously did not copy over the file locking settings from a file into the new File Access Property List that it creates. This would make it difficult to match the file locking settings between an external file and its parent file. * Fix missing NOT from if check in HL folder (#4036) * Fix the datatype passed to H5*exists_async APIs in tests. (#4033) Add a new testing function to verify C_BOOL values. * Add deb and rpm binaries to snapshots (#4035) * Update and Add general INSTALL (#4016) * Improve performance of flushing single objects (#4017) Improve performance of flushing a single object, and remove metadata cache flush markers * Fix memory leak in H5LTopen_file_image when H5LT_FILE_IMAGE_DONT_COPY flag is used (#4021) When the H5LT_FILE_IMAGE_DONT_COPY flag is passed to H5LTopen_file_image, the internally-allocated udata structure gets leaked as the core file driver doesn't have a way to determine when or if it needs to call the 'udata_free' callback. This has been fixed by freeing the udata structure when the 'image_free' callback gets made during file close, where the file is holding the last reference to the udata structure. * Fix allocating too much memory in dset API test (#4041) * Don't try to load general-19 warnings file for icc (#4042) The Autools Classic Intel compiler configuration attempts to load a file named `general-19` from the intel-warnings/classic directory, which does not exist. This removes the attempted load of the file. * Remove unused AIX cross-compile cache overrides (#4043) The ibm-aix Autotools config file had some unmaintained and unnecessary Autoconf cache overrides. These have been removed. * Consolidate Autotools linux files (#4044) There are many architecture-specific linux files in the config directory, all of which simply redirect to linux-gnulibc1. This change renames linux-gnulibc1 to linux-gnu and deletes the more specific files. * Remove check for gettimeofday + tz in CMake (#4045) This is not used in the library * Remove limitations on preset generators (#4051) * Fix issue with FAPL file locking setting inheriting test (#4053) Fixes an issue where the HDF5_USE_FILE_LOCKING environment variable being set can interfere with the file locking setting that the test expects to be returned. * Bump the github-actions group with 2 updates (#4054) Bumps the github-actions group with 2 updates: [actions/download-artifact](https://github.com/actions/download-artifact) and [github/codeql-action](https://github.com/github/codeql-action). * Fix VOL-compatibility issues in External Link API test (#4039) Fix link API tests with incorrect filename * Add upddated cmake tools from source location (#4040) * Add options to allow tools type selection and naming (#4046) * Improve error messages when tools attempt to use non-enabled S3 and HDFS VFDs. (#4047) * Correct several 1.15/1.15.0 references to 1.14/1.14.4. * Ignore HDF5Examples/CMakeUserPresets.json

Fix link API tests with incorrect filename

mattjala added 3 commits February 22, 2024 11:51

Fix link API tests with incorrect filename

5ea725a

Generalize link name checks

c48019b

Fix link iteration test counting skip as a failure

a902fa5

mattjala added Merge - To 1.14 Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Component - C Library Core C library issues (usually in the src directory) Type - Improvement Improvements that don't add a new feature or functionality labels Feb 23, 2024

mattjala requested review from lrknox, derobins, byrnHDF, fortnern, jhendersonHDF, qkoziol, vchoi-hdfgroup, bmribler, glennsong09 and brtnfld as code owners February 23, 2024 15:52

fortnern reviewed Feb 23, 2024

View reviewed changes

Add short-circuit to name checks

fe26973

mattjala force-pushed the api_link_test_fix branch from d366b84 to fe26973 Compare February 23, 2024 22:05

Committing clang-format changes

e039ede

fortnern reviewed Feb 27, 2024

View reviewed changes

mattjala and others added 2 commits February 27, 2024 12:44

Short-circuit name checks, remove unneeded flag checks

1c7cf47

Committing clang-format changes

be16a40

fortnern approved these changes Feb 27, 2024

View reviewed changes

jhendersonHDF approved these changes Feb 28, 2024

View reviewed changes

derobins merged commit 8276bdb into HDFGroup:develop Mar 4, 2024

lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Mar 4, 2024

Fix VOL-compatibility issues in External Link API test (HDFGroup#4039)

402e121

Fix link API tests with incorrect filename

mattjala deleted the api_link_test_fix branch September 20, 2024 14:47

jhendersonHDF pushed a commit to LifeboatLLC/hdf5_lifeboat that referenced this pull request Dec 10, 2024

Fix VOL-compatibility issues in External Link API test (HDFGroup#4039)

cc8d305

Fix link API tests with incorrect filename

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix VOL-compatibility issues in External Link API test #4039

Fix VOL-compatibility issues in External Link API test #4039

mattjala commented Feb 23, 2024

fortnern Feb 23, 2024

fortnern Feb 23, 2024

fortnern Feb 23, 2024

mattjala Feb 23, 2024

fortnern Feb 27, 2024

mattjala Feb 27, 2024 •

edited

Loading

fortnern Feb 27, 2024

Fix VOL-compatibility issues in External Link API test #4039

Fix VOL-compatibility issues in External Link API test #4039

Conversation

mattjala commented Feb 23, 2024

fortnern Feb 23, 2024

Choose a reason for hiding this comment

fortnern Feb 23, 2024

Choose a reason for hiding this comment

fortnern Feb 23, 2024

Choose a reason for hiding this comment

mattjala Feb 23, 2024

Choose a reason for hiding this comment

fortnern Feb 27, 2024

Choose a reason for hiding this comment

mattjala Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

fortnern Feb 27, 2024

Choose a reason for hiding this comment

mattjala Feb 27, 2024 •

edited

Loading