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

Garden test cleanup #587

Merged
merged 9 commits into from
Jan 30, 2024
Merged

Garden test cleanup #587

merged 9 commits into from
Jan 30, 2024

Conversation

mjcarroll
Copy link
Contributor

Split from #513 for easier review.

This cleans up physics tests in a few ways:

  • Consolidate TestUtilities.hh and test/Utils.hh
  • Move test headers to test/include/test
  • Move mock headers to test/include/mock
  • Create an INTERFACE library for test/mock headers that sets common compiler definitions as well as common libraries to link against
  • Remove all instances of hardcoded world names or resource names and instead use constants.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jan 9, 2024
@mjcarroll mjcarroll changed the title Mjcarroll/test cleanup Garden test cleanup Jan 9, 2024
This cleans up physics tests in a few ways:

Consolidate TestUtilities.hh and test/Utils.hh
Move test headers to test/include/test
Move mock headers to test/include/mock
Create an INTERFACE library for test/mock headers that sets common compiler definitions as well as common libraries to link against
Remove all instances of hardcoded world names or resource names and instead use constants.

Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Copy link
Contributor Author

A question here (maybe for @scpeters @ahcorde). There are worlds/sdf files that are spread out among the various implementations. Can these be deduplicated/consolidated?

Copy link

codecov bot commented Jan 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5367620) 78.64% compared to head (e84feed) 78.47%.

❗ Current head e84feed differs from pull request most recent head 1c806fd. Consider uploading reports for the commit 1c806fd to get more accurate results

Additional details and impacted files
@@               Coverage Diff               @@
##           gz-physics6     #587      +/-   ##
===============================================
- Coverage        78.64%   78.47%   -0.17%     
===============================================
  Files              141      139       -2     
  Lines             7713     7611     -102     
===============================================
- Hits              6066     5973      -93     
+ Misses            1647     1638       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mjcarroll mjcarroll requested a review from ahcorde January 9, 2024 18:28
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Nice clean up! Just some minor comments

dartsim/src/Worlds.hh Outdated Show resolved Hide resolved
test/common_test/collisions.cc Show resolved Hide resolved
test/include/test/Resources.hh Outdated Show resolved Hide resolved
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I'm not finished reviewing this, but here's a batch of comments

CMakeLists.txt Outdated Show resolved Hide resolved
dartsim/src/Worlds.hh Outdated Show resolved Hide resolved
dartsim/src/Worlds.hh Outdated Show resolved Hide resolved
test/common_test/CMakeLists.txt Outdated Show resolved Hide resolved
@mjcarroll
Copy link
Contributor Author

@scpeters I have removed anything that was duplicated (or closely duplicated). This required some small changes in the tpe tests to match the common world files.

@mjcarroll mjcarroll merged commit 8e1aca6 into gz-physics6 Jan 30, 2024
9 checks passed
@mjcarroll mjcarroll deleted the mjcarroll/test_cleanup branch January 30, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants