Upgrade build_coverage.sh script with new functionality
#39836
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
c21b609 to
95f7ae4
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces new flags for quiet mode and coverage accumulation, support for multiple test targets, and a path correction to the build_coverage.sh script. There's a potential issue with how multiple test targets are handled and a critical issue with the updated lcov command that could break coverage generation.
0bf86bf to
4c63d14
Compare
|
PR #39836: Size comparison from e4c8fb6 to 4c63d14 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
4c63d14 to
d7ccf98
Compare
|
PR #39836: Size comparison from 1f94f79 to 859e7bb Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces new functionalities to the build_coverage.sh script, including a quiet flag, coverage data accumulation, and support for multiple targets. I've suggested improvements related to handling empty target strings and ensuring the script's portability by checking for the realpath command before using it.
859e7bb to
6874fb7
Compare
|
PR #39836: Size comparison from ce90b61 to 6874fb7 Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
heinsaar
approved these changes
Jul 7, 2025
|
PR #39836: Size comparison from ce90b61 to bd04981 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the build_coverage.sh script by adding several new features for local development: a quiet flag, a flag to control coverage data accumulation, and support for running multiple test targets at once. It also fixes an issue with the reported path to the coverage report. The changes are well-implemented and improve the script's usability. I have one suggestion to improve the robustness of the script by using the established $CHIP_ROOT variable instead of $PWD for exclude paths, making it independent of the directory it's run from.
juandediosg
approved these changes
Jul 7, 2025
|
PR #39836: Size comparison from ce90b61 to c1a32e4 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
86e4ce8 to
d33ef3e
Compare
|
PR #39836: Size comparison from 1e27099 to d33ef3e Full report (28 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, telink, tizen)
|
d33ef3e to
3bd7cd5
Compare
|
PR #39836: Size comparison from 961f1c3 to 3bd7cd5 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
This will suppresses the details of the script which are too long.
This was actually breaking the CI pipeline
To make sure no new dependency is used.
3bd7cd5 to
3ef8170
Compare
|
PR #39836: Size comparison from 050f895 to 3ef8170 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
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 three functionalities to
build_coverage.shscript and a small fixAll these changes have apparent effect only when using the script locally.
-qor--quiet)This will decrease verbosity level, omitting long outputs.
-aor--accumulate)This is a fix to previous incorrect behaviour occurring when running the script more than one time, the coverage results were accumulated over multiple runs. Though this doesn't affect the coverage percentage reported by the script, but because of this, for example, it was impossible to observe the effect of removing unit tests. Now this accumulating behaviour is by default disabled, and with the
-aflag it is possible to add current data to the data of the previous run if needed.Now it is possible to specify multiple targets separated by space.
Related issues
N/A
Testing
This PR will not have any effect when actually generating the online report, and it doesn't change any production code.