Skip to content
This repository was archived by the owner on Aug 28, 2024. It is now read-only.

Add --ignore-files option to exclude files from code coverage using path patterns #496

Merged
merged 9 commits into from
Aug 1, 2024
Merged

Add --ignore-files option to exclude files from code coverage using path patterns #496

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7c03004
Add --ignore-files option, update tests
gogolon Jul 29, 2024
87dbbac
Simplify checking if any glob matches the source
gogolon Jul 29, 2024
06d0e78
Move logic to _getPathFilter
gogolon Jul 30, 2024
634c39b
Remove unused variable, avoid excessive calls to canonicalize
gogolon Jul 30, 2024
736378a
Flatten ifs
gogolon Jul 30, 2024
d8c4d33
Avoid exceeding line length limit
gogolon Jul 30, 2024
4fe06f5
Add missing expects
gogolon Jul 31, 2024
d066ce9
Merge
gogolon Jul 31, 2024
b5586c8
Remove junk
gogolon Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 1.8.1-wip

- Require Dart ^3.4
- Add --ignore-files option allowing to exclude files from coverage reports using glob patterns

## 1.8.0

Expand Down
102 changes: 58 additions & 44 deletions bin/format_coverage.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:io';

import 'package:args/args.dart';
import 'package:coverage/coverage.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;

