Catch2 update #246
Catch2 update #246
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 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. |
|
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 = new IPAddress();
#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.
While this surely makes the test pass, I'm quite sure the test is not about self 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.
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.
According to Gcc online docs at 3.7.2 GCC_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 machine GCC_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
andreagilardoni
force-pushed
the
cmake-improvements
branch
from
6050833 to
e97e5e2
Compare
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.