Make easier getting coverage data for devices #63826

nordic-piks · 2023-10-11T19:48:30Z

The idea is to enable easy coverage data gathering for tests executed on devices.
Most devices do not have coverage support enabled by default, yet they can obtain coverage for some tests.
From now coverage can be enabled like this:
./zephyr/scripts/twister -T xxx --coverage --platform nrf52840dk_nrf52840 --device-testing --device-serial /dev/ttyACM0 ----extra-args CONFIG_FORCE_COVERAGE=y

There is also possible optimization of the coverage data dump process i.e.
not using an additional buffer to collect gcda data, instead print them directly on the console.
This can be achieved by adding:
--extra-args CONFIG_COVERAGE_GCOV_HEAP_SIZE=0
This allows reduction of used RAM, thus more gcov data can fit the memory.

Regardless of used buffer size, there is another optimization added:
reduced size of data sent on console - no space between hex characters.

An error can occur when getting coverage from the console - received data may be corrupted.
In that case, report failures but continue to process valid data.
Include the status of coverage data processing in the twister return code.

EDIT:
Useful example of how to run Twister and get a sample coverage report from a device:
./zephyr/scripts/twister --ninja --inline-logs --outdir twister-build --jobs 1 -T zephyr/samples/hello_world --coverage --platform nrf52840dk_nrf52840 --coverage-tool gcovr --device-testing --device-serial /dev/ttyACM0 --extra-args CONFIG_COVERAGE_GCOV_HEAP_SIZE=0 --extra-args CONFIG_FORCE_COVERAGE=y --extra-args CONFIG_MAIN_STACK_SIZE=4096 -v -v

The result will be at twister-build/coverage/index.html

Additionally, please be sure that there is an env variable ZEPHYR_SDK_INSTALL_DIR which points to Zephyr SDK.

jfischer-no · 2023-10-12T10:44:48Z

boards/shields/coverage_support/Kconfig.shield

+# Copyright (c) 2023 Nordic Semiconductor ASA
+# SPDX-License-Identifier: Apache-2.0
+
+config SHIELD_COVERAGE_SUPPORT
+	def_bool $(shields_list_contains,coverage_support)
+	select HAS_COVERAGE_SUPPORT


You probably want to use snippets, https://docs.zephyrproject.org/latest/build/snippets/index.html

Thanks for the suggestion, I did not know that possibility.
However, when I started to use snippets I found an easier way, just using KCONFIGs directly.
I will stick to that simple solution until more settings do not need to be adjusted to get coverage.

hakehuang · 2023-10-12T13:48:21Z

subsys/testsuite/Kconfig

@@ -72,14 +72,6 @@ config COVERAGE_GCOV

 endchoice

-config COVERAGE_GCOV_HEAP_SIZE


remove this feature totally may not good, as we some time expect to have coverage data dumpped by debugger, without the uart driver. maybe move this to a none-default option?

Ok.
The code still uses a buffer by default. I added the possibility to set the buffer size to 0, which will lead to dumping data directly to the console.
While I was thinking about your comment I found how a debugger dump can be prepared - https://www.thanassis.space/coverage.html (I did not try it but as the buffer is available, that solution should be possible)

this is the way most coverage tools get coverage result back. so thanks for keep this feature

nordic-piks · 2023-10-16T06:04:57Z

I see github action (twister build) - https://github.com/zephyrproject-rtos/zephyr/actions/runs/6517518442/job/17702105148?pr=63826 - hanging after "Set up runner", with no obvious reason. How can I deal with that? Can it be simply rerun?
(I expect all CI will be green, other similar actions were green)

addressed

nordic-piks · 2023-10-19T10:14:32Z

During review I got question about exact command to execute testing with coverage - here there is:
./zephyr/scripts/twister --ninja --inline-logs --outdir twister-build --jobs 1 -T zephyr/samples/hello_world --coverage --platform nrf52840dk_nrf52840 --coverage-tool gcovr --device-testing --device-serial /dev/ttyACM0 --extra-args CONFIG_COVERAGE_GCOV_HEAP_SIZE=0 --extra-args CONFIG_FORCE_COVERAGE=y -v -v --extra-args CONFIG_MAIN_STACK_SIZE=4096

There result will be at twister-build/coverage/index.html

gopiotr · 2023-10-19T13:45:16Z

During review I got question about exact command to execute testing with coverage - here there is: ./zephyr/scripts/twister --ninja --inline-logs --outdir twister-build --jobs 1 -T zephyr/samples/hello_world --coverage --platform nrf52840dk_nrf52840 --coverage-tool gcovr --device-testing --device-serial /dev/ttyACM0 --extra-args CONFIG_COVERAGE_GCOV_HEAP_SIZE=0 --extra-args CONFIG_FORCE_COVERAGE=y -v -v --extra-args CONFIG_MAIN_STACK_SIZE=4096

