Update unit tests to work with asserts enabled (#322)

Description
-----------
This PR solves the Issue #321. 

**Learning:** Unit tests are supposed to be built with asserts enabled
during the development process. But when these are built for coverage
analysis these should be built with asserts disabled. This is because
the lcov tool would consider all the branches in the asserts as
uncovered.

This PR:
1. Update the unit tests to work with asserts enabled
2. Update the code to have 100% branch coverage. To achieve this, one
branch which was logically unreachable, was excluded from the coverage
analysis.
3. Updates the CI checks to build the code with asserts to check if all
the unit tests are working as expected. Along with this it will also
build the code without asserts to check coverage.
4. Updates the command in the README.md that users should use to build
the unit tests. This command has asserts disabled, as asserts are only
required during the development process of the unit tests.

Test Steps
-----------
The unit tests are building and running successfully with asserts. They
also give 100% coverage without asserts.

Checklist:
----------
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have tested my changes. No regression in existing tests.
- [x] I have modified and/or added unit-tests to cover the code changes
in this Pull Request.

Related Issue
-----------
#321
<!-- If any, please provide issue ID. -->
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

Author: DakshitBabbar

.github/.cSpellWords.txt

@@ -52,3 +52,4 @@ Werror
 Wextra
 Wsign
 Wunused
+DCOV

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ jobs:
         uses: actions/checkout@v3
 
       - env:
-          stepName: Build CoreMQTT
+          stepName: Build CoreMQTT with asserts
         run: |
           # ${{ env.stepName }}
           echo -e "::group::${{ env.bashInfo }} ${{ env.stepName }} ${{ env.bashEnd }}"
@@ -32,7 +32,29 @@ jobs:
           -DCMAKE_BUILD_TYPE=Debug \
           -DBUILD_CLONE_SUBMODULES=ON \
           -DUNITTEST=1 \
-          -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
+          -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
+          make -C build/ all
+          echo "::endgroup::"
+
+          echo -e "${{ env.bashPass }} ${{env.stepName}} ${{ env.bashEnd }}"
+
+      - name: Run System Tests with asserts
+        run:  ctest --test-dir build -E system --output-on-failure
+
+      - env:
+          stepName: Build CoreMQTT without asserts for coverage analysis
+        run: |
+          # ${{ env.stepName }}
+          echo -e "::group::${{ env.bashInfo }} ${{ env.stepName }} ${{ env.bashEnd }}"
+
+          sudo apt-get install -y lcov
+          cmake -S test -B build/ \
+          -G "Unix Makefiles" \
+          -DCMAKE_BUILD_TYPE=Debug \
+          -DBUILD_CLONE_SUBMODULES=ON \
+          -DUNITTEST=1 \
+          -DCOV_ANALYSIS=1 \
+          -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
           make -C build/ all
           echo "::endgroup::"
 

README.md

@@ -176,7 +176,8 @@ git submodule update --checkout --init --recursive test/unit-test/CMock
               -DCMAKE_BUILD_TYPE=Debug  \
               -DBUILD_CLONE_SUBMODULES=ON \
               -DUNITTEST=1 \
-              -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
+              -DCOV_ANALYSIS=1 \
+              -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
     ```
     For Mac machines:
 
@@ -186,7 +187,8 @@ git submodule update --checkout --init --recursive test/unit-test/CMock
               -DCMAKE_BUILD_TYPE=RelWithDebInfo  \
               -DBUILD_CLONE_SUBMODULES=ON \
               -DUNITTEST=1 \
-              -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DNDEBUG -DLIBRARY_LOG_LEVEL=LOG_DEBUG' \
+              -DCOV_ANALYSIS=1 \
+              -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DLIBRARY_LOG_LEVEL=LOG_DEBUG' \
               -DCMAKE_C_STANDARD=99
     ```
 

source/core_mqtt.c

@@ -869,9 +869,14 @@ static int32_t sendMessageVector( MQTTContext_t * pContext,
         }
 
         /* Some of the bytes from this vector were sent as well, update the length
-         * and the pointer to data in this vector. */
+         * and the pointer to data in this vector. One branch in the following
+         * condition logically cannot be reached as the iterator would always be
+         * bounded if the sendResult is positive. If it were not then the assert
+         * above in the function will be triggered and the flow will never reach
+         * here. Hence for that sake the branches on this condition are excluded
+         * from coverage analysis */
         if( ( sendResult > 0 ) &&
-            ( pIoVectIterator <= &( pIoVec[ ioVecCount - 1U ] ) ) )
+            ( pIoVectIterator <= &( pIoVec[ ioVecCount - 1U ] ) ) ) /* LCOV_EXCL_BR_LINE */
         {
             pIoVectIterator->iov_base = ( const void * ) &( ( ( const uint8_t * ) pIoVectIterator->iov_base )[ sendResult ] );
             pIoVectIterator->iov_len -= ( size_t ) sendResult;

test/unit-test/core_mqtt_utest.c

@@ -1555,7 +1555,7 @@ void test_MQTT_Connect_sendConnect1( void )
     size_t packetSize;
 
     setupTransportInterface( &transport );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
     setupNetworkBuffer( &networkBuffer );
 
     memset( &mqttContext, 0x0, sizeof( mqttContext ) );
@@ -1674,7 +1674,7 @@ void test_MQTT_Connect_ProperWIllInfo( void )
     MQTTPublishInfo_t willInfo;
 
     setupTransportInterface( &transport );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
     setupNetworkBuffer( &networkBuffer );
 
     memset( &mqttContext, 0x0, sizeof( mqttContext ) );
@@ -1730,7 +1730,7 @@ void test_MQTT_Connect_ProperWIllInfoWithNoPayload( void )
     MQTTPublishInfo_t willInfo;
 
     setupTransportInterface( &transport );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
     setupNetworkBuffer( &networkBuffer );
 
     memset( &mqttContext, 0x0, sizeof( mqttContext ) );
@@ -1785,7 +1785,7 @@ void test_MQTT_Connect_ProperWIllInfoWithPayloadButZeroLength( void )
     MQTTPublishInfo_t willInfo;
 
     setupTransportInterface( &transport );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
     setupNetworkBuffer( &networkBuffer );
 
     memset( &mqttContext, 0x0, sizeof( mqttContext ) );
@@ -3476,7 +3476,8 @@ void test_MQTT_Publish7( void )
 
     setupTransportInterface( &transport );
     setupNetworkBuffer( &networkBuffer );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
+    transport.send = transportSendFailure;
 
     memset( &mqttContext, 0x0, sizeof( mqttContext ) );
     memset( &publishInfo, 0x0, sizeof( publishInfo ) );
@@ -3508,7 +3509,7 @@ void test_MQTT_Publish8( void )
 
     setupTransportInterface( &transport );
     setupNetworkBuffer( &networkBuffer );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
 
     memset( &mqttContext, 0x0, sizeof( mqttContext ) );
     memset( &publishInfo, 0x0, sizeof( publishInfo ) );
@@ -3794,7 +3795,7 @@ void test_MQTT_Publish_Send_Timeout( void )
 
     setupNetworkBuffer( &networkBuffer );
     setupTransportInterface( &transport );
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
 
     /* Set the transport send function to the mock that always returns zero
      * bytes for the test. */
@@ -5359,8 +5360,9 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths3( void )
     context.waitingForPingResp = false;
     /* Set expected return values in the loop. */
     resetProcessLoopParams( &expectParams );
-
+    size_t packetSize = MQTT_PACKET_PINGREQ_SIZE;
     MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess );
