Skip to content

Normalize Compact and Expanded reporters #1325

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 1 commit into from
Aug 22, 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
9 changes: 7 additions & 2 deletions pkgs/test_core/lib/src/runner/configuration/reporters.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,13 @@ final _allReporters = <String, ReporterDetails>{
printPath: config.paths.length > 1 ||
Directory(config.paths.single).existsSync(),
printPlatform: config.suiteDefaults.runtimes.length > 1)),
'compact': ReporterDetails('A single line, updated continuously.',
(_, engine, sink) => CompactReporter.watch(engine, sink)),
'compact': ReporterDetails(
'A single line, updated continuously.',
(config, engine, sink) => CompactReporter.watch(engine, sink,
color: config.color,
printPath: config.paths.length > 1 ||
Directory(config.paths.single).existsSync(),
printPlatform: config.suiteDefaults.runtimes.length > 1)),
'json': ReporterDetails(
'A machine-readable format (see https://bit.ly/2Z7J0OH).',
(_, engine, sink) => JsonReporter.watch(engine, sink)),
Expand Down
72 changes: 48 additions & 24 deletions pkgs/test_core/lib/src/runner/reporter/compact.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import 'package:test_api/src/utils.dart'; // ignore: implementation_imports
import 'package:test_api/src/utils.dart' as utils;

