From 28f1105def044ec02c3040f3d1953408da785ea6 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 4 Jun 2020 08:11:41 +0200 Subject: [PATCH 1/3] Install valgrind and run unit tests via valgrind in order to check for memory leaks --- .github/workflows/unit-tests.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 9eb35c8f7..b91b8f2f4 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -25,13 +25,17 @@ jobs: - name: Checkout uses: actions/checkout@v2 + - name: Install valgrind + run: | + sudo apt-get install valgrind + - name: Run unit tests run: | mkdir "$BUILD_PATH" cd "$BUILD_PATH" cmake .. make - bin/testArduinoIoTCloud + valgrind --leak-check=yes bin/testArduinoIoTCloud - name: Check code coverage run: | From 11a8f579da944f831bb95773405a123fa1c3c83f Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 4 Jun 2020 08:18:31 +0200 Subject: [PATCH 2/3] Let valgrind return a non-zero error code (0 = success on bash) if there are memory leaks because otherwise it returns the return code of the software its running --- .github/workflows/unit-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index b91b8f2f4..a4246edc1 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -35,7 +35,7 @@ jobs: cd "$BUILD_PATH" cmake .. make - valgrind --leak-check=yes bin/testArduinoIoTCloud + valgrind --leak-check=yes --error-exitcode=1 bin/testArduinoIoTCloud - name: Check code coverage run: | From 61822067bd1b06661b92a4c11de8678719ea7c72 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Thu, 4 Jun 2020 08:22:33 +0200 Subject: [PATCH 3/3] Fixing memory leaks in unit tests --- extras/test/src/test_callback.cpp | 4 ++-- extras/test/src/test_decode.cpp | 16 +++++++++------- extras/test/src/test_encode.cpp | 16 +++++++++------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/extras/test/src/test_callback.cpp b/extras/test/src/test_callback.cpp index f95e4c6da..447b73d61 100644 --- a/extras/test/src/test_callback.cpp +++ b/extras/test/src/test_callback.cpp @@ -162,7 +162,7 @@ SCENARIO("After a connection/reconnection an incoming cbor payload is processed SCENARIO("Primitive property: After a connection/reconnection an incoming cbor payload is processed and the synchronization callback is executed. The sync callback applies the AUTO_SYNC policy (the most recent value between the local one and the cloud one is finally assigned to the property). The onUpdate function is called if the cloud value is the most recent one. In this scenario the most updated value is the cloud one.") { GIVEN("CloudProtocol::V2") { bool test = true; - ArduinoCloudProperty *p = new CloudWrapperBool(test); + std::unique_ptr p(new CloudWrapperBool(test)); sync_callback_called = false; change_callback_called = false; @@ -192,7 +192,7 @@ SCENARIO("Primitive property: After a connection/reconnection an incoming cbor p SCENARIO("Primitive property: After a connection/reconnection an incoming cbor payload is processed and the synchronization callback is executed. The sync callback apply the AUTO_SYNC policy (the most recent value between the local one and the cloud one is finally assigned to the property). The onUpdate function is called if the cloud value is the most recent one. In this scenario the most updated value is the local one.") { GIVEN("CloudProtocol::V2") { bool test = true; - ArduinoCloudProperty *p = new CloudWrapperBool(test); + std::unique_ptr p(new CloudWrapperBool(test)); sync_callback_called = false; change_callback_called = false; diff --git a/extras/test/src/test_decode.cpp b/extras/test/src/test_decode.cpp index 9b89bb177..43c66b4cc 100644 --- a/extras/test/src/test_decode.cpp +++ b/extras/test/src/test_decode.cpp @@ -8,6 +8,8 @@ #include +#include + #include #include #include "types/CloudWrapperBool.h" @@ -483,15 +485,15 @@ SCENARIO("Arduino Cloud Properties are decoded", "[ArduinoCloudThing::decode]") String str_test; str_test = "str_test"; - ArduinoCloudProperty *i = new CloudWrapperInt(int_test); - ArduinoCloudProperty *b = new CloudWrapperBool(bool_test); - ArduinoCloudProperty *f = new CloudWrapperFloat(float_test); - ArduinoCloudProperty *s = new CloudWrapperString(str_test); + std::unique_ptr i(new CloudWrapperInt(int_test)); + std::unique_ptr b(new CloudWrapperBool(bool_test)); + std::unique_ptr f(new CloudWrapperFloat(float_test)); + std::unique_ptr s(new CloudWrapperString(str_test)); - thing.addPropertyReal(*b, "bool_test", Permission::ReadWrite).onSync(CLOUD_WINS); - thing.addPropertyReal(*i, "int_test", Permission::ReadWrite).onSync(CLOUD_WINS); + thing.addPropertyReal(*b, "bool_test", Permission::ReadWrite).onSync(CLOUD_WINS); + thing.addPropertyReal(*i, "int_test", Permission::ReadWrite).onSync(CLOUD_WINS); thing.addPropertyReal(*f, "float_test", Permission::ReadWrite).onSync(CLOUD_WINS); - thing.addPropertyReal(*s, "str_test", Permission::ReadWrite).onSync(CLOUD_WINS); + thing.addPropertyReal(*s, "str_test", Permission::ReadWrite).onSync(CLOUD_WINS); thing.updateTimestampOnLocallyChangedProperties(); diff --git a/extras/test/src/test_encode.cpp b/extras/test/src/test_encode.cpp index f81d6701c..6198b675b 100644 --- a/extras/test/src/test_encode.cpp +++ b/extras/test/src/test_encode.cpp @@ -8,6 +8,8 @@ #include +#include + #include #include #include "types/CloudWrapperBool.h" @@ -377,15 +379,15 @@ SCENARIO("Arduino Cloud Properties are encoded", "[ArduinoCloudThing::encode]") String str_test; str_test = "str_test"; - ArduinoCloudProperty *i = new CloudWrapperInt(int_test); - ArduinoCloudProperty *b = new CloudWrapperBool(bool_test); - ArduinoCloudProperty *f = new CloudWrapperFloat(float_test); - ArduinoCloudProperty *s = new CloudWrapperString(str_test); + std::unique_ptr i(new CloudWrapperInt(int_test)); + std::unique_ptr b(new CloudWrapperBool(bool_test)); + std::unique_ptr f(new CloudWrapperFloat(float_test)); + std::unique_ptr s(new CloudWrapperString(str_test)); - thing.addPropertyReal(*i, "int_test", Permission::ReadWrite); - thing.addPropertyReal(*b, "bool_test", Permission::ReadWrite); + thing.addPropertyReal(*i, "int_test", Permission::ReadWrite); + thing.addPropertyReal(*b, "bool_test", Permission::ReadWrite); thing.addPropertyReal(*f, "float_test", Permission::ReadWrite); - thing.addPropertyReal(*s, "str_test", Permission::ReadWrite); + thing.addPropertyReal(*s, "str_test", Permission::ReadWrite); thing.updateTimestampOnLocallyChangedProperties();