-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add boilerplate unit test code to currently untested modules #14993
Conversation
@hazzlim, thank you for your changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, only one question/comment.
55469f1
to
0a0acb1
Compare
0a0acb1
to
5d2ea38
Compare
Unit tests for drivers are currently not checked for style compliance. We should be checking coding style in unit tests, so we remove them from the .codecheckignore file.
Currently there are no stub implementations of the analogin_api.c functions. As the AnalogIn class makes use of these functions, these stub definitions are required in order to build AnalogIn.cpp and generate .gcno files to be used to generate code coverage metrics.
In pursuit of increasing unit test coverage of Mbed OS, we add the boilerplate code for unit testing of AnalogIn.cpp and an empty test source file. This serves two purposes - it allows us to report on currently untested sources in code coverage data, and it makes it a little easier for developers to write unit tests for these sources. The 'empty' test file contains a main function that simply returns. This is required to allow CMake's add_executable() to be used to pull in the source file for the SUT, which ensures that this source file is built and therefore instrumented to generate coverage data. The alternative that was explored was to instead use Google Test's TEST() macro and prefix the test name with 'DISABLED_' to skip it, but that resulted in the test being reported as skipped, which was deemed undesireable for these 'empty' tests.
5d2ea38
to
c6312a3
Compare
CI started |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -35,4 +35,3 @@ | |||
^UNITTESTS | |||
^storage/blockdevice/tests/UNITTESTS | |||
^storage/kvstore/tests/UNITTESTS | |||
^drivers/tests/UNITTESTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an observation: There seem to be no style changes needed in this commit. The style checking CI is passing. If there were files out of style in drivers/tests/UNITTESTS, this commit would have needed fixes to get those files back in style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jaeden, noted. I did run astyle on them but they turned out to all be fine, luckily.
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Summary of changes
In pursuit of increasing unit test coverage of MbedOS, we add the
boilerplate code for unit testing of AnalogIn.cpp and an empty test
source file. This serves two purposes - it allows us to report on
currently untested sources in code coverage data, and it makes it a
little easier for developers to write unit tests for these sources.
The 'empty' test file contains a main function that simply returns. This
is required to allow CMake's add_executable() to be used to pull in the
source file for the SUT, which ensures that this source file is built
and therefore instrumented to generate coverage data. The alternative
that was explored was to instead use Google Test's TEST() macro and
prefix the test name with 'DISABLED_' to skip it, but that resulted in
the test being reported as skipped, which was deemed undesireable for
these 'empty' tests.
Additionally, stubs for analogin_api.c functions are added to the Mbed-stubs-hal library, to allow AnalogIn.cpp to build & to be used by the future unit tests.
Impact of changes
Migration actions required
None.
Documentation
None.
Pull request type
Test results
Reviewers
@Patater @rwalton-arm @ARMmbed/mbed-os-core