From 43ac0e33f0181830ebe238fa6266a870597f78c9 Mon Sep 17 00:00:00 2001 From: Jakub Lange <31567861+gogolon@users.noreply.github.com> Date: Thu, 1 Aug 2024 02:32:42 +0200 Subject: [PATCH] Add --ignore-files option to exclude files from code coverage using path patterns (#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 --- CHANGELOG.md | 1 + bin/format_coverage.dart | 102 ++++++++++++++++------------ lib/src/formatter.dart | 36 ++++++++-- pubspec.yaml | 1 + test/lcov_test.dart | 114 ++++++++++++++++++++++++++++---- test/run_and_collect_test.dart | 2 +- test/test_files/test_app.dart | 7 ++ test/test_files/test_app.g.dart | 5 ++ 8 files changed, 203 insertions(+), 65 deletions(-) create mode 100644 test/test_files/test_app.g.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index c8920f53..ef4e97e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Require Dart ^3.4 - Fix bug where some ranges were able to bypass the `--scope-output` filters. +- Add --ignore-files option allowing to exclude files from coverage reports using glob patterns ## 1.8.0 diff --git a/bin/format_coverage.dart b/bin/format_coverage.dart index 540f1b1d..d24f60d9 100644 --- a/bin/format_coverage.dart +++ b/bin/format_coverage.dart @@ -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. @@ -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, @@ -42,6 +44,7 @@ class Environment { bool prettyPrintFunc; bool prettyPrintBranch; List? reportOn; + List? ignoreFiles; String? sdkRoot; bool verbose; int workers; @@ -76,6 +79,8 @@ Future main(List 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) @@ -88,12 +93,15 @@ Future main(List 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); @@ -124,50 +132,54 @@ Future main(List arguments) async { Environment parseArgs(List arguments) { final parser = ArgParser(); - parser.addOption('sdk-root', abbr: 's', help: 'path to the SDK root'); - 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); @@ -261,6 +273,7 @@ Environment parseArgs(List arguments) { } final checkIgnore = args['check-ignore'] as bool; + final ignoredGlobs = args['ignore-files'] as List; final verbose = args['verbose'] as bool; return Environment( baseDirectory: baseDirectory, @@ -276,6 +289,7 @@ Environment parseArgs(List arguments) { prettyPrintFunc: prettyPrintFunc, prettyPrintBranch: prettyPrintBranch, reportOn: reportOn, + ignoreFiles: ignoredGlobs, sdkRoot: sdkRoot, verbose: verbose, workers: workers); diff --git a/lib/src/formatter.dart b/lib/src/formatter.dart index a7a96c1d..a37df73b 100644 --- a/lib/src/formatter.dart +++ b/lib/src/formatter.dart @@ -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'; @@ -76,8 +77,12 @@ extension FileHitMapsFormatter on Map { Resolver resolver, { String? basePath, List? reportOn, + Set? ignoreGlobs, }) { - final pathFilter = _getPathFilter(reportOn); + final pathFilter = _getPathFilter( + reportOn: reportOn, + ignoreGlobs: ignoreGlobs, + ); final buf = StringBuffer(); for (final entry in entries) { final v = entry.value; @@ -136,10 +141,14 @@ extension FileHitMapsFormatter on Map { Resolver resolver, Loader loader, { List? reportOn, + Set? 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; @@ -192,10 +201,23 @@ const _prefix = ' '; typedef _PathFilter = bool Function(String path); -_PathFilter _getPathFilter(List? reportOn) { - if (reportOn == null) return (String path) => true; +_PathFilter _getPathFilter({List? reportOn, Set? 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; + }; } diff --git a/pubspec.yaml b/pubspec.yaml index e3c50ba1..b68f928c 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -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 diff --git a/test/lcov_test.dart b/test/lcov_test.dart index d2af9f99..ca62117c 100644 --- a/test/lcov_test.dart +++ b/test/lcov_test.dart @@ -7,14 +7,18 @@ 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(); +final _sampleGeneratedFileUri = + p.toUri(p.absolute(_sampleGeneratedPath)).toString(); final _isolateLibFileUri = p.toUri(p.absolute(_isolateLibPath)).toString(); void main() { @@ -22,6 +26,7 @@ void main() { final hitmap = await _getHitMap(); expect(hitmap, contains(_sampleAppFileUri)); + expect(hitmap, contains(_sampleGeneratedFileUri)); expect(hitmap, contains(_isolateLibFileUri)); expect(hitmap, contains('package:coverage/src/util.dart')); @@ -31,19 +36,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'); }); @@ -51,6 +56,7 @@ void main() { final hitmap = await _getHitMap(); expect(hitmap, contains(_sampleAppFileUri)); + expect(hitmap, contains(_sampleGeneratedFileUri)); expect(hitmap, contains(_isolateLibFileUri)); expect(hitmap, contains('package:coverage/src/util.dart')); @@ -59,17 +65,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'); }); @@ -85,6 +91,7 @@ void main() { .format(hitmap.map((key, value) => MapEntry(key, value.lineHits))); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); }); @@ -96,6 +103,7 @@ void main() { final res = hitmap.formatLcov(resolver); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); }); @@ -107,6 +115,7 @@ void main() { final res = hitmap.formatLcov(resolver, reportOn: ['lib/', 'test/']); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); }); @@ -118,10 +127,46 @@ void main() { final res = hitmap.formatLcov(resolver, reportOn: ['lib/']); expect(res, isNot(contains(p.absolute(_sampleAppPath)))); + expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); expect(res, isNot(contains(p.absolute(_isolateLibPath)))); 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, contains(p.absolute(_sampleAppPath))); + expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); + expect(res, contains(p.absolute(_isolateLibPath))); + 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, contains(p.absolute(_sampleAppPath))); + expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); + expect(res, contains(p.absolute(_isolateLibPath))); + }); + test('formatLcov() uses paths relative to basePath', () async { final hitmap = await _getHitMap(); @@ -146,6 +191,7 @@ void main() { .format(hitmap.map((key, value) => MapEntry(key, value.lineHits))); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); @@ -169,6 +215,7 @@ void main() { final res = await hitmap.prettyPrint(resolver, Loader()); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); @@ -193,6 +240,7 @@ void main() { .prettyPrint(resolver, Loader(), reportOn: ['lib/', 'test/']); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); }); @@ -205,10 +253,48 @@ void main() { await hitmap.prettyPrint(resolver, Loader(), reportOn: ['lib/']); expect(res, isNot(contains(p.absolute(_sampleAppPath)))); + expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); expect(res, isNot(contains(p.absolute(_isolateLibPath)))); 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, contains(p.absolute(_sampleAppPath))); + expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); + expect(res, contains(p.absolute(_isolateLibPath))); + 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, contains(p.absolute(_sampleAppPath))); + expect(res, isNot(contains(p.absolute(_sampleGeneratedPath)))); + expect(res, contains(p.absolute(_isolateLibPath))); + }); + test('prettyPrint() functions', () async { final hitmap = await _getHitMap(); @@ -217,6 +303,7 @@ void main() { await hitmap.prettyPrint(resolver, Loader(), reportFuncs: true); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); @@ -235,6 +322,7 @@ void main() { await hitmap.prettyPrint(resolver, Loader(), reportBranches: true); expect(res, contains(p.absolute(_sampleAppPath))); + expect(res, contains(p.absolute(_sampleGeneratedPath))); expect(res, contains(p.absolute(_isolateLibPath))); expect(res, contains(p.absolute(p.join('lib', 'src', 'util.dart')))); diff --git a/test/run_and_collect_test.dart b/test/run_and_collect_test.dart index e21e353e..733fc744 100644 --- a/test/run_and_collect_test.dart +++ b/test/run_and_collect_test.dart @@ -73,7 +73,7 @@ class ThrowingResolver implements Resolver { void checkIgnoredLinesInFilesCache( Map>?> 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; diff --git a/test/test_files/test_app.dart b/test/test_files/test_app.dart index 231486a8..1e507033 100644 --- a/test/test_files/test_app.dart +++ b/test/test_files/test_app.dart @@ -12,6 +12,8 @@ import 'package:coverage/src/util.dart'; import 'test_app_isolate.dart'; +part 'test_app.g.dart'; + Future main() async { for (var i = 0; i < 10; i++) { for (var j = 0; j < 10; j++) { @@ -19,6 +21,11 @@ Future main() async { if (sum != (i + j)) { throw 'bad method!'; } + + final multiplication = usedGeneratedMethod(i, j); + if (multiplication != i * j) { + throw 'bad generated method!'; + } } } diff --git a/test/test_files/test_app.g.dart b/test/test_files/test_app.g.dart new file mode 100644 index 00000000..c80718ae --- /dev/null +++ b/test/test_files/test_app.g.dart @@ -0,0 +1,5 @@ +part of 'test_app.dart'; + +int usedGeneratedMethod(int a, int b) { + return a * b; +}