Increase unit test code coverage of controller/ by 2.3%
#39941
+188
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Test that the initial value of the last received sequence number is zero. - Test that the initial value of the newest unacknowledged sent sequence number is zero. - Test that sending a packet updates the unacknowledged sequence number correctly and that receiving an acknowledgment updates the state as expected.
There was a problem hiding this comment.
Code Review
This pull request increases unit test coverage for AutoCommissioner and BtpEngine by adding several new test cases. The approach of using a test accessor class to test private functionality is sound. My review focuses on improving code clarity, consistency with the project's coding style, and fixing minor typos. I've provided specific suggestions for these improvements.
5dd2c20 to
af8316a
Compare
|
PR #39941: Size comparison from 1446be8 to 2869de6 Full report (9 builds for cc13x4_26x4, cc32xx, stm32, tizen)
|
|
PR #39941: Size comparison from 1446be8 to e38318a Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
approved these changes
Jul 10, 2025
heinsaar
approved these changes
Jul 11, 2025
|
PR #39941: Size comparison from eae1a24 to e97f8fc Full report (59 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
juandediosg
reviewed
Jul 11, 2025
|
|
||
| namespace Test { | ||
|
|
||
| class AutoCommissionerTestAccess |
There was a problem hiding this comment.
Maybe consider adding a short comment.
| EXPECT_EQ(stage, kCleanup); | ||
| } | ||
|
|
||
| TEST_F(AutoCommissionerTest, NextStageReadCommissioningInfo) < F363 /td> |
There was a problem hiding this comment.
Maybe consider adding a short comment.
| EXPECT_EQ(nextStageReadCommissioningInfo, nextStageSendNOC); | ||
| } | ||
|
|
||
| TEST_F(AutoCommissionerTest, NextStageConfigureTCAcknowledgments) |
There was a problem hiding this comment.
Maybe consider adding a short comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds five new test cases to
TestAutoCommissioner.cppto improve test coverage of the commissioning stage progression in theGetNextCommissioningStageInternalfunction.The new tests verify the following scenarios:
NextCommissioningStage: Ensures the linear and expected transition between stages.NextStageStopCommissioning: Ensures that if commissioning is manually stopped, the next stage correctly transitions tokCleanup.NextCommissioningStageAfterError: Ensures that if commissioning fails due to an error, the next stage correctly transitions tokCleanup.NextStageReadCommissioningInfo: Checks that the stage transitions correctly based on the state ofDeviceCommissioningInfoand UTC requirements.NextStageConfigureTCAcknowledgments: Verifies the stage transition is correct depending on theDeviceCommissioningInfo,CommissioningParametersconfiguration andTimeZonearguments.Implementation Notes
Testing of the private
GetNextCommissioningStageInternalfunction is accomplished via aAutoCommissionerTestAccessclass, in accordance with the testing guidelines defined in the Connected Home IP unit testing documentation.Related issues
Fixes: #37233
Testing
This PR only adds new unit tests. The sole change to production code is the addition of a
friendfunction to grant the tests access.Coverage Impact of
AutoCommissioner.cpp/controllerfolder