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

Unit test the bulk of the public API. #88

Merged
merged 31 commits into from
Aug 19, 2022
Merged

Unit test the bulk of the public API. #88

merged 31 commits into from
Aug 19, 2022

Conversation

Anders429
Copy link
Owner

@Anders429 Anders429 commented Aug 16, 2022

No description provided.

@Anders429
Copy link
Owner Author

Those tests on Archetype's serde impls alone added 19% coverage. I suspect that other Archetype tests will result in similar gains, since so much code goes through the Archetype methods.

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2022

Codecov Report

Merging #88 (b77db73) into master (3a0a447) will increase coverage by 55.09%.
The diff coverage is 99.39%.

@@             Coverage Diff             @@
##           master      #88       +/-   ##
===========================================
+ Coverage   39.94%   95.04%   +55.09%     
===========================================
  Files          56       57        +1     
  Lines        5970     9215     +3245     
===========================================
+ Hits         2385     8758     +6373     
+ Misses       3585      457     -3128     
Impacted Files Coverage Δ
src/world/entry.rs 95.27% <0.00%> (+95.27%) ⬆️
src/registry/serde.rs 98.27% <96.93%> (+98.27%) ⬆️
src/archetypes/impl_serde.rs 98.13% <97.74%> (+98.13%) ⬆️
src/entity/allocator/impl_serde.rs 98.76% <99.05%> (+98.76%) ⬆️
src/archetype/identifier/impl_serde.rs 100.00% <100.00%> (+100.00%) ⬆️
src/archetype/identifier/mod.rs 94.88% <100.00%> (+5.18%) ⬆️
src/archetype/impl_serde.rs 99.82% <100.00%> (+99.82%) ⬆️
src/archetype/mod.rs 97.40% <100.00%> (+86.90%) ⬆️
src/archetypes/mod.rs 100.00% <100.00%> (+51.42%) ⬆️
src/entities/mod.rs 100.00% <100.00%> (+100.00%) ⬆️
... and 53 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Anders429
Copy link
Owner Author

I'm not worried about the valgrind issue for now. It seems that rayon leaks some memory from its thread pool, but that memory won't grow or cause any major issues. It's not caused directly by this library, but by what rayon is doing. Likely related to this.

@Anders429
Copy link
Owner Author

This is a very significant improvement from what it was before. While this doesn't actually resolve #71, since there is still some more granular testing that can be done, I'm going to go ahead and merge this into master. It will be nice to have some actual unit test coverage for changes going forward.

@Anders429 Anders429 changed the title Fully unit test the code. Unit test the bulk of the public API. Aug 19, 2022
@Anders429 Anders429 marked this pull request as ready for review August 19, 2022 23:08
@Anders429 Anders429 merged commit 473c1da into master Aug 19, 2022
@Anders429 Anders429 deleted the unit branch August 19, 2022 23:08
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.

2 participants