Skip to content

Chrome coverage support #1155

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

Merged
merged 4 commits into from
Jan 24, 2020
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
4 changes: 3 additions & 1 deletion pkgs/test/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
## 1.11.2-dev
## 1.12.0-dev

* Bump minimum SDK to `2.4.0` for safer usage of for-loop elements.
* Deprecate `PhantomJS` and provide warning when used. Support for `PhantomJS`
will be removed in version `2.0.0`.
* Support coverage collection for the Chrome platform. See `README.md` for usage
Copy link
Contributor

Choose a reason for hiding this comment

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

1.12.0 since this is a new feature 🎉

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

details.

## 1.11.1

Expand Down
7 changes: 4 additions & 3 deletions pkgs/test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ specifying it at all, meaning the test order will remain as-is.

### Collecting Code Coverage
To collect code coverage, you can run tests with the `--coverage <directory>`
argument. The directory specified can be an absolute or relative path.
argument. The directory specified can be an absolute or relative path.
If a directory does not exist at the path specified, a directory will be
created. If a directory does exist, files may be overwritten with the latest
coverage data, if they conflict.
Expand All @@ -196,7 +196,8 @@ and the resulting coverage files will be outputted in the directory specified.
The files can then be formatted using the `package:coverage`
`format_coverage` executable.

Coverage gathering is currently only implemented for tests run in the Dart VM.
Coverage gathering is currently only implemented for tests run on the Dart VM or
Chrome.

### Restricting Tests to Certain Platforms

Expand Down Expand Up @@ -765,7 +766,7 @@ A configuration file can do much more than just set global defaults. See

Tests can be debugged interactively using platforms' built-in development tools.
Tests running on browsers can use those browsers' development consoles to inspect
the document, set breakpoints, and step through code. Those running on the Dart
the document, set breakpoints, and step through code. Those running on the Dart
VM use [the Dart Observatory][observatory]'s .

[observatory]: https://dart-lang.github.io/observatory/
Expand Down
23 changes: 15 additions & 8 deletions pkgs/test/lib/src/runner/browser/browser_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@ import 'dart:convert';
import 'package:async/async.dart';
import 'package:pool/pool.dart';
import 'package:stream_channel/stream_channel.dart';
import 'package:web_socket_channel/web_socket_channel.dart';

import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_imports
import 'package:test_api/src/util/stack_trace_mapper.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/runner_suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/environment.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports

import 'package:test_core/src/runner/application_exception.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/environment.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/plugin/platform_helpers.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/runner_suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
import 'package:web_socket_channel/web_socket_channel.dart';

import '../executable_settings.dart';
import 'browser.dart';
Expand Down Expand Up @@ -238,8 +236,17 @@ class BrowserManager {
});

