Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork130
Catch2 update#246
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Catch2 update#246
Uh oh!
There was an error while loading.Please reload this page.
Conversation
In order to update catch2 the following changes were made:- CMakeLists.txt: import catch2 with FetchContent_Declare, this will make it more flexible and easier to update- removed main for tests, since it is not mandatory and it doesn't add any value of having it- renaming includes in all the files
codecov-commenter commentedFeb 13, 2025
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## master #246 +/- ##======================================= Coverage 95.53% 95.53% ======================================= Files 16 16 Lines 1075 1076 +1 =======================================+ Hits 1027 1028 +1 Misses 48 48 ☔ View full report in Codecov by Sentry. |
andreagilardoni commentedFeb 13, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
One of the reasons why I suggest introducing this new version of Catch2 is because I noticed that valgrind is unable to detect memory leaks caused by missing virtual destructor. A practical example is the following. You can test this by running the tests this way #include<catch.hpp>#include<api/IPAddress.h>TEST_CASE("Testing polymorphic IPAddress memory free","[printable-delete-01]"){ arduino::Printable* p =newIPAddress();#pragma GCC diagnostic push#pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor"delete p;#pragma GCC diagnostic pop} |
test/src/String/test_move.cpp Outdated
| #if defined(GCC_VERSION) && GCC_VERSION >= 13 | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| String b; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
While this surely makes the test pass, I'm quite sure the test is not aboutself assignment anymore 🙃 If there's an issue with memory leaking from this we should solve it at core level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I missed the point of this test case, I'll drop this commit. I'll look if I can improve the preprocessor checks, since I am failing this locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
According toGcc online docs at 3.7.2GCC_VERSION is not defined in the way we are using it. I think we could check against__GNUC__ only.
I don't understand why on my local machineGCC_VERSION is not defined, but__GNUC__ is.
error: moving ‘a’ of type ‘arduino::String’ to itself [-Werror=self-move]Which may happen if if GCC_VERSION is not defined, and instead __GNUC__is defined
since for catch2 '[' is not a valid character in the tag name
6050833 toe97e5e2Compare
This PR aims to provide a newer version for catch2 that is directly integrated in CMake with improved functionalities.
This also solves the issue of
-Werror=self-movehappening ontest/src/String/test_move.cpp:45:7happening on some compilers.