Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Collect test names in lcov reports #84106

Merged
merged 4 commits into from
Feb 7, 2025
Merged

Collect test names in lcov reports #84106

merged 4 commits into from
Feb 7, 2025

Conversation

jeremybettis
Copy link
Contributor

@jeremybettis jeremybettis commented Jan 16, 2025 •
edited
Loading

If the flag --coverage-by-instance is passed to twister, pass the test name to lcov so that the coverage reports will include test details.

Here is an example without the test names populated:
42AEQv8ZynYtoK7

And with:
w4t2fuyT64kWDRy

@jeremybettis
Copy link
Contributor Author

Ran twister on 6448 test cases

With -coverage-by-testsuite

real    102m34.790s
user    4588m59.765s
sys     415m0.437s

Without -coverage-by-testsuite

real    87m56.910s
user    4126m7.868s
sys     320m30.187s

So not a lot faster, but some.

@jeremybettis jeremybettis force-pushed the lcov-per-test branch 2 times, most recently from fd2daac to 7aeff42 Compare January 17, 2025 23:07
@jeremybettis jeremybettis marked this pull request as ready for review January 17, 2025 23:49
@zephyrbot zephyrbot added the area: Twister Twister label Jan 17, 2025
@golowanow
Copy link
Member

@jeremybettis, nice to see you are also going in the same direction as in #66345 (RFC #67058)

From my understanding, this PR's --coverage-by-testsuite mode is similar to --coverage-split part of #66345
but this PR doesn't make summary report for the default gcovr, e.g.

twister -p unit_testing  --coverage  --coverage-by-testsuite -T tests/unit
...
ERROR   - Merge coverage not implemented for <twisterlib.coverage.Gcovr object at 0x7fbf68a75e80>
INFO - Saving reports...
INFO - Run completed

.. This should help with parallelism,

well, when I try it looks opposite for me (~10% slower), e.g.:

time ./scripts/twister -vv -p unit_testing  --coverage --coverage-tool lcov --coverage-by-testsuite -T tests/unit 
	real    1m32.694s   
	user    14m35.421s  
	sys     1m17.638s   

time ./scripts/twister -vv -p unit_testing  --coverage --coverage-tool lcov -T tests/unit
	real    1m19.423s   
	user    11m26.782s  
	sys     1m8.083s

@nashif nashif mentioned this pull request Jan 18, 2025
Merged
@nashif
Copy link
Member

nashif commented Jan 18, 2025

Also, the coverage reports will include test details.

this is nice, didn't know lcov can do that.

hakehuang
hakehuang previously approved these changes Jan 19, 2025
View reviewed changes
@jeremybettis
Copy link
Contributor Author

I'm resolving the merge conflicts, don't look at this PR.

@jeremybettis jeremybettis changed the title Run lcov on each test Collect test names in lcov reports Feb 6, 2025
@jeremybettis
Copy link
Contributor Author

Ok, I've rebased, and I went ahead and split up my change into 4 commits, and they are all very small.

@jeremybettis
twister: Fix --disable-coverage-aggregation help text
c9afcbf 
There is no flag named --coverage-split, switch help to
--coverage-per-instance

Signed-off-by: Jeremy Bettis <jbettis@google.com>
@jeremybettis
twister: Implement coverage file merging for lcov
010e975 
Instead of capturing the coverage data twice with
--coverage-per-instance, just merge the lcov files in the aggregation
report.

Signed-off-by: Jeremy Bettis <jbettis@google.com>
@jeremybettis
twister: Collect test names in lcov
c85b251 
When using --coverage-per-instance, collect the test names in lcov, and
add --show-details to the html report to display those test names.

Signed-off-by: Jeremy Bettis <jbettis@google.com>
@jeremybettis
twister: Check lcov exit codes
565fb1c 
Don't ignore lcov exit codes, but instead log and return if one of the
lcov commands fails.

Signed-off-by: Jeremy Bettis <jbettis@google.com>
golowanow
golowanow reviewed Feb 6, 2025
View reviewed changes
scripts/pylib/twister/twisterlib/coverage.py Show resolved Hide resolved
scripts/pylib/twister/twisterlib/coverage.py
"-output-directory", os.path.join(outdir, "coverage")] + files
"-output-directory", os.path.join(outdir, "coverage")]
if self.coverage_per_instance:
cmd.append("--show-details")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps genhtml also needs --description-file generated with a summary for test names and descriptions to show it in the report.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the tests have descriptions?

@kartben kartben merged commit e88499c into zephyrproject-rtos:main Feb 7, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@golowanow golowanow golowanow left review comments

@nashif nashif nashif approved these changes

@hakehuang hakehuang hakehuang approved these changes

@gchwier gchwier Awaiting requested review from gchwier

@KamilxPaszkiet KamilxPaszkiet Awaiting requested review from KamilxPaszkiet

@LukaszMrugala LukaszMrugala Awaiting requested review from LukaszMrugala

@PerMac PerMac Awaiting requested review from PerMac

Assignees

@nashif nashif

Labels
area: Twister Twister
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeremybettis @golowanow @nashif @hakehuang @kartben @zephyrbot