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

API test updates #3018

Merged
merged 19 commits into from
May 26, 2023
Merged

API test updates #3018

merged 19 commits into from
May 26, 2023

Conversation

jhendersonHDF
Copy link
Collaborator

@jhendersonHDF jhendersonHDF commented May 26, 2023

Most of the changes in this PR just replace macros from test/API/H5_api_tests_disabled.h that were disabling API tests with appropriate checks for VOL connector capability flags. However, there are a few bug fixes and CMake code updates to implement support for pulling in, building and testing external VOL connectors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file hook up external built VOL connectors into the API tests

@@ -49,6 +49,22 @@ if (BUILD_STATIC_LIBS)
endif ()
H5_SET_LIB_OPTIONS (${HDF5_TEST_LIB_TARGET} ${HDF5_TEST_LIB_NAME} STATIC 0)
set_target_properties (${HDF5_TEST_LIB_TARGET} PROPERTIES FOLDER libraries/test)

if (HDF5_EXPORTED_TARGETS)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@byrnHDF I just realized for the purposes of the API tests I was unconditionally installing the test lib here and below. Would it be appropriate to surround it with if (HDF5_TEST_API_INSTALL) or should we have something else since there may be other reasons to install the test lib?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file mostly just separated the code for generating different datatypes into sub-functions, but it also fixed a bug in enum. datatype generation where we tried to insert duplicate name/value pairs into the datatype.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file hook up external built VOL connectors into the API tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file mirror a check that was added to H5Aopen where we want to make sure we return failure when an invalid AAPL is passed in. The API tests were failing for the native VOL connector due to checking this.

CMakeVOL.cmake Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This new file contains all the code to generate CMake options and variables for specifying external VOL connectors and then pulling in, building those connectors and setting up for later testing of the connectors.

@glennsong09 glennsong09 added Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - Testing Code in test or testpar directories, GitHub workflows Type - Improvement Improvements that don't add a new feature or functionality labels May 26, 2023
those semi-colons should be escaped with a single backslash so that CMake
doesn't parse the string as a list. If `cmake` is run from a shell, extra care
may need to be taken when escaping the semi-colons depending on how the
shell interprets backslashes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

'semicolons' shouldn't have a '-'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@jhendersonHDF
Copy link
Collaborator Author

jhendersonHDF commented May 26, 2023

The EOS workflow appears to be failing due to too many requests being made, likely because the same actions are running in the feature/api_tests feature branch and those end up running two sets of the same actions for some reason, so I believe there are 122 checks running simultaneously, which is a bit overkill

@derobins derobins merged commit 79bb60c into develop May 26, 2023
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request Jul 27, 2023
* Remove macros from api tests (HDFGroup#2929)
* Remove macros and undefined callbacks (HDFGroup#2959)
* Remove remaining macros from H5_api_tests_disabled.h (HDFGroup#2968)
* Put some vol capability checks in testpar tests and remove remaining warnings (HDFGroup#2995)
* API tests datatype generation cleanup
* Clean up API tests' random datatype generation and fix bug with enum
datatype generation
* Init parallel API tests with MPI_THREAD_MULTIPLE
* HDF5 API tests - Check VOL connector registration
* Determine whether a VOL connector failed to load before running API
tests
* Cleanup some usages of H5VL_CAP_FLAG_CREATION_ORDER in API tests
* Remove some now-unused macros from H5_api_tests_disabled.h
* Enable HDF5 API tests by default
* Implement CMake option to install HDF5 API tests
* Check for invalid AAPL from H5Acreate
* Enable building of VOL connectors alongside HDF5 in CMake
* Prepend CMake VOL URL option indices with 0s so they come in order
* Don't turn on API tests by default yet
* Document VOL connector FetchContent functionality
* Add release note for API test updates
* Only install testing library if API tests are installed
* Fix grammar
derobins pushed a commit that referenced this pull request Jul 27, 2023
* Remove macros from api tests (#2929)
* Remove macros and undefined callbacks (#2959)
* Remove remaining macros from H5_api_tests_disabled.h (#2968)
* Put some vol capability checks in testpar tests and remove remaining warnings (#2995)
* API tests datatype generation cleanup
* Clean up API tests' random datatype generation and fix bug with enum
datatype generation
* Init parallel API tests with MPI_THREAD_MULTIPLE
* HDF5 API tests - Check VOL connector registration
* Determine whether a VOL connector failed to load before running API
tests
* Cleanup some usages of H5VL_CAP_FLAG_CREATION_ORDER in API tests
* Remove some now-unused macros from H5_api_tests_disabled.h
* Enable HDF5 API tests by default
* Implement CMake option to install HDF5 API tests
* Check for invalid AAPL from H5Acreate
* Enable building of VOL connectors alongside HDF5 in CMake
* Prepend CMake VOL URL option indices with 0s so they come in order
* Don't turn on API tests by default yet
* Document VOL connector FetchContent functionality
* Add release note for API test updates
* Only install testing library if API tests are installed
* Fix grammar
@jhendersonHDF jhendersonHDF deleted the feature/api_tests branch September 19, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Testing Code in test or testpar directories, GitHub workflows Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants