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

Fix/tests #2

Merged
merged 6 commits into from
Dec 7, 2023
Merged

Fix/tests #2

merged 6 commits into from
Dec 7, 2023

Conversation

nathanhhughes
Copy link
Collaborator

@Schmluk Found a somewhat workable solution for handling the full-stream precision that yaml-cpp uses under the hood for floating-point representations. Other notes:

  • Brings gtest_discover_tests back (I'm hoping I found the right arguments to avoid the weird linking issue that happens in catkin that made me remove it originally) so we can use ctest
  • Fixes parsing negative numbers on new-ish verisons of yaml-cpp (you can't do as<unsigned> on a literal with a negative sign)
  • Adds some minor fixes for unit tests that were previously passing because of global config settings set in other tests (which we should maybe more explicitly fix, but gtest_discover_tests isolates each test into a single executable already).

Lmk what you think about the float formatting. We can also talk in-person about the what else I tried, I don't think there's an easy way to get around the edge-cases to my solution (i.e., string fields that are look like really long floats) without a much more substantial overhaul to support tracking which scalars are floats.

@nathanhhughes nathanhhughes requested a review from Schmluk December 3, 2023 19:11
@nathanhhughes nathanhhughes self-assigned this Dec 3, 2023
Copy link
Collaborator

@Schmluk Schmluk left a comment

Choose a reason for hiding this comment

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

Looks great! Probably a good thing to have floats formatted to human readable sizes by default 👍

@nathanhhughes nathanhhughes merged commit 05c7f87 into main Dec 7, 2023
2 checks passed
@nathanhhughes nathanhhughes deleted the fix/tests branch December 7, 2023 15:46
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