From ae5f79e059ffb330b848fb0a18f6be13332f185e Mon Sep 17 00:00:00 2001 From: Reuben Cartwright Date: Tue, 30 Jul 2024 16:59:10 +0100 Subject: [PATCH 1/5] coremqtt-agent: Add FreeRTOS Command Pool tests This commit adds 7 new unit tests for the file freertos_command_pool.c. Signed-off-by: Reuben Cartwright --- .../integration/tests/CMakeLists.txt | 26 ++ .../tests/test_freertos_command_pool.cpp | 251 ++++++++++++++++++ release_changes/202409111546.change | 1 + 3 files changed, 278 insertions(+) create mode 100644 components/aws_iot/coremqtt_agent/integration/tests/test_freertos_command_pool.cpp create mode 100644 release_changes/202409111546.change diff --git a/components/aws_iot/coremqtt_agent/integration/tests/CMakeLists.txt b/components/aws_iot/coremqtt_agent/integration/tests/CMakeLists.txt index d53f8a3..2ac570b 100644 --- a/components/aws_iot/coremqtt_agent/integration/tests/CMakeLists.txt +++ b/components/aws_iot/coremqtt_agent/integration/tests/CMakeLists.txt @@ -51,3 +51,29 @@ target_link_libraries(mqtt-agent-task-test trusted-firmware-m-mock ) iot_reference_arm_corstone3xx_add_test(mqtt-agent-task-test) + +add_executable(freertos-command-pool-test + test_freertos_command_pool.cpp + ../src/freertos_command_pool.c +) +target_include_directories(freertos-command-pool-test + PRIVATE + . + ../../library_mocks/inc + + ../inc +) +target_link_libraries(freertos-command-pool-test + PRIVATE + fff + backoff-algorithm-mock + coremqtt-agent-mock + coremqtt-agent-test-config-mocks + coremqtt-mock + freertos-kernel-mock + freertos-plus-tcp-mock + helpers-logging-mock + mbedtls-mock + trusted-firmware-m-mock +) +iot_reference_arm_corstone3xx_add_test(freertos-command-pool-test) diff --git a/components/aws_iot/coremqtt_agent/integration/tests/test_freertos_command_pool.cpp b/components/aws_iot/coremqtt_agent/integration/tests/test_freertos_command_pool.cpp new file mode 100644 index 0000000..1a10cd6 --- /dev/null +++ b/components/aws_iot/coremqtt_agent/integration/tests/test_freertos_command_pool.cpp @@ -0,0 +1,251 @@ +/* Copyright 2024 Arm Limited and/or its affiliates + * + * SPDX-License-Identifier: MIT + */ + +#include "fff.h" + +#include "gtest/gtest.h" + + +extern "C" { +#include "FreeRTOSConfig.h" +#include "core_mqtt_config.h" +#include "FreeRTOS.h" +#include "logging_stack.h" +#include "queue.h" +#include "freertos_command_pool.h" + +/* Directly copy-paste mock headers from the file under test's directory. + * Otherwise, the non-mock files are detected. */ + +/* freertos_agent_message.h */ +#ifndef FREERTOS_AGENT_MESSAGE_H + #define FREERTOS_AGENT_MESSAGE_H + +#include +#include +#include "core_mqtt_agent_message_interface.h" +#include "fff.h" +#include "queue.h" + + struct MQTTAgentMessageContext + { + QueueHandle_t queue; + }; + + DECLARE_FAKE_VALUE_FUNC( bool, + Agent_MessageSend, + MQTTAgentMessageContext_t *, + MQTTAgentCommand_t * const *, + uint32_t ); + DECLARE_FAKE_VALUE_FUNC( bool, + Agent_MessageReceive, + MQTTAgentMessageContext_t *, + MQTTAgentCommand_t * *, + uint32_t ); +#endif /* FREERTOS_AGENT_MESSAGE_H */ + +/* freertos_agent_message.c */ + +DEFINE_FAKE_VALUE_FUNC( bool, + Agent_MessageSend, + MQTTAgentMessageContext_t *, + MQTTAgentCommand_t * const *, + uint32_t ); +DEFINE_FAKE_VALUE_FUNC( bool, + Agent_MessageReceive, + MQTTAgentMessageContext_t *, + MQTTAgentCommand_t * *, + uint32_t ); + +/* Functions usually defined by main.c */ +DEFINE_FAKE_VOID_FUNC( vAssertCalled, + const char *, + unsigned long ); +DEFINE_FAKE_VOID_FUNC_VARARG( SdkLogError, + const char *, + ... ); +DEFINE_FAKE_VOID_FUNC_VARARG( SdkLogWarn, + const char *, + ... ); +DEFINE_FAKE_VOID_FUNC_VARARG( SdkLogInfo, + const char *, + ... ); +DEFINE_FAKE_VOID_FUNC_VARARG( SdkLogDebug, + const char *, + ... ); +} + +DEFINE_FFF_GLOBALS + +#define ASSERTION_FAILURE 1 + +/* Mocks for vAssertCalled */ +void throw_assertion_failure( const char * pcFile, + unsigned long ulLine ) +{ + /* + * Behaviour wanted: + * - Encounters assertion fail, stops running any more code. E.g. does not go to next line. + * - But checks all assertions in the google test program hold. + */ + throw ( ASSERTION_FAILURE ); +} +void do_nothing_on_assertion_failure( const char * pcFile, + unsigned long ulLine ) /* do nothing */ +{ +} + +class TestFreertosCommandPool : public ::testing::Test { +public: + TestFreertosCommandPool() + { + /* May be called but not used in the test suite.*/ + RESET_FAKE( SdkLogError ); + RESET_FAKE( SdkLogDebug ); + RESET_FAKE( SdkLogWarn ); + RESET_FAKE( SdkLogInfo ); + + /* Used in the test suite. */ + RESET_FAKE( xQueueCreateStatic ); + RESET_FAKE( Agent_MessageSend ); + RESET_FAKE( Agent_MessageReceive ); + RESET_FAKE( vAssertCalled ); /* used to trap errors. */ + vAssertCalled_fake.custom_fake = throw_assertion_failure; + Agent_MessageSend_fake.return_val = true; /* success for InitializePool. */ + } +}; + +void expect_no_errors( void ) +{ + EXPECT_EQ( vAssertCalled_fake.call_count, 0 ); + EXPECT_EQ( SdkLogError_fake.call_count, 0 ); +} +void expect_errors( void ) +{ + EXPECT_NE( vAssertCalled_fake.call_count + SdkLogError_fake.call_count, 0 ); +} + +TEST_F( TestFreertosCommandPool, correct_command_pool_initialisation_causes_no_errors ) +{ + Agent_MessageSend_fake.return_val = true; + QueueDefinition queue = { 10 }; + QueueHandle_t handle = &queue; + xQueueCreateStatic_fake.return_val = handle; + + Agent_InitializePool(); + + expect_no_errors(); +} + +TEST_F( TestFreertosCommandPool, initialisation_errors_if_queue_creation_fails ) +{ + vAssertCalled_fake.custom_fake = do_nothing_on_assertion_failure; + Agent_MessageSend_fake.return_val = true; + QueueHandle_t handle; /* invalid handle */ + xQueueCreateStatic_fake.return_val = handle; + + Agent_InitializePool(); + + expect_errors(); +} + +TEST_F( TestFreertosCommandPool, errors_if_initialisation_cannot_send_items_to_queue ) +{ + vAssertCalled_fake.custom_fake = do_nothing_on_assertion_failure; + Agent_MessageSend_fake.return_val = false; + QueueDefinition queue = { 10 }; + QueueHandle_t handle = &queue; + xQueueCreateStatic_fake.return_val = handle; + + Agent_InitializePool(); + + expect_errors(); +} + +TEST_F( TestFreertosCommandPool, get_command_tries_to_obtain_a_command_if_the_command_pool_is_not_full ) +{ + Agent_MessageReceive_fake.return_val = true; + Agent_MessageSend_fake.return_val = true; + QueueDefinition queue = { 10 }; + QueueHandle_t handle = &queue; + xQueueCreateStatic_fake.return_val = handle; + + Agent_InitializePool(); + expect_no_errors(); + EXPECT_EQ( Agent_MessageReceive_fake.call_count, 0 ); + + /* 20ms is used as the block time */ + Agent_GetCommand( 20 ); + + /* This test cannot run if the pool cannot contain anything. */ + EXPECT_NE( MQTT_COMMAND_CONTEXTS_POOL_SIZE, 0 ); + EXPECT_NE( Agent_MessageReceive_fake.call_count, 0 ); +} + +TEST_F( TestFreertosCommandPool, does_not_try_to_get_command_if_pool_not_initialized ) +{ + Agent_MessageReceive_fake.return_val = true; + Agent_MessageSend_fake.return_val = true; + + /* We expect MessageReceive to never be called on unsafe memory. */ + try{ + Agent_GetCommand( 20 ); + } + catch( int num ) { + if( num != ASSERTION_FAILURE ) + { + throw ( num ); + } + } + expect_errors(); + EXPECT_EQ( Agent_MessageReceive_fake.call_count, 0 ); +} + +TEST_F( TestFreertosCommandPool, tries_to_wait_the_correct_amount_of_time_to_receive_a_message ) +{ + QueueDefinition queue = { 10 }; + QueueHandle_t handle = &queue; + + xQueueCreateStatic_fake.return_val = handle; /* used to intialise pool. */ + uint32_t blockDurationMs = 20; + + Agent_InitializePool(); + expect_no_errors(); + + Agent_MessageReceive_fake.return_val = true; + EXPECT_EQ( Agent_MessageReceive_fake.call_count, 0 ); + Agent_GetCommand( blockDurationMs ); + EXPECT_NE( Agent_MessageReceive_fake.call_count, 0 ); + EXPECT_EQ( Agent_MessageReceive_fake.arg2_val, blockDurationMs ); +} + +/* This checks that memory locations are not accessed without checking first. */ +TEST_F( TestFreertosCommandPool, trying_to_release_a_bad_command_pointer_does_not_cause_crashes ) +{ + QueueDefinition queue = { 10 }; + QueueHandle_t handle = &queue; + + xQueueCreateStatic_fake.return_val = handle; + + Agent_InitializePool(); + expect_no_errors(); + + Agent_ReleaseCommand( nullptr ); + expect_no_errors(); +} + +TEST_F( TestFreertosCommandPool, failing_to_release_command_returns_false ) +{ + QueueDefinition queue = { 10 }; + QueueHandle_t handle = &queue; + + xQueueCreateStatic_fake.return_val = handle; + + Agent_InitializePool(); + expect_no_errors(); + + EXPECT_FALSE( Agent_ReleaseCommand( nullptr ) ); + expect_no_errors(); +} diff --git a/release_changes/202409111546.change b/release_changes/202409111546.change new file mode 100644 index 0000000..8e73977 --- /dev/null +++ b/release_changes/202409111546.change @@ -0,0 +1 @@ +coremqtt-agent-unit-test: Add tests for freertos_command_pool.c. From 29f194056a20e3492c0036f5e256e7792c4c8c46 Mon Sep 17 00:00:00 2001 From: Reuben Cartwright Date: Tue, 10 Sep 2024 15:35:09 +0100 Subject: [PATCH 2/5] fix-mocks: Fix FreeRTOSConfig.h mock FreeRTOSConfig.h has been corrected to #include "fff.h". Additionally defines configASSERT within FreeRTOS.h. Signed-off-by: Reuben Cartwright --- .../tests/config_mocks/inc/FreeRTOSConfig.h | 2 ++ .../freertos_kernel/library_mocks/inc/FreeRTOS.h | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/components/aws_iot/coremqtt_agent/integration/tests/config_mocks/inc/FreeRTOSConfig.h b/components/aws_iot/coremqtt_agent/integration/tests/config_mocks/inc/FreeRTOSConfig.h index bcb73ba..95c93e6 100644 --- a/components/aws_iot/coremqtt_agent/integration/tests/config_mocks/inc/FreeRTOSConfig.h +++ b/components/aws_iot/coremqtt_agent/integration/tests/config_mocks/inc/FreeRTOSConfig.h @@ -27,6 +27,8 @@ #ifndef FREERTOS_CONFIG_H #define FREERTOS_CONFIG_H +#include "fff.h" + DECLARE_FAKE_VOID_FUNC( vAssertCalled, const char *, unsigned long ); diff --git a/components/freertos_kernel/library_mocks/inc/FreeRTOS.h b/components/freertos_kernel/library_mocks/inc/FreeRTOS.h index 6e45d25..7dedcf2 100644 --- a/components/freertos_kernel/library_mocks/inc/FreeRTOS.h +++ b/components/freertos_kernel/library_mocks/inc/FreeRTOS.h @@ -36,7 +36,22 @@ typedef int StaticQueue_t; +/* +Definitions found in FreeTOSConfig.h. +Because for testing `freertos_command_pool.c` we cannot +directly include FreeRTOSConfig.h (this causes build failure), +nor can we prototype the configAssert macro from within the file. +*/ +#ifndef FREERTOS_CONFIG_H +#define FREERTOS_CONFIG_H + +DECLARE_FAKE_VOID_FUNC( vAssertCalled, + const char *, + unsigned long ); +#define configASSERT( x ) if( ( x ) == 0 ) vAssertCalled( __FILE__, __LINE__ ); + +#endif /* FREERTOS_CONFIG_H */ /* This file also contains some definitions usually found in * 'FreeRTOSConfig_target.h'. From 8b31831e6e7db5ce8e87f8cc4100a3978062ad7b Mon Sep 17 00:00:00 2001 From: Reuben Cartwright Date: Wed, 11 Sep 2024 15:56:27 +0100 Subject: [PATCH 3/5] aws-iot-fix: Remove unneeded .h include `mqtt_agent_task.h` included `transport_interface_api.h` unnecessarily for unit testing. This has been extracted into the test file `test_mqtt_agent_task.cpp` instead. Signed-off-by: Reuben Cartwright --- .../coremqtt_agent/integration/inc/mqtt_agent_task.h | 7 ------- .../integration/tests/test_mqtt_agent_task.cpp | 1 + 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/components/aws_iot/coremqtt_agent/integration/inc/mqtt_agent_task.h b/components/aws_iot/coremqtt_agent/integration/inc/mqtt_agent_task.h index 0cb7185..61c5926 100644 --- a/components/aws_iot/coremqtt_agent/integration/inc/mqtt_agent_task.h +++ b/components/aws_iot/coremqtt_agent/integration/inc/mqtt_agent_task.h @@ -33,13 +33,6 @@ #include "FreeRTOS.h" #include "task.h" -#ifdef UNIT_TESTING - -/* Transport interface header file. - * Needed to compile UNIT_TESTING function prototypes. */ - #include "transport_interface_api.h" -#endif /* UNIT_TESTING */ - /* MQTT library includes. */ #include "core_mqtt_config.h" #include "core_mqtt_agent.h" diff --git a/components/aws_iot/coremqtt_agent/integration/tests/test_mqtt_agent_task.cpp b/components/aws_iot/coremqtt_agent/integration/tests/test_mqtt_agent_task.cpp index 2815f0c..d7b50d6 100644 --- a/components/aws_iot/coremqtt_agent/integration/tests/test_mqtt_agent_task.cpp +++ b/components/aws_iot/coremqtt_agent/integration/tests/test_mqtt_agent_task.cpp @@ -23,6 +23,7 @@ extern "C" { #include "psa/crypto.h" #include "psa/error.h" #include "task.h" +#include "transport_interface_api.h" #include "queue.h" /* From 63e91740390d4b12a4b2f2687b1d14ff49fc50db Mon Sep 17 00:00:00 2001 From: Reuben Cartwright Date: Wed, 11 Sep 2024 16:30:36 +0100 Subject: [PATCH 4/5] unit-testing: Refactor mocks Makes CMakeLists.txt use `BUILD_TESTING` as well as `CMAKE_CROSSCOMPILING`. Signed-off-by: Reuben Cartwright --- .../integration/src/mqtt_agent_task.c | 84 +++++++++++-------- .../connectivity/iot_socket/CMakeLists.txt | 4 +- .../library_mocks/inc/FreeRTOS.h | 22 ++--- .../freertos_pkcs11_psa/CMakeLists.txt | 10 ++- 4 files changed, 68 insertions(+), 52 deletions(-) diff --git a/components/aws_iot/coremqtt_agent/integration/src/mqtt_agent_task.c b/components/aws_iot/coremqtt_agent/integration/src/mqtt_agent_task.c index b147e7a..910a7aa 100644 --- a/components/aws_iot/coremqtt_agent/integration/src/mqtt_agent_task.c +++ b/components/aws_iot/coremqtt_agent/integration/src/mqtt_agent_task.c @@ -208,6 +208,54 @@ extern SubscriptionElement_t xGlobalSubscriptionList[ SUBSCRIPTION_MANAGER_MAX_S /*-----------------------------------------------------------*/ +/** + * @brief Retry logic to establish a connection to the MQTT broker. + * + * If the connection fails, keep retrying with exponentially increasing + * timeout value, until max retries, max timeout or successful connect. + * + * @param[in] pNetworkContext Network context to connect on. + * @return int pdFALSE if connection failed after retries. + */ +STATIC BaseType_t prvSocketConnect( NetworkContext_t * pxNetworkContext ); + +/** + * @brief Initializes an MQTT context, including transport interface and + * network buffer. + * + * @return `MQTTSuccess` if the initialization succeeds, else `MQTTBadParameter`. + */ +STATIC MQTTStatus_t prvMQTTInit( void ); + +/** + * @brief Sends an MQTT Connect packet over the already connected TCP socket. + * This function takes no inputs, but does rely on the global 'xConnectInfo' + * variable, which contains information such as passwords and user identifiers, + * and whether to start a clean MQTT session. + * + * @return `MQTTSuccess` if connection succeeds, else appropriate error code + * from MQTT_Connect. + */ +STATIC MQTTStatus_t prvMQTTConnect( void ); + +/** + * @brief Disconnects from the MQTT broker. + * Initiates an MQTT disconnect and then teardown underlying TCP connection. + */ +STATIC void prvDisconnectFromMQTTBroker( void ); + +/** + * @brief Task for MQTT agent. + * Task runs MQTT agent command loop, which returns only when the user disconnects + * MQTT, terminates agent, or the MQTT connection is broken. If the MQTT connection is broken, the task + * tries to reconnect to the broker. + * + * @param[in] pParam Can be used to pass down functionality to the agent task + */ +STATIC void prvMQTTAgentTask( void * pParam ); + +/*-----------------------------------------------------------*/ + STATIC uint32_t prvGetTimeMs( void ) { TickType_t xTickCount = 0; @@ -243,15 +291,6 @@ STATIC UBaseType_t prvGetRandomNumber( void ) return uxRandomValue; } -/** - * @brief Retry logic to establish a connection to the MQTT broker. - * - * If the connection fails, keep retrying with exponentially increasing - * timeout value, until max retries, max timeout or successful connect. - * - * @param[in] pNetworkContext Network context to connect on. - * @return int pdFALSE if connection failed after retries. - */ STATIC BaseType_t prvSocketConnect( NetworkContext_t * pxNetworkContext ) { BaseType_t xConnected = pdFAIL; @@ -391,12 +430,6 @@ STATIC void prvReSubscriptionCommandCallback( MQTTAgentCommandContext_t * pxComm } } -/** - * @brief Initializes an MQTT context, including transport interface and - * network buffer. - * - * @return `MQTTSuccess` if the initialization succeeds, else `MQTTBadParameter`. - */ STATIC MQTTStatus_t prvMQTTInit( void ) { TransportInterface_t xTransport = { 0 }; @@ -512,15 +545,6 @@ STATIC MQTTStatus_t prvHandleResubscribe( void ) return xResult; } -/** - * @brief Sends an MQTT Connect packet over the already connected TCP socket. - * - * @param[in] pxMQTTContext MQTT context pointer. - * @param[in] xCleanSession If a clean session should be established. - * - * @return `MQTTSuccess` if connection succeeds, else appropriate error code - * from MQTT_Connect. - */ STATIC MQTTStatus_t prvMQTTConnect( void ) { MQTTStatus_t xResult; @@ -593,10 +617,6 @@ STATIC void prvDisconnectCommandCallback( MQTTAgentCommandContext_t * pxCommandC } } -/** - * @brief Disconnects from the MQTT broker. - * Initiates an MQTT disconnect and then teardown underlying TCP connection. - */ STATIC void prvDisconnectFromMQTTBroker( void ) { static MQTTAgentCommandContext_t xCommandContext = { 0 }; @@ -629,14 +649,6 @@ STATIC void prvDisconnectFromMQTTBroker( void ) } } -/** - * @brief Task for MQTT agent. - * Task runs MQTT agent command loop, which returns only when the user disconnects - * MQTT, terminates agent, or the mqtt connection is broken. If the mqtt connection is broken, the task - * tries to reconnect to the broker. - * - * @param[in] pParam Can be used to pass down functionality to the agent task - */ STATIC void prvMQTTAgentTask( void * pParam ) { BaseType_t xResult; diff --git a/components/connectivity/iot_socket/CMakeLists.txt b/components/connectivity/iot_socket/CMakeLists.txt index 8d5ae16..aedfe52 100644 --- a/components/connectivity/iot_socket/CMakeLists.txt +++ b/components/connectivity/iot_socket/CMakeLists.txt @@ -2,7 +2,9 @@ # # SPDX-License-Identifier: MIT -if(CMAKE_CROSSCOMPILING) +if(BUILD_TESTING AND NOT CMAKE_CROSSCOMPILING) + # left empty for future mocks. +else () set(iot_socket_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/library CACHE INTERNAL diff --git a/components/freertos_kernel/library_mocks/inc/FreeRTOS.h b/components/freertos_kernel/library_mocks/inc/FreeRTOS.h index 7dedcf2..9c9364e 100644 --- a/components/freertos_kernel/library_mocks/inc/FreeRTOS.h +++ b/components/freertos_kernel/library_mocks/inc/FreeRTOS.h @@ -36,20 +36,20 @@ typedef int StaticQueue_t; -/* -Definitions found in FreeTOSConfig.h. -Because for testing `freertos_command_pool.c` we cannot -directly include FreeRTOSConfig.h (this causes build failure), -nor can we prototype the configAssert macro from within the file. -*/ +/* + * Definitions found in FreeTOSConfig.h. + * Because for testing `freertos_command_pool.c` we cannot + * directly include FreeRTOSConfig.h (this causes build failure), + * nor can we prototype the configAssert macro from within the file. + */ #ifndef FREERTOS_CONFIG_H -#define FREERTOS_CONFIG_H + #define FREERTOS_CONFIG_H -DECLARE_FAKE_VOID_FUNC( vAssertCalled, - const char *, - unsigned long ); -#define configASSERT( x ) if( ( x ) == 0 ) vAssertCalled( __FILE__, __LINE__ ); + DECLARE_FAKE_VOID_FUNC( vAssertCalled, + const char *, + unsigned long ); + #define configASSERT( x ) if( ( x ) == 0 ) vAssertCalled( __FILE__, __LINE__ ); #endif /* FREERTOS_CONFIG_H */ diff --git a/components/security/freertos_pkcs11_psa/CMakeLists.txt b/components/security/freertos_pkcs11_psa/CMakeLists.txt index ae4551b..de92b45 100644 --- a/components/security/freertos_pkcs11_psa/CMakeLists.txt +++ b/components/security/freertos_pkcs11_psa/CMakeLists.txt @@ -2,11 +2,13 @@ # # SPDX-License-Identifier: MIT -if(CMAKE_CROSSCOMPILING) +if(BUILD_TESTING AND NOT CMAKE_CROSSCOMPILING) + # left empty for future mocks. +else () set(freertos_pkcs11_psa_SOURCE_DIR - ${CMAKE_CURRENT_LIST_DIR}/library - CACHE INTERNAL - "Path to FreeRTOS PKCS#11 to PSA shim layer source code" + ${CMAKE_CURRENT_LIST_DIR}/library + CACHE INTERNAL + "Path to FreeRTOS PKCS#11 to PSA shim layer source code" ) add_subdirectory(integration) From 520744e7d25c079298c6d97f9576e36934dd4c95 Mon Sep 17 00:00:00 2001 From: Reuben Cartwright Date: Thu, 19 Sep 2024 11:49:50 +0100 Subject: [PATCH 5/5] docs: Document unit testing with configASSERT This commit adds an explanation in `unit_testing.md` on how configASSERT is handled in `freertos_command_pool.c` tests and other test files. The intended behaviour is that an assertion failure causes the rest of the code to stop running, but can be handled by a test and does not stop the rest of the test's checks running. Signed-off-by: Reuben Cartwright --- docs/unit_testing.md | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/docs/unit_testing.md b/docs/unit_testing.md index dec9dc1..fbf0535 100644 --- a/docs/unit_testing.md +++ b/docs/unit_testing.md @@ -413,6 +413,101 @@ This is testing the function `initialize_network`, however this function does no > :bulb: This can be avoided by using a Test Driven Development (TDD) approach which will ensure that all functions developed can be tested. Writing unit test cases for existing modules does not guarantee that the behavior can be tested without observing implementation details. +### Testing with configASSERT + +We define the macro configASSERT to be: + +```c +#define configASSERT( x ) if( ( x ) == 0 ) vAssertCalled( __FILE__, __LINE__ ); +``` +Where `vAssertCalled` is a void-returning fake function. + +We want to test the below function: +```c +MQTTAgentCommand_t * Agent_GetCommand( uint32_t blockTimeMs ) +{ + MQTTAgentCommand_t * structToUse = NULL; + bool structRetrieved = false; + + /* Check queue has been created. */ + configASSERT( initStatus == QUEUE_INITIALIZED ); + + /* Retrieve a struct from the queue. */ + structRetrieved = Agent_MessageReceive( &commandStructMessageCtx, &( structToUse ), blockTimeMs ); + + if( !structRetrieved ) + { + LogDebug( ( "No command structure available.\n" ) ); + } + + return structToUse; +} +``` + +We want to test that if the function is called with the command pool uninitialized, then it will error and not call `Agent_MessageReceive`. + +A naive test would be: + +```cpp +TEST_F(TestFreertosCommandPool, does_not_try_to_get_command_if_pool_not_initialized) { + Agent_MessageReceive_fake.return_val = true; + Agent_MessageSend_fake.return_val = true; + + // We expect MessageReceive to never be called on unsafe memory. + Agent_GetCommand(20); + expect_errors(); + EXPECT_EQ(Agent_MessageReceive_fake.call_count, 0); +} +``` +The above will not work. If we do not define a custom fake for `vAssertCalled`, the program will not stop at the line `configASSERT` fails, but instead continue to the next line and call `Agent_MessageReceive`, which will cause the test to fail. + +The desired behaviour is: + +1. Failing an assertion causes the function under test to stop running. +2. GoogleTest still checks other assertions (such as `Agent_MessageReceive_fake.call_count equals zero`). + +The method in use at the moment is to: + +1. Define an `ASSERTION_FAILURE` error code and a custom fake for `vAssertCalled`. +2. Define a custom fake for `vAssertCalled` +```cpp +#define ASSERTION_FAILURE 1 + +/* Mocks for vAssertCalled */ +void throw_assertion_failure ( const char * pcFile, + unsigned long ulLine ) { + /* + Behaviour wanted: + - Encounters assertion fail, stops running any more code. E.g. does not go to next line. + - But checks all assertions in the google test program hold. + */ + throw (ASSERTION_FAILURE); +} +``` +3. Assign the `vAssertCalled` custom fake for every test (by default). Within the initialisation for `TestFreertosCommandPool()`: +```cpp + vAssertCalled_fake.custom_fake = throw_assertion_failure; +``` +4. Within tests expected to fail an assertion, catch the assertion. +```cpp +TEST_F(TestFreertosCommandPool, does_not_try_to_get_command_if_pool_not_initialized) { + Agent_MessageReceive_fake.return_val = true; + Agent_MessageSend_fake.return_val = true; + + // We expect MessageReceive to never be called on unsafe memory. + try{ + Agent_GetCommand(20); + } catch (int num) { + if (num != ASSERTION_FAILURE) { + throw (num); + } + } + expect_errors(); + EXPECT_EQ(Agent_MessageReceive_fake.call_count, 0); +} +``` + +The above example is from the file [`test_freertos_command_pool.cpp`](../components/aws_iot/coremqtt_agent/integration/tests/test_freertos_command_pool.cpp). ### Integrating unit test file with CMake