[cmake-developers] Support for coverage.py coverage on the topic stage

Rolf Eike Beer eike at sf-mail.de
Sat Sep 28 05:36:45 EDT 2013


Am Freitag, 27. September 2013, 11:30:52 schrieb Patrick Reynolds:
> Hi folks,
> 
> I just pushed a topic to the stage that adds support for coverage.py python
> coverage. It actually should work for anything the exports cobertura
> coverage.xml files.
> 
> The topic is here:
> 
> http://public.kitware.com/gitweb?p=stage/cmake.git;a=shortlog;h=refs/heads/A
> dd-coverage.py-Coverage

You are doomed! Now that you want to introduce a new coverage handler I'll 
complain as long until you add a testcase for it, i.e. putting a sample output 
file in the Tests/ directory and verify that CTest will generate the expected 
results ;)

But there are some things about your code and even about older code. First, 
you do not set error from cont.error in 
cmCTestCoverageHandler::ProcessHandler().

Second, this whole gathering code there will badly fail once 2 coverage 
systems are present and the second one gives an error. Say gcov finds 2 files. 
Then python has an error, returns -1. The file count is 1, no error is 
propagated. This is not your fault, but please fix this in one commit first 
before adding your new handler. And maybe make this block from gcov to mumps 
somehow use an array, vector, list, or something and just iterate over it, so 
you only have to add your handler into the array.

Then I'm very unhappy with the coverage database being in the source 
directory. One setup I daily use at work is having one checkout and run 2 
builds from it with different compilers. In your case the coverage results will 
mix. Even worse, they will always sum up, because usually noone deletes a file 
from the source directory before running a test.

Another important question: can python handle the situation where 2 test 
programs run in parallel and want to write to the coverage file? Or will they 
just happily trash each others results? That's a problem I fixed in 2.8.12 
which would have happened for all other coverage generators.

Another thing totally unrelated to your code I see when looking at the code is 
that the Bullseye coverage handling seems broken:

int cmCTestCoverageHandler::HandleBullseyeCoverage(
  cmCTestCoverageHandlerContainer* cont)
{
  const char* covfile = cmSystemTools::GetEnv("COVFILE");
  if(!covfile || strlen(covfile) == 0)
    {
    cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
               " COVFILE environment variable not found, not running "
               " bullseye\n");
    return 0;

/// nothing happened, return 0

    }
  cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
             " run covsrc with COVFILE=["
             << covfile
             << "]" << std::endl);
  if(!this->RunBullseyeSourceSummary(cont))
    {
    cmCTestLog(this->CTest, ERROR_MESSAGE,
               "Error running bullseye summary.\n");


//// error case, return 0

    return 0;
    }
  cmCTestLog(this->CTest, DEBUG, "HandleBullseyeCoverage return 1 "
             << std::endl);

//// default, probably everything fine: return 1

  return 1;
}


  if(this->HandleBullseyeCoverage(&cont))
    {
    return cont.Error;
    }

Break if everything is fine, continue on error or when no files are found? All 
other handlers support being run in parallel, why not Bullseye?

Eike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20130928/e6c08177/attachment.sig>


More information about the cmake-developers mailing list