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

Add HDF5IO tests for add_read PR #118

Open
wants to merge 33 commits into
base: add_read
Choose a base branch
from
Open

Conversation

oruebel
Copy link
Contributor

@oruebel oruebel commented Dec 26, 2024

This PR targets #85 to complete open ToDo items to add unit tests.

Add unit tests for public HDF5IO-specific functions (i.e., not inherited from BaseIO)

  • HDF5IO::getH5ObjectType
  • HDF5IO::getNativeType
  • HDF5IO::getH5Type

Add unit tests for new publicHDF5IO functions (which are core functions new inBaseIO). While some of these are partially covered by the DEFINE_FIELDS read tests, it will be useful to more fully test these separately

  • HDF5IO::readDataset (incl. with start, count, stride, and block parameters set). This is to test that reading subsets of larger arrays works as expected.
  • HDF5IO::open(FileMode mode) test that file modes are being set correctly
  • HDF5IO::readAttribute
  • HDF5IO::getStorageObjectType
  • HDF5IO::getGroupObjects
  • HDF5IO::attributeExists
  • HDF5IO::objectExists

New TODO items identified in unit tests:

  • Update HDF5IO::readDataset to cover all numeric types in BaseDataType
  • Debug reading of strings and update theTEST_CASE("HDF5IO; read dataset", "[hdf5io]") and TEST_CASE("HDF5IO; read dataset subset", "[hdf5io]").
    • Fixed writing of fixed length strings (the data type was not being set correctly)
    • Fixed reading of fixed length strings
    • Fixed writing of variable length strings via HDF5RecordingData::writeDataBlock
    • Fixed reading of variable length string datasets
    • Fixed reading of empty string datasets

@oruebel oruebel marked this pull request as draft December 26, 2024 00:25
@oruebel oruebel mentioned this pull request Dec 26, 2024
70 tasks
@oruebel oruebel marked this pull request as ready for review December 26, 2024 01:35
@oruebel oruebel requested a review from stephprince December 26, 2024 01:35
@oruebel
Copy link
Contributor Author

oruebel commented Jan 3, 2025

@stephprince there is still an issue with reading strings, but otherwise this PR is ready for you to take a look at.

@oruebel
Copy link
Contributor Author

oruebel commented Jan 3, 2025

I have confirmed now that the fixed length string dataset is being written correctly. However, reading of fixed length string data still fails. I have not investigated the issues with variable length strings yet.

@oruebel
Copy link
Contributor Author

oruebel commented Jan 3, 2025

Ok, fixed length strings seem to work now for both write and read. Next up, variable length strings.

@oruebel
Copy link
Contributor Author

oruebel commented Jan 3, 2025

@stephprince I've now fixed the writing/reading of fixed and variable length strings. This PR is ready for you to take a look at. I think the other TODO items for #85 are probably best done in additional PRs, otherwise this PR will become to large.

The last remaining issue for this PR is to fix the address sanitizer test. It looks like the sanitizer doesn't like something about the changes in HDF5RecordingData::writeDataBlock . Getting string datasets to work for write/read is a bit tricky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant