diff --git a/CHANGELOG.md b/CHANGELOG.md index 09faed80..c8920f53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## 1.8.1-wip - Require Dart ^3.4 +- Fix bug where some ranges were able to bypass the `--scope-output` filters. ## 1.8.0 diff --git a/lib/src/collect.dart b/lib/src/collect.dart index 51b56255..646a7e12 100644 --- a/lib/src/collect.dart +++ b/lib/src/collect.dart @@ -178,11 +178,8 @@ Future> _getAllCoverage( continue; } for (final script in scripts.scripts!) { - final uri = Uri.parse(script.uri!); - if (uri.scheme != 'package') continue; - final scope = uri.path.split('/').first; // Skip scripts which should not be included in the report. - if (!scopedOutput.contains(scope)) continue; + if (!scopedOutput.includesScript(script.uri)) continue; late final SourceReport scriptReport; try { scriptReport = await service.getSourceReport( @@ -203,7 +200,8 @@ Future> _getAllCoverage( includeDart, functionCoverage, reportLines, - coverableLineCache); + coverableLineCache, + scopedOutput); allCoverage.addAll(coverage); } } else { @@ -229,7 +227,8 @@ Future> _getAllCoverage( includeDart, functionCoverage, reportLines, - coverableLineCache); + coverableLineCache, + scopedOutput); allCoverage.addAll(coverage); } } @@ -312,7 +311,8 @@ Future>> _processSourceReport( bool includeDart, bool functionCoverage, bool reportLines, - Map>? coverableLineCache) async { + Map>? coverableLineCache, + Set scopedOutput) async { final hitMaps = {}; final scripts = {}; final libraries = {}; @@ -362,8 +362,14 @@ Future>> _processSourceReport( for (var range in report.ranges!) { final scriptRef = report.scripts![range.scriptIndex!]; - final scriptUriString = scriptRef.uri!; - final scriptUri = Uri.parse(scriptUriString); + final scriptUriString = scriptRef.uri; + if (!scopedOutput.includesScript(scriptUriString)) { + // Sometimes a range's script can be different to the function's script + // (eg mixins), so we have to re-check the scope filter. + // See https://github.com/dart-lang/coverage/issues/495 + continue; + } + final scriptUri = Uri.parse(scriptUriString!); // If we have a coverableLineCache, use it in the same way we use // SourceReportCoverage.misses: to add zeros to the coverage result for all @@ -497,3 +503,19 @@ class StdoutLog extends Log { @override void severe(String message) => print(message); } + +extension _ScopedOutput on Set { + bool includesScript(String? scriptUriString) { + if (scriptUriString == null) return false; + + // If the set is empty, it means the user didn't specify a --scope-output + // flag, so allow everything. + if (isEmpty) return true; + + final scriptUri = Uri.parse(scriptUriString); + if (scriptUri.scheme != 'package') return false; + + final scope = scriptUri.pathSegments.first; + return contains(scope); + } +} diff --git a/test/collect_coverage_mock_test.dart b/test/collect_coverage_mock_test.dart index e36a8367..a919d9fa 100644 --- a/test/collect_coverage_mock_test.dart +++ b/test/collect_coverage_mock_test.dart @@ -116,36 +116,39 @@ void main() { test('Collect coverage, report lines', () async { final service = _mockService(3, 51); - when(service.getSourceReport('isolate', ['Coverage'], - forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [12], - misses: [47], - ), - ), - _range( - 1, - SourceReportCoverage( - hits: [95], - misses: [52], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ScriptRef( - uri: 'package:bar/bar.dart', - id: 'bar', - ), - ], - )); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + _range( + 1, + SourceReportCoverage( + hits: [95], + misses: [52], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); final jsonResult = await collect(Uri(), false, false, false, null, serviceOverrideForTesting: service); @@ -171,25 +174,28 @@ void main() { ), ], )); - when(service.getSourceReport('isolate', ['Coverage'], - scriptId: 'foo', forceCompile: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [15], - misses: [32], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ], - )); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + scriptId: 'foo', + forceCompile: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [15], + misses: [32], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ], + )); when(service.getObject('isolate', 'foo')) .thenAnswer((_) async => _script([ [12, 15, 7], @@ -207,27 +213,29 @@ void main() { test('Collect coverage, scoped output, library filters', () async { final service = _mockService(3, 57); - when(service.getSourceReport('isolate', ['Coverage'], - forceCompile: true, - reportLines: true, - libraryFilters: ['package:foo/'])) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [12], - misses: [47], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ], - )); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + forceCompile: true, + reportLines: true, + libraryFilters: ['package:foo/'], + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ], + )); final jsonResult = await collect(Uri(), false, false, false, {'foo'}, serviceOverrideForTesting: service); @@ -243,55 +251,61 @@ void main() { 'isolateGroupA': ['isolate1', 'isolate2'], 'isolateGroupB': ['isolate3'], }); - when(service.getSourceReport('isolate1', ['Coverage'], - forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [12], - misses: [47], - ), - ), - _range( - 1, - SourceReportCoverage( - hits: [95], - misses: [52], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ScriptRef( - uri: 'package:bar/bar.dart', - id: 'bar', - ), - ], - )); - when(service.getSourceReport('isolate3', ['Coverage'], - forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [34], - misses: [61], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:baz/baz.dart', - id: 'baz', - ), - ], - )); + when(service.getSourceReport( + 'isolate1', + ['Coverage'], + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + _range( + 1, + SourceReportCoverage( + hits: [95], + misses: [52], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); + when(service.getSourceReport( + 'isolate3', + ['Coverage'], + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [34], + misses: [61], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:baz/baz.dart', + id: 'baz', + ), + ], + )); final jsonResult = await collect(Uri(), false, false, false, null, serviceOverrideForTesting: service); @@ -313,55 +327,61 @@ void main() { 'isolateGroupA': ['isolate1', 'isolate2'], 'isolateGroupB': ['isolate3'], }); - when(service.getSourceReport('isolate1', ['Coverage'], - forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [12], - misses: [47], - ), - ), - _range( - 1, - SourceReportCoverage( - hits: [95], - misses: [52], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ScriptRef( - uri: 'package:bar/bar.dart', - id: 'bar', - ), - ], - )); - when(service.getSourceReport('isolate3', ['Coverage'], - forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [34], - misses: [61], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:baz/baz.dart', - id: 'baz', - ), - ], - )); + when(service.getSourceReport( + 'isolate1', + ['Coverage'], + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + _range( + 1, + SourceReportCoverage( + hits: [95], + misses: [52], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); + when(service.getSourceReport( + 'isolate3', + ['Coverage'], + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [34], + misses: [61], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:baz/baz.dart', + id: 'baz', + ), + ], + )); final jsonResult = await collect(Uri(), false, false, false, null, serviceOverrideForTesting: service); @@ -409,28 +429,36 @@ void main() { ), ], )); - when(service.getSourceReport('isolate', ['Coverage'], - scriptId: 'foo', forceCompile: true, reportLines: true)) - .thenThrow(FakeSentinelException()); - when(service.getSourceReport('isolate', ['Coverage'], - scriptId: 'bar', forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [95], - misses: [52], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:bar/bar.dart', - id: 'bar', - ), - ], - )); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + scriptId: 'foo', + forceCompile: true, + reportLines: true, + )).thenThrow(FakeSentinelException()); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + scriptId: 'bar', + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [95], + misses: [52], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); final jsonResult = await collect( Uri(), false, false, false, {'foo', 'bar'}, @@ -460,36 +488,39 @@ void main() { // Expect that getSourceReport's librariesAlreadyCompiled param is not set // when coverableLineCache is non-null but the service version is too old. final service = _mockService(4, 12); - when(service.getSourceReport('isolate', ['Coverage'], - forceCompile: true, reportLines: true)) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [12], - misses: [47], - ), - ), - _range( - 1, - SourceReportCoverage( - hits: [95], - misses: [52], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ScriptRef( - uri: 'package:bar/bar.dart', - id: 'bar', - ), - ], - )); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + forceCompile: true, + reportLines: true, + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + _range( + 1, + SourceReportCoverage( + hits: [95], + misses: [52], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); final coverableLineCache = >{}; final jsonResult = await collect(Uri(), false, false, false, null, @@ -507,39 +538,40 @@ void main() { // Expect that on the first getSourceReport call, librariesAlreadyCompiled // is empty. final service = _mockService(4, 13); - when( - service.getSourceReport('isolate', ['Coverage'], - forceCompile: true, - reportLines: true, - librariesAlreadyCompiled: [])) - .thenAnswer((_) async => SourceReport( - ranges: [ - _range( - 0, - SourceReportCoverage( - hits: [12], - misses: [47], - ), - ), - _range( - 1, - SourceReportCoverage( - hits: [95], - misses: [52], - ), - ), - ], - scripts: [ - ScriptRef( - uri: 'package:foo/foo.dart', - id: 'foo', - ), - ScriptRef( - uri: 'package:bar/bar.dart', - id: 'bar', - ), - ], - )); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + forceCompile: true, + reportLines: true, + librariesAlreadyCompiled: [], + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + _range( + 1, + SourceReportCoverage( + hits: [95], + misses: [52], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); final coverableLineCache = >{}; final jsonResult = await collect(Uri(), false, false, false, null, @@ -563,14 +595,16 @@ void main() { // seen. The response won't contain any misses for these libraries, // because they won't be force compiled. We'll also return a 3rd library, // which will contain misses, as it hasn't been compiled yet. - when( - service.getSourceReport('isolate', ['Coverage'], - forceCompile: true, - reportLines: true, - librariesAlreadyCompiled: [ - 'package:foo/foo.dart', - 'package:bar/bar.dart' - ])).thenAnswer((_) async => SourceReport( + when(service.getSourceReport( + 'isolate', + ['Coverage'], + forceCompile: true, + reportLines: true, + librariesAlreadyCompiled: [ + 'package:foo/foo.dart', + 'package:bar/bar.dart' + ], + )).thenAnswer((_) async => SourceReport( ranges: [ _range( 0, @@ -628,5 +662,54 @@ void main() { 'package:baz/baz.dart': {36, 81}, }); }); + + test( + 'Collect coverage, scoped output, library filters, ' + 'handles SourceReports that contain unfiltered ranges', () async { + // Regression test for https://github.com/dart-lang/coverage/issues/495 + final service = _mockService(3, 57); + when(service.getSourceReport( + 'isolate', + ['Coverage'], + forceCompile: true, + reportLines: true, + libraryFilters: ['package:foo/'], + )).thenAnswer((_) async => SourceReport( + ranges: [ + _range( + 0, + SourceReportCoverage( + hits: [12], + misses: [47], + ), + ), + _range( + 1, + SourceReportCoverage( + hits: [86], + misses: [91], + ), + ), + ], + scripts: [ + ScriptRef( + uri: 'package:foo/foo.dart', + id: 'foo', + ), + ScriptRef( + uri: 'package:bar/bar.dart', + id: 'bar', + ), + ], + )); + + final jsonResult = await collect(Uri(), false, false, false, {'foo'}, + serviceOverrideForTesting: service); + final result = await HitMap.parseJson( + jsonResult['coverage'] as List>); + + expect(result.length, 1); + expect(result['package:foo/foo.dart']?.lineHits, {12: 1, 47: 0}); + }); }); }