From 83ef279297b32c75fc3f15878ae12754c9bef1d7 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 8 Mar 2022 10:04:22 -0800 Subject: [PATCH 1/8] Migrate tests to package_config.json --- test/collect_coverage_test.dart | 3 ++- test/lcov_test.dart | 22 +++++++++++----------- tool/test_and_collect.sh | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/test/collect_coverage_test.dart b/test/collect_coverage_test.dart index 7261eb75..222de541 100644 --- a/test/collect_coverage_test.dart +++ b/test/collect_coverage_test.dart @@ -299,7 +299,8 @@ void main() { await outputFile.writeAsString(coverageResults, flush: true); final parsedResult = await HitMap.parseFiles([outputFile], - packagesPath: '.packages', checkIgnoredLines: true); + packagesPath: '.dart_tool/package_config.json', + checkIgnoredLines: true); // This file has ignore:coverage-file. expect(parsedResult, isNot(contains(_sampleAppFileUri))); diff --git a/test/lcov_test.dart b/test/lcov_test.dart index b6995896..40cc6401 100644 --- a/test/lcov_test.dart +++ b/test/lcov_test.dart @@ -79,7 +79,7 @@ void main() { test('format()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); // ignore: deprecated_member_use_from_same_package final formatter = LcovFormatter(resolver); @@ -94,7 +94,7 @@ void main() { test('formatLcov()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = hitmap.formatLcov(resolver); expect(res, contains(p.absolute(_sampleAppPath))); @@ -105,7 +105,7 @@ void main() { test('formatLcov() includes files in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = hitmap.formatLcov(resolver, reportOn: ['lib/', 'test/']); expect(res, contains(p.absolute(_sampleAppPath))); @@ -116,7 +116,7 @@ void main() { test('formatLcov() excludes files not in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = hitmap.formatLcov(resolver, reportOn: ['lib/']); expect(res, isNot(contains(p.absolute(_sampleAppPath)))); @@ -127,7 +127,7 @@ void main() { test('formatLcov() uses paths relative to basePath', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = hitmap.formatLcov(resolver, basePath: p.absolute('lib')); expect( @@ -140,7 +140,7 @@ void main() { test('format()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); // ignore: deprecated_member_use_from_same_package final formatter = PrettyPrintFormatter(resolver, Loader()); @@ -167,7 +167,7 @@ void main() { test('prettyPrint()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = await hitmap.prettyPrint(resolver, Loader()); expect(res, contains(p.absolute(_sampleAppPath))); @@ -190,7 +190,7 @@ void main() { test('prettyPrint() includes files in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = await hitmap .prettyPrint(resolver, Loader(), reportOn: ['lib/', 'test/']); @@ -202,7 +202,7 @@ void main() { test('prettyPrint() excludes files not in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = await hitmap.prettyPrint(resolver, Loader(), reportOn: ['lib/']); @@ -214,7 +214,7 @@ void main() { test('prettyPrint() functions', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = await hitmap.prettyPrint(resolver, Loader(), reportFuncs: true); @@ -232,7 +232,7 @@ void main() { test('prettyPrint() branches', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.packages'); + final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); final res = await hitmap.prettyPrint(resolver, Loader(), reportBranches: true); diff --git a/tool/test_and_collect.sh b/tool/test_and_collect.sh index 1d9e7fec..484841a9 100755 --- a/tool/test_and_collect.sh +++ b/tool/test_and_collect.sh @@ -30,7 +30,7 @@ if [[ $(dart --version 2>&1 ) =~ '(dev)' ]]; then --lcov \ --in=var/coverage.json \ --out=var/lcov.info \ - --packages=.packages \ + --packages=.dart_tool/package_config.json \ --report-on=lib \ --check-ignore fi From e695a6f22d6ed81b2761751a43ba5431e0405aaa Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 10 Mar 2022 11:32:50 -0800 Subject: [PATCH 2/8] Swap --packages flag for --package --- CHANGELOG.md | 4 ++++ README.md | 7 ++++--- bin/format_coverage.dart | 23 ++++++++++++----------- lib/src/hitmap.dart | 21 +++++++++++++++------ lib/src/resolver.dart | 32 ++++++++++++++++++++++++++++++-- test/collect_coverage_test.dart | 3 +-- test/lcov_test.dart | 22 +++++++++++----------- test/resolver_test.dart | 15 +++++++++++---- tool/test_and_collect.sh | 1 - 9 files changed, 88 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef3500d5..de111d40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ is collected. * Add a `branchHits` field to `HitMap`. * Add support for scraping the service URI from the new Dart VM service message. +* Replace `--packages` flag with `--package`, which takes the package's root + directory, instead of the .package file. +* Deprecate the packagesPath parameter and add packagePath instead, in + `HitMap.parseJson`, `HitMap.parseFiles`, `createHitmap`, and `parseCoverage`. ## 1.1.0 - 2022-1-18 diff --git a/README.md b/README.md index 3d038df8..e507380e 100644 --- a/README.md +++ b/README.md @@ -44,17 +44,18 @@ all isolates are paused before collecting coverage. #### Formatting coverage data ``` -pub global run coverage:format_coverage --packages=app_package/.packages -i coverage.json +pub global run coverage:format_coverage --package=app_package -i coverage.json ``` or if the `pub global run` exectuables are on your PATH, ``` -format_coverage --packages=app_package/.packages -i coverage.json +format_coverage --package=app_package -i coverage.json ``` where `app_package` is the path to the package whose coverage is being -collected. If `--sdk-root` is set, Dart SDK coverage will also be output. +collected (defaults to the current working directory). If `--sdk-root` is set, +Dart SDK coverage will also be output. #### Ignore lines from coverage diff --git a/bin/format_coverage.dart b/bin/format_coverage.dart index 63fc48c7..2e05d450 100644 --- a/bin/format_coverage.dart +++ b/bin/format_coverage.dart @@ -18,7 +18,7 @@ class Environment { required this.input, required this.lcov, required this.output, - required this.packagesPath, + required this.packagePath, required this.prettyPrint, required this.prettyPrintFunc, required this.prettyPrintBranch, @@ -35,7 +35,7 @@ class Environment { String input; bool lcov; IOSink output; - String? packagesPath; + String? packagePath; bool prettyPrint; bool prettyPrintFunc; bool prettyPrintBranch; @@ -54,7 +54,7 @@ Future main(List arguments) async { print(' # files: ${files.length}'); print(' # workers: ${env.workers}'); print(' sdk-root: ${env.sdkRoot}'); - print(' package-spec: ${env.packagesPath}'); + print(' package-path: ${env.packagePath}'); print(' report-on: ${env.reportOn}'); print(' check-ignore: ${env.checkIgnore}'); } @@ -63,7 +63,7 @@ Future main(List arguments) async { final hitmap = await HitMap.parseFiles( files, checkIgnoredLines: env.checkIgnore, - packagesPath: env.packagesPath, + packagePath: env.packagePath, ); // All workers are done. Process the data. @@ -74,7 +74,7 @@ Future main(List arguments) async { String output; final resolver = env.bazel ? BazelResolver(workspacePath: env.bazelWorkspace) - : Resolver(packagesPath: env.packagesPath, sdkRoot: env.sdkRoot); + : Resolver(packagePath: env.packagePath, sdkRoot: env.sdkRoot); final loader = Loader(); if (env.prettyPrint) { output = await hitmap.prettyPrint(resolver, loader, @@ -116,7 +116,8 @@ Environment parseArgs(List arguments) { final parser = ArgParser(); parser.addOption('sdk-root', abbr: 's', help: 'path to the SDK root'); - parser.addOption('packages', help: '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'); @@ -184,10 +185,10 @@ Environment parseArgs(List arguments) { } } - final packagesPath = args['packages'] as String?; - if (packagesPath != null) { - if (!FileSystemEntity.isFileSync(packagesPath)) { - fail('Package spec "${args["packages"]}" not found, or not a file.'); + final packagePath = args['package'] as String?; + if (packagePath != null) { + if (!FileSystemEntity.isDirectorySync(packagePath)) { + fail('Package spec "${args["package"]}" not found, or not a directory.'); } } @@ -253,7 +254,7 @@ Environment parseArgs(List arguments) { input: input, lcov: lcov, output: output, - packagesPath: packagesPath, + packagePath: packagePath, prettyPrint: prettyPrint, prettyPrintFunc: prettyPrintFunc, prettyPrintBranch: prettyPrintBranch, diff --git a/lib/src/hitmap.dart b/lib/src/hitmap.dart index f94754e7..0e6cdebd 100644 --- a/lib/src/hitmap.dart +++ b/lib/src/hitmap.dart @@ -39,9 +39,11 @@ class HitMap { static Future> parseJson( List> jsonResult, { bool checkIgnoredLines = false, - String? packagesPath, + @Deprecated('Use packagePath') String? packagesPath, + String? packagePath, }) async { - final resolver = Resolver(packagesPath: packagesPath); + final resolver = await Resolver.create( + packagesPath: packagesPath, packagePath: packagePath); final loader = Loader(); // Map of source file to map of line to hit count for that line. @@ -159,7 +161,8 @@ class HitMap { static Future> parseFiles( Iterable files, { bool checkIgnoredLines = false, - String? packagesPath, + @Deprecated('Use packagePath') String? packagesPath, + String? packagePath, }) async { final globalHitmap = {}; for (var file in files) { @@ -171,6 +174,7 @@ class HitMap { jsonResult.cast>(), checkIgnoredLines: checkIgnoredLines, packagesPath: packagesPath, + packagePath: packagePath, )); } } @@ -239,12 +243,14 @@ class _HitInfo { Future>> createHitmap( List> jsonResult, { bool checkIgnoredLines = false, - String? packagesPath, + @Deprecated('Use packagePath') String? packagesPath, + String? packagePath, }) async { final result = await HitMap.parseJson( jsonResult, checkIgnoredLines: checkIgnoredLines, packagesPath: packagesPath, + packagePath: packagePath, ); return result.map((key, value) => MapEntry(key, value.lineHits)); } @@ -276,10 +282,13 @@ Future>> parseCoverage( Iterable files, int _, { bool checkIgnoredLines = false, - String? packagesPath, + @Deprecated('Use packagePath') String? packagesPath, + String? packagePath, }) async { final result = await HitMap.parseFiles(files, - checkIgnoredLines: checkIgnoredLines, packagesPath: packagesPath); + checkIgnoredLines: checkIgnoredLines, + packagesPath: packagesPath, + packagePath: packagePath); return result.map((key, value) => MapEntry(key, value.lineHits)); } diff --git a/lib/src/resolver.dart b/lib/src/resolver.dart index 8794d1fa..4072ad50 100644 --- a/lib/src/resolver.dart +++ b/lib/src/resolver.dart @@ -10,10 +10,30 @@ import 'package:path/path.dart' as p; /// [Resolver] resolves imports with respect to a given environment. class Resolver { - Resolver({this.packagesPath, this.sdkRoot}) - : _packages = packagesPath != null ? _parsePackages(packagesPath) : null; + Resolver( + {this.packagesPath, + this.packagePath, + this.sdkRoot, + Map? packages}) + : _packages = packages; + + static Future create({ + @Deprecated('Use packagePath') String? packagesPath, + String? packagePath, + String? sdkRoot, + }) async { + return Resolver( + packagesPath: packagesPath, + packagePath: packagePath, + sdkRoot: sdkRoot, + packages: packagesPath != null + ? _parsePackages(packagesPath) + : (packagePath != null ? await _parsePackage(packagePath) : null), + ); + } final String? packagesPath; + final String? packagePath; final String? sdkRoot; final List failed = []; final Map? _packages; @@ -112,6 +132,14 @@ class Resolver { return packageMap; } } + + static Future?> _parsePackage(String packagePath) async { + final parsed = await findPackageConfig(Directory(packagePath)); + if (parsed == null) return null; + return { + for (var package in parsed.packages) package.name: package.packageUriRoot + }; + } } /// Bazel URI resolver. diff --git a/test/collect_coverage_test.dart b/test/collect_coverage_test.dart index 222de541..0a056ea2 100644 --- a/test/collect_coverage_test.dart +++ b/test/collect_coverage_test.dart @@ -299,8 +299,7 @@ void main() { await outputFile.writeAsString(coverageResults, flush: true); final parsedResult = await HitMap.parseFiles([outputFile], - packagesPath: '.dart_tool/package_config.json', - checkIgnoredLines: true); + packagePath: '.', checkIgnoredLines: true); // This file has ignore:coverage-file. expect(parsedResult, isNot(contains(_sampleAppFileUri))); diff --git a/test/lcov_test.dart b/test/lcov_test.dart index 40cc6401..a32f3fdc 100644 --- a/test/lcov_test.dart +++ b/test/lcov_test.dart @@ -79,7 +79,7 @@ void main() { test('format()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); // ignore: deprecated_member_use_from_same_package final formatter = LcovFormatter(resolver); @@ -94,7 +94,7 @@ void main() { test('formatLcov()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = hitmap.formatLcov(resolver); expect(res, contains(p.absolute(_sampleAppPath))); @@ -105,7 +105,7 @@ void main() { test('formatLcov() includes files in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = hitmap.formatLcov(resolver, reportOn: ['lib/', 'test/']); expect(res, contains(p.absolute(_sampleAppPath))); @@ -116,7 +116,7 @@ void main() { test('formatLcov() excludes files not in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = hitmap.formatLcov(resolver, reportOn: ['lib/']); expect(res, isNot(contains(p.absolute(_sampleAppPath)))); @@ -127,7 +127,7 @@ void main() { test('formatLcov() uses paths relative to basePath', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = hitmap.formatLcov(resolver, basePath: p.absolute('lib')); expect( @@ -140,7 +140,7 @@ void main() { test('format()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); // ignore: deprecated_member_use_from_same_package final formatter = PrettyPrintFormatter(resolver, Loader()); @@ -167,7 +167,7 @@ void main() { test('prettyPrint()', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = await hitmap.prettyPrint(resolver, Loader()); expect(res, contains(p.absolute(_sampleAppPath))); @@ -190,7 +190,7 @@ void main() { test('prettyPrint() includes files in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = await hitmap .prettyPrint(resolver, Loader(), reportOn: ['lib/', 'test/']); @@ -202,7 +202,7 @@ void main() { test('prettyPrint() excludes files not in reportOn list', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = await hitmap.prettyPrint(resolver, Loader(), reportOn: ['lib/']); @@ -214,7 +214,7 @@ void main() { test('prettyPrint() functions', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = await hitmap.prettyPrint(resolver, Loader(), reportFuncs: true); @@ -232,7 +232,7 @@ void main() { test('prettyPrint() branches', () async { final hitmap = await _getHitMap(); - final resolver = Resolver(packagesPath: '.dart_tool/package_config.json'); + final resolver = await Resolver.create(packagePath: '.'); final res = await hitmap.prettyPrint(resolver, Loader(), reportBranches: true); diff --git a/test/resolver_test.dart b/test/resolver_test.dart index c66e6949..203af613 100644 --- a/test/resolver_test.dart +++ b/test/resolver_test.dart @@ -37,7 +37,7 @@ foo:file:///${d.sandbox}/foo/lib }); test('can be created from a package_config.json', () async { - final resolver = Resolver( + final resolver = await Resolver.create( packagesPath: p.join(d.sandbox, 'foo', '.dart_tool', 'package_config.json')); expect(resolver.resolve('package:foo/foo.dart'), @@ -45,16 +45,23 @@ foo:file:///${d.sandbox}/foo/lib }); test('can be created from a .packages file', () async { + final resolver = await Resolver.create( + packagesPath: p.join(d.sandbox, 'foo', '.packages')); + expect(resolver.resolve('package:foo/foo.dart'), + '${d.sandbox}/foo/lib/foo.dart'); + }); + + test('can be created from a package directory', () async { final resolver = - Resolver(packagesPath: p.join(d.sandbox, 'foo', '.packages')); + await Resolver.create(packagePath: p.join(d.sandbox, 'foo')); expect(resolver.resolve('package:foo/foo.dart'), '${d.sandbox}/foo/lib/foo.dart'); }); test('errors if the packagesFile is an unknown format', () async { expect( - () => - Resolver(packagesPath: p.join(d.sandbox, 'foo', '.bad.packages')), + () => await Resolver.create( + packagesPath: p.join(d.sandbox, 'foo', '.bad.packages')), throwsA(isA())); }); }); diff --git a/tool/test_and_collect.sh b/tool/test_and_collect.sh index 484841a9..ff0d396a 100755 --- a/tool/test_and_collect.sh +++ b/tool/test_and_collect.sh @@ -30,7 +30,6 @@ if [[ $(dart --version 2>&1 ) =~ '(dev)' ]]; then --lcov \ --in=var/coverage.json \ --out=var/lcov.info \ - --packages=.dart_tool/package_config.json \ --report-on=lib \ --check-ignore fi From 428d652341c92dc3f9133c5f68b40943274ce9d2 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 10 Mar 2022 12:27:12 -0800 Subject: [PATCH 3/8] Fix analysis errors --- lib/src/hitmap.dart | 1 + lib/src/resolver.dart | 2 +- test/resolver_test.dart | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/src/hitmap.dart b/lib/src/hitmap.dart index 0e6cdebd..6b9b6226 100644 --- a/lib/src/hitmap.dart +++ b/lib/src/hitmap.dart @@ -173,6 +173,7 @@ class HitMap { globalHitmap.merge(await HitMap.parseJson( jsonResult.cast>(), checkIgnoredLines: checkIgnoredLines, + // ignore: deprecated_member_use_from_same_package packagesPath: packagesPath, packagePath: packagePath, )); diff --git a/lib/src/resolver.dart b/lib/src/resolver.dart index 4072ad50..6ef09b01 100644 --- a/lib/src/resolver.dart +++ b/lib/src/resolver.dart @@ -18,7 +18,7 @@ class Resolver { : _packages = packages; static Future create({ - @Deprecated('Use packagePath') String? packagesPath, + String? packagesPath, String? packagePath, String? sdkRoot, }) async { diff --git a/test/resolver_test.dart b/test/resolver_test.dart index 203af613..5eadd47e 100644 --- a/test/resolver_test.dart +++ b/test/resolver_test.dart @@ -60,7 +60,7 @@ foo:file:///${d.sandbox}/foo/lib test('errors if the packagesFile is an unknown format', () async { expect( - () => await Resolver.create( + () async => await Resolver.create( packagesPath: p.join(d.sandbox, 'foo', '.bad.packages')), throwsA(isA())); }); From 2825085c926216861128dc7c4ffb067348caada8 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 22 Mar 2022 09:52:53 -0700 Subject: [PATCH 4/8] Keep the --packages flag, but delete support for .package in resolver --- CHANGELOG.md | 4 ++-- bin/format_coverage.dart | 29 +++++++++++++++++++++++------ lib/src/resolver.dart | 31 +++++-------------------------- test/resolver_test.dart | 16 +++------------- 4 files changed, 33 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de111d40..cfd73e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,8 @@ is collected. * Add a `branchHits` field to `HitMap`. * Add support for scraping the service URI from the new Dart VM service message. -* Replace `--packages` flag with `--package`, which takes the package's root - directory, instead of the .package file. +* Add a `--package` flag, which takes the package's root directory, instead of + the .package file. Deprecate the `--packages` flag. * Deprecate the packagesPath parameter and add packagePath instead, in `HitMap.parseJson`, `HitMap.parseFiles`, `createHitmap`, and `parseCoverage`. diff --git a/bin/format_coverage.dart b/bin/format_coverage.dart index 2e05d450..a7e2177b 100644 --- a/bin/format_coverage.dart +++ b/bin/format_coverage.dart @@ -18,6 +18,7 @@ class Environment { required this.input, required this.lcov, required this.output, + required this.packagesPath, required this.packagePath, required this.prettyPrint, required this.prettyPrintFunc, @@ -35,7 +36,8 @@ class Environment { String input; bool lcov; IOSink output; - String? packagePath; + String? packagesPath; + String packagePath; bool prettyPrint; bool prettyPrintFunc; bool prettyPrintBranch; @@ -55,6 +57,7 @@ Future main(List arguments) async { print(' # workers: ${env.workers}'); print(' sdk-root: ${env.sdkRoot}'); print(' package-path: ${env.packagePath}'); + print(' packages-path: ${env.packagesPath}'); print(' report-on: ${env.reportOn}'); print(' check-ignore: ${env.checkIgnore}'); } @@ -63,6 +66,8 @@ Future main(List arguments) async { final hitmap = await HitMap.parseFiles( files, checkIgnoredLines: env.checkIgnore, + // ignore: deprecated_member_use_from_same_package + packagesPath: env.packagesPath, packagePath: env.packagePath, ); @@ -74,7 +79,11 @@ Future main(List arguments) async { String output; final resolver = env.bazel ? BazelResolver(workspacePath: env.bazelWorkspace) - : Resolver(packagePath: env.packagePath, sdkRoot: env.sdkRoot); + : Resolver( + packagesPath: env.packagesPath, + packagePath: env.packagePath, + sdkRoot: env.sdkRoot, + ); final loader = Loader(); if (env.prettyPrint) { output = await hitmap.prettyPrint(resolver, loader, @@ -116,6 +125,8 @@ 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'); @@ -185,13 +196,18 @@ Environment parseArgs(List arguments) { } } - final packagePath = args['package'] as String?; - if (packagePath != null) { - if (!FileSystemEntity.isDirectorySync(packagePath)) { - fail('Package spec "${args["package"]}" not found, or not a directory.'); + final packagesPath = args['packages'] as String?; + if (packagesPath != null) { + if (!FileSystemEntity.isFileSync(packagesPath)) { + fail('Package spec "${args["packages"]}" not found, or not a file.'); } } + final packagePath = args['package'] as String; + if (!FileSystemEntity.isDirectorySync(packagePath)) { + fail('Package spec "${args["package"]}" not found, or not a directory.'); + } + if (args['in'] == null) fail('No input files given.'); final input = p.absolute(p.normalize(args['in'] as String)); if (!FileSystemEntity.isDirectorySync(input) && @@ -254,6 +270,7 @@ Environment parseArgs(List arguments) { input: input, lcov: lcov, output: output, + packagesPath: packagesPath, packagePath: packagePath, prettyPrint: prettyPrint, prettyPrintFunc: prettyPrintFunc, diff --git a/lib/src/resolver.dart b/lib/src/resolver.dart index 6ef09b01..f948c5d7 100644 --- a/lib/src/resolver.dart +++ b/lib/src/resolver.dart @@ -2,7 +2,6 @@ // 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 'dart:convert'; import 'dart:io'; import 'package:package_config/package_config.dart'; @@ -106,31 +105,11 @@ class Resolver { static Map _parsePackages(String packagesPath) { final content = File(packagesPath).readAsStringSync(); - try { - final parsed = - PackageConfig.parseString(content, Uri.base.resolve(packagesPath)); - return { - for (var package in parsed.packages) - package.name: package.packageUriRoot - }; - } on FormatException catch (_) { - // It was probably an old style .packages file - final lines = LineSplitter.split(content); - final packageMap = {}; - for (var line in lines) { - if (line.startsWith('#')) continue; - final firstColon = line.indexOf(':'); - if (firstColon == -1) { - throw FormatException( - 'Unexpected package config format, expected an old style ' - '.packages file or new style package_config.json file.', - content); - } - packageMap[line.substring(0, firstColon)] = - Uri.parse(line.substring(firstColon + 1, line.length)); - } - return packageMap; - } + final parsed = + PackageConfig.parseString(content, Uri.base.resolve(packagesPath)); + return { + for (var package in parsed.packages) package.name: package.packageUriRoot + }; } static Future?> _parsePackage(String packagePath) async { diff --git a/test/resolver_test.dart b/test/resolver_test.dart index 5eadd47e..1d397d8e 100644 --- a/test/resolver_test.dart +++ b/test/resolver_test.dart @@ -11,12 +11,8 @@ void main() { group('Default Resolver', () { setUp(() async { await d.dir('foo', [ - d.file('.packages', ''' -# Fake for testing! -foo:file:///${d.sandbox}/foo/lib -'''), - d.file('.bad.packages', 'thisIsntAPackagesFile!'), d.dir('.dart_tool', [ + d.file('bad_package_config.json', 'thisIsntAPackageConfigFile!'), d.file('package_config.json', ''' { "configVersion": 2, @@ -44,13 +40,6 @@ foo:file:///${d.sandbox}/foo/lib '${d.sandbox}/foo/lib/foo.dart'); }); - test('can be created from a .packages file', () async { - final resolver = await Resolver.create( - packagesPath: p.join(d.sandbox, 'foo', '.packages')); - expect(resolver.resolve('package:foo/foo.dart'), - '${d.sandbox}/foo/lib/foo.dart'); - }); - test('can be created from a package directory', () async { final resolver = await Resolver.create(packagePath: p.join(d.sandbox, 'foo')); @@ -61,7 +50,8 @@ foo:file:///${d.sandbox}/foo/lib test('errors if the packagesFile is an unknown format', () async { expect( () async => await Resolver.create( - packagesPath: p.join(d.sandbox, 'foo', '.bad.packages')), + packagesPath: p.join( + d.sandbox, 'foo', '.dart_tool', 'bad_package_config.json')), throwsA(isA())); }); }); From 3f840f52cb2ffae6a317a17c81061a4b2f2a124e Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 22 Mar 2022 10:28:34 -0700 Subject: [PATCH 5/8] Make separate private constructor for Resolver --- bin/format_coverage.dart | 2 +- lib/src/resolver.dart | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bin/format_coverage.dart b/bin/format_coverage.dart index a7e2177b..540f1b1d 100644 --- a/bin/format_coverage.dart +++ b/bin/format_coverage.dart @@ -79,7 +79,7 @@ Future main(List arguments) async { String output; final resolver = env.bazel ? BazelResolver(workspacePath: env.bazelWorkspace) - : Resolver( + : await Resolver.create( packagesPath: env.packagesPath, packagePath: env.packagePath, sdkRoot: env.sdkRoot, diff --git a/lib/src/resolver.dart b/lib/src/resolver.dart index c8b1a07d..940dcf11 100644 --- a/lib/src/resolver.dart +++ b/lib/src/resolver.dart @@ -9,7 +9,12 @@ import 'package:path/path.dart' as p; /// [Resolver] resolves imports with respect to a given environment. class Resolver { - Resolver( + @Deprecated('Use Resolver.create') + Resolver({this.packagesPath, this.sdkRoot}) + : _packages = packagesPath != null ? _parsePackages(packagesPath) : null, + packagePath = null; + + Resolver._( {this.packagesPath, this.packagePath, this.sdkRoot, @@ -21,7 +26,7 @@ class Resolver { String? packagePath, String? sdkRoot, }) async { - return Resolver( + return Resolver._( packagesPath: packagesPath, packagePath: packagePath, sdkRoot: sdkRoot, From 6c665ac6a510dda13b0d4a514160359e6f9754de Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 22 Mar 2022 12:26:11 -0700 Subject: [PATCH 6/8] Improve resolver test coverage --- test/resolver_test.dart | 47 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/resolver_test.dart b/test/resolver_test.dart index 02068fac..f34e694c 100644 --- a/test/resolver_test.dart +++ b/test/resolver_test.dart @@ -42,6 +42,18 @@ void main() { d.file('foo.dart', 'final foo = "bar";'), ]), ]).create(); + + await d.dir('sdk', [ + d.dir('io', [ + d.file('io.dart', 'final io = "hello";'), + ]), + d.dir('io_patch', [ + d.file('io.dart', 'final patch = true;'), + ]), + d.dir('io_dev', [ + d.file('io.dart', 'final dev = true;'), + ]), + ]).create(); }); test('can be created from a package_config.json', () async { @@ -68,6 +80,41 @@ void main() { d.sandbox, 'foo', '.dart_tool', 'bad_package_config.json')), throwsA(isA())); }); + + test('resolves dart: URIs', () async { + final resolver = await Resolver.create( + packagePath: p.join(d.sandbox, 'foo'), + sdkRoot: p.join(d.sandbox, 'sdk')); + expect(resolver.resolve('dart:io'), + p.join(d.sandbox, 'sdk', 'io', 'io.dart')); + expect(resolver.resolve('dart:io-patch/io.dart'), null); + expect(resolver.resolve('dart:io-dev/io.dart'), + p.join(d.sandbox, 'sdk', 'io_dev', 'io.dart')); + }); + + test('cannot resolve SDK URIs if sdkRoot is null', () async { + final resolver = + await Resolver.create(packagePath: p.join(d.sandbox, 'foo')); + expect(resolver.resolve('dart:convert'), null); + }); + + test('cannot resolve package URIs if packagePath is null', () async { + // ignore: deprecated_member_use_from_same_package + final resolver = Resolver(); + expect(resolver.resolve('package:foo/foo.dart'), null); + }); + + test('cannot resolve package URIs if packagePath is not found', () async { + final resolver = + await Resolver.create(packagePath: p.join(d.sandbox, 'foo')); + expect(resolver.resolve('package:baz/baz.dart'), null); + }); + + test('cannot resolve unexpected URI schemes', () async { + final resolver = + await Resolver.create(packagePath: p.join(d.sandbox, 'foo')); + expect(resolver.resolve('thing:foo/foo.dart'), null); + }); }); group('Bazel resolver', () { From eea4d5aa23632eeba68ba5e15567a0ba5b9f9616 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 22 Mar 2022 13:25:10 -0700 Subject: [PATCH 7/8] Switch readme example commands from `pub` to `dart pub` --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index e507380e..5269e64e 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Tools dart pub global activate coverage -Consider adding the `pub global run` executables directory to your path. +Consider adding the `dart pub global run` executables directory to your path. See [Running a script from your PATH](https://dart.dev/tools/pub/cmd/pub-global#running-a-script-from-your-path) for more details. @@ -24,10 +24,10 @@ for more details. ``` dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN script.dart -pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates +dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates ``` -or if the `pub global run` executables are on your PATH, +or if the `dart pub global run` executables are on your PATH, ``` collect_coverage --uri=http://... -o coverage.json --resume-isolates @@ -44,10 +44,10 @@ all isolates are paused before collecting coverage. #### Formatting coverage data ``` -pub global run coverage:format_coverage --package=app_package -i coverage.json +dart pub global run coverage:format_coverage --package=app_package -i coverage.json ``` -or if the `pub global run` exectuables are on your PATH, +or if the `dart pub global run` exectuables are on your PATH, ``` format_coverage --package=app_package -i coverage.json @@ -70,7 +70,7 @@ collect_coverage: ``` dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN script.dart -pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage +dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage ``` To gather branch level coverage information, pass `--branch-coverage` to *both* @@ -78,7 +78,7 @@ collect_coverage and the Dart command you're gathering coverage from: ``` dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN --branch-coverage script.dart -pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --branch-coverage +dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --branch-coverage ``` Branch coverage requires Dart VM 2.17.0, with service API v3.56. Function, @@ -87,5 +87,5 @@ those flags: ``` dart --pause-isolates-on-exit --disable-service-auth-codes --enable-vm-service=NNNN --branch-coverage script.dart -pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage --branch-coverage +dart pub global run coverage:collect_coverage --uri=http://... -o coverage.json --resume-isolates --function-coverage --branch-coverage ``` From 6143820e6f24fb9251707bf1262f04801602bda3 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 24 Mar 2022 15:04:45 -0700 Subject: [PATCH 8/8] 1.2.0 is published, so move changes to 1.3.0-dev --- CHANGELOG.md | 10 ++++++---- pubspec.yaml | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30549c0c..4568c09d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.3.0-dev +* Add a `--package` flag, which takes the package's root directory, instead of + the .package file. Deprecate the `--packages` flag. +* Deprecate the packagesPath parameter and add packagePath instead, in + `HitMap.parseJson`, `HitMap.parseFiles`, `createHitmap`, and `parseCoverage`. + ## 1.2.0 * Support branch level coverage information, when running tests in the Dart VM. @@ -15,10 +21,6 @@ * Add a `branchHits` field to `HitMap`. * Add support for scraping the service URI from the new Dart VM service message. * Correctly parse package_config files on Windows when the root URI is relative. -* Add a `--package` flag, which takes the package's root directory, instead of - the .package file. Deprecate the `--packages` flag. -* Deprecate the packagesPath parameter and add packagePath instead, in - `HitMap.parseJson`, `HitMap.parseFiles`, `createHitmap`, and `parseCoverage`. ## 1.1.0 - 2022-1-18 diff --git a/pubspec.yaml b/pubspec.yaml index 0f372f4c..f6117ee1 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: coverage -version: 1.2.0 +version: 1.3.0-dev description: Coverage data manipulation and formatting homepage: https://github.com/dart-lang/coverage