[Tests] Include branch coverage info in coverage test #10511

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:lcov changing 4 files +71 −16

achow101 commented at 9:18 PM on June 2, 2017: member

This PR adds an option to configure to include branch coverage statistics be gathered during make cov. It also disables logging when coverage is enabled so that the logging branches are not included in the coverage stats. This has a side effect of causing -Wunused-variable warnings during compile time when coverage is enabled.

sipa commented at 10:53 PM on June 2, 2017: member

Here is a possible hack to force a variable list of arguments to be treated as 'used' for the purpose of -Wunused-variable:

static inline void MarkUsed() {}
template<typename T, typename... Args> static inline void MarkUsed(const T& t, const Args&... args)
{
    (void)t;
    MarkUsed(args...);
}

#ifdef USE_COVERAGE
#define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0)
#define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0)
#else

fanquake added the label Tests on Jun 2, 2017
achow101 force-pushed on Jun 5, 2017
achow101 commented at 7:06 PM on June 5, 2017: member

Used @sipa's hack to shut up the warnings.
achow101 force-pushed on Jun 5, 2017
gmaxwell commented at 10:12 AM on June 6, 2017: contributor

Cool. Concept ACK.

Have `make cov` optionally include branch coverage statistics

Added an option to configure to allow for branch coverage statistics gathering.

Disabled logprint macro when coverage testing is on so that unnecessary branches are not analyzed.

c8914b9dbb

achow101 force-pushed on Jun 7, 2017
in contrib/filter-lcov.py:19 in 67a24f3e52 outdated
```
  14 | +
  15 | +in_remove = False
  16 | +with open(tracefile, 'r') as f:
  17 | +    with open(outfile, 'w') as wf:
  18 | +        for line in f:
  19 | +            if pattern in line:
```
sipa commented at 2:25 AM on June 9, 2017:

Perhaps instead use line.startswith("SF:" + pattern) instead? That would prevent matching on variables that accidentally match the pattern name.

achow101 commented at 5:56 PM on June 9, 2017:

Done

in contrib/filter-lcov.py:23 in 67a24f3e52 outdated

  18 | +        for line in f:
  19 | +            if pattern in line:
  20 | +                in_remove = True
  21 | +            if not in_remove:
  22 | +                wf.write(line)
  23 | +            if 'end_of_record' in line:

sipa commented at 2:25 AM on June 9, 2017:

Perhaps use line == 'end_of_record' instead? Same reason as above.

achow101 commented at 5:56 PM on June 9, 2017:

done

achow101 force-pushed on Jun 9, 2017
achow101 force-pushed on Jun 9, 2017
sipa commented at 7:18 PM on June 12, 2017: member

utACK 405b86a92aee4f2ddb6710bfe07ff714f2afcfa2

in contrib/filter-lcov.py:1 in 405b86a92a outdated

   0 | @@ -0,0 +1,24 @@
   1 | +#!/usr/bin/env python3

MarcoFalke commented at 11:26 AM on June 13, 2017:

explicit license is missing:

# Copyright (c) 2017 Bitcoin Core Developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

achow101 commented at 5:51 PM on June 13, 2017:

done

in contrib/filter-lcov.py:23 in 405b86a92a outdated
```
  15 | +in_remove = False
  16 | +with open(tracefile, 'r') as f:
  17 | +    with open(outfile, 'w') as wf:
  18 | +        for line in f:
  19 | +            if line.startswith("SF:") and pattern in line:
  20 | +                in_remove = True
```
MarcoFalke commented at 11:29 AM on June 13, 2017:

Not sure why you need the var in_remove. Couldn't you write wf.write(line);continue; here?
achow101 commented at 5:49 PM on June 13, 2017:

The format of the file is

SF:<path to file> <bunch of lines> end_of_record <repeat>

In order to know that we are in a <bunch of lines> section that should be removed (because the SF:... line matches our removal pattern, we need to have a variable to remember that we shouldn't be writing these lines to the output file. That is what in_remove is for.
MarcoFalke commented at 11:29 AM on June 13, 2017: member

Concept ACK
MarcoFalke requested review from theuni on Jun 13, 2017

Replace lcov -r commands with faster way

Instead of using lcov -r (which is extremely slow), first use a python script to perform bulk cleanup of the /usr/include/* coverage. Then use lcov -a to remove the duplicate entries. This has the same effect of lcov -r but runs significantly faster

368c10d125

achow101 force-pushed on Jun 13, 2017
theuni commented at 8:11 PM on June 13, 2017: member

The lcov stuff was added ages ago, when lcov's --no-external was too new to rely on. I think it'd be ok now to require >=1.10 and forego all of the filtering.
MarcoFalke commented at 6:13 PM on June 19, 2017: member

@achow101 Are you still working on this?
achow101 commented at 6:17 PM on June 19, 2017: member

@MarcoFalke Yes
achow101 commented at 12:49 AM on June 20, 2017: member

Would it be okay with everyone if I just merged #10565 into this PR? It all falls under "improvements to coverage data".
sipa commented at 8:32 PM on June 20, 2017: member

@achow101 Have you tried the approach @theuni suggested above (using --no-external, and removing all filtering)?
achow101 commented at 11:59 PM on June 20, 2017: member

@sipa I am thinking of doing that and combining #10565 into this PR. If I remove filtering here, it would just be added back in by #10565.
MarcoFalke commented at 7:49 PM on June 22, 2017: member

Needs rebase.
achow101 commented at 8:13 PM on June 22, 2017: member

Closing this as #10565 contains this PR (I totally forgot) except for the copyright header I added last commit :/
achow101 closed this on Jun 22, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Review Requested
theuni

Labels

Tests

Linked (view graph)

#10565 [coverage] Remove subtrees and benchmarks from coverage report