cocotb icon indicating copy to clipboard operation
cocotb copied to clipboard

Add coverage for cocotb_tools

Open oscargus opened this issue 8 months ago • 10 comments

I've been quite confused that the coverage was not changed when making commits towards runner.py...

One may consider adding the tests as well.

oscargus avatar Apr 30 '25 12:04 oscargus

Outside of the Runner where we can use pytest-coverage to collect coverage, the other things in that package can't easily have coverage collected on them. We also don't have exhaustive tests for every feature so trying to obtain high coverage in the Runner is not easy. And that goes without mentioning Runners for simulators we don't have access to yet, like DSim. So it begs the question of why collect coverage? Enabling coverage comes with the goal of actually trying to reach 100% coverage and that's a mountain of work that's low priority right now.

ktbarrett avatar Apr 30 '25 14:04 ktbarrett

So it begs the question of why collect coverage?

Valid point. My main reason was to confirm that my modifications were actually executed, especially for things that I didn't have the simulator set up for.

One may also ask: why not collect coverage. :-)

For Dsim, I agree that it is a problem, but then it would at least be clear that Dsim is not tested (without reading the scripts etc). It would also be a constant reminder (for people checking the coverage) to possibly do something about it. But maybe I am overestimating the coverage as a driver for development (I have in many other projects started out by increasing test coverage as a way to learn the code base and project).

Admittedly, it is runner.py which is the primary interest for me. I guess that one can only collect that file in cocotb_runners, but not sure if the failures are related.

oscargus avatar Apr 30 '25 15:04 oscargus

Does specifying --cov=cocotb_tools.runner work? That's fine by be.

We can enable more once we have some idea of how we can actually collect coverage on those modules. They are commandline scripts which mostly require calling the script with coverage module to enable collection. That's a big change to the Makefiles and Runner to make that happen. And the other module is a module designed to enable coverage for cocotb, if we can somehow figure out a way to enable coverage for that file, that file become pointless.

As for the failures:

  • Icarus v12_0 random hangs during build. It's super annoying. We are not sure why and we can't reproduce it locally.
  • The other tests looked like they timed out waiting for licenses. We only have so many licenses and all concurrent CI runs have to share them. The CI tests can sometimes time out due to license contention.

ktbarrett avatar May 01 '25 14:05 ktbarrett

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.30%. Comparing base (e63082f) to head (ac325a8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4646      +/-   ##
==========================================
- Coverage   76.96%   75.30%   -1.66%     
==========================================
  Files          53       68      +15     
  Lines        8316     9455    +1139     
  Branches     2033     2057      +24     
==========================================
+ Hits         6400     7120     +720     
- Misses       1437     1805     +368     
- Partials      479      530      +51     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 01 '25 14:05 codecov[bot]

And Questa fails randomly until #4593 is in.

ktbarrett avatar May 02 '25 18:05 ktbarrett

Are we sure that specifying --cov more than once works correctly? It seems like coverage decreases because of pytest tests coverage not being collected anymore. Also, I don't see runner.py in the coverage results.

ktbarrett avatar May 04 '25 20:05 ktbarrett

I am quite sure: https://pytest-cov.readthedocs.io/en/latest/config.html#reference

That is the only interpretation of "multi-allowed" that makes sense to me, but I can be wrong.

Yeah, I do not really know what to try now. I thought the pyproject changes was gonna do it, but not.

oscargus avatar May 05 '25 12:05 oscargus

This is quite strange. Only using the pyproject.toml-info lead to the tests being included (maybe one should remove the nox-line?), but not runner.py.

oscargus avatar May 05 '25 13:05 oscargus

It looks like it's collecting coverage data in tests/ as well and strangely picking up a single line from one file? Maybe we should exclude the tests/ and examples/ directories until we can figure out what's going on there.

Also I still don't see runner.py in the codecov file explorer for this PR.

ktbarrett avatar May 05 '25 14:05 ktbarrett

If I ever get coverage for runner.py, I will sort out the other issues. ;-)

oscargus avatar May 06 '25 06:05 oscargus