lcov icon indicating copy to clipboard operation
lcov copied to clipboard

HTML report tree structure should reflect directory structure of code

Open henry2cox opened this issue 4 years ago • 14 comments

In current versions, the 'genhtml' report has three levels:

  • 'top': which contains a table of directories which contain source code files
  • 'directory' which contains a table of all the source files containing coverpoints in that directory
  • source code detail: containing information for a particular source file (this class of pages can be suppressed)

In a more complicated project, it is convenient for the HTML structure to match the directory structure - for example, most projects have some logic behind their physical structure - e.g., so a particular submodule is in one directory tree and its parent module is the parent directory. When the project team is larger, individuals or individual teams tend to be responsible for particular modules - which correspond to particular (sets of) directories - and they tend to be interested in the coverage holes in the code that they are responsible for. This will also enable the project lead/program manager to more easily see and summarize the status of each module in the system.

I propose to add a "--hierarchical" flag to genhtml, to support this (default 'off' - to maintain backward compatibility).

A result of this change is that some pages may hold both a directory table and a file table - similar to the MS file explorer window. At least in our code bases, there are relatively few non-leaf directories which contain code - and the ones that exist tend to be module integration packages. Another result is that the number of directory pages can be quite large.

henry2cox avatar Sep 26 '20 11:09 henry2cox

I committed and pushed the enhancement to my repo/branch: https://github.com/henry2cox/lcov.git branch 'diffcov_initial'

henry2cox avatar Oct 03 '20 11:10 henry2cox

@henry2cox thanks for working on this issue. The CKI project (https://cki-project.org/) would also be interested on the --hierarchical option.

bgoncalv avatar May 24 '21 06:05 bgoncalv

That’s great.

The feature is implemented and ready for you to try – if you download my lcov branch. My branch also implements a number of other features which you may or may not find useful – differential coverage and date/owner binning chief among them. If you think of other features or find any bugs: please let me know

Henry

henry2cox avatar May 24 '21 10:05 henry2cox

That’s great. The feature is implemented and ready for you to try – if you download my lcov branch. My branch also implements a number of other features which you may or may not find useful – differential coverage and date/owner binning chief among them. If you think of other features or find any bugs: please let me know Henry

Yes, I did some tests from your branch, using the 3e7bc717dbce4aa807822187de8c3bd1ac8c7bf9 as I wanted to use the feature, but keeping lcov as much close as possible to the official one. It did work well for our needs.

bgoncalv avatar May 24 '21 11:05 bgoncalv

@henry2cox I've hit the following issue when running lcov: "Undefined subroutine &lcovutil::rmtree called at /usr/local/bin/../lib/lcovutil.pm line 203."

Finished .info-file creation
Undefined subroutine &lcovutil::rmtree called at /usr/local/bin/../lib/lcovutil.pm line 203.

I've tried to add "use File::Path qw( rmtree );", but ran into another issue :-)

Finished .info-file creation
Removing temporary directories.
lcov: No root path(s) specified

 at /usr/local/bin/../lib/lcovutil.pm line 284.

The command line is lcov --capture --output-file test.info

bgoncalv avatar Jul 28 '21 14:07 bgoncalv

Thanks for finding my rmtree bug. I fixed that one and the one that was causing the second problem. Fix was pushed just now.

henry2cox avatar Jul 28 '21 15:07 henry2cox

thanks for the fixes, it is working well now.

bgoncalv avatar Jul 29 '21 11:07 bgoncalv

@henry2cox I tried to run genhtml from your branch but ran into the following error:

Can't locate DateTime.pm in @INC (you may need to install the DateTime module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.26.1 /usr/local/share/perl/5.26.1 /usr/lib/x86_64-linux-gnu/perl5/5.26 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.26 /usr/share/perl/5.26 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) at ./bin/genhtml line 85.
BEGIN failed--compilation aborted at ./bin/genhtml line 85.

Sorry in advance - I know nothing about perl. Any ideas how I can resolve this? I am on Ubuntu 18.04.

alokpr avatar Mar 03 '22 17:03 alokpr

$ perl -MCPAN -e shell cpan> install DateTime

Should do it.

(you may need privileges to install new perl modules – so you may need to talk to your IT folks – if you are not your own IT 😊)

I can’t remember if it is a standard part or not – but you may also have to install the “DateTime::Format::W3CDTF” module. I don’t think it is part of the perl base install. (You need this to enable parsing file annotations from git or perforce.)

There is very comprehensive online documentation for how to install and use CPAN.

Hope this helps.

Henry

PS: there are a couple of enhancements and/or cleanups that I will push fairly soon. (Delay is to verify that we have everything we need to satisfy the use models of a few internal teams…once that is done, then I planned to update the module.) Check back in a week or two.

henry2cox avatar Mar 03 '22 18:03 henry2cox

Thanks for the prompt response. Installing libdatetime-format-w3cdtf-perl and libjson-perl packages also worked. Looking forward to the enhancements.

alokpr avatar Mar 03 '22 19:03 alokpr

Sorry – forgot about JSON. That is used for the ‘criteria’ script feature which we use for our Jenkins integration. It is also used in some performance profiling (which you don’t have).

It would no doubt be possible to conditionally load both the datetime and json packages only when those other features are used … but I didn’t have much incentive to do that, as all of the current (internal) groups use all those features…and because we got the IT guys to install the required packages.

I’m curious about your application (what you are trying to use the tool for) and also about your experience + any suggestions for improvement. It is not hard to guess my (work) email address if you happen to read the documentation or look more closely at the code.

Henry

henry2cox avatar Mar 03 '22 20:03 henry2cox

I am mainly interested in the genhtml hierarchical display option. We have a monorepo with different owners for various directories where providing hierarchical code coverage numbers is very useful. I just tested it on our repository and it is exactly what we needed. Thanks a lot for providing this option. Would you be contributing back to the upstream?

alokpr avatar Mar 03 '22 22:03 alokpr

  • Would you be contributing back to the upstream?

I had tried - initiated a pull request about a year and a half ago – but no joy. Meantime: our development continued – and I have no interest in trying to decouple some smaller features to submit individually. Happy to support someone else if they want to try – but I won’t be doing it myself. (Wholesale adoption/merge should be straightforward as this is entirely backward compatible. At this point it seems unlikely, though.)

If you have a few minutes, I suggest you try adding the ‘date bin’ options to your report. Should be fairly easy if you are using git or perforce – depending on the complexity of your build (does your repo hierarchy more-or-less match your build layout?). The age bins give quick insight into your project: is coverage of older code (say, more than a year old) higher or lower than new code? Are you getting better or worse? From there – it is a slippery slope to some more advanced features and methodology.

henry2cox avatar Mar 03 '22 22:03 henry2cox

I will checkout the date-bin feature. However we already have this analytics for the global coverage number from gitlab-ci. Every CI pipeline run publishes the coverage report (in cobertura format) and gitlab automatically generates the analytics.

alokpr avatar Mar 03 '22 23:03 alokpr

This is fixed in #169 - and can be closed when that one is merged.

henry2cox avatar Nov 10 '22 11:11 henry2cox

this is fixed in sha 5f659f63801ef, merged to master. Closing isssue.

henry2cox avatar Dec 08 '22 12:12 henry2cox