Integ tests/more cases of mqtt restore session #1083
Conversation
| /* Terminate TLS session and TCP connection network connection to discard current MQTT session | ||
| * that was created as a "clean session". */ | ||
| ( void ) Openssl_Disconnect( &networkContext ); | ||
|
|
||
| /* Establish a new MQTT connection over TLS with the broker with the "clean session" flag set to 0 | ||
| * to start a persistent session with the broker. */ | ||
|
|
||
| /* Create the TLS+TCP connection with the broker. */ | ||
| TEST_ASSERT_EQUAL( OPENSSL_SUCCESS, Openssl_Connect( &networkContext, | ||
| &serverInfo, | ||
| &opensslCredentials, | ||
| TRANSPORT_SEND_RECV_TIMEOUT_MS, | ||
| TRANSPORT_SEND_RECV_TIMEOUT_MS ) ); | ||
| TEST_ASSERT_NOT_EQUAL( -1, networkContext.socketDescriptor ); | ||
| TEST_ASSERT_NOT_NULL( networkContext.pSsl ); | ||
|
|
||
| /* Establish a new MQTT connection for starting a persistent session with the broker | ||
| * by setting the "clean session" flag to 0. */ | ||
| establishMqttSession( &context, &networkContext, false, &persistentSession ); | ||
| TEST_ASSERT_FALSE( persistentSession ); |
There was a problem hiding this comment.
I don't think this part is necessary. Resending a publish is from the client side, and I'd expect it to be the same whether or not the session actually was resumed by the broker. Also, since this is QoS 1, the broker is not going to have any records of this publish to begin with, regardless if it's a clean or dirty session.
There was a problem hiding this comment.
Yeah, it's not necessary for the duplicate packet to be sent only for unacked PUBLISH packet in a persistent session. Though, I am not sure how the state transition would behave when a PUBLISH packet is resent even though the first attempt had succeeded.
| * Tests that the library is able to support resending the PUBLISH packet with the DUP flag | ||
| * set in the restored session. | ||
| */ | ||
| void test_MQTT_Restore_Session_Resend_Unacked_Publish_QoS1( void ) |
There was a problem hiding this comment.
I think this test should be called something more along the lines of test_MQTT_PublishToResend_QoS1, since MQTT_PublishToResend is really the only part of the library we're testing; the actual resending is done by the application
There was a problem hiding this comment.
We are also testing that the state transitions of the records are occurring correctly when resending the PUBLISH packet ID
There was a problem hiding this comment.
Re-thinking about it, I agree with your point of not tying resending of unacked PUBLISH packet with persistent session in the test name. Have changed the name of both PUBLISH resend tests
| @@ -384,7 +400,8 @@ static void eventCallback( MQTTContext_t * pContext, | |||
| assert( pContext != NULL ); | |||
| assert( pPacketInfo != NULL ); | |||
|
|
|||
| if( pPacketInfo->type == disconnectOnPacketType ) | |||
| if( ( pPacketInfo->type == disconnectOnPacketType ) || | |||
There was a problem hiding this comment.
Nit: You could just have if( ( pPacketInfo->type & 0xF0U ) == ( disconnectOnPacketType & 0xF0U ) ) if you want a shorter condition
Co-authored-by: Muneeb Ahmed <[email protected]>
…re-cases-of-mqtt-restore-session
Co-authored-by: Muneeb Ahmed <[email protected]>
|
/cbmc run checks |
Add integration tests for: 1) Resending unacked PUBLISH packets (Qos 1 and QoS 2) 2) Receiving duplicate PUBLISH packets (QoS 1 and QoS 2) from the broker in restored session
Add integration tests for: 1) Resending unacked PUBLISH packets (Qos 1 and QoS 2) 2) Receiving duplicate PUBLISH packets (QoS 1 and QoS 2) from the broker in restored session
Add integration tests for: 1) Resending unacked PUBLISH packets (Qos 1 and QoS 2) 2) Receiving duplicate PUBLISH packets (QoS 1 and QoS 2) from the broker in restored session
Add integration tests for: 1) Resending unacked PUBLISH packets (Qos 1 and QoS 2) 2) Receiving duplicate PUBLISH packets (QoS 1 and QoS 2) from the broker in restored session
Add integration tests for cases #1 and #3 of MQTT session restoration feature that were pointed in #1078 (comment)
Description of changes:
Add tests for the following cases:
MQTT_PublishToResendfunction to resend packetsBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.