From 6f125d624efbbdeb71cb513b18396e22f8f64db5 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 16 Jun 2022 09:15:17 -0700 Subject: [PATCH 01/10] Fix branch coverage test --- test/collect_coverage_test.dart | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/collect_coverage_test.dart b/test/collect_coverage_test.dart index e8cd5b85..26c1fe74 100644 --- a/test/collect_coverage_test.dart +++ b/test/collect_coverage_test.dart @@ -166,8 +166,18 @@ void main() { 28: 'fooAsync', 38: 'isolateTask' }); - expect(isolateFile?.branchHits, - {11: 1, 12: 1, 15: 0, 19: 1, 23: 1, 28: 1, 32: 0, 38: 1, 42: 1}); + expect(isolateFile?.branchHits, { + 11: 1, + 12: 1, + 15: 0, + 19: 1, + 23: 1, + 28: 1, + if (platformVersionCheck(2, 18)) 29: 1, + 32: 0, + 38: 1, + 42: 1 + }); }, skip: !platformVersionCheck(2, 17)); test('HitMap.parseJson, old VM without branch coverage', () async { From bb4b04e85b0bac62bde8d8030264292e9ef33e17 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 16 Jun 2022 09:27:36 -0700 Subject: [PATCH 02/10] Ignore implicit functions in function coverage --- lib/src/collect.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/collect.dart b/lib/src/collect.dart index a2249a70..2743f663 100644 --- a/lib/src/collect.dart +++ b/lib/src/collect.dart @@ -258,7 +258,7 @@ Future _processFunction(VmService service, IsolateRef isolateRef, Script script, FuncRef funcRef, HitMap hits) async { final func = await service.getObject(isolateRef.id!, funcRef.id!) as Func; final location = func.location; - if (location != null) { + if (!(func.implicit ?? false) && location != null) { final funcName = await _getFuncName(service, isolateRef, func); final tokenPos = location.tokenPos!; final line = _getLineFromTokenPos(script, tokenPos); From 0e5537fbf34c967ba64b95c360467b97df67c16d Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 16 Jun 2022 11:49:56 -0700 Subject: [PATCH 03/10] Add a dedicated function coverage test. --- lib/src/collect.dart | 1 - test/collect_coverage_test.dart | 6 +- test/function_coverage_test.dart | 125 +++++++++++++++++++++ test/test_files/function_coverage_app.dart | 56 +++++++++ test/test_files/test_library.dart | 11 ++ test/test_files/test_library_part.dart | 9 ++ 6 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 test/function_coverage_test.dart create mode 100644 test/test_files/function_coverage_app.dart create mode 100644 test/test_files/test_library.dart create mode 100644 test/test_files/test_library_part.dart diff --git a/lib/src/collect.dart b/lib/src/collect.dart index 2743f663..c1d337b5 100644 --- a/lib/src/collect.dart +++ b/lib/src/collect.dart @@ -262,7 +262,6 @@ Future _processFunction(VmService service, IsolateRef isolateRef, final funcName = await _getFuncName(service, isolateRef, func); final tokenPos = location.tokenPos!; final line = _getLineFromTokenPos(script, tokenPos); - if (line == null) { stderr.writeln( 'tokenPos $tokenPos in function ${funcRef.name} has no line mapping ' diff --git a/test/collect_coverage_test.dart b/test/collect_coverage_test.dart index 26c1fe74..91d64801 100644 --- a/test/collect_coverage_test.dart +++ b/test/collect_coverage_test.dart @@ -157,11 +157,10 @@ void main() { 68: 1 }; expect(isolateFile?.lineHits, expectedHits); - expect(isolateFile?.funcHits, {11: 1, 19: 1, 21: 0, 23: 1, 28: 1, 38: 1}); + expect(isolateFile?.funcHits, {11: 1, 19: 1, 23: 1, 28: 1, 38: 1}); expect(isolateFile?.funcNames, { 11: 'fooSync', 19: 'BarClass.BarClass', - 21: 'BarClass.x=', 23: 'BarClass.baz', 28: 'fooAsync', 38: 'isolateTask' @@ -225,11 +224,10 @@ void main() { 68: 1 }; expect(isolateFile?.lineHits, expectedHits); - expect(isolateFile?.funcHits, {11: 1, 19: 1, 21: 0, 23: 1, 28: 1, 38: 1}); + expect(isolateFile?.funcHits, {11: 1, 19: 1, 23: 1, 28: 1, 38: 1}); expect(isolateFile?.funcNames, { 11: 'fooSync', 19: 'BarClass.BarClass', - 21: 'BarClass.x=', 23: 'BarClass.baz', 28: 'fooAsync', 38: 'isolateTask' diff --git a/test/function_coverage_test.dart b/test/function_coverage_test.dart new file mode 100644 index 00000000..82651b73 --- /dev/null +++ b/test/function_coverage_test.dart @@ -0,0 +1,125 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// 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:async'; +import 'dart:convert' show json, LineSplitter, utf8; +import 'dart:io'; + +import 'package:coverage/coverage.dart'; +import 'package:coverage/src/util.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +import 'test_util.dart'; + +final _collectAppPath = p.join('bin', 'collect_coverage.dart'); +final _funcCovApp = p.join('test', 'test_files', 'function_coverage_app.dart'); +final _sampleAppFileUri = p.toUri(p.absolute(_funcCovApp)).toString(); + +void main() { + test('Function coverage', () async { + final resultString = await _collectCoverage(); + final jsonResult = json.decode(resultString) as Map; + final coverage = jsonResult['coverage'] as List; + final hitMap = await HitMap.parseJson( + coverage.cast>(), + ); + + // function_coverage_app.dart. + expect(hitMap, contains(_sampleAppFileUri)); + final isolateFile = hitMap[_sampleAppFileUri]!; + expect(isolateFile.funcHits, { + 7: 1, + 12: 0, // TODO(#398): This abstract method should be ignored. + 19: 1, + 21: 1, + 25: 1, + 29: 1, + 36: 1, + 42: 1, + 47: 1, + }); + expect(isolateFile.funcNames, { + 7: 'normalFunction', + 12: 'BaseClass.abstractMethod', + 19: 'SomeClass.SomeClass', + 21: 'SomeClass.normalMethod', + 25: 'SomeClass.staticMethod', + 29: 'SomeClass.abstractMethod', + 36: 'SomeExtension.extensionMethod', + 42: 'OtherClass.otherMethod', + 47: 'main', + }); + + // test_library.dart. + final testLibraryPath = + p.absolute(p.join('test', 'test_files', 'test_library.dart')); + final testLibraryUri = p.toUri(testLibraryPath).toString(); + expect(hitMap, contains(testLibraryUri)); + final libraryfile = hitMap[testLibraryUri]!; + expect(libraryfile.funcHits, {9: 1}); + expect(libraryfile.funcNames, {9: 'libraryFunction'}); + + // test_library_part.dart. + final testLibraryPartPath = + p.absolute(p.join('test', 'test_files', 'test_library_part.dart')); + final testLibraryPartUri = p.toUri(testLibraryPartPath).toString(); + expect(hitMap, contains(testLibraryPartUri)); + final libraryPartFile = hitMap[testLibraryPartUri]!; + // TODO(#399): Fix handling of part files then re-enable this check. + // expect(libraryPartFile.funcHits, {7: 1}); + // expect(libraryPartFile.funcNames, {7: 'otherLibraryFunction'}); + }); +} + +Future _collectCoverage() async { + expect(FileSystemEntity.isFileSync(_funcCovApp), isTrue); + + final openPort = await getOpenPort(); + + // Run the sample app with the right flags. + final sampleProcess = await Process.start(Platform.resolvedExecutable, [ + '--enable-vm-service=$openPort', + '--pause_isolates_on_exit', + _funcCovApp + ]); + + // Capture the VM service URI. + final serviceUriCompleter = Completer(); + sampleProcess.stdout + .transform(utf8.decoder) + .transform(LineSplitter()) + .listen((line) { + if (!serviceUriCompleter.isCompleted) { + final serviceUri = extractVMServiceUri(line); + if (serviceUri != null) { + serviceUriCompleter.complete(serviceUri); + } + } + }); + final serviceUri = await serviceUriCompleter.future; + + // Run the collection tool. + final toolResult = await Process.run(Platform.resolvedExecutable, [ + _collectAppPath, + '--function-coverage', + '--uri', + '$serviceUri', + '--resume-isolates', + '--wait-paused' + ]).timeout(timeout, onTimeout: () { + throw 'We timed out waiting for the tool to finish.'; + }); + + print(toolResult.stderr); + if (toolResult.exitCode != 0) { + print(toolResult.stdout); + fail('Tool failed with exit code ${toolResult.exitCode}.'); + } + + await sampleProcess.exitCode; + await sampleProcess.stderr.drain(); + + return toolResult.stdout as String; +} diff --git a/test/test_files/function_coverage_app.dart b/test/test_files/function_coverage_app.dart new file mode 100644 index 00000000..db0eea15 --- /dev/null +++ b/test/test_files/function_coverage_app.dart @@ -0,0 +1,56 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// 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 'test_library.dart'; + +int normalFunction() { + return 123; +} + +abstract class BaseClass { + int abstractMethod(); +} + +class SomeClass extends BaseClass { + // Creates an implicit getter and setter that should be ignored. + int x; + + SomeClass() : x = 123; + + int normalMethod() { + return 123; + } + + static int staticMethod() { + return 123; + } + + @override + int abstractMethod() { + return 123; + } +} + +extension SomeExtension on SomeClass { + int extensionMethod() { + return 123; + } +} + +class OtherClass { + int otherMethod() { + return 123; + } +} + +main() { + print(normalFunction()); + print(SomeClass().normalMethod()); + print(SomeClass.staticMethod()); + print(SomeClass().extensionMethod()); + print(SomeClass().abstractMethod()); + print(OtherClass().otherMethod()); + print(libraryFunction()); + print(otherLibraryFunction()); +} diff --git a/test/test_files/test_library.dart b/test/test_files/test_library.dart new file mode 100644 index 00000000..33542942 --- /dev/null +++ b/test/test_files/test_library.dart @@ -0,0 +1,11 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// 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. + +library test_library; + +part 'test_library_part.dart'; + +int libraryFunction() { + return 123; +} diff --git a/test/test_files/test_library_part.dart b/test/test_files/test_library_part.dart new file mode 100644 index 00000000..d587ff25 --- /dev/null +++ b/test/test_files/test_library_part.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// 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. + +part of test_library; + +int otherLibraryFunction() { + return 123; +} From d23bdd9ca08d8610fbe25b0dcc235fd13a5920fa Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 16 Jun 2022 14:42:11 -0700 Subject: [PATCH 04/10] Fix analysis --- test/function_coverage_test.dart | 6 +++--- test/test_files/function_coverage_app.dart | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/function_coverage_test.dart b/test/function_coverage_test.dart index 82651b73..a653abde 100644 --- a/test/function_coverage_test.dart +++ b/test/function_coverage_test.dart @@ -32,7 +32,7 @@ void main() { expect(isolateFile.funcHits, { 7: 1, 12: 0, // TODO(#398): This abstract method should be ignored. - 19: 1, + 16: 1, 21: 1, 25: 1, 29: 1, @@ -43,7 +43,7 @@ void main() { expect(isolateFile.funcNames, { 7: 'normalFunction', 12: 'BaseClass.abstractMethod', - 19: 'SomeClass.SomeClass', + 16: 'SomeClass.SomeClass', 21: 'SomeClass.normalMethod', 25: 'SomeClass.staticMethod', 29: 'SomeClass.abstractMethod', @@ -66,8 +66,8 @@ void main() { p.absolute(p.join('test', 'test_files', 'test_library_part.dart')); final testLibraryPartUri = p.toUri(testLibraryPartPath).toString(); expect(hitMap, contains(testLibraryPartUri)); - final libraryPartFile = hitMap[testLibraryPartUri]!; // TODO(#399): Fix handling of part files then re-enable this check. + // final libraryPartFile = hitMap[testLibraryPartUri]!; // expect(libraryPartFile.funcHits, {7: 1}); // expect(libraryPartFile.funcNames, {7: 'otherLibraryFunction'}); }); diff --git a/test/test_files/function_coverage_app.dart b/test/test_files/function_coverage_app.dart index db0eea15..e3ffe096 100644 --- a/test/test_files/function_coverage_app.dart +++ b/test/test_files/function_coverage_app.dart @@ -13,11 +13,11 @@ abstract class BaseClass { } class SomeClass extends BaseClass { + SomeClass() : x = 123; + // Creates an implicit getter and setter that should be ignored. int x; - SomeClass() : x = 123; - int normalMethod() { return 123; } @@ -44,7 +44,7 @@ class OtherClass { } } -main() { +void main() { print(normalFunction()); print(SomeClass().normalMethod()); print(SomeClass.staticMethod()); From ad45d9ee46277954ef32d7d6978dc64cc70bcbee Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 21 Jun 2022 15:37:19 -0700 Subject: [PATCH 05/10] Ignore abstract functions --- lib/src/collect.dart | 48 ++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/src/collect.dart b/lib/src/collect.dart index f23783f8..d2da9937 100644 --- a/lib/src/collect.dart +++ b/lib/src/collect.dart @@ -255,27 +255,6 @@ int? _getLineFromTokenPos(Script script, int tokenPos) { return null; } -Future _processFunction(VmService service, IsolateRef isolateRef, - Script script, FuncRef funcRef, HitMap hits) async { - final func = await service.getObject(isolateRef.id!, funcRef.id!) as Func; - final location = func.location; - if (location != null) { - final funcName = await _getFuncName(service, isolateRef, func); - final tokenPos = location.tokenPos!; - final line = _getLineFromTokenPos(script, tokenPos); - - if (line == null) { - if (_debugTokenPositions) { - stderr.writeln( - 'tokenPos $tokenPos in function ${funcRef.name} has no line ' - 'mapping for script ${script.uri!}'); - } - return; - } - hits.funcNames![line] = funcName; - } -} - /// Returns a JSON coverage list backward-compatible with pre-1.16.0 SDKs. Future>> _getCoverageJson( VmService service, @@ -288,6 +267,28 @@ Future>> _getCoverageJson( final scripts = {}; final libraries = {}; final needScripts = functionCoverage || !reportLines; + + Future processFunction( + Script script, FuncRef funcRef, HitMap hits) async { + final func = await service.getObject(isolateRef.id!, funcRef.id!) as Func; + final location = func.location; + if (!(func.isAbstract ?? false) && location != null) { + final funcName = await _getFuncName(service, isolateRef, func); + final tokenPos = location.tokenPos!; + final line = _getLineFromTokenPos(script, tokenPos); + + if (line == null) { + if (_debugTokenPositions) { + stderr.writeln( + 'tokenPos $tokenPos in function ${funcRef.name} has no line ' + 'mapping for script ${script.uri!}'); + } + return; + } + hits.funcNames![line] = funcName; + } + } + for (var range in report.ranges!) { final scriptRef = report.scripts![range.scriptIndex!]; final scriptUri = Uri.parse(scriptRef.uri!); @@ -322,7 +323,7 @@ Future>> _getCoverageJson( await service.getObject(isolateRef.id!, libRef.id!) as Library; if (library.functions != null) { for (var funcRef in library.functions!) { - await _processFunction(service, isolateRef, script!, funcRef, hits); + await processFunction(script!, funcRef, hits); } } if (library.classes != null) { @@ -331,8 +332,7 @@ Future>> _getCoverageJson( await service.getObject(isolateRef.id!, classRef.id!) as Class; if (clazz.functions != null) { for (var funcRef in clazz.functions!) { - await _processFunction( - service, isolateRef, script!, funcRef, hits); + await processFunction(script!, funcRef, hits); } } } From e52fe03329c73f2c71681fd943655ecf174710d3 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 6 Jul 2022 15:42:49 -0700 Subject: [PATCH 06/10] Use the new vm_service version --- pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubspec.yaml b/pubspec.yaml index a60cc19d..50b57be3 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -13,7 +13,7 @@ dependencies: path: ^1.8.0 source_maps: ^0.10.10 stack_trace: ^1.10.0 - vm_service: ^9.0.0 + vm_service: ^10.0.0 dev_dependencies: build_runner: ^2.1.10 From e76abeb03246185010463b3b7951a351aa4d7d4f Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 6 Jul 2022 15:51:22 -0700 Subject: [PATCH 07/10] Fix test --- lib/src/collect.dart | 2 +- test/function_coverage_test.dart | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/collect.dart b/lib/src/collect.dart index a1fed147..b0df8436 100644 --- a/lib/src/collect.dart +++ b/lib/src/collect.dart @@ -284,7 +284,7 @@ Future>> _getCoverageJson( Future processFunction(FuncRef funcRef) async { final func = await service.getObject(isolateRef.id!, funcRef.id!) as Func; - if (func.implicit ?? false || func.abstract ?? false) { + if ((func.implicit ?? false) || (func.isAbstract ?? false)) { return; } final location = func.location; diff --git a/test/function_coverage_test.dart b/test/function_coverage_test.dart index 5eb71be5..5ef79d40 100644 --- a/test/function_coverage_test.dart +++ b/test/function_coverage_test.dart @@ -32,7 +32,6 @@ void main() { final isolateFile = hitMap[_sampleAppFileUri]!; expect(isolateFile.funcHits, { 7: 1, - 12: 0, // TODO(#398): This abstract method should be ignored. 16: 1, 21: 1, 25: 1, @@ -43,7 +42,6 @@ void main() { }); expect(isolateFile.funcNames, { 7: 'normalFunction', - 12: 'BaseClass.abstractMethod', 16: 'SomeClass.SomeClass', 21: 'SomeClass.normalMethod', 25: 'SomeClass.staticMethod', From 622bb6918e2a3b82d1f2fc3a0cdd8e96926ad45b Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 20 Jul 2022 09:01:41 -0700 Subject: [PATCH 08/10] vm_service abstract update was actually a minor version bump --- pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubspec.yaml b/pubspec.yaml index 50b57be3..532683f6 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -13,7 +13,7 @@ dependencies: path: ^1.8.0 source_maps: ^0.10.10 stack_trace: ^1.10.0 - vm_service: ^10.0.0 + vm_service: ^9.2.0 dev_dependencies: build_runner: ^2.1.10 From 2f1dfac86824c060e7cab869debc521e9689ef94 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 20 Jul 2022 12:32:37 -0700 Subject: [PATCH 09/10] Remove deprecated analysis option --- analysis_options.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index 462c5ebd..71eda7f8 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -22,7 +22,6 @@ linter: - control_flow_in_finally - empty_statements - hash_and_equals - - invariant_booleans - iterable_contains_unrelated_type - list_remove_unrelated_type - literal_only_boolean_expressions From efb54723097c8e768004b6f811d47a7ce8fc6628 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 20 Jul 2022 13:26:01 -0700 Subject: [PATCH 10/10] Fix function_coverage_test on older SDKs --- test/function_coverage_test.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/function_coverage_test.dart b/test/function_coverage_test.dart index 5ef79d40..e5fd9951 100644 --- a/test/function_coverage_test.dart +++ b/test/function_coverage_test.dart @@ -32,6 +32,7 @@ void main() { final isolateFile = hitMap[_sampleAppFileUri]!; expect(isolateFile.funcHits, { 7: 1, + if (!platformVersionCheck(2, 19)) 12: 0, 16: 1, 21: 1, 25: 1, @@ -42,6 +43,7 @@ void main() { }); expect(isolateFile.funcNames, { 7: 'normalFunction', + if (!platformVersionCheck(2, 19)) 12: 'BaseClass.abstractMethod', 16: 'SomeClass.SomeClass', 21: 'SomeClass.normalMethod', 25: 'SomeClass.staticMethod',