-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add test suite for the Hermes VFD #416
Conversation
# because of the way dynamic plugin loading works in HDF5. When searching | ||
# for a VFD, HDF5 will dlopen and dlclose every shared library in | ||
# HDF5_PLUGIN_PATH looking for a valid VFD, and when it goes through this | ||
# process with libhermes.so it resets some statically initialized variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish you could split the setence for clarity.
When searching for a VFD, HDF5 will dlopen and dlclose every shared library in
HDF5_PLUGIN_PATH looking for a valid VFD. When HDF5 goes through this
process with libhermes.so, it resets some statically initialized variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
endforeach() | ||
endfunction() | ||
|
||
add_test(NAME "TestVfd" COMMAND hermes_vfd_test --reporter compact -d yes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for using Vfd instead of VFD?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It's easier to type.
- I tend to use that convention because when you capitalize an acronym in camel case the acronym runs into the next word and is harder to read in my opinion. For example, in
VFDTest
the D and T run together whereasVfdTest
has a clear separation between the two words.
add_test(NAME "TestVfd" COMMAND hermes_vfd_test --reporter compact -d yes) | ||
set_vfd_test_properties("TestVfd") | ||
set_property(TEST "TestVfd" APPEND | ||
PROPERTY ENVIRONMENT HDF5_DRIVER_CONFIG=true\ 65536) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is \ 65536
for after true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
65 KiB page size. If you're curious about the \
, if I pass the config string as "true 65536"
then CMake retains the quotes in the value of the environment variable. Using \
escapes the space and sets the value of the variable to true 65536
without the quotes.
add_test(NAME ${IOR_TEST_NAME} COMMAND ${IOR_EXE} -a HDF5 -w -W -r -R) | ||
set_vfd_test_properties(${IOR_TEST_NAME}) | ||
set_property(TEST ${IOR_TEST_NAME} APPEND | ||
PROPERTY ENVIRONMENT HDF5_DRIVER_CONFIG=true\ 262144) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is \ 262144
for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
256 KiB page size.
return result; | ||
} | ||
|
||
/** Returns a string in the range ["0", "upper_bound") */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it "upper_bound-1"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using interval notation. The [
includes 0 on the left, but the )
on the right means up to but not including upper_bound
.
Closes #378. Closes #385. Closes #386.
libhdf5_hermes_vfd.so
must beLD_PRELOAD
ed in addition to settingHDF5_DRIVER=hermes
.