Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#9267 closed defect (implemented)

Usability tweaks for code coverage

Reported by: andrea Owned by:
Priority: Low Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client testing
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The new coverage tools introduced in #8949 have a few rough edges:

  • The cov-diff script wants two directories full of .gcov files, but the coverage script dumps them all in the repo root. There should be a way to specify a target directory to put them in.
  • If you modify the code, rebuild and re-run the test suite, the gcov instrumentation tries to merge the states into the existing gcda file and fails. There should be a makefile target to reset the coverage data by deleting just the gcda (not the gcno) files.

Child Tickets

Change History (9)

comment:1 Changed 6 years ago by andrea

Status: newneeds_review

See my coverage_script_improvements branch for fixes for these, and improved unit test coverage (to 100%) of replaycache.c I used to test them.

comment:2 Changed 6 years ago by andrea

Also: it is not necessarily true that gcov -o src_or_lib_foo_a-bar.o src/or/bar.c only writes to bar.gcov; it may also emit .gcov files for referenced headers if they contain executable lines such as inline functions. Since these may be generated more than once for the same source file, they would be overwritten by the repeated invocations of gcov in the old version of contrib/coverage. In this new version, they are not moved to the destination directory since they do not match "${BN}.gcov" for any basename which occurs.

comment:3 Changed 6 years ago by nickm

Awesome; thanks! merged. Should I close this ticket, or leave it open for the issue you just mentioned?

comment:4 in reply to:  3 Changed 6 years ago by andrea

Replying to nickm:

Awesome; thanks! merged. Should I close this ticket, or leave it open for the issue you just mentioned?

Well, I'm not sure if the issue I just mentioned is really an issue or not, since those cases are rare and there isn't any very clear way to merge the results for multiple object files referencing the same header. Possibly, though, we could systematically rename the .gcov files to include the object filename as well as the source filename when we move them so we'd end up with multiple gcov files for the same header. With a bit more effort, a merge script could combine the counts.

comment:5 Changed 6 years ago by nickm

Status: needs_reviewnew

comment:6 Changed 6 years ago by arma

Andrea: for posterity, the d1059a93 commit shouldn't have had a changes/ file, since it was modifying a thing that had never been in any released version of Tor.

(If anything, it should have modified the changes/fancy_testing file.)

Thanks!

comment:7 Changed 5 years ago by nickm

Closing this, opening a new one for for merging .gcov files.

comment:8 Changed 5 years ago by nickm

Resolution: implemented
Status: newclosed

comment:9 Changed 5 years ago by nickm

Added the new tickets as #11145 and #11146

Note: See TracTickets for help on using tickets.