Add CI tests for fake platform #12870
Merged
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
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
hawk248,
holbrookt,
jelderton,
jepenven-silabs,
jmartinez-silabs,
LuDuda,
mlepage-google,
mrjerryjohns,
msandstedt,
mspang and
pan-apple
I think it is platform independent. All its implementation functions are just returning unsupported. |
This will add CI cost and time. Can we get this same value using Linux? |
|
PR #12870: Size comparison from 586aca7 to f430c24 Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
Not sure about the potential of the fake platform. Its implementation is much more pure and simple than Linux. Sometime it can help us analysis whether a unit-tests failure is platform dependent or not. The discussion may be outside the scope of this PR. We already got the code, and lots of scripts are using it, then we should introduce CI to cover it. |
|
The CI fails as expected. https://github.com/project-chip/connectedhomeip/runs/4480120957?check_suite_focus=true |
yunhanw-google
approved these changes
Dec 10, 2021
kghost
force-pushed
the
fake-platform
branch
from
f430c24 to
d03b13c
Compare
|
PR #12870: Size comparison from de0af9a to d03b13c Full report (30 builds for efr32, esp32, k32w, mbed, nrfconnect, p6, qpg, telink)
|
|
@woody-apple I believe fake platform was primarely used to test mdns, so that it provides some abstractions that are not a real underlying system in #8946 Generally it is intended to test that calls to platform code occur as expected.Realistically I believe it is to be considered a 'validate that mdns works'. |
|
In #12887 I also enabled build_examples to be able to build and run tests for both current platform and fake. |
woody-apple
approved these changes
Dec 10, 2021
|
We should pick a better name than fake if it’s for mdns, I’m still curious why Linux doesn’t cover this… |
xylophone21
approved these changes
Dec 11, 2021
|
PR #12870: Size comparison from 20d5ede to eefbc91 Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
One difference is that it uses the config defaults, from which every actual platform diverges. This caught a mistake in a test I added last week. |
kghost
force-pushed
the
fake-platform
branch
2 times, most recently
from
ced4419 to
ec0ecb1
Compare
|
PR #12870: Size comparison from 41700bd to ec0ecb1 Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
kghost
force-pushed
the
fake-platform
branch
from
ec0ecb1 to
9f1c6f8
Compare
|
PR #12870: Size comparison from bf7feea to 9f1c6f8 Full report (29 builds for efr32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
Apologies all - missed this before I went on vacation. Fake platform is for unit testing - it's a stub platform that can catch and check data that's sent to the platform layer. Right now the only test implemented is mdns, but it's not mdns specific. It is used to test that the data / commands etc. sent to the platform layer are correct. |
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.
Problem
Fake platform is not verified by GitHub CI.
Change overview
Run CI tests for fake platform on GitHub Action
Testing
Verified using unit-tests