try {
controller = deserializeSuite(path, currentPlatform(_runtime),
suiteConfig, await _environment, suiteChannel, message);
controller = deserializeSuite(
path,
currentPlatform(_runtime),
suiteConfig,
await _environment,
suiteChannel,
message, gatherCoverage: () async {
var browser = _browser;
if (browser is Chrome) return browser.gatherCoverage();
return {};
Copy link
Contributor

Choose a reason for hiding this comment

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

should we warn here or anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

This will produce an empty result which is consistent with the current behavior. I might follow up with a PR that warns at the time of arg parsing.

});

controller.channel('test.browser.mapper').sink.add(mapper?.serialize());

Expand Down
91 changes: 88 additions & 3 deletions pkgs/test/lib/src/runner/browser/chrome.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,21 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:coverage/coverage.dart';
import 'package:http/http.dart' as http;
import 'package:path/path.dart' as p;
import 'package:pedantic/pedantic.dart';
import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_imports
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';

import '../executable_settings.dart';
import 'browser.dart';
import 'default_settings.dart';

// TODO(nweiz): move this into its own package?
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Are we using it? Should we file an issue to migrate to that package?

Copy link
Member Author

Choose a reason for hiding this comment

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

/// A class for running an instance of Chrome.
///
/// Most of the communication with the browser is expected to happen via HTTP,
Expand All @@ -28,11 +32,16 @@ class Chrome extends Browser {
@override
final Future<Uri> remoteDebuggerUrl;

final Future<WipConnection> _tabConnection;
final Map<String, String> _idToUrl;

/// Starts a new instance of Chrome open to the given [url], which may be a
/// [Uri] or a [String].
factory Chrome(Uri url, {ExecutableSettings settings, bool debug = false}) {
settings ??= defaultSettings[Runtime.chrome];
var remoteDebuggerCompleter = Completer<Uri>.sync();
var connectionCompleter = Completer<WipConnection>();
var idToUrl = <String, String>{};
return Chrome._(() async {
var tryPort = ([int port]) async {
var dir = createTempDir();
Expand Down Expand Up @@ -73,6 +82,8 @@ class Chrome extends Browser {
if (port != null) {
remoteDebuggerCompleter.complete(
getRemoteDebuggerUrl(Uri.parse('http://localhost:$port')));

connectionCompleter.complete(_connect(process, port, idToUrl));
} else {
remoteDebuggerCompleter.complete(null);
}
Expand All @@ -85,9 +96,83 @@ class Chrome extends Browser {

if (!debug) return tryPort();
return getUnusedPort<Process>(tryPort);
}, remoteDebuggerCompleter.future);
}, remoteDebuggerCompleter.future, connectionCompleter.future, idToUrl);
}

Chrome._(Future<Process> Function() startBrowser, this.remoteDebuggerUrl)
/// Returns a Dart based hit-map containing coverage report, suitable for use
/// with `package:coverage`.
Future<Map<String, dynamic>> gatherCoverage() async {
var tabConnection = await _tabConnection;
var response = await tabConnection.debugger.connection
.sendCommand('Profiler.takePreciseCoverage', {});
var result = response.result['result'];
var coverage = await parseChromeCoverage(
(result as List).cast(),
_sourceProvider,
_sourceMapProvider,
_sourceUriProvider,
);
return coverage;
}

Chrome._(Future<Process> Function() startBrowser, this.remoteDebuggerUrl,
this._tabConnection, this._idToUrl)
: super(startBrowser);

Future<Uri> _sourceUriProvider(String sourceUrl, String scriptId) async {
var script = _idToUrl[scriptId];
if (script == null) return null;
var uri = Uri.parse(script);
var path = p.join(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you could do

var path = p.joinAll([
  ...uri.pathSegments.sublist(1, uri.pathSegments.length - 1),
  sourceUrl,
]);

p.joinAll(uri.pathSegments.sublist(1, uri.pathSegments.length - 1)),
sourceUrl);
return path.contains('/packages/')
? Uri(scheme: 'package', path: path.split('/packages/').last)
: Uri.file(p.absolute(path));
}

Future<String> _sourceMapProvider(String scriptId) async {
var script = _idToUrl[scriptId];
if (script == null) return null;
var mapResponse = await http.get('$script.map');
if (mapResponse.statusCode != HttpStatus.ok) return null;
return mapResponse.body;
}

Future<String> _sourceProvider(String scriptId) async {
var script = _idToUrl[scriptId];
if (script == null) return null;
var scriptResponse = await http.get(script);
if (scriptResponse.statusCode != HttpStatus.ok) return null;
return scriptResponse.body;
}
}

Future<WipConnection> _connect(
Process process, int port, Map<String, String> idToUrl) async {
// Wait for Chrome to be in a ready state.
await process.stderr
.transform(utf8.decoder)
.transform(LineSplitter())
.firstWhere((line) => line.startsWith('DevTools listening'));

var chromeConnection = ChromeConnection('localhost', port);
var tab = (await chromeConnection.getTabs()).first;
var tabConnection = await tab.connect();

// Enable debugging.
await tabConnection.debugger.enable();

// Coverage reports are in terms of scriptIds so keep note of URLs.
tabConnection.debugger.onScriptParsed.listen((data) {
var script = data.script;
if (script.url.isNotEmpty) idToUrl[script.scriptId] = script.url;
});

// Enable coverage collection.
await tabConnection.debugger.connection.sendCommand('Profiler.enable', {});
await tabConnection.debugger.connection.sendCommand(
'Profiler.startPreciseCoverage', {'detailed': true, 'callCount': false});

return tabConnection;
}
4 changes: 3 additions & 1 deletion pkgs/test/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: test
version: 1.11.2-dev
version: 1.12.0-dev
description: A full featured library for writing and running Dart tests.
homepage: https://github.com/dart-lang/test/blob/master/pkgs/test

Expand All @@ -10,6 +10,7 @@ dependencies:
analyzer: ">=0.36.0 <0.40.0"
async: ^2.0.0
boolean_selector: ">=1.0.0 <3.0.0"
coverage: ^0.13.4
http_multi_server: ^2.0.0
io: ^0.3.0
js: ^0.6.0
Expand All @@ -28,6 +29,7 @@ dependencies:
stream_channel: ">=1.7.0 <3.0.0"
typed_data: ^1.0.0
web_socket_channel: ^1.0.0
webkit_inspection_protocol: ^0.5.0
yaml: ^2.0.0
# Use an exact version until the test_api and test_core package are stable.
test_api: 0.2.14
Expand Down
44 changes: 29 additions & 15 deletions pkgs/test/test/runner/coverage_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,26 @@ import 'package:test_descriptor/test_descriptor.dart' as d;

import 'package:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:test_process/test_process.dart';

import '../io.dart';

void main() {
group('with the --coverage flag,', () {
test('gathers coverage for VM tests', () async {
Directory coverageDirectory;

Future<void> _validateCoverage(
TestProcess test, String coveragePath) async {
expect(test.stdout, emitsThrough(contains('+1: All tests passed!')));
await test.shouldExit(0);

final coverageFile = File(p.join(coverageDirectory.path, coveragePath));
final coverage = await coverageFile.readAsString();
final jsonCoverage = json.decode(coverage);
expect(jsonCoverage['coverage'], isNotEmpty);
}

setUp(() async {
await d.file('test.dart', '''
import 'package:test/test.dart';

Expand All @@ -27,24 +41,24 @@ void main() {
}
''').create();

final coverageDirectory =
Directory(p.join(Directory.current.path, 'test_coverage'));
expect(await coverageDirectory.exists(), isFalse,
reason:
'Coverage directory exists, cannot safely run coverage tests. Delete the ${coverageDirectory.path} directory to fix.');
coverageDirectory =
await Directory.systemTemp.createTemp('test_coverage');
});

tearDown(() async {
await coverageDirectory.delete(recursive: true);
});

test('gathers coverage for VM tests', () async {
var test =
await runTest(['--coverage', coverageDirectory.path, 'test.dart']);
expect(test.stdout, emitsThrough(contains('+1: All tests passed!')));
await test.shouldExit(0);

final coverageFile =
File(p.join(coverageDirectory.path, 'test.dart.vm.json'));
final coverage = await coverageFile.readAsString();
final jsonCoverage = json.decode(coverage);
expect(jsonCoverage['coverage'], isNotEmpty);
await _validateCoverage(test, 'test.dart.vm.json');
});

await coverageDirectory.delete(recursive: true);
test('gathers coverage for Chrome tests', () async {
var test = await runTest(
['--coverage', coverageDirectory.path, 'test.dart', '-p', 'chrome']);
await _validateCoverage(test, 'test.dart.chrome.json');
});
});
}