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

Docs and Tests for Format #97

Merged
merged 4 commits into from
May 18, 2022
Merged

Docs and Tests for Format #97

merged 4 commits into from
May 18, 2022

Conversation

ardasener
Copy link
Contributor

This pr will add a test suite and documentation comments for the members of sparsebase::format namespace. The test suites includes 3 different types of tests for each concrete class derived from sparsebase::format::Format:

  1. Basics: Stores some data on the format instance and checks the integrity and the dimensions of the data.
  2. Ownership: Creates two format instances using the same data with only one them set to owned. Then attempts to delete both instances. In a successful run, only the owned instance will deallocate the memory hence no runtime errors will be encountered. Note that I found no way to verify if the memory is actually deallocated properly, as a result I suggest adding a "memory leak check" step to our Github Actions configuration (which can use an external program like valgrind).
  3. Release: Creates a format instance set to owned. Then releases the data using the release_xxx() member functions. Then both the data and the format instance is deleted. Since the data is released from the instance a double delete should not occur in a successful run.

@AmroAlJundi AmroAlJundi self-requested a review May 17, 2022 12:40
@ardasener ardasener added the type: docs Related to documentation and information label May 18, 2022
@ardasener ardasener merged commit fda7b81 into develop May 18, 2022
@ardasener ardasener deleted the feature/format_docs_tests branch May 18, 2022 12:05
@ardasener ardasener added state: approved Approved to proceed. Ready to be merged and removed state: review needed labels May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: approved Approved to proceed. Ready to be merged type: docs Related to documentation and information type: testing Related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants