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.

Sign up for GitHub

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

Add h5dump tests that require subfiling VFD #3879

Merged
merged 2 commits into from
Feb 1, 2024
Merged

Add h5dump tests that require subfiling VFD #3879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9d2128
Add h5dump tests that require subfiling VFD
hyoklee Dec 1, 2023
5054cc4
Address @brtnfld review
hyoklee Dec 21, 2023
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
45 changes: 44 additions & 1 deletion tools/test/h5dump/CMakeVFDTests.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,51 @@ set (HDF5_VFD_H5DUMP_FILES
packedbits
)

set (HDF5_SF_VFD_H5DUMP_FILES
test_subfiling_stripe_sizes.h5
)

set (HDF5_SF2_VFD_H5DUMP_FILES
test_subfiling_precreate_rank_0.h5
)

foreach (vfdtest ${VFD_LIST})
if (vfdtest STREQUAL "subfiling")
foreach (h5_tfile ${HDF5_SF_VFD_H5DUMP_FILES})
file(COPY "${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}" DESTINATION "${PROJECT_BINARY_DIR}/${vfdtest}")
execute_process(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this doing a process that could be done by a CMake command? We should not be using platform specific commands by choice. We should not assume that something is only ever going to be on one platform.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why use the inode in the first place - the files are unique without the number. I don't see the advantage of the complexity.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The inode is used by the Subfiling VFD to generate unique filenames, so the CMake code would need to be able to generate the names of the other resulting files when creating files with the Subfiling VFD during testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

But isn't that in the original name? Seems like there needs to be more documentation on this test, because it isn't obvious.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The problem is we can't add generated files here easily because the inode value for the file could change, meaning that when the Subfiling VFD tries to open test_subfiling_stripe_sizes.h5, it will also look for
test_subfiling_stripe_sizes.h5.subfile_XXXXXXXX_1_of_1 and test_subfiling_stripe_sizes.h5.subfile_XXXXXXXX.config, but we wouldn't have a way of predicting what the inode value for XXXXXXXX would be and so we have to grab the inode value. On the other hand, we probably don't necessarily need a generated file at all to test Subfiling with h5dump if we just create the file on the fly.

COMMAND ls -i ${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}
OUTPUT_VARIABLE OUTPUT_VALUE
OUTPUT_STRIP_TRAILING_WHITESPACE
)
string(REGEX MATCH "^ *([0-9]+) *" INODE_VALUE "${OUTPUT_VALUE}")
string(STRIP ${INODE_VALUE} INODE_STR)
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}.subfile_1_of_1" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.subfile_${INODE_STR}_1_of_1" "HDF5_SF_VFD_H5DUMP_files")
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}.subfile.config" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.subfile_${INODE_STR}.config" "HDF5_SF_VFD_H5DUMP_files")
endforeach ()
foreach (h5_tfile ${HDF5_SF2_VFD_H5DUMP_FILES})
file(COPY "${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}" DESTINATION "${PROJECT_BINARY_DIR}/${vfdtest}")
execute_process(
COMMAND ls -i ${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}
OUTPUT_VARIABLE OUTPUT_VALUE
OUTPUT_STRIP_TRAILING_WHITESPACE
)
string(REGEX MATCH "^ *([0-9]+) *" INODE_VALUE "${OUTPUT_VALUE}")
string(STRIP ${INODE_VALUE} INODE_STR)
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}.subfile_1_of_2" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.subfile_${INODE_STR}_1_of_2" "HDF5_SF2_VFD_H5DUMP_files")
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}.subfile_2_of_2" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.subfile_${INODE_STR}_2_of_2" "HDF5_SF2_VFD_H5DUMP_files")
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}.subfile.config" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.subfile_${INODE_STR}.config" "HDF5_SF2_VFD_H5DUMP_files")
endforeach ()
endif ()
foreach (h5_tfile ${HDF5_VFD_H5DUMP_FILES})
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/testfiles/${h5_tfile}.h5" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.h5" "HDF5_VFD_H5DUMP_files")
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/expected/${h5_tfile}.ddl" "${PROJECT_BINARY_DIR}/${vfdtest}/${h5_tfile}.ddl" "HDF5_VFD_H5DUMP_files")
endforeach ()
endforeach ()

add_custom_target(HDF5_VFD_H5DUMP_files ALL COMMENT "Copying files needed by HDF5_VFD_H5DUMP tests" DEPENDS ${HDF5_VFD_H5DUMP_files_list})
add_custom_target(HDF5_SF_VFD_H5DUMP_files ALL COMMENT "Copying files needed by HDF5_SF_VFD_H5DUMP tests" DEPENDS ${HDF5_SF_VFD_H5DUMP_files_list})
add_custom_target(HDF5_SF2_VFD_H5DUMP_files ALL COMMENT "Copying files needed by HDF5_SF2_VFD_H5DUMP tests" DEPENDS ${HDF5_SF2_VFD_H5DUMP_files_list})

##############################################################################
##############################################################################
Expand Down Expand Up @@ -69,6 +106,12 @@ endmacro ()

# Run test with different Virtual File Driver
foreach (vfd ${VFD_LIST})
# test for signed/unsigned datasets
if (vfd STREQUAL "subfiling")
ADD_VFD_H5DUMP_TEST (${vfd} filedriver_subfiling 0 --enable-error-stack=2 --filedriver=subfiling test_subfiling_stripe_sizes.h5)
ADD_VFD_H5DUMP_TEST (${vfd} vfd_name_subfiling 0 --enable-error-stack=2 --vfd-name=subfiling test_subfiling_stripe_sizes.h5)
ADD_VFD_H5DUMP_TEST (${vfd} vfd_value_subfiling 0 --enable-error-stack=2 --vfd-value=12 test_subfiling_stripe_sizes.h5)
ADD_VFD_H5DUMP_TEST (${vfd} vfd_value_subfiling_2 0 --enable-error-stack=2 --vfd-value=12 -d DSET -s 0 -S 100 -c 10 test_subfiling_precreate_rank_0.h5)
endif ()
# test for signed/unsigned datasets
ADD_VFD_H5DUMP_TEST (${vfd} packedbits 0 --enable-error-stack packedbits.h5)
endforeach ()
Binary file added tools/test/h5dump/testfiles/test_subfiling_precreate_rank_0.h5
View file
Open in desktop
Binary file not shown.
2 changes: 2 additions & 0 deletions tools/test/h5dump/testfiles/test_subfiling_precreate_rank_0.h5.subfile.config
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where were these test files taken from? I'd expect the configuration files to have aggregator_count, hdf5_file, subfile_dir fields and a list of the names of all the subfiles.

Copy link
Member Author

@hyoklee hyoklee Dec 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I removed them manually because they did not affect the h5dump test result. Test files are based on subfiling test under testpar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, it's probably best to keep the original config files intact so that future versions of the VFD don't have issues with the test files if it requires certain fields in the future for verification purposes. That said, I think this is fine for now.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
stripe_size=22495773
subfile_count=2
Binary file added tools/test/h5dump/testfiles/test_subfiling_precreate_rank_0.h5.subfile_1_of_2
View file
Open in desktop
Binary file not shown.
Binary file added tools/test/h5dump/testfiles/test_subfiling_precreate_rank_0.h5.subfile_2_of_2
View file
Open in desktop
Binary file not shown.
Binary file added tools/test/h5dump/testfiles/test_subfiling_stripe_sizes.h5
View file
Open in desktop
Binary file not shown.
2 changes: 2 additions & 0 deletions tools/test/h5dump/testfiles/test_subfiling_stripe_sizes.h5.subfile.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
stripe_size=12149997
subfile_count=1
Binary file added tools/test/h5dump/testfiles/test_subfiling_stripe_sizes.h5.subfile_1_of_1
View file
Open in desktop
Binary file not shown.