Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add --cover-exclude-file to exclude files from --cover-inclusive. #924
Conversation
jszakmeister
reviewed
Jun 17, 2015
| @@ -66,6 +67,11 @@ def options(self, parser, env): | ||
| "discovering holes in test coverage if not all " | ||
| "files are imported by the test suite. " | ||
| "[NOSE_COVER_INCLUSIVE]") | ||
| + parser.add_option('--cover-exclude-file', action='append', | ||
| + default=env.get('NOSE_COVER_EXCLUDE_FILE'), |
jszakmeister
Jun 17, 2015
Contributor
If I'm reading things correctly, this would only work for a single file?
OddBloke
Jun 17, 2015
Contributor
Nope, this could be a comma-separated list; there's even a test for that. :)
jszakmeister
reviewed
Jun 17, 2015
| + if isinstance(options.cover_exclude_file, (list, tuple)): | ||
| + cover_exclude_file = options.cover_exclude_file | ||
| + else: | ||
| + cover_exclude_file = [options.cover_exclude_file] |
jszakmeister
Jun 17, 2015
Contributor
I suspect this if/else has to do with the default for the option. I'm not sure I like the fact that it works only for a single file, but I think the answer is to provide a better default value than do this song and dance.
jszakmeister
reviewed
Jun 17, 2015
| + c = _get_coverage_obj([], env) | ||
| + assert_false(c.wantFile('foo.py')) | ||
| + assert_false(c.wantFile('bar.py')) | ||
| + assert_true(c.wantFile('baz.py')) |
OddBloke
Jun 17, 2015
Contributor
Nope, this should work for multiple files; the option is probably named
poorly. I'll fix that up this afternoon.
On Wed, Jun 17, 2015 at 10:34 AM John Szakmeister notifications@github.com
wrote:
In unit_tests/test_cover_plugin.py
#924 (comment):
assert_true(c.wantFile('bar.py'))
- def test_want_file_inclusive_multiple_args(self):
c = _get_coverage_obj(['--cover-inclusive','--cover-exclude-file=foo.py','--cover-exclude-file=bar.py'])assert_false(c.wantFile('foo.py'))assert_false(c.wantFile('bar.py'))assert_true(c.wantFile('baz.py'))
env = {'NOSE_COVER_INCLUSIVE': 'yes','NOSE_COVER_EXCLUDE_FILE': 'bar.py,foo.py'}c = _get_coverage_obj([], env)assert_false(c.wantFile('foo.py'))assert_false(c.wantFile('bar.py'))assert_true(c.wantFile('baz.py'))Thank you for even considering to write tests!
—
Reply to this email directly or view it on GitHub
https://github.com/nose-devs/nose/pull/924/files#r32609335.
jszakmeister
Jun 17, 2015
Contributor
I'm not sure why you replied here, but I was referring to pulling the value from the environment variable--where you just shove a string in and don't do any splitting, parsing, etc. I see how it would work correctly outside of that.
OddBloke
reviewed
Jun 17, 2015
| + cover_exclude_file = options.cover_exclude_file | ||
| + else: | ||
| + cover_exclude_file = [options.cover_exclude_file] | ||
| + for pkgs in [tolist(x) for x in cover_exclude_file]: |
OddBloke
Jun 18, 2015
Contributor
Yep, that's to handle cases like '--cover-exclude-file foo,bar --cover-exclude-file bar'.
extend() (on the line below this), takes a list so this makes sense.
OddBloke commentedJun 11, 2015
This addresses half of issue #22.