-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
parseChromeCoverage #281
parseChromeCoverage #281
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
// Copyright (c) 2020, 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 'package:coverage/src/util.dart'; | ||
import 'package:source_maps/parser.dart'; | ||
|
||
/// Returns a Dart based hit-map containing coverage report for the provided | ||
/// Chrome [preciseCoverage]. | ||
/// | ||
/// [sourceProvider] returns the source content for the Chrome scriptId, or null | ||
/// if not available. | ||
/// | ||
/// [sourceMapProvider] returns the associated source map content for the Chrome | ||
/// scriptId, or null if not available. | ||
/// | ||
/// [sourceUriProvider] returns the uri for the provided sourceUrl and | ||
/// associated scriptId. | ||
/// | ||
/// Chrome coverage information for which the corresponding source map or source | ||
/// content is null will be ignored. | ||
Future<Map<String, dynamic>> parseChromeCoverage( | ||
List<Map<String, dynamic>> preciseCoverage, | ||
Future<String> Function(String scriptId) sourceProvider, | ||
Future<String> Function(String scriptId) sourceMapProvider, | ||
Future<Uri> Function(String sourceUrl, String scriptId) sourceUriProvider, | ||
) async { | ||
final coverageReport = <Uri, Map<int, bool>>{}; | ||
for (Map<String, dynamic> entry in preciseCoverage) { | ||
final String scriptId = entry['scriptId']; | ||
|
||
final mapResponse = await sourceMapProvider(scriptId); | ||
if (mapResponse == null) continue; | ||
grouma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
final SingleMapping mapping = parse(mapResponse); | ||
|
||
final compiledSource = await sourceProvider(scriptId); | ||
if (compiledSource == null) continue; | ||
|
||
final coverageInfo = _coverageInfoFor(entry); | ||
final offsetCoverage = _offsetCoverage(coverageInfo, compiledSource.length); | ||
final coveredPositions = _coveredPositions(compiledSource, offsetCoverage); | ||
|
||
for (var lineEntry in mapping.lines) { | ||
for (var columnEntry in lineEntry.entries) { | ||
if (columnEntry.sourceUrlId == null) continue; | ||
final sourceUrl = mapping.urls[columnEntry.sourceUrlId]; | ||
|
||
// Ignore coverage information for the SDK. | ||
if (sourceUrl.startsWith('org-dartlang-sdk:')) continue; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How stable is this? Where is it defined? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It comes from the SDK. I don't believe there is a way to programatically consume this value. We have similar logic in |
||
|
||
final uri = await sourceUriProvider(sourceUrl, scriptId); | ||
final coverage = coverageReport.putIfAbsent(uri, () => <int, bool>{}); | ||
|
||
coverage[columnEntry.sourceLine + 1] = coveredPositions | ||
.contains(_Position(lineEntry.line + 1, columnEntry.column + 1)); | ||
} | ||
} | ||
} | ||
|
||
final coverageHitMaps = <Uri, Map<int, int>>{}; | ||
coverageReport.forEach((uri, coverage) { | ||
final hitMap = <int, int>{}; | ||
for (var line in coverage.keys.toList()..sort()) { | ||
hitMap[line] = coverage[line] ? 1 : 0; | ||
} | ||
coverageHitMaps[uri] = hitMap; | ||
}); | ||
grouma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
final allCoverage = <Map<String, dynamic>>[]; | ||
coverageHitMaps.forEach((uri, hitMap) { | ||
allCoverage.add(toScriptCoverageJson(uri, hitMap)); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [optional] final allCoverage = [
for(var hitMap in coverageHitMaps.entries)
toScriptCoverageJson(hitMap.key, hitMap.value),
]; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The SDK constraint is wide on this package so I don't believe we can use this feature yet. |
||
return <String, dynamic>{'type': 'CodeCoverage', 'coverage': allCoverage}; | ||
} | ||
|
||
/// Returns all covered positions in a provided source. | ||
Set<_Position> _coveredPositions( | ||
String compiledSource, List<bool> offsetCoverage) { | ||
final positions = Set<_Position>(); | ||
// Line is 1 based. | ||
var line = 1; | ||
// Column is 1 based. | ||
var column = 0; | ||
for (var offset = 0; offset < compiledSource.length; offset++) { | ||
if (compiledSource[offset] == '\n') { | ||
line++; | ||
column = 0; | ||
} else { | ||
column++; | ||
} | ||
if (offsetCoverage[offset]) positions.add(_Position(line, column)); | ||
} | ||
return positions; | ||
} | ||
|
||
/// Returns coverage information for a Chrome entry. | ||
List<_CoverageInfo> _coverageInfoFor(Map<String, dynamic> entry) { | ||
final result = <_CoverageInfo>[]; | ||
for (Map<String, dynamic> functions in entry['functions']) { | ||
for (Map<String, dynamic> range in functions['ranges']) { | ||
result.add(_CoverageInfo( | ||
range['startOffset'], | ||
range['endOffset'], | ||
range['count'] > 0, | ||
)); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [optional] return [
for (Map<String, dynamic> functions in entry['functions'])
for (Map<String, dynamic> range in functions['ranges'])
_CoverageInfo(
range['startOffset'],
range['endOffset'],
range['count'] > 0,
),
]; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment. |
||
return result; | ||
} | ||
|
||
/// Returns the coverage information for each offset. | ||
List<bool> _offsetCoverage(List<_CoverageInfo> coverageInfo, int sourceLength) { | ||
final offsetCoverage = List.filled(sourceLength, false); | ||
|
||
// Sort coverage information by their size. | ||
// Coverage information takes granularity as precedence. | ||
coverageInfo.sort((a, b) => | ||
(b.endOffset - b.startOffset).compareTo(a.endOffset - a.startOffset)); | ||
|
||
for (var range in coverageInfo) { | ||
for (var i = range.startOffset; i < range.endOffset; i++) { | ||
offsetCoverage[i] = range.isCovered; | ||
} | ||
} | ||
|
||
return offsetCoverage; | ||
} | ||
|
||
class _CoverageInfo { | ||
_CoverageInfo(this.startOffset, this.endOffset, this.isCovered); | ||
|
||
/// 0 based byte offset. | ||
final int startOffset; | ||
|
||
/// 0 based byte offset. | ||
final int endOffset; | ||
|
||
final bool isCovered; | ||
} | ||
|
||
/// A covered position in a source file where [line] and [column] are 1 based. | ||
class _Position { | ||
_Position(this.line, this.column); | ||
|
||
final int line; | ||
final int column; | ||
|
||
@override | ||
int get hashCode => hash2(line, column); | ||
|
||
@override | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add a blank line before the |
||
bool operator ==(dynamic o) => | ||
o is _Position && o.line == line && o.column == column; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// Copyright (c) 2020, 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:convert'; | ||
import 'dart:io'; | ||
|
||
import 'package:coverage/coverage.dart'; | ||
import 'package:test/test.dart'; | ||
|
||
Future<String> sourceMapProvider(String scriptId) async { | ||
// The scriptId for the main_test.ddc.js in the sample report is 37. | ||
if (scriptId != '37') return null; | ||
return File('test/test_files/main_test.ddc.js.map').readAsString(); | ||
} | ||
|
||
Future<String> sourceProvider(String scriptId) async { | ||
// The scriptId for the main_test.ddc.js in the sample report is 37. | ||
if (scriptId != '37') return null; | ||
return File('test/test_files/main_test.ddc.js').readAsString(); | ||
} | ||
|
||
Future<Uri> sourceUriProvider(String sourceUrl, String scriptId) async => | ||
Uri.parse(sourceUrl); | ||
|
||
void main() { | ||
test('reports correctly', () async { | ||
final preciseCoverage = json.decode( | ||
await File('test/test_files/chrome_precise_report.txt').readAsString()); | ||
|
||
final report = await parseChromeCoverage( | ||
// ignore: avoid_as | ||
(preciseCoverage as List).cast(), | ||
sourceProvider, | ||
sourceMapProvider, | ||
sourceUriProvider, | ||
); | ||
|
||
final coverage = report['coverage']; | ||
expect(coverage.length, equals(1)); | ||
|
||
final sourceReport = coverage.first; | ||
expect(sourceReport['source'], equals('main_test.dart')); | ||
|
||
final Map<int, int> expectedHits = { | ||
5: 1, | ||
6: 1, | ||
7: 1, | ||
8: 0, | ||
10: 1, | ||
11: 1, | ||
13: 1, | ||
14: 1, | ||
15: 1, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Would this be shorter without a trailing comma? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No difference. |
||
}; | ||
|
||
final List<int> hitMap = sourceReport['hits']; | ||
expect(hitMap.length, equals(expectedHits.keys.length * 2)); | ||
for (var i = 0; i < hitMap.length; i += 2) { | ||
expect(expectedHits[hitMap[i]], equals(hitMap[i + 1])); | ||
} | ||
}); | ||
} |
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this is done on the Dart team, but the general policy we use for Flutter is to use a single year for all files (in our case the earliest date we had in any of them, which was 2013). We used to use a policy that new files got the current year, old files preserved their headers, but that changed a few years back.
If current-year is still the right approach for Dart packages, then LGTM. Otherwise we should probably (in a separate PR), set these all consistently to 2013 in this repo, which is the earliest date I see in our headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most other packages I touch follow the current-year pattern.