Chrome coverage support #1155
Chrome coverage support #1155
Conversation
|
|
||
| import '../executable_settings.dart'; | ||
| import 'browser.dart'; | ||
| import 'default_settings.dart'; | ||
|
|
||
| // TODO(nweiz): move this into its own package? |
There was a problem hiding this comment.
There was a problem hiding this comment.
Are we using it? Should we file an issue to migrate to that package?
| @@ -73,6 +82,8 @@ class Chrome extends Browser { | |||
| if (port != null) { | |||
| remoteDebuggerCompleter.complete( | |||
| getRemoteDebuggerUrl(Uri.parse('http://localhost:$port'))); | |||
|
|
|||
| connectionCompleter.complete(await _connect(process, port, idToUrl)); | |||
There was a problem hiding this comment.
I don't think this await is necessary, Completer.complete takes a FutureOr.
| @@ -3,6 +3,8 @@ | |||
| * 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 | |||
There was a problem hiding this comment.
1.12.0 since this is a new feature 🎉
| message, gatherCoverage: () async { | ||
| var browser = _browser; | ||
| if (browser is Chrome) return browser.gatherCoverage(); | ||
| return {}; |
There was a problem hiding this comment.
should we warn here or anything?
There was a problem hiding this comment.
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.
| var script = _idToUrl[scriptId]; | ||
| if (script == null) return null; | ||
| var uri = Uri.parse(script); | ||
| var path = p.join( |
There was a problem hiding this comment.
nit: you could do
var path = p.joinAll([
...uri.pathSegments.sublist(1, uri.pathSegments.length - 1),
sourceUrl,
]);
|
What a nice surprise for a Friday afternoon! Thank you! |
|
We're pretty excited to use this .. will there be a 1.12 release with coverage soon? |
|
Likely next week (busy this week traveling for work). I need to make sure this solution scales for our internal use and so far it has been extremely promising. I have uncovered an issue in which invalid source maps can cause a crash. Once I fix that up I'll publish a new version of Glad you folks are excited :D |
Support coverage collection for the Chrome platform. Coverage information is output to
.chrome.jsonin a format suitable for consumption bypackage:coverage.Closes #36