+    MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &packetSize );
     MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess );
 
     mqttStatus = MQTT_ProcessLoop( &context );
@@ -5417,8 +5419,9 @@ void test_MQTT_ProcessLoop_handleKeepAlive_Error_Paths4( void )
 
     MQTT_ProcessIncomingPacketTypeAndLength_ExpectAnyArgsAndReturn( MQTTNeedMoreBytes );
     MQTT_ProcessIncomingPacketTypeAndLength_ReturnThruPtr_pIncomingPacket( &incomingPacket );
-
+    size_t packetSize = MQTT_PACKET_PINGREQ_SIZE;
     MQTT_GetPingreqPacketSize_ExpectAnyArgsAndReturn( MQTTSuccess );
+    MQTT_GetPingreqPacketSize_ReturnThruPtr_pPacketSize( &packetSize );
     MQTT_SerializePingreq_ExpectAnyArgsAndReturn( MQTTSuccess );
 
     mqttStatus = MQTT_ProcessLoop( &context );
@@ -5975,7 +5978,7 @@ void test_MQTT_Subscribe_error_paths1( void )
     subscribeInfo.qos = MQTTQoS0;
     setupTransportInterface( &transport );
     transport.send = transportSendFailure;
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
 
     /* Initialize context. */
     mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );
@@ -6450,7 +6453,7 @@ void test_MQTT_Unsubscribe_error_path1( void )
 
     transport.send = transportSendFailure;
     transport.recv = transportRecvFailure;
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
 
     /* Initialize context. */
     mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );
@@ -6492,7 +6495,7 @@ void test_MQTT_Unsubscribe_error_path2( void )
     setupTransportInterface( &transport );
     transport.send = transportSendFailure;
     transport.recv = transportRecvFailure;
-    transport.writev = transportWritevFail;
+    transport.writev = NULL;
 
     /* Initialize context. */
     mqttStatus = MQTT_Init( &context, &transport, getTime, eventCallback, &networkBuffer );