Skip to content

Commit

Permalink
Update code to resolve lcov issues and mocks failing on Mac (#307)
Browse files Browse the repository at this point in the history
<!--- Title -->

Description
-----------
<!--- Describe your changes in detail. -->
**Issue1:** 
Mocked function calls jump to real implementation instead of mocks on
mac
**Solution:**
Convert the symbols of the mocked implementation from weak to strong

**Issue2:**
lcov generating wrong coverage reports on mac
**Solution:**
Rectifying network buffer size to be of the appropriate value otherwise
memset clears out more memory spaces than required which leads to
reseting the line coverage counters.

**Issue3:**
log statements being taken as branches
**Solution:**
Removing the ternary operation to figure out if there is a '/' in front
of the file name or not. This can be done as this feature is just for
debugging purposes.

**Issue4:**
Some tests failing randomly on mac
**Solution:**
Initialise variables at places where necessary 

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

---------

Co-authored-by: Dakshit Babbar <dakshba@amazon.com>
  • Loading branch information
DakshitBabbar and Dakshit Babbar authored Sep 17, 2024
1 parent c0c05f9 commit afe000c
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 36 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ or the following:
```
cmake -S test -B build/ \
-G "Unix Makefiles" \
-DCMAKE_BUILD_TYPE=Debug \
-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'
Expand Down
8 changes: 0 additions & 8 deletions source/core_mqtt.c
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,6 @@ static MQTTPubAckType_t getAckFromPacketType( uint8_t packetType )
ackType = MQTTPubrel;
break;

case MQTT_PACKET_TYPE_PUBCOMP:
default:

/* This function is only called after checking the type is one of
Expand Down Expand Up @@ -1245,13 +1244,6 @@ static uint8_t getAckTypeToSend( MQTTPublishState_t state )
packetTypeByte = MQTT_PACKET_TYPE_PUBCOMP;
break;

case MQTTPubAckPending:
case MQTTPubCompPending:
case MQTTPubRecPending:
case MQTTPubRelPending:
case MQTTPublishDone:
case MQTTPublishSend:
case MQTTStateNull:
default:
/* Take no action for states that do not require sending an ack. */
break;
Expand Down
24 changes: 9 additions & 15 deletions source/core_mqtt_state.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ static bool validateTransitionPublish( MQTTPublishState_t currentState,
isValid = newState == MQTTPubRecPending;
break;

case MQTTQoS0:
default:
/* QoS 0 is checked before calling this function. */
break;
Expand All @@ -290,13 +289,6 @@ static bool validateTransitionPublish( MQTTPublishState_t currentState,

break;

case MQTTPubAckSend:
case MQTTPubCompPending:
case MQTTPubCompSend:
case MQTTPubRecSend:
case MQTTPubRelPending:
case MQTTPubRelSend:
case MQTTPublishDone:
default:
/* For a PUBLISH, we should not start from any other state. */
break;
Expand Down Expand Up @@ -400,14 +392,16 @@ static bool validateTransitionAck( MQTTPublishState_t currentState,
( newState == MQTTPubCompPending );
break;

case MQTTPublishDone:
/* Done state should transition to invalid since it will be removed from the record. */
case MQTTPublishSend:
/* If an ack was sent/received we shouldn't have been in this state. */
case MQTTStateNull:
/* If an ack was sent/received the record should exist. */
default:
/* Invalid. */

/* 1. MQTTPublishDone - state should transition to invalid since it
* will be removed from the record.
* 2. MQTTPublishSend - If an ack was sent/received we shouldn't
* have been in this state.
* 3. MQTTStateNull - If an ack was sent/received the record should
* exist.
* 4. Any other state is invalid.
*/
break;
}

Expand Down
6 changes: 2 additions & 4 deletions test/unit-test/core_mqtt_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,7 @@ void test_MQTT_Connect_happy_path2()
MQTTContext_t mqttContext = { 0 };
MQTTConnectInfo_t connectInfo = { 0 };
uint32_t timeout = 2;
bool sessionPresent;
bool sessionPresent = false;
MQTTStatus_t status;
TransportInterface_t transport = { 0 };
MQTTFixedBuffer_t networkBuffer = { 0 };
Expand Down Expand Up @@ -2051,7 +2051,7 @@ void test_MQTT_Connect_happy_path3()
MQTTConnectInfo_t connectInfo = { 0 };
MQTTPublishInfo_t willInfo = { 0 };
uint32_t timeout = 2;
bool sessionPresent;
bool sessionPresent = false;
MQTTStatus_t status;
TransportInterface_t transport = { 0 };
MQTTFixedBuffer_t networkBuffer = { 0 };
Expand Down Expand Up @@ -2126,7 +2126,6 @@ void test_MQTT_Connect_happy_path4()
mqttContext.outgoingPublishRecords[ 0 ].qos = MQTTQoS2;
mqttContext.outgoingPublishRecords[ 0 ].publishState = MQTTPublishSend;
mqttContext.incomingPublishRecords[ MQTT_STATE_ARRAY_MAX_COUNT - 1 ].packetId = 1;
mqttContext.networkBuffer.size = 5000;

incomingPacket.type = MQTT_PACKET_TYPE_CONNACK;
incomingPacket.remainingLength = 2;
Expand Down Expand Up @@ -4358,7 +4357,6 @@ void test_MQTT_ProcessLoop_Timer_Overflow( void )
setupTransportInterface( &transport );
setupNetworkBuffer( &networkBuffer );

networkBuffer.size = 1000;
incomingPacket.type = MQTT_PACKET_TYPE_PUBLISH;
incomingPacket.remainingLength = MQTT_SAMPLE_REMAINING_LENGTH;

Expand Down
8 changes: 1 addition & 7 deletions test/unit-test/logging/logging_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,9 @@
#error "Please define LIBRARY_LOG_NAME for the library."
#endif

/**
* @brief Macro to extract only the file name from file path to use for metadata in
* log messages.
*/
#define FILENAME ( strrchr( __FILE__, '/' ) ? strrchr( __FILE__, '/' ) + 1 : __FILE__ )

/* Metadata information to prepend to every log message. */
#define LOG_METADATA_FORMAT "[%s] [%s:%d] " /**< @brief Format of metadata prefix in log messages as `[<Logging-Level>] [<Library-Name>] [<File-Name>:<Line-Number>]` */
#define LOG_METADATA_ARGS LIBRARY_LOG_NAME, FILENAME, __LINE__ /**< @brief Arguments into the metadata logging prefix format. */
#define LOG_METADATA_ARGS LIBRARY_LOG_NAME, __FILE__, __LINE__ /**< @brief Arguments into the metadata logging prefix format. */

#if !defined( DISABLE_LOGGING )

Expand Down
1 change: 0 additions & 1 deletion tools/cmock/project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,4 @@
:includes_c_post_header:
- <cmock_opaque_types.h>
:treat_externs: :exclude # Now the extern-ed functions will be mocked.
:weak: __attribute__((weak))
:treat_externs: :include

0 comments on commit afe000c

Please sign in to comment.