Add --ignore-files option to exclude files from code coverage using path patterns #496
Conversation
| @@ -124,50 +127,54 @@ Future<void> main(List<String> arguments) async { | |||
| Environment parseArgs(List<String> arguments) { | |||
| final parser = ArgParser(); | |||
|
|
|||
| parser.addOption('sdk-root', abbr: 's', help: 'path to the SDK root'); | |||
There was a problem hiding this comment.
careful making unrelated changes. Makes reviews harder.
bin/format_coverage.dart
Outdated
| @@ -66,6 +68,7 @@ Future<void> main(List<String> arguments) async { | |||
| final hitmap = await HitMap.parseFiles( | |||
| files, | |||
| checkIgnoredLines: env.checkIgnore, | |||
| ignoreGlobs: env.ignoreFiles, | |||
There was a problem hiding this comment.
Why not add this logic in the same place as reportOn? You could just add it to the _getPathFilter function in formatter.dart. Having the logic in two totally different places makes it a bit hard to read.
There was a problem hiding this comment.
At first I wanted to put it in the same place where // coverage:ignore-file is handled, I forgot about reportsOn. I moved the logic to _getPathFilter, it's much better now. Additionally the ignore-files option can now be used without check-ignore flag, which I think makes more sense than the previous approach.
| ignoreGlobs: {Glob('**/*.g.dart'), Glob('**/util.dart')}, | ||
| ); | ||
|
|
||
| expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); |
There was a problem hiding this comment.
I think you should be able to add an extra check for this file to all the other tests in this file: expect(res, contains(p.absolute(_sampleGeneratedPath)));. That way you're checking both the contains and doesn't contain cases.
At the moment I think these tests could pass accidentally by, for example, mistyping the _sampleGeneratedPath, since every test that checks it is checking that it's not in the report.
There was a problem hiding this comment.
I added this check as well as some additional checks in globs related tests to make the expected behaviour more explicit
…ath patterns (dart-archive/coverage#496) * Add --ignore-files option, update tests * Simplify checking if any glob matches the source * Move logic to _getPathFilter * Remove unused variable, avoid excessive calls to canonicalize * Flatten ifs * Avoid exceeding line length limit * Add missing expects * Remove junk
This makes it possible to exclude files which match glob patterns from the report, which was discussed in dart-lang/tools#510 and dart-lang/tools#463. The
.yamlconfiguration part which was requested in dart-lang/tools#510 has not been implemented, but it can be done separately after this is merged.