There result will be at twister-build/coverage/index.html

Thanks for sharing this. Please change PR description, because I believe, that most of reviewers start from reading PR description and testing basing on information from this. Another useful tip is necessity of exporting ZEPHYR_SDK_INSTALL_DIR variable with a path to Zephyr SDK. Without this Twister cannot find proper path to gcov tool and raises "not saying much" error like:

  File "/home/redbeard/zephyrproject/zephyr/scripts/pylib/twister/twisterlib/coverage.py", line 116, in generate
    ret = self._generate(outdir, coveragelog)
  File "/home/redbeard/zephyrproject/zephyr/scripts/pylib/twister/twisterlib/coverage.py", line 233, in _generate
    subprocess.call(["gcovr", "-r", self.base_dir, "--gcov-executable",
  File "/usr/lib/python3.8/subprocess.py", line 340, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.8/subprocess.py", line 1639, in _execute_child
    self.pid = _posixsubprocess.fork_exec(
TypeError: expected str, bytes or os.PathLike object, not NoneType

gopiotr

Looks good to me - I believe that this can be very useful especially for measuring coverage in driver tests. I don't want to stop or delay this feature, but I think that it could be helpful for potential users, if some information about measuring coverage on real devices will be placed in documentation here:
https://docs.zephyrproject.org/latest/develop/test/coverage.html

nordic-piks · 2023-10-19T14:08:56Z

During review I got question about exact command to execute testing with coverage - here there is: ./zephyr/scripts/twister --ninja --inline-logs --outdir twister-build --jobs 1 -T zephyr/samples/hello_world --coverage --platform nrf52840dk_nrf52840 --coverage-tool gcovr --device-testing --device-serial /dev/ttyACM0 --extra-args CONFIG_COVERAGE_GCOV_HEAP_SIZE=0 --extra-args CONFIG_FORCE_COVERAGE=y -v -v --extra-args CONFIG_MAIN_STACK_SIZE=4096
There result will be at twister-build/coverage/index.html

Thanks for sharing this. Please change PR description, because I believe, that most of reviewers start from reading PR description and testing basing on information from this. Another useful tip is necessity of exporting ZEPHYR_SDK_INSTALL_DIR variable with a path to Zephyr SDK. Without this Twister cannot find proper path to gcov tool and raises "not saying much" error like:
  File "/home/redbeard/zephyrproject/zephyr/scripts/pylib/twister/twisterlib/coverage.py", line 116, in generate
    ret = self._generate(outdir, coveragelog)
  File "/home/redbeard/zephyrproject/zephyr/scripts/pylib/twister/twisterlib/coverage.py", line 233, in _generate
    subprocess.call(["gcovr", "-r", self.base_dir, "--gcov-executable",
  File "/usr/lib/python3.8/subprocess.py", line 340, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.8/subprocess.py", line 1639, in _execute_child
    self.pid = _posixsubprocess.fork_exec(
TypeError: expected str, bytes or os.PathLike object, not NoneType

Thanks, updated the PR description with both information.

subsys/testsuite/coverage/coverage.c

nashif · 2023-10-23T14:08:16Z

subsys/testsuite/Kconfig


 config COVERAGE_DUMP
 	bool "Dump coverage data on exit"
 	depends on COVERAGE_GCOV
 	help
 	  Dump collected coverage information to console on exit.

+config FORCE_COVERAGE


why do we need this if you can just set 'CONFIG_HAS_COVERAGE_SUPPORT'?

Thanks for the question ;)
That was actually my first attempt, which failed as this config cannot be directly modified:
https://docs.zephyrproject.org/latest/kconfig.html#CONFIG_HAS_COVERAGE_SUPPORT

I decided to add another config that influences HAS_COVERAGE_SUPPORT to describe my intention clearly:
force coverage data generation knowing that it is not supported by design by this platform.
I agree with the current design, that this config is a property of the platform, not an on/off feature.

Do you have suggestions on how this can be implemented differently?
There are already multiple platforms that have this property, I did not want to influence them.

ok, with FORCE_COVERAGE, HAS_COVERAGE_SUPPORT will have no meaning, so, my suggestion is to expand the help message for th is kconfig saying that there is no guarantee coverage data will be produced and that things might fail, etc.

Thanks, added more information in the help message + rebased.

When there is no buffer for gcov hex data, dump them directly to console. This saves RAM which can be used to hold more gcov data. Reduce number of gcov hex data send by console - do not send space between every two digits. Signed-off-by: Piotr Kosycarz <piotr.kosycarz@nordicsemi.no>

When coverage is enabled, gather console logs longer to receive gcov data which are send after test output. Signed-off-by: Piotr Kosycarz <piotr.kosycarz@nordicsemi.no>

Do not break processing when incorrect coverage data are received. Instead, report failures but still process valid data. Include coverage processing status within twister retrun code. Signed-off-by: Piotr Kosycarz <piotr.kosycarz@nordicsemi.no>

nordic-piks force-pushed the coverage_for_target branch 2 times, most recently from 8d1dedb to 6142c89 Compare October 12, 2023 08:04

nordic-piks marked this pull request as ready for review October 12, 2023 10:17

nordic-piks requested review from nashif and erwango as code owners October 12, 2023 10:17

zephyrbot added area: Testsuite Testsuite area: Twister Twister area: Shields Shields (add-on boards) labels Oct 12, 2023

zephyrbot requested review from aaronemassey, asemjonovs, avisconti, galak, gchwier, golowanow, gopiotr, hakehuang, jeremybettis, jfischer-no, kartben, PerMac and yperess October 12, 2023 10:18

zephyrbot assigned nashif Oct 12, 2023

jfischer-no previously requested changes Oct 12, 2023

View reviewed changes

hakehuang reviewed Oct 12, 2023

View reviewed changes

nordic-piks force-pushed the coverage_for_target branch 2 times, most recently from 944bc5e to 81ffa98 Compare October 14, 2023 12:06

hakehuang self-requested a review October 18, 2023 03:49

hakehuang previously approved these changes Oct 18, 2023

View reviewed changes

gopiotr previously approved these changes Oct 19, 2023

View reviewed changes

fabiobaltieri added this to the v3.6.0 milestone Oct 19, 2023

golowanow reviewed Oct 19, 2023

View reviewed changes

subsys/testsuite/coverage/coverage.c Show resolved Hide resolved

golowanow previously approved these changes Oct 19, 2023

View reviewed changes

nashif requested changes Oct 23, 2023

View reviewed changes

nordic-piks added 3 commits October 24, 2023 17:47

scripts: twister: extend test timeout to get gcov data

bcb1477

When coverage is enabled, gather console logs longer to receive gcov data which are send after test output. Signed-off-by: Piotr Kosycarz <piotr.kosycarz@nordicsemi.no>

nordic-piks dismissed stale reviews from golowanow, gopiotr, and hakehuang via 9b21c80 October 24, 2023 15:48

nordic-piks force-pushed the coverage_for_target branch from 81ffa98 to 9b21c80 Compare October 24, 2023 15:48

nashif approved these changes Oct 24, 2023

View reviewed changes

hakehuang approved these changes Oct 25, 2023

View reviewed changes

carlescufi merged commit 8c5a15a into zephyrproject-rtos:main Oct 25, 2023
24 checks passed

nordic-piks deleted the coverage_for_target branch October 25, 2023 08:00

This was referenced Oct 25, 2023

testsuite: coverage: failed to parse coverage data on qemu_x86 #64396

Closed

testsuite: coverage: Fix gcov broken output on qemu_x86 #64397

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make easier getting coverage data for devices #63826

Make easier getting coverage data for devices #63826

nordic-piks commented Oct 11, 2023 •

edited

Loading

jfischer-no Oct 12, 2023

nordic-piks Oct 14, 2023

hakehuang Oct 12, 2023

nordic-piks Oct 14, 2023

hakehuang Oct 14, 2023

nordic-piks commented Oct 16, 2023

nordic-piks commented Oct 19, 2023

gopiotr commented Oct 19, 2023

gopiotr left a comment •

edited

Loading

nordic-piks commented Oct 19, 2023

nashif Oct 23, 2023

nordic-piks Oct 23, 2023

nashif Oct 24, 2023

nordic-piks Oct 24, 2023

		@@ -72,14 +72,6 @@ config COVERAGE_GCOV

		endchoice

		config COVERAGE_GCOV_HEAP_SIZE

Make easier getting coverage data for devices #63826

Make easier getting coverage data for devices #63826

Conversation

nordic-piks commented Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordic-piks commented Oct 16, 2023

nordic-piks commented Oct 19, 2023

gopiotr commented Oct 19, 2023

gopiotr left a comment • edited Loading

Choose a reason for hiding this comment

nordic-piks commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordic-piks commented Oct 11, 2023 •

edited

Loading

gopiotr left a comment •

edited

Loading