Measure code coverage

[frederic-mahe]

The purpose of this issue is to document a simple way to measure code coverage with gcc and gcov. Code coverage will be used to identify blind spots in swarm's test suite. As explained in the gcov introduction:

Software developers also use coverage testing in concert with testsuites, to make sure software is actually good enough for a release. Testsuites can verify that a program works as expected; a coverage program tests to see how much of the program is exercised by the testsuite.

cd /tmp/

## temporary repositories
git clone https://github.com/frederic-mahe/swarm-tests.git
git clone https://github.com/torognes/swarm.git
cd ./swarm/src/
git checkout swarm3

## instrument the code (GCC only)
make \
    CXXFLAGS='-O0 -ggdb3 -fprofile-arcs -ftest-coverage -msse2 -mtune=core2' \
    LINKFLAGS="-g -lgcov --coverage"

## run the tests
(cd ../../swarm-tests/
 bash run_all_tests.sh ../swarm/src/swarm
)

## get the coverage values and annotated source code
gcov *.cc

## code lines that are not reached during the tests are marked "#####"
less derep.cc.gcov

Initial reported coverage is 75.25% (3,155 lines of code). I've already started to write tests for the most obvious cases. Let's see what coverage percentage we can reach.

[frederic-mahe]

Hi @torognes I would like to write a test to activate line 121 in db.cc:

      145:  117:      if (opt_usearch_abundance)
        -:  118:        {
        -:  119:          /* print semicolon if the abundance is not at either end */
        3:  120:          if ((sp->abundance_start > 0) && (sp->abundance_end < hdrlen))
    #####:  121:            fprintf(stream, ";");
        -:  122:          
        -:  123:          /* print remaining part */
        3:  124:          fprintf(stream, "%.*s", hdrlen - sp->abundance_end, h + sp->abundance_end);
        -:  125:        }

my first idea was to write an abundance value in the middle of the header string:

printf ">s;size=1;length=1\nA\n" | ./swarm -z

but that does not work. Any idea?

[torognes]

I think your idea is right, but that part of the code, the function fprint_id_noabundance, is only used when writing identifiers to the internal structure file (-i) and statistics file (-s), so you'll need to output to one of those files. For example, like this:

printf ">s;size=1;length=1\nA\n" | ./swarm -z -o /dev/null -s - 2> /dev/null

The result is:

1   1   s;length=1  1   1   0   0

Line 121 is executed to print the semicolon between s and length=1.

[frederic-mahe]

Great, I've added tests to trigger that function https://github.com/frederic-mahe/swarm-tests/commit/2c2cc9324f651b473d9f888170e5435316b5e00d. Thanks!

[frederic-mahe]

I cannot activate line 603 in swarm.cc with my tests:

https://github.com/torognes/swarm/blob/b584e94b8eb8a92e8e7e5868ee2f597c0094182b/src/swarm.cc#L603

  if (opt_output_file)
    {
      outfile = fopen_output(opt_output_file);
      if (! outfile)
        fatal("Unable to open output file for writing.");
    }
  else
    outfile = stdout;

I assumed that a simple swarm command without any output file would activate that line, but my tests with gdb show that it is not the case. Any idea?

[torognes]

Actually that line cannot be activated. The variable opt_output_file is initialised to point to a dash (-) at start and then changed to point to a filename if any is given on the command line. It will never be null. I'll remove that line of code and rewrite some nearby code as well.

If a filename is - then stdin or stdout will be opened instead at a later stage, depending on whether it is an input or output file.

[frederic-mahe]

Thanks, gdb shows that opt_output_file is - by default but I wasn't 100% sure. That's a good exercise, I am getting better at reading c++ code.

[frederic-mahe]

As of today, our code coverage is 85.83%, for 3,507 lines of code (excluding third-party code). We've gained 10% since we've started to work on that. Most of the remaining uncovered code is in search8.cc and search16.cc in what I think to be debug functions (e.g., dprofile_fill8). Lines contained between preprocessor instructions are ignored by gcov:

#if 0
dprofile_dump16(dprofile_word);
#endif

Maybe we should set the code for dprofile_fill8 and dprofile_fill16 between preprocessor instructions too?

[torognes]

Looks good!

The dprofile_fill8 and dprofile_fill16 functions (in search8.cc and search16.cc) are not debug functions but functions called instead of dprofile_shuffle8 and dprofile_shuffle16 (in ssse3.cc), respectively, on x86_64 cpus without SSSE3. So they are only used for old cpus with just SSE2. Very rare these days. To get code coverage on these you need to run the software on a cpu without SSSE3.

They are all compiled into the code and the program chooses at run-time if it should use the SSE2 or SSSE3 version depending on the cpu it is running on.

We could remove support for these old processors and check for SSSE3 at startup, but see this and this question.

[frederic-mahe]

We could remove support for these old processors and check for SSSE3 at startup

No, no need to remove these functions, I am glad we have support for older CPUs. I do not mean to discard code just to improve our coverage score. Anyway, we can report a raw coverage score, and explain that most of the remaining uncovered code is hardware-specific, or linked to extreme conditions (out-of-memory, thread failure, etc) that cannot be easily reproduced.

Besides that, there are still a few uncovered lines of code in variants.cc, that I think I can reach with carefully crafted tests. I'll open a specific issue for that, and close this one.