topic/jsiwek/brofiler

[jsiwek]

This branch is in bro, btest, bro-testing, and bro-testing-private repos and implements scripting-layer coverage analysis for the test suites. Bro was augmented to output script-layer statement execution data in the presence of the BROFILER_FILE environment variable and some minor scripts were added to the test suites to aggregate those data files into coverage.log files. That data seems like they'll generally help identify areas of scripts that lack testing coverage, but a goal of getting to 100% seems unreasonable since some statements are just naturally difficult or impossible to reach (which is sometimes a good thing!) and I haven't yet thought of a good way to automatically identify those or even manually maintaining a list of such statements might not be great since location information can change when scripts get modified. Let me know if there's any feedback on anything that can be improved.

For the 2.0 release the coverage numbers I get are:

• 52% coverage rate for the btest suite
• 45% coverage rate for the external/bro-testing suite
• 62% coverage rate for btest and external combined

[robin]

• 52% coverage rate for the btest suite
• 45% coverage rate for the external/bro-testing suite
• 62% coverage rate for btest and external combined

That's not too bad already! Does it increase further with the private
traces?

to aggregate those data files into coverage.log files. That data seems
like they'll generally help identify areas of scripts that lack testing
coverage, but a goal of getting to 100% seems unreasonable since some
statements are just naturally difficult or impossible to reach (which is
sometimes a good thing!)

Hmm ... yeah. On the other hand, I think we need some hard criteria to
aim for as otherwise we never know whether we have written sufficient
tests (or for new functionality, what we need to write tests for). The
number would keep going up and down as we progress.

Could we insert markers right in the scripts that tag blocks that
aren't counted towards the covaerage? Like a whitelist, but not
external (because, as you say, locations chage), but coming with the
script itself. For example:

    if ( xxx ) { # @no-test
        <rarely executed code that can't be easily tested>
    }

The "@no-test" would need to be part of a comment and always
associated with the preceding block start.

Robin

[jsiwek]

That's not too bad already! Does it increase further with the private
traces?

Just to 64%.

Could we insert markers right in the scripts that tag blocks that
aren't counted towards the covaerage?

I'll see what I can do.

[jsiwek]

Ok, added the ability to tag statement blocks so they aren't counted towards coverage. I didn't go through to blacklist anything yet, seems like that can be a task for later, so I'll switch back to a merge request (in bro, btest, bro-testing, bro-testing-private repos) since I don't have any ideas on what more needs done here to enable testing coverage analysis.

[robin]

Merged, but I've disabled brofiling in btest.cfg as I'm getting errors: the initial unit tests work fine, but at some point everything starts failing suddenly with errors like this:

scripts.base.frameworks.logging.ascii-empty ... failed
  % 'bro -b /da/home/robin/bro/master/testing/btest/.tmp/scripts.base.frameworks.logging.ascii-empty/ascii-empty.bro' failed 
  % cat .stderr
  terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_S_construct null not valid

I'm guessing this might be a problem with the coverage file at some point getting into bad shape.

Can you try current master and see if you get that as well?

I don't think I changed anything relevant. I renamed BROFILER_FILE to BRO_PROFILER_FILE for consistency with other env variables though.

[jsiwek]

I don't think I changed anything relevant. I renamed BROFILER_FILE to BRO_PROFILER_FILE for consistency with other env variables though.

Looks like a few places didn't get adapted to the new name:

$ grep -R BROFILER_FILE .
./testing/btest/coverage/coverage-blacklist.bro:# @TEST-EXEC: BROFILER_FILE=coverage bro -b %INPUT
./testing/external/bro-testing/btest.cfg:BROFILER_FILE=%(testbase)s/.tmp/script-coverage
./testing/external/subdir-btest.cfg:BROFILER_FILE=%(testbase)s/.tmp/script-coverage
./testing/scripts/btest-bg-run:BROFILER_FILE=`mktemp -t script-coverage` $BTEST_PATH/btest-bg-run $@

The last one is the reason for the errors -- the tests involving communication go through that wrapper script so that each Bro process gets a unique coverage file to write to, otherwise they may write simultaneously to the same file. Want to try again after fixing those?

[robin]

On Thu, Jan 26, 2012 at 15:43 -0000, you wrote:

Looks like a few places didn't get adapted to the new name:

Ok, will try again (and should have just done the grep!)

Robin

--
Robin Sommer * Phone +1 (510) 722-6541 * robin@…
ICSI/LBNL * Fax +1 (510) 666-2956 * www.icir.org

[robin]

Fixes committed, but still getting two test failures:

coverage.bare-mode-errors ... failed
  % 'btest-diff unique_errors' failed unexpectedly (exit code 1)
  % cat .diag
  == File ===============================
  
  error: Failed to open BRO_PROFILER_FILE destination '' for writing
  == Diff ===============================
  --- /tmp/test-diff.25104.unique_errors.baseline.tmp   2012-01-27 04:46:01.120424720 +0000
  +++ /tmp/test-diff.25104.unique_errors.tmp    2012-01-27 04:46:01.140425480 +0000
  @@ -0,0 +1,2 @@
  +
  +error: Failed to open BRO_PROFILER_FILE destination '' for writing
  =======================================

  % cat .stderr
  <<< [24919] bro -b /da/home/robin/bro/master/testing/btest/../../scripts/policy/frameworks/control/controllee.bro
  received termination signal
  error: Failed to open BRO_PROFILER_FILE destination '' for writing
  
  >>>

I am still missing something?

[jsiwek]

In [0065cf6148ab76ae9dceebba4dcdbe7e9b556e07/bro]:

Fix mktemp portability issue affecting test coverage. (addresses #752)

Also update Makefiles to behave better for brief targets and in absence
of external test repos.

[jsiwek]

I am still missing something?

Looked like mktemp portability issues. There's a fix and other minor Makefile tweaks in topic/jsiwek/brofiler in bro, bro-testing, and bro-testing-private repos for you to try out and merge.

[robin]

In [c607785cec2b548ea8084be2eea27812fa603725/bro]:

Merge remote-tracking branch 'origin/topic/jsiwek/brofiler'

• origin/topic/jsiwek/brofiler: Fix mktemp portability issue affecting test coverage. (addresses #752)

Closes #752.

[robin]

Works now, thanks.

I like this!