Replace cover-function-only by cover-include-pattern #1348
Conversation
|
Could I ask for some explanation either in the commit or on the PR as to what this is intending to do? |
src/goto-instrument/cover.cpp
Outdated
| // cover entry point function only | ||
| std::string cover_include_pattern=cmdline.get_value("cover-include-pattern"); | ||
| if(cmdline.isset("cover-function-only")) | ||
| cover_include_pattern=config.main; |
There was a problem hiding this comment.
for regex we should escape ., else there could be a problem with correct matching, may also be required for other characters that can be part of a function name, maybe '<', '>' ?
There was a problem hiding this comment.
This appears to be a robust way of doing this: https://stackoverflow.com/a/40195721/958004
peterschrammel
force-pushed
the
cleanup/cover-function-only
branch
from
9d3b30a to
8641287
Compare
|
@thk123, amended the commit message. |
There was a problem hiding this comment.
Somewhat incredibly, there are no tests in cbmc/regression using the --cover-function-only flag. Could you add some in the cbmc-cover directory:
- A default entry point
- A default entry point specified by the
--functionflag - A function specified with the
--functionflag using the minimal format:--function Class.functionName - A function specified with the
--functionflag using the complex format that includes types etc (I'm not an expert on how this works, it might be appropriate to test static methods, non-static methods, methods that take and return arrays, generic methods)
As a first step you should probably bump the test-gen submodule (which does have --cover-function-only tests) and check they pass with this change.
|
@peterschrammel thanks for the commit message - makes sense 👍 |
|
These options have never been made available in CBMC's CLI (as many others...). There are tests in test-gen, but not as systematic as you suggest... |
|
@peterschrammel Ah that makes sense - well OK I won't block on that, but I think a PR to test-gen that brings this in should probably test complex format style of specifying functions since I believe this is what the platform uses (and would have caught the regex escape problem) |
peterschrammel
force-pushed
the
cleanup/cover-function-only
branch
from
8641287 to
0525585
Compare
cover-function-only (instrumentating coverage goals in the entry point function only) is now implemented with the help of cover-include-pattern (regex matching of functions to be instrumented). This is to avoid potential consistencies due to providing two mechanisms for filtering functions. cover-function-only has priority over the given cover-include-pattern.
peterschrammel
force-pushed
the
cleanup/cover-function-only
branch
from
0525585 to
215c7ae
Compare
|
Will approve once test-gen#1031 is working as expected. |
|
@thk123 What's the state of TG on this one? |
|
Sorry been away and then busy - current state of TG is failing however... |
|
Closing because obsolete (subsumed by #1419). |
Companion: https://github.com/diffblue/test-gen/pull/1031