import '../../util/io.dart';
import '../configuration.dart';
import '../engine.dart';
import '../load_exception.dart';
import '../load_suite.dart';
Expand All @@ -23,40 +22,41 @@ import '../reporter.dart';
/// A reporter that prints test results to the console in a single
/// continuously-updating line.
class CompactReporter implements Reporter {
final _config = Configuration.current;
/// Whether the reporter should emit terminal color escapes.
final bool _color;

/// The terminal escape for green text, or the empty string if this is Windows
/// or not outputting to a terminal.
String get _green => _config.color ? '\u001b[32m' : '';
final String _green;

/// The terminal escape for red text, or the empty string if this is Windows
/// or not outputting to a terminal.
String get _red => _config.color ? '\u001b[31m' : '';
final String _red;

/// The terminal escape for yellow text, or the empty string if this is
/// Windows or not outputting to a terminal.
String get _yellow => _config.color ? '\u001b[33m' : '';
final String _yellow;

/// The terminal escape for gray text, or the empty string if this is
/// Windows or not outputting to a terminal.
String get _gray => _config.color ? '\u001b[1;30m' : '';
final String _gray;

/// The terminal escape for bold text, or the empty string if this is
/// Windows or not outputting to a terminal.
String get _bold => _config.color ? '\u001b[1m' : '';
final String _bold;

/// The terminal escape for removing test coloring, or the empty string if
/// this is Windows or not outputting to a terminal.
String get _noColor => _config.color ? '\u001b[0m' : '';

/// Whether the path to each test's suite should be printed.
final bool _printPath = Configuration.current.paths.length > 1 ||
Directory(Configuration.current.paths.single).existsSync();
final String _noColor;

/// The engine used to run the tests.
final Engine _engine;

final StringSink _sink;
/// Whether the path to each test's suite should be printed.
final bool _printPath;

/// Whether the platform each test is running on should be printed.
final bool _printPlatform;

/// A stopwatch that tracks the duration of the full run.
final _stopwatch = Stopwatch();
Expand All @@ -69,9 +69,10 @@ class CompactReporter implements Reporter {

/// The size of `_engine.passed` last time a progress notification was
/// printed.
int? _lastProgressPassed;
int _lastProgressPassed = 0;

/// The size of `_engine.skipped` last time a progress notification was printed.
/// The size of `_engine.skipped` last time a progress notification was
/// printed.
int? _lastProgressSkipped;

/// The size of `_engine.failed` last time a progress notification was
Expand Down Expand Up @@ -101,16 +102,39 @@ class CompactReporter implements Reporter {
/// The set of all subscriptions to various streams.
final _subscriptions = <StreamSubscription>{};

final StringSink _sink;

/// Watches the tests run by [engine] and prints their results to the
/// terminal.
static CompactReporter watch(Engine engine, StringSink sink) =>
CompactReporter._(engine, sink);

CompactReporter._(this._engine, this._sink) {
///
/// If [color] is `true`, this will use terminal colors; if it's `false`, it
/// won't. If [printPath] is `true`, this will print the path name as part of
/// the test description. Likewise, if [printPlatform] is `true`, this will
/// print the platform as part of the test description.
static CompactReporter watch(Engine engine, StringSink sink,
{required bool color,
required bool printPath,
required bool printPlatform}) =>
CompactReporter._(engine, sink,
color: color, printPath: printPath, printPlatform: printPlatform);

CompactReporter._(this._engine, this._sink,
{required bool color,
required bool printPath,
required bool printPlatform})
: _printPath = printPath,
_printPlatform = printPlatform,
_color = color,
_green = color ? '\u001b[32m' : '',
_red = color ? '\u001b[31m' : '',
_yellow = color ? '\u001b[33m' : '',
_gray = color ? '\u001b[1;30m' : '',
_bold = color ? '\u001b[1m' : '',
_noColor = color ? '\u001b[0m' : '' {
_subscriptions.add(_engine.onTestStarted.listen(_onTestStarted));

/// Convert the future to a stream so that the subscription can be paused or
/// canceled.
// Convert the future to a stream so that the subscription can be paused or
// canceled.
_subscriptions.add(_engine.success.asStream().listen(_onDone));
}

Expand Down Expand Up @@ -158,7 +182,7 @@ class CompactReporter implements Reporter {
_stopwatchStarted = true;
_stopwatch.start();

/// Keep updating the time even when nothing else is happening.
// Keep updating the time even when nothing else is happening.
_subscriptions.add(Stream.periodic(Duration(seconds: 1))
.listen((_) => _progressLine(_lastProgressMessage!)));
}
Expand Down Expand Up @@ -222,7 +246,7 @@ class CompactReporter implements Reporter {
}

// TODO - what type is this?
_sink.writeln(indent(error.toString(color: _config.color)));
_sink.writeln(indent(error.toString(color: _color)));

// Only print stack traces for load errors that come from the user's code.
if (error.innerError is! IOException &&
Expand Down Expand Up @@ -376,7 +400,7 @@ class CompactReporter implements Reporter {
name = '${liveTest.suite.path}: $name';
}

if (_config.suiteDefaults.runtimes.length > 1) {
if (_printPlatform) {
name = '[${liveTest.suite.platform.runtime.name}] $name';
}

Expand Down
22 changes: 12 additions & 10 deletions pkgs/test_core/lib/src/runner/reporter/expanded.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ class ExpandedReporter implements Reporter {

final StringSink _sink;

// TODO(nweiz): Get configuration from [Configuration.current] once we have
// cross-platform imports.
/// Watches the tests run by [engine] and prints their results to the
/// terminal.
///
Expand All @@ -96,13 +94,16 @@ class ExpandedReporter implements Reporter {
/// the test description. Likewise, if [printPlatform] is `true`, this will
/// print the platform as part of the test description.
static ExpandedReporter watch(Engine engine, StringSink sink,
{bool color = true, bool printPath = true, bool printPlatform = true}) {
return ExpandedReporter._(engine, sink,
color: color, printPath: printPath, printPlatform: printPlatform);
}
{required bool color,
required bool printPath,
required bool printPlatform}) =>
ExpandedReporter._(engine, sink,
color: color, printPath: printPath, printPlatform: printPlatform);

ExpandedReporter._(this._engine, this._sink,
{bool color = true, bool printPath = true, bool printPlatform = true})
{required bool color,
required bool printPath,
required bool printPlatform})
: _printPath = printPath,
_printPlatform = printPlatform,
_color = color,
Expand All @@ -114,8 +115,8 @@ class ExpandedReporter implements Reporter {
_noColor = color ? '\u001b[0m' : '' {
_subscriptions.add(_engine.onTestStarted.listen(_onTestStarted));

/// Convert the future to a stream so that the subscription can be paused or
/// canceled.
// Convert the future to a stream so that the subscription can be paused or
// canceled.
_subscriptions.add(_engine.success.asStream().listen(_onDone));
}

Expand All @@ -134,6 +135,7 @@ class ExpandedReporter implements Reporter {
@override
void resume() {
if (!_paused) return;

_stopwatch.start();

for (var subscription in _subscriptions) {
Expand Down Expand Up @@ -206,7 +208,7 @@ class ExpandedReporter implements Reporter {
}

// TODO - what type is this?
_sink.writeln(indent((error as dynamic).toString(color: _color) as String));
_sink.writeln(indent(error.toString(color: _color)));

// Only print stack traces for load errors that come from the user's code.
if (error.innerError is! FormatException && error.innerError is! String) {
Expand Down
24 changes: 12 additions & 12 deletions pkgs/test_core/lib/src/runner/reporter/json.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ class JsonReporter implements Reporter {
/// The engine used to run the tests.
final Engine _engine;

final StringSink _sink;

/// A stopwatch that tracks the duration of the full run.
final _stopwatch = Stopwatch();

Expand All @@ -45,12 +43,6 @@ class JsonReporter implements Reporter {
/// when the reporter is paused.
var _stopwatchStarted = false;

/// Whether the reporter is paused.
var _paused = false;

/// The set of all subscriptions to various streams.
final _subscriptions = <StreamSubscription>{};

/// An expando that associates unique IDs with [LiveTest]s.
final _liveTestIDs = <LiveTest, int>{};

Expand All @@ -63,15 +55,23 @@ class JsonReporter implements Reporter {
/// The next ID to associate with a [LiveTest].
var _nextID = 0;

/// Whether the reporter is paused.
var _paused = false;

/// The set of all subscriptions to various streams.
final _subscriptions = <StreamSubscription>{};
Copy link
Member

Choose a reason for hiding this comment

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

[nit] move this back if it wasn't touched

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed this because all 3 reporters have these same 3 fields directly before the constructor. If you look at a diff between them this minimizes the noise (although admittedly that is less useful in this case because this reporter is more different overall)


final StringSink _sink;

/// Watches the tests run by [engine] and prints their results as JSON.
static JsonReporter watch(Engine engine, StringSink sink) =>
JsonReporter._(engine, sink);

JsonReporter._(this._engine, this._sink) : _config = Configuration.current {
_subscriptions.add(_engine.onTestStarted.listen(_onTestStarted));

/// Convert the future to a stream so that the subscription can be paused or
/// canceled.
// Convert the future to a stream so that the subscription can be paused or
// canceled.
_subscriptions.add(_engine.success.asStream().listen(_onDone));

_subscriptions.add(_engine.onSuiteAdded.listen(null, onDone: () {
Expand Down Expand Up @@ -146,8 +146,8 @@ class JsonReporter implements Reporter {
liveTest.suite.path!)
});

/// Convert the future to a stream so that the subscription can be paused or
/// canceled.
// Convert the future to a stream so that the subscription can be paused or
// canceled.
_subscriptions.add(
liveTest.onComplete.asStream().listen((_) => _onComplete(liveTest)));

Expand Down