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

Apply --scope-output filters after getSourceReport #498

Merged
merged 6 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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

Expand Down
40 changes: 31 additions & 9 deletions lib/src/collect.dart
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,8 @@ Future<Map<String, dynamic>> _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(
Expand All @@ -203,7 +200,8 @@ Future<Map<String, dynamic>> _getAllCoverage(
includeDart,
functionCoverage,
reportLines,
coverableLineCache);
coverableLineCache,
scopedOutput);
allCoverage.addAll(coverage);
}
} else {
Expand All @@ -229,7 +227,8 @@ Future<Map<String, dynamic>> _getAllCoverage(
includeDart,
functionCoverage,
reportLines,
coverableLineCache);
coverableLineCache,
scopedOutput);
allCoverage.addAll(coverage);
}
}
Expand Down Expand Up @@ -312,7 +311,8 @@ Future<List<Map<String, dynamic>>> _processSourceReport(
bool includeDart,
bool functionCoverage,
bool reportLines,
Map<String, Set<int>>? coverableLineCache) async {
Map<String, Set<int>>? coverableLineCache,
Set<String> scopedOutput) async {
final hitMaps = <Uri, HitMap>{};
final scripts = <ScriptRef, Script>{};
final libraries = <LibraryRef>{};
Expand Down Expand Up @@ -362,8 +362,14 @@ Future<List<Map<String, dynamic>>> _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
Expand Down Expand Up @@ -497,3 +503,19 @@ class StdoutLog extends Log {
@override
void severe(String message) => print(message);
}

extension _ScopedOutput on Set<String> {
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this return false if the set is empty? If not, can you leave a comment explaining why we're considering an empty set to include all scripts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! That makes sense.


final scriptUri = Uri.parse(scriptUriString);
if (scriptUri.scheme != 'package') return false;

final scope = scriptUri.pathSegments.first;
return contains(scope);
}
}
Loading