/// [Environment] stores gathered arguments information.
Expand All @@ -24,6 +25,7 @@ class Environment {
required this.prettyPrintFunc,
required this.prettyPrintBranch,
required this.reportOn,
required this.ignoreFiles,
required this.sdkRoot,
required this.verbose,
required this.workers,
Expand All @@ -42,6 +44,7 @@ class Environment {
bool prettyPrintFunc;
bool prettyPrintBranch;
List<String>? reportOn;
List<String>? ignoreFiles;
String? sdkRoot;
bool verbose;
int workers;
Expand Down Expand Up @@ -76,6 +79,8 @@ Future<void> main(List<String> arguments) async {
print('Done creating global hitmap. Took ${clock.elapsedMilliseconds} ms.');
}

final ignoreGlobs = env.ignoreFiles?.map(Glob.new).toSet();

String output;
final resolver = env.bazel
? BazelResolver(workspacePath: env.bazelWorkspace)
Expand All @@ -88,12 +93,15 @@ Future<void> main(List<String> arguments) async {
if (env.prettyPrint) {
output = await hitmap.prettyPrint(resolver, loader,
reportOn: env.reportOn,
ignoreGlobs: ignoreGlobs,
reportFuncs: env.prettyPrintFunc,
reportBranches: env.prettyPrintBranch);
} else {
assert(env.lcov);
output = hitmap.formatLcov(resolver,
reportOn: env.reportOn, basePath: env.baseDirectory);
reportOn: env.reportOn,
ignoreGlobs: ignoreGlobs,
basePath: env.baseDirectory);
}

env.output.write(output);
Expand Down Expand Up @@ -124,50 +132,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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

careful making unrelated changes. Makes reviews harder.

parser.addOption('packages',
help: '[DEPRECATED] path to the package spec file');
parser.addOption('package',
help: 'root directory of the package', defaultsTo: '.');
parser.addOption('in', abbr: 'i', help: 'input(s): may be file or directory');
parser.addOption('out',
abbr: 'o', defaultsTo: 'stdout', help: 'output: may be file or stdout');
parser.addMultiOption('report-on',
help: 'which directories or files to report coverage on');
parser.addOption('workers',
abbr: 'j', defaultsTo: '1', help: 'number of workers');
parser.addOption('bazel-workspace',
defaultsTo: '', help: 'Bazel workspace directory');
parser.addOption('base-directory',
abbr: 'b',
help: 'the base directory relative to which source paths are output');
parser.addFlag('bazel',
defaultsTo: false, help: 'use Bazel-style path resolution');
parser.addFlag('pretty-print',
abbr: 'r',
negatable: false,
help: 'convert line coverage data to pretty print format');
parser.addFlag('pretty-print-func',
abbr: 'f',
parser
..addOption('sdk-root', abbr: 's', help: 'path to the SDK root')
..addOption('packages', help: '[DEPRECATED] path to the package spec file')
..addOption('package',
help: 'root directory of the package', defaultsTo: '.')
..addOption('in', abbr: 'i', help: 'input(s): may be file or directory')
..addOption('out',
abbr: 'o', defaultsTo: 'stdout', help: 'output: may be file or stdout')
..addMultiOption('report-on',
help: 'which directories or files to report coverage on')
..addOption('workers',
abbr: 'j', defaultsTo: '1', help: 'number of workers')
..addOption('bazel-workspace',
defaultsTo: '', help: 'Bazel workspace directory')
..addOption('base-directory',
abbr: 'b',
help: 'the base directory relative to which source paths are output')
..addFlag('bazel',
defaultsTo: false, help: 'use Bazel-style path resolution')
..addFlag('pretty-print',
abbr: 'r',
negatable: false,
help: 'convert line coverage data to pretty print format')
..addFlag('pretty-print-func',
abbr: 'f',
negatable: false,
help: 'convert function coverage data to pretty print format')
..addFlag('pretty-print-branch',
negatable: false,
help: 'convert branch coverage data to pretty print format')
..addFlag('lcov',
abbr: 'l',
negatable: false,
help: 'convert coverage data to lcov format')
..addFlag('verbose', abbr: 'v', negatable: false, help: 'verbose output')
..addFlag(
'check-ignore',
abbr: 'c',
negatable: false,
help: 'convert function coverage data to pretty print format');
parser.addFlag('pretty-print-branch',
negatable: false,
help: 'convert branch coverage data to pretty print format');
parser.addFlag('lcov',
abbr: 'l',
negatable: false,
help: 'convert coverage data to lcov format');
parser.addFlag('verbose',
abbr: 'v', negatable: false, help: 'verbose output');
parser.addFlag(
'check-ignore',
abbr: 'c',
negatable: false,
help: 'check for coverage ignore comments.'
' Not supported in web coverage.',
);
parser.addFlag('help', abbr: 'h', negatable: false, help: 'show this help');
help: 'check for coverage ignore comments.'
' Not supported in web coverage.',
)
..addMultiOption(
'ignore-files',
defaultsTo: [],
help: 'Ignore files by glob patterns',
)
..addFlag('help', abbr: 'h', negatable: false, help: 'show this help');

final args = parser.parse(arguments);

Expand Down Expand Up @@ -261,6 +273,7 @@ Environment parseArgs(List<String> arguments) {
}

final checkIgnore = args['check-ignore'] as bool;
final ignoredGlobs = args['ignore-files'] as List<String>;
final verbose = args['verbose'] as bool;
return Environment(
baseDirectory: baseDirectory,
Expand All @@ -276,6 +289,7 @@ Environment parseArgs(List<String> arguments) {
prettyPrintFunc: prettyPrintFunc,
prettyPrintBranch: prettyPrintBranch,
reportOn: reportOn,
ignoreFiles: ignoredGlobs,
sdkRoot: sdkRoot,
verbose: verbose,
workers: workers);
Expand Down
36 changes: 29 additions & 7 deletions lib/src/formatter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;

import 'hitmap.dart';
Expand Down Expand Up @@ -76,8 +77,12 @@ extension FileHitMapsFormatter on Map<String, HitMap> {
Resolver resolver, {
String? basePath,
List<String>? reportOn,
Set<Glob>? ignoreGlobs,
}) {
final pathFilter = _getPathFilter(reportOn);
final pathFilter = _getPathFilter(
reportOn: reportOn,
ignoreGlobs: ignoreGlobs,
);
final buf = StringBuffer();
for (final entry in entries) {
final v = entry.value;
Expand Down Expand Up @@ -136,10 +141,14 @@ extension FileHitMapsFormatter on Map<String, HitMap> {
Resolver resolver,
Loader loader, {
List<String>? reportOn,
Set<Glob>? ignoreGlobs,
bool reportFuncs = false,
bool reportBranches = false,
}) async {
final pathFilter = _getPathFilter(reportOn);
final pathFilter = _getPathFilter(
reportOn: reportOn,
ignoreGlobs: ignoreGlobs,
);
final buf = StringBuffer();
for (final entry in entries) {
final v = entry.value;
Expand Down Expand Up @@ -192,10 +201,23 @@ const _prefix = ' ';

typedef _PathFilter = bool Function(String path);

_PathFilter _getPathFilter(List<String>? reportOn) {
if (reportOn == null) return (String path) => true;
_PathFilter _getPathFilter({List<String>? reportOn, Set<Glob>? ignoreGlobs}) {
if (reportOn == null && ignoreGlobs == null) return (String path) => true;

final absolutePaths = reportOn.map(p.canonicalize).toList();
return (String path) =>
absolutePaths.any((item) => p.canonicalize(path).startsWith(item));
final absolutePaths = reportOn?.map(p.canonicalize).toList();

return (String path) {
final canonicalizedPath = p.canonicalize(path);

if (absolutePaths != null &&
!absolutePaths.any(canonicalizedPath.startsWith)) {
return false;
}
if (ignoreGlobs != null &&
ignoreGlobs.any((glob) => glob.matches(canonicalizedPath))) {
return false;
}

return true;
};
}
1 change: 1 addition & 0 deletions pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ environment:

dependencies:
args: ^2.0.0
glob: ^2.1.2
logging: ^1.0.0
package_config: ^2.0.0
path: ^1.8.0
Expand Down
92 changes: 79 additions & 13 deletions test/lcov_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import 'dart:io';

import 'package:coverage/coverage.dart';
import 'package:coverage/src/util.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:test_process/test_process.dart';

final _sampleAppPath = p.join('test', 'test_files', 'test_app.dart');
final _sampleGeneratedPath = p.join('test', 'test_files', 'test_app.g.dart');
final _isolateLibPath = p.join('test', 'test_files', 'test_app_isolate.dart');

final _sampleAppFileUri = p.toUri(p.absolute(_sampleAppPath)).toString();
Expand All @@ -31,19 +33,19 @@ void main() {
final sampleAppFuncNames = sampleAppHitMap?.funcNames;
final sampleAppBranchHits = sampleAppHitMap?.branchHits;

expect(sampleAppHitLines, containsPair(46, greaterThanOrEqualTo(1)),
expect(sampleAppHitLines, containsPair(53, greaterThanOrEqualTo(1)),
reason: 'be careful if you modify the test file');
expect(sampleAppHitLines, containsPair(50, 0),
expect(sampleAppHitLines, containsPair(57, 0),
reason: 'be careful if you modify the test file');
expect(sampleAppHitLines, isNot(contains(32)),
expect(sampleAppHitLines, isNot(contains(39)),
reason: 'be careful if you modify the test file');
expect(sampleAppHitFuncs, containsPair(45, 1),
expect(sampleAppHitFuncs, containsPair(52, 1),
reason: 'be careful if you modify the test file');
expect(sampleAppHitFuncs, containsPair(49, 0),
expect(sampleAppHitFuncs, containsPair(56, 0),
reason: 'be careful if you modify the test file');
expect(sampleAppFuncNames, containsPair(45, 'usedMethod'),
expect(sampleAppFuncNames, containsPair(52, 'usedMethod'),
reason: 'be careful if you modify the test file');
expect(sampleAppBranchHits, containsPair(41, 1),
expect(sampleAppBranchHits, containsPair(48, 1),
reason: 'be careful if you modify the test file');
});

Expand All @@ -59,17 +61,17 @@ void main() {
final sampleAppHitFuncs = sampleAppHitMap?.funcHits;
final sampleAppFuncNames = sampleAppHitMap?.funcNames;

expect(sampleAppHitLines, containsPair(46, greaterThanOrEqualTo(1)),
expect(sampleAppHitLines, containsPair(53, greaterThanOrEqualTo(1)),
reason: 'be careful if you modify the test file');
expect(sampleAppHitLines, containsPair(50, 0),
expect(sampleAppHitLines, containsPair(57, 0),
reason: 'be careful if you modify the test file');
expect(sampleAppHitLines, isNot(contains(32)),
expect(sampleAppHitLines, isNot(contains(39)),
reason: 'be careful if you modify the test file');
expect(sampleAppHitFuncs, containsPair(45, 1),
expect(sampleAppHitFuncs, containsPair(52, 1),
reason: 'be careful if you modify the test file');
expect(sampleAppHitFuncs, containsPair(49, 0),
expect(sampleAppHitFuncs, containsPair(56, 0),
reason: 'be careful if you modify the test file');
expect(sampleAppFuncNames, containsPair(45, 'usedMethod'),
expect(sampleAppFuncNames, containsPair(52, 'usedMethod'),
reason: 'be careful if you modify the test file');
});

Expand Down Expand Up @@ -122,6 +124,37 @@ void main() {
expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart'))));
});

test('formatLcov() excludes files matching glob patterns', () async {
final hitmap = await _getHitMap();

final resolver = await Resolver.create(packagePath: '.');
final res = hitmap.formatLcov(
resolver,
ignoreGlobs: {Glob('**/*.g.dart'), Glob('**/util.dart')},
);

expect(res, isNot(contains(p.absolute(_sampleGeneratedPath))));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this check as well as some additional checks in globs related tests to make the expected behaviour more explicit

expect(
res,
isNot(contains(p.absolute(p.join('lib', 'src', 'util.dart')))),
);
});

test(
'formatLcov() excludes files matching glob patterns regardless of their'
'presence on reportOn list', () async {
final hitmap = await _getHitMap();

final resolver = await Resolver.create(packagePath: '.');
final res = hitmap.formatLcov(
resolver,
reportOn: ['test/'],
ignoreGlobs: {Glob('**/*.g.dart')},
);

expect(res, isNot(contains(p.absolute(_sampleGeneratedPath))));
});

test('formatLcov() uses paths relative to basePath', () async {
final hitmap = await _getHitMap();

Expand Down Expand Up @@ -209,6 +242,39 @@ void main() {
expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart'))));
});

test('prettyPrint() excludes files matching glob patterns', () async {
final hitmap = await _getHitMap();

final resolver = await Resolver.create(packagePath: '.');
final res = await hitmap.prettyPrint(
resolver,
Loader(),
ignoreGlobs: {Glob('**/*.g.dart'), Glob('**/util.dart')},
);

expect(res, isNot(contains(p.absolute(_sampleGeneratedPath))));
expect(
res,
isNot(contains(p.absolute(p.join('lib', 'src', 'util.dart')))),
);
});

test(
'prettyPrint() excludes files matching glob patterns regardless of'
'their presence on reportOn list', () async {
final hitmap = await _getHitMap();

final resolver = await Resolver.create(packagePath: '.');
final res = await hitmap.prettyPrint(
resolver,
Loader(),
reportOn: ['test/'],
ignoreGlobs: {Glob('**/*.g.dart')},
);

expect(res, isNot(contains(p.absolute(_sampleGeneratedPath))));
});

test('prettyPrint() functions', () async {
final hitmap = await _getHitMap();

Expand Down
2 changes: 1 addition & 1 deletion test/run_and_collect_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class ThrowingResolver implements Resolver {

void checkIgnoredLinesInFilesCache(
Map<String, List<List<int>>?> ignoredLinesInFilesCache) {
expect(ignoredLinesInFilesCache.length, 3);
expect(ignoredLinesInFilesCache.length, 4);
final keys = ignoredLinesInFilesCache.keys.toList();
final testAppKey =
keys.where((element) => element.endsWith('test_app.dart')).single;
Expand Down
Loading