Updated AppContext, LoopbackMessagingContext, and UDPMessagingContext to serve as PW test fixture classes. #34036
Conversation
This file contains 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
… reflect PW migration.
…ved LoopbackMessagingContext data to heap.
github-actions
bot
added
documentation
feasel0
changed the title
feasel0
changed the title
|
PR #34036: Size comparison from 3f59179 to d43b087 Full report (27 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, mbed, nxp, qpg, stm32, tizen)
|
|
PR #34036: Size comparison from 3f59179 to a02c9d7 Full report (62 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, esp32, linux, mbed, nxp, qpg, stm32, telink, tizen)
|
jlatusek
suggested changes
Jun 21, 2024
|
PR #34036: Size comparison from 2af3379 to 4fefac4 Full report (79 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, linux, mbed, nxp, psoc6, qpg, stm32, telink, tizen)
|
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille and
chrisdecenzo
|
PR #34036: Size comparison from e5acdc9 to 789d6c0 Full report (49 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
|
PR #34036: Size comparison from f8a633e to f61c712 Full report (20 builds for cc13x4_26x4, cc32xx, mbed, nrfconnect, nxp, qpg, stm32, tizen)
|
|
PR #34036: Size comparison from f8a633e to 3d37fc4 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34036: Size comparison from f199ba4 to f4f25fc Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34036: Size comparison from 35eba86 to a85954e Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
feasel0
changed the title
andy31415
reviewed
Jul 5, 2024
andy31415
reviewed
Jul 5, 2024
andy31415
reviewed
Jul 5, 2024
andy31415
approved these changes
Jul 5, 2024
…easel0/connectedhomeip into feature/appcontext-refactoring
|
PR #34036: Size comparison from 35eba86 to 71f1caf Full report (8 builds for cc32xx, mbed, qpg, stm32, tizen)
|
|
PR #34036: Size comparison from 35eba86 to 7c35ed2 Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jlatusek
approved these changes
Jul 9, 2024
lazarkov
pushed a commit
to lazarkov/connectedhomeip
that referenced
this pull request
Jul 10, 2024
… to serve as PW test fixture classes. (project-chip#34036) * Modified MessagingContext and its subclasses as well as AppContext to reflect PW migration. * Modifed all tests that use LoopbackMessagingContext or AppContext. Moved LoopbackMessagingContext data to heap. * Removed unneeded scope from call to parent setup/teardown * Made some helper functions into class methods. * Restyled by prettier-markdown * Reverted TestICDManager back to using LoopbackMessagingContext * fixing merge conflicts * fixing merge conflicts * Fixed duplicate code and missing semicolon * Trying to update TestReadHandler_DataVersionFiltersTruncated * Restyled by whitespace * Restyled by clang-format * fixed problem with the new TestReadHandler_DataVersionFiltersTruncated * fix in tracing * Changes to MessagingContext member variable initialization in response to code review * Used std::unique_ptr for MessagingContext.mpData --------- Co-authored-by: Restyled.io <commits@restyled.io>
austina-csa
pushed a commit
to austina-csa/connectedhomeip
that referenced
this pull request
Jul 10, 2024
… to serve as PW test fixture classes. (project-chip#34036) * Modified MessagingContext and its subclasses as well as AppContext to reflect PW migration. * Modifed all tests that use LoopbackMessagingContext or AppContext. Moved LoopbackMessagingContext data to heap. * Removed unneeded scope from call to parent setup/teardown * Made some helper functions into class methods. * Restyled by prettier-markdown * Reverted TestICDManager back to using LoopbackMessagingContext * fixing merge conflicts * fixing merge conflicts * Fixed duplicate code and missing semicolon * Trying to update TestReadHandler_DataVersionFiltersTruncated * Restyled by whitespace * Restyled by clang-format * fixed problem with the new TestReadHandler_DataVersionFiltersTruncated * fix in tracing * Changes to MessagingContext member variable initialization in response to code review * Used std::unique_ptr for MessagingContext.mpData --------- Co-authored-by: Restyled.io <commits@restyled.io>
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.
AppContext, LoopbackMessagingContext, and UDPMessagingContext have member functions that resemble the setup/teardown functions of pigweed (SetUp, TearDown, SetUpTestSuite, TearDownTestSuite), however we are not currently using these classes as actual pigweed test fixture classes. Instead each unit test is creating a test fixture class that contains a pointer to AppContext (or LoopbackMessagingContext, or UDPMessagingContext), and then it dynamically creates the object when the test runs, and deletes it when it's finished. This leads to a lot of redundant code, since each unit test has to do this.
This PR modifies LoopbackMessagingContext and UDPMessagingContext so that they inherit from
::testing::Test. Since AppContext inherits from LoopbackMessagingContext, it too can be used as a pigweed test fixture class.This PR also modifies all the unit tests that are dynamically creating AppContext, LoopbackMessagingContext, or UDPMessagingContext objects so that they instead inherit from the class directly. This leads to a significant reduction of code, and in many cases the unit test doesn't even need to define a fixture class at all (it can just do
using TestSomething = AppContext;, or whichever context class it's using).To address the issue of the memory footprint, this PR also modifies MessagingContext and LoopbackMessagingContext so that they store their member variables on the heap, since this was a concern that may have been preventing us from adopting this method earlier.
Here is a summary of what these classes do.
::testing::Testis what each pigweed unit test (with a fixture) must inherit. The goal is to move this inheritance somewhere else in order to simplify the unit tests.Right now we have 25 test sources that use AppContext (in
src/app/testsandsrc/controller/tests), we have 8 test sources that use LoopbackMessagingContext (insrc/messaging/testsandsrc/protocols/secure_channel/tests), and we have 1 test source that uses UDPMessagingContext (insrc/messaging/tests).