From 68858c76c091e0855b07c136724077b1def8d85a Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 17 Mar 2022 21:10:20 -0700 Subject: [PATCH 01/14] initial implementation of a github actions reporter --- .../src/runner/configuration/reporters.dart | 14 +- .../lib/src/runner/reporter/github.dart | 188 ++++++++++++++++++ pkgs/test_core/lib/src/util/io.dart | 6 + 3 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 pkgs/test_core/lib/src/runner/reporter/github.dart diff --git a/pkgs/test_core/lib/src/runner/configuration/reporters.dart b/pkgs/test_core/lib/src/runner/configuration/reporters.dart index d3d4604d1..c72e7db17 100644 --- a/pkgs/test_core/lib/src/runner/configuration/reporters.dart +++ b/pkgs/test_core/lib/src/runner/configuration/reporters.dart @@ -11,6 +11,7 @@ import '../engine.dart'; import '../reporter.dart'; import '../reporter/compact.dart'; import '../reporter/expanded.dart'; +import '../reporter/github.dart'; import '../reporter/json.dart'; /// Constructs a reporter for the provided engine with the provided @@ -43,6 +44,11 @@ final _allReporters = { printPath: config.paths.length > 1 || Directory(config.paths.single.testPath).existsSync(), printPlatform: config.suiteDefaults.runtimes.length > 1)), + 'github': ReporterDetails( + 'Custom output for Github Actions.', + (config, engine, sink) => GithubReporter.watch(engine, sink, + printPath: config.paths.length > 1 || + Directory(config.paths.single.testPath).existsSync())), 'json': ReporterDetails( 'A machine-readable format (see ' 'https://dart.dev/go/test-docs/json_reporter.md).', @@ -52,9 +58,11 @@ final _allReporters = { final defaultReporter = inTestTests ? 'expanded' - : canUseSpecialChars - ? 'compact' - : 'expanded'; + : githubContext + ? 'github' + : canUseSpecialChars + ? 'compact' + : 'expanded'; /// **Do not call this function without express permission from the test package /// authors**. diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart new file mode 100644 index 000000000..311235699 --- /dev/null +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -0,0 +1,188 @@ +// Copyright (c) 2022, 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. + +// ignore_for_file: implementation_imports + +import 'dart:async'; + +import 'package:test_api/src/backend/live_test.dart'; +import 'package:test_api/src/backend/message.dart'; +import 'package:test_api/src/backend/state.dart'; +import 'package:test_api/src/backend/util/pretty_print.dart'; + +import '../engine.dart'; +import '../load_suite.dart'; +import '../reporter.dart'; + +/// A reporter that prints test output using formatting for Github Actions. +/// +/// See +/// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions +/// for a description of the output format, and +/// https://github.com/dart-lang/test/issues/1415 for discussions about this +/// implementation. +class GithubReporter implements Reporter { + /// The engine used to run the tests. + final Engine _engine; + + /// Whether the path to each test's suite should be printed. + final bool _printPath; + + /// Whether the reporter is paused. + var _paused = false; + + /// The set of all subscriptions to various streams. + final _subscriptions = {}; + + final StringSink _sink; + final _helper = _GithubHelper(); + + final Map> _testMessages = {}; + + /// Watches the tests run by [engine] and prints their results as JSON. + static GithubReporter watch(Engine engine, StringSink sink, + {required bool printPath}) => + GithubReporter._(engine, sink, printPath); + + GithubReporter._(this._engine, this._sink, this._printPath) { + _subscriptions.add(_engine.onTestStarted.listen(_onTestStarted)); + _subscriptions.add(_engine.success.asStream().listen(_onDone)); + } + + @override + void pause() { + if (_paused) return; + _paused = true; + + for (var subscription in _subscriptions) { + subscription.pause(); + } + } + + @override + void resume() { + if (!_paused) return; + _paused = false; + + for (var subscription in _subscriptions) { + subscription.resume(); + } + } + + void _cancel() { + for (var subscription in _subscriptions) { + subscription.cancel(); + } + _subscriptions.clear(); + } + + /// A callback called when the engine begins running [liveTest]. + void _onTestStarted(LiveTest liveTest) { + // Convert the future to a stream so that the subscription can be paused or + // canceled. + _subscriptions.add( + liveTest.onComplete.asStream().listen((_) => _onComplete(liveTest))); + + // Collect messages from tests as they are emitted. + _subscriptions.add(liveTest.onMessage.listen((message) { + _testMessages.putIfAbsent(liveTest, () => []).add(message); + })); + } + + /// A callback called when [liveTest] finishes running. + void _onComplete(LiveTest test) { + final errors = test.errors; + final messages = _testMessages[test] ?? []; + final skipped = test.state.result == Result.skipped; + final failed = errors.isNotEmpty; + + void emitMessages(List messages) { + for (var message in messages) { + _sink.writeln(message.text); + } + } + + void emitErrors(List errors) { + for (var error in errors) { + _sink.writeln('${error.error}'); + _sink.writeln(error.stackTrace.toString().trimRight()); + } + } + + final isLoadSuite = test.suite is LoadSuite; + if (isLoadSuite) { + // Don't emit any info for 'loadSuite' tests, unless they contain errors. + if (errors.isNotEmpty || messages.isNotEmpty) { + _sink.writeln('${test.suite.path}:'); + emitMessages(messages); + emitErrors(errors); + } + + return; + } + + final prefix = failed + ? _GithubHelper.failedIcon + : skipped + ? _GithubHelper.skippedIcon + : _GithubHelper.passedIcon; + final statusSuffix = failed + ? ' (failed)' + : skipped + ? ' (skipped)' + : ''; + + var name = test.test.name; + if (_printPath && test.suite.path != null) { + name = '${test.suite.path}: $name'; + } + _sink.writeln(_helper.startGroup('$prefix $name$statusSuffix')); + emitMessages(messages); + emitErrors(errors); + _sink.writeln(_helper.endGroup); + } + + void _onDone(bool? success) { + _cancel(); + + _sink.writeln(); + + final hadFailures = _engine.failed.isNotEmpty; + String message = + '${_engine.passed.length} ${pluralize('test', _engine.passed.length)} passed'; + if (_engine.failed.isNotEmpty) { + message += ', ${_engine.failed.length} failed'; + } + if (_engine.skipped.isNotEmpty) { + message += ', ${_engine.skipped.length} skipped'; + } + message += '.'; + _sink.writeln(hadFailures ? _helper.error(message) : message); + } + + // todo: do we need to bake in awareness about tests that haven't completed + // yet? + + // ignore: unused_element + String _normalizeTestResult(LiveTest liveTest) { + // For backwards-compatibility, report skipped tests as successes. + if (liveTest.state.result == Result.skipped) return 'success'; + // if test is still active, it was probably cancelled + if (_engine.active.contains(liveTest)) return 'error'; + return liveTest.state.result.toString(); + } +} + +class _GithubHelper { + static const String passedIcon = '✅'; + static const String failedIcon = '❌'; + static const String skippedIcon = '☑️'; + + _GithubHelper(); + + String startGroup(String title) => '::group::$title'; + final String endGroup = '::endgroup::'; + + String error(String message) => '::error::$message'; +} diff --git a/pkgs/test_core/lib/src/util/io.dart b/pkgs/test_core/lib/src/util/io.dart index 657ec1e39..56a4dcd0c 100644 --- a/pkgs/test_core/lib/src/util/io.dart +++ b/pkgs/test_core/lib/src/util/io.dart @@ -87,6 +87,12 @@ final _tempDir = Platform.environment.containsKey('_UNITTEST_TEMP_DIR') bool get canUseSpecialChars => (!Platform.isWindows || stdout.supportsAnsiEscapes) && !inTestTests; +/// Detect whether we're running in a Github Actions context. +/// +/// See +/// https://docs.github.com/en/actions/learn-github-actions/environment-variables. +bool get githubContext => Platform.environment['GITHUB_ACTIONS'] == 'true'; + /// Creates a temporary directory and returns its path. String createTempDir() => Directory(_tempDir).createTempSync('dart_test_').resolveSymbolicLinksSync(); From 84c3797c5873792dfde03adcd0c9a7cfbba9bad4 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 17 Mar 2022 21:36:37 -0700 Subject: [PATCH 02/14] remove newlines from test titles --- .../lib/src/runner/reporter/github.dart | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index 311235699..9b276b4a9 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -41,8 +41,11 @@ class GithubReporter implements Reporter { final Map> _testMessages = {}; /// Watches the tests run by [engine] and prints their results as JSON. - static GithubReporter watch(Engine engine, StringSink sink, - {required bool printPath}) => + static GithubReporter watch( + Engine engine, + StringSink sink, { + required bool printPath, + }) => GithubReporter._(engine, sink, printPath); GithubReporter._(this._engine, this._sink, this._printPath) { @@ -158,7 +161,11 @@ class GithubReporter implements Reporter { message += ', ${_engine.skipped.length} skipped'; } message += '.'; - _sink.writeln(hadFailures ? _helper.error(message) : message); + _sink.writeln( + hadFailures + ? _helper.error(message) + : '${_GithubHelper.celebrationIcon} $message', + ); } // todo: do we need to bake in awareness about tests that haven't completed @@ -177,11 +184,12 @@ class GithubReporter implements Reporter { class _GithubHelper { static const String passedIcon = '✅'; static const String failedIcon = '❌'; - static const String skippedIcon = '☑️'; + static const String skippedIcon = '❎'; + static const String celebrationIcon = '🎉'; _GithubHelper(); - String startGroup(String title) => '::group::$title'; + String startGroup(String title) => '::group::${title.replaceAll('\n', ' ')}'; final String endGroup = '::endgroup::'; String error(String message) => '::error::$message'; From 4554787b59b26fbfd7e9bd74e3f1d0bbb08b24f0 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Fri, 18 Mar 2022 08:01:08 -0700 Subject: [PATCH 03/14] some output tweaks --- pkgs/test_core/lib/src/runner/reporter/github.dart | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index 9b276b4a9..aff87c908 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -101,6 +101,7 @@ class GithubReporter implements Reporter { final failed = errors.isNotEmpty; void emitMessages(List messages) { + // todo: escape some github commands? ::group::, ::endgroup::, ::error::, ... for (var message in messages) { _sink.writeln(message.text); } @@ -113,6 +114,9 @@ class GithubReporter implements Reporter { } } + // TODO: how to recognize (setUpAll) and (tearDownAll)? + // And are they reported in the 'passed test' count from the engine? + final isLoadSuite = test.suite is LoadSuite; if (isLoadSuite) { // Don't emit any info for 'loadSuite' tests, unless they contain errors. @@ -152,19 +156,18 @@ class GithubReporter implements Reporter { _sink.writeln(); final hadFailures = _engine.failed.isNotEmpty; - String message = - '${_engine.passed.length} ${pluralize('test', _engine.passed.length)} passed'; + var message = '${_engine.passed.length} ' + '${pluralize('test', _engine.passed.length)} passed'; if (_engine.failed.isNotEmpty) { message += ', ${_engine.failed.length} failed'; } if (_engine.skipped.isNotEmpty) { message += ', ${_engine.skipped.length} skipped'; } - message += '.'; _sink.writeln( hadFailures ? _helper.error(message) - : '${_GithubHelper.celebrationIcon} $message', + : '$message ${_GithubHelper.celebrationIcon}', ); } From e86c5b04ef4853e84820c3396be984bfc588c421 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sat, 19 Mar 2022 10:55:16 -0700 Subject: [PATCH 04/14] better visualize setUpAll, tearDownAll, and loadSuite LiveTests --- pkgs/test_api/lib/src/backend/declarer.dart | 7 +- .../lib/src/runner/reporter/github.dart | 93 +++++++++---------- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/pkgs/test_api/lib/src/backend/declarer.dart b/pkgs/test_api/lib/src/backend/declarer.dart index 7a37552c6..27e614453 100644 --- a/pkgs/test_api/lib/src/backend/declarer.dart +++ b/pkgs/test_api/lib/src/backend/declarer.dart @@ -14,6 +14,9 @@ import 'invoker.dart'; import 'metadata.dart'; import 'test.dart'; +const setUpAllName = '(setUpAll)'; +const tearDownAllName = '(tearDownAll)'; + /// A class that manages the state of tests as they're declared. /// /// A nested tree of Declarers tracks the current group, set-up, and tear-down @@ -357,7 +360,7 @@ class Declarer { Test? get _setUpAll { if (_setUpAlls.isEmpty) return null; - return LocalTest(_prefix('(setUpAll)'), _metadata.change(timeout: _timeout), + return LocalTest(_prefix(setUpAllName), _metadata.change(timeout: _timeout), () { return runZoned( () => Future.forEach(_setUpAlls, (setUp) => setUp()), @@ -374,7 +377,7 @@ class Declarer { if (_setUpAlls.isEmpty && _tearDownAlls.isEmpty) return null; return LocalTest( - _prefix('(tearDownAll)'), _metadata.change(timeout: _timeout), () { + _prefix(tearDownAllName), _metadata.change(timeout: _timeout), () { return runZoned(() => Invoker.current!.runTearDowns(_tearDownAlls), // Make the declarer visible to running scaffolds so they can add to // the declarer's `tearDownAll()` list. diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index aff87c908..65b5c4e37 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -9,6 +9,7 @@ import 'dart:async'; import 'package:test_api/src/backend/live_test.dart'; import 'package:test_api/src/backend/message.dart'; import 'package:test_api/src/backend/state.dart'; +import 'package:test_api/src/backend/declarer.dart'; import 'package:test_api/src/backend/util/pretty_print.dart'; import '../engine.dart'; @@ -99,41 +100,27 @@ class GithubReporter implements Reporter { final messages = _testMessages[test] ?? []; final skipped = test.state.result == Result.skipped; final failed = errors.isNotEmpty; - - void emitMessages(List messages) { - // todo: escape some github commands? ::group::, ::endgroup::, ::error::, ... - for (var message in messages) { - _sink.writeln(message.text); - } - } - - void emitErrors(List errors) { - for (var error in errors) { - _sink.writeln('${error.error}'); - _sink.writeln(error.stackTrace.toString().trimRight()); - } - } - - // TODO: how to recognize (setUpAll) and (tearDownAll)? - // And are they reported in the 'passed test' count from the engine? - - final isLoadSuite = test.suite is LoadSuite; - if (isLoadSuite) { - // Don't emit any info for 'loadSuite' tests, unless they contain errors. - if (errors.isNotEmpty || messages.isNotEmpty) { - _sink.writeln('${test.suite.path}:'); - emitMessages(messages); - emitErrors(errors); - } - - return; - } - + final loadSuite = test.suite is LoadSuite; + final setUpAll = test.individualName == setUpAllName; + final tearDownAll = test.individualName == tearDownAllName; + + // // Don't emit any info for 'loadSuite' tests unless they contain errors. + // if (isLoadSuite && (errors.isEmpty && messages.isEmpty)) { + // return; + // } + + var defaultIcon = loadSuite + ? _GithubHelper.loadSuite + : setUpAll + ? _GithubHelper.setUpAll + : tearDownAll + ? _GithubHelper.tearDownAll + : _GithubHelper.passed; final prefix = failed - ? _GithubHelper.failedIcon + ? _GithubHelper.failed : skipped - ? _GithubHelper.skippedIcon - : _GithubHelper.passedIcon; + ? _GithubHelper.skipped + : defaultIcon; final statusSuffix = failed ? ' (failed)' : skipped @@ -144,9 +131,16 @@ class GithubReporter implements Reporter { if (_printPath && test.suite.path != null) { name = '${test.suite.path}: $name'; } + // TODO: have a visual indication when passed tests still contain contents + // in the group? _sink.writeln(_helper.startGroup('$prefix $name$statusSuffix')); - emitMessages(messages); - emitErrors(errors); + for (var message in messages) { + _sink.writeln(message.text); + } + for (var error in errors) { + _sink.writeln('${error.error}'); + _sink.writeln(error.stackTrace.toString().trimRight()); + } _sink.writeln(_helper.endGroup); } @@ -156,24 +150,24 @@ class GithubReporter implements Reporter { _sink.writeln(); final hadFailures = _engine.failed.isNotEmpty; - var message = '${_engine.passed.length} ' - '${pluralize('test', _engine.passed.length)} passed'; + final message = StringBuffer('${_engine.passed.length} ' + '${pluralize('test', _engine.passed.length)} passed'); if (_engine.failed.isNotEmpty) { - message += ', ${_engine.failed.length} failed'; + message.write(', ${_engine.failed.length} failed'); } if (_engine.skipped.isNotEmpty) { - message += ', ${_engine.skipped.length} skipped'; + message.write(' (${_engine.skipped.length} skipped)'); } + message.write('.'); _sink.writeln( hadFailures - ? _helper.error(message) - : '$message ${_GithubHelper.celebrationIcon}', + ? _helper.error(message.toString()) + : '${_GithubHelper.passed} $message', ); } // todo: do we need to bake in awareness about tests that haven't completed - // yet? - + // yet? some reporters seem to, but not all // ignore: unused_element String _normalizeTestResult(LiveTest liveTest) { // For backwards-compatibility, report skipped tests as successes. @@ -185,10 +179,15 @@ class GithubReporter implements Reporter { } class _GithubHelper { - static const String passedIcon = '✅'; - static const String failedIcon = '❌'; - static const String skippedIcon = '❎'; - static const String celebrationIcon = '🎉'; + static const String passed = '✅'; + static const String skipped = ' ⃞'; + static const String failed = '❌'; + + // char sets avilable at https://www.compart.com/en/unicode/ + // todo: ⛔ ⊝ ⏹ ⃞, ⏺ ⏩ ⏪ ∅, + static const String loadSuite = '⏺'; + static const String setUpAll = '⏩'; + static const String tearDownAll = '⏪'; _GithubHelper(); From 0dbb2b93d7552df5e22b73176b242932eea306da Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sat, 19 Mar 2022 13:02:47 -0700 Subject: [PATCH 05/14] tweak the icons used --- .../lib/src/runner/reporter/github.dart | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index 65b5c4e37..7d6f6f002 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -104,18 +104,16 @@ class GithubReporter implements Reporter { final setUpAll = test.individualName == setUpAllName; final tearDownAll = test.individualName == tearDownAllName; - // // Don't emit any info for 'loadSuite' tests unless they contain errors. - // if (isLoadSuite && (errors.isEmpty && messages.isEmpty)) { - // return; - // } - - var defaultIcon = loadSuite - ? _GithubHelper.loadSuite - : setUpAll - ? _GithubHelper.setUpAll - : tearDownAll - ? _GithubHelper.tearDownAll - : _GithubHelper.passed; + // Don't emit any info for 'loadSuite' tests unless they contain errors. + if (loadSuite && (errors.isEmpty && messages.isEmpty)) { + return; + } + + var defaultIcon = setUpAll + ? _GithubHelper.setUpAll + : tearDownAll + ? _GithubHelper.tearDownAll + : _GithubHelper.passed; final prefix = failed ? _GithubHelper.failed : skipped @@ -180,14 +178,13 @@ class GithubReporter implements Reporter { class _GithubHelper { static const String passed = '✅'; - static const String skipped = ' ⃞'; + static const String skipped = '∅'; static const String failed = '❌'; // char sets avilable at https://www.compart.com/en/unicode/ // todo: ⛔ ⊝ ⏹ ⃞, ⏺ ⏩ ⏪ ∅, - static const String loadSuite = '⏺'; - static const String setUpAll = '⏩'; - static const String tearDownAll = '⏪'; + static const String setUpAll = '⏺'; + static const String tearDownAll = '⏺'; _GithubHelper(); From cb3e6d400f301bff186afe1bfb188c0031a651fb Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sat, 19 Mar 2022 16:40:03 -0700 Subject: [PATCH 06/14] more teaks to icons --- .../lib/src/runner/reporter/github.dart | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index 7d6f6f002..cb989f3f0 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -92,6 +92,9 @@ class GithubReporter implements Reporter { _subscriptions.add(liveTest.onMessage.listen((message) { _testMessages.putIfAbsent(liveTest, () => []).add(message); })); + + // Add a spacer between pre-test output and the test results. + _sink.writeln(); } /// A callback called when [liveTest] finishes running. @@ -101,19 +104,18 @@ class GithubReporter implements Reporter { final skipped = test.state.result == Result.skipped; final failed = errors.isNotEmpty; final loadSuite = test.suite is LoadSuite; - final setUpAll = test.individualName == setUpAllName; - final tearDownAll = test.individualName == tearDownAllName; + final synthetic = loadSuite || + test.individualName == setUpAllName || + test.individualName == tearDownAllName; - // Don't emit any info for 'loadSuite' tests unless they contain errors. - if (loadSuite && (errors.isEmpty && messages.isEmpty)) { + // Don't emit any info for 'loadSuite', setUpAll, or tearDownAll tests + // unless they contain errors or other info. + if (synthetic && (errors.isEmpty && messages.isEmpty)) { return; } - var defaultIcon = setUpAll - ? _GithubHelper.setUpAll - : tearDownAll - ? _GithubHelper.tearDownAll - : _GithubHelper.passed; + var defaultIcon = + synthetic ? _GithubHelper.synthetic : _GithubHelper.passed; final prefix = failed ? _GithubHelper.failed : skipped @@ -126,8 +128,10 @@ class GithubReporter implements Reporter { : ''; var name = test.test.name; - if (_printPath && test.suite.path != null) { - name = '${test.suite.path}: $name'; + if (!loadSuite) { + if (_printPath && test.suite.path != null) { + name = '${test.suite.path}: $name'; + } } // TODO: have a visual indication when passed tests still contain contents // in the group? @@ -178,13 +182,13 @@ class GithubReporter implements Reporter { class _GithubHelper { static const String passed = '✅'; - static const String skipped = '∅'; + static const String skipped = ' ⃞'; static const String failed = '❌'; // char sets avilable at https://www.compart.com/en/unicode/ - // todo: ⛔ ⊝ ⏹ ⃞, ⏺ ⏩ ⏪ ∅, - static const String setUpAll = '⏺'; - static const String tearDownAll = '⏺'; + // todo: ⛔, ⃞ + // ⊝, ⏹, ⃞, ⏺, ⏩, ⏪, ∅, ❎ + static const String synthetic = '⏺'; _GithubHelper(); From cea45efa7bae1e48d4290eff6c168992709d6de8 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sun, 20 Mar 2022 09:59:39 -0700 Subject: [PATCH 07/14] adjust success method and icons --- .../lib/src/runner/reporter/github.dart | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index cb989f3f0..99a4912f3 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -52,6 +52,9 @@ class GithubReporter implements Reporter { GithubReporter._(this._engine, this._sink, this._printPath) { _subscriptions.add(_engine.onTestStarted.listen(_onTestStarted)); _subscriptions.add(_engine.success.asStream().listen(_onDone)); + + // Add a spacer between pre-test output and the test results. + _sink.writeln(); } @override @@ -92,9 +95,6 @@ class GithubReporter implements Reporter { _subscriptions.add(liveTest.onMessage.listen((message) { _testMessages.putIfAbsent(liveTest, () => []).add(message); })); - - // Add a spacer between pre-test output and the test results. - _sink.writeln(); } /// A callback called when [liveTest] finishes running. @@ -158,17 +158,17 @@ class GithubReporter implements Reporter { message.write(', ${_engine.failed.length} failed'); } if (_engine.skipped.isNotEmpty) { - message.write(' (${_engine.skipped.length} skipped)'); + message.write(', ${_engine.skipped.length} skipped'); } message.write('.'); _sink.writeln( hadFailures ? _helper.error(message.toString()) - : '${_GithubHelper.passed} $message', + : '${_GithubHelper.success} $message', ); } - // todo: do we need to bake in awareness about tests that haven't completed + // TODO: Do we need to bake in awareness about tests that haven't completed // yet? some reporters seem to, but not all // ignore: unused_element String _normalizeTestResult(LiveTest liveTest) { @@ -182,14 +182,15 @@ class GithubReporter implements Reporter { class _GithubHelper { static const String passed = '✅'; - static const String skipped = ' ⃞'; + static const String skipped = '❎'; static const String failed = '❌'; - // char sets avilable at https://www.compart.com/en/unicode/ - // todo: ⛔, ⃞ - // ⊝, ⏹, ⃞, ⏺, ⏩, ⏪, ∅, ❎ + // Char sets avilable at https://www.compart.com/en/unicode/: + // ⛔, ⏹, ⏺, ⏩, ⏪, ∅, ❎, 🚫 static const String synthetic = '⏺'; + static const String success = '🎉'; + _GithubHelper(); String startGroup(String title) => '::group::${title.replaceAll('\n', ' ')}'; From 2cbf943ad634924aaca2dda141d08d28579a5a01 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 22 Mar 2022 13:40:42 -0700 Subject: [PATCH 08/14] add tests --- .../test/runner/github_reporter_test.dart | 392 ++++++++++++++++++ pkgs/test/test/runner/runner_test.dart | 1 + .../src/runner/configuration/reporters.dart | 2 +- .../lib/src/runner/reporter/github.dart | 37 +- 4 files changed, 412 insertions(+), 20 deletions(-) create mode 100644 pkgs/test/test/runner/github_reporter_test.dart diff --git a/pkgs/test/test/runner/github_reporter_test.dart b/pkgs/test/test/runner/github_reporter_test.dart new file mode 100644 index 000000000..e971e207c --- /dev/null +++ b/pkgs/test/test/runner/github_reporter_test.dart @@ -0,0 +1,392 @@ +// Copyright (c) 2022, 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. + +@TestOn('vm') + +import 'dart:async'; + +import 'package:test_descriptor/test_descriptor.dart' as d; + +import 'package:test/test.dart'; + +import '../io.dart'; + +void main() { + setUpAll(precompileTestExecutable); + + test('reports when no tests are run', () async { + await d.file('test.dart', 'void main() {}').create(); + + var test = await runTest(['test.dart'], reporter: 'github'); + expect(test.stdout, emitsThrough(contains('0 tests passed'))); + await test.shouldExit(79); + }); + + test('runs several successful tests and reports when each completes', () { + return _expectReport(''' + test('success 1', () {}); + test('success 2', () {}); + test('success 3', () {});''', ''' + ::group::✅ success 1 + ::endgroup:: + ::group::✅ success 2 + ::endgroup:: + ::group::✅ success 3 + ::endgroup:: + 🎉 3 tests passed.'''); + }); + + test('runs several failing tests and reports when each fails', () { + return _expectReport(''' + test('failure 1', () => throw TestFailure('oh no')); + test('failure 2', () => throw TestFailure('oh no')); + test('failure 3', () => throw TestFailure('oh no'));''', ''' + ::group::❌ failure 1 (failed) + oh no + test.dart 6:33 main. + ::endgroup:: + ::group::❌ failure 2 (failed) + oh no + test.dart 7:33 main. + ::endgroup:: + ::group::❌ failure 3 (failed) + oh no + test.dart 8:33 main. + ::endgroup:: + ::error::0 tests passed, 3 failed.'''); + }); + + test('includes the full stack trace with --verbose-trace', () async { + await d.file('test.dart', ''' +import 'dart:async'; + +import 'package:test/test.dart'; + +void main() { + test("failure", () => throw "oh no"); +} +''').create(); + + var test = + await runTest(['--verbose-trace', 'test.dart'], reporter: 'github'); + expect(test.stdout, emitsThrough(contains('dart:async'))); + await test.shouldExit(1); + }); + + test('runs failing tests along with successful tests', () { + return _expectReport(''' + test('failure 1', () => throw TestFailure('oh no')); + test('success 1', () {}); + test('failure 2', () => throw TestFailure('oh no')); + test('success 2', () {});''', ''' + ::group::❌ failure 1 (failed) + oh no + test.dart 6:33 main. + ::endgroup:: + ::group::✅ success 1 + ::endgroup:: + ::group::❌ failure 2 (failed) + oh no + test.dart 8:33 main. + ::endgroup:: + ::group::✅ success 2 + ::endgroup:: + ::error::2 tests passed, 2 failed.'''); + }); + + test('always prints the full test name', () { + return _expectReport( + ''' + test( + 'really gosh dang long test name. Even longer than that. No, yet ' + 'longer. A little more... okay, that should do it.', + () {});''', + ''' + ::group::✅ really gosh dang long test name. Even longer than that. No, yet longer. A little more... okay, that should do it. + ::endgroup::''', + useContains: true, + ); + }); + + test('gracefully handles multiple test failures in a row', () { + return _expectReport(''' + // This completer ensures that the test isolate isn't killed until all + // errors have been thrown. + var completer = Completer(); + test('failures', () { + Future.microtask(() => throw 'first error'); + Future.microtask(() => throw 'second error'); + Future.microtask(() => throw 'third error'); + Future.microtask(completer.complete); + }); + test('wait', () => completer.future);''', ''' + ::group::❌ failures (failed) + first error + test.dart 10:34 main.. + second error + test.dart 11:34 main.. + third error + test.dart 12:34 main.. + ::endgroup:: + ::group::✅ wait + ::endgroup:: + ::error::1 test passed, 1 failed.'''); + }); + + group('print:', () { + test('handles multiple prints', () { + return _expectReport( + ''' + test('test', () { + print("one"); + print("two"); + print("three"); + print("four"); + });''', + ''' + ::group::✅ test + one + two + three + four + ::endgroup::''', + useContains: true, + ); + }); + + test('handles a print after the test completes', () { + return _expectReport(''' + // This completer ensures that the test isolate isn't killed until all + // prints have happened. + var testDone = Completer(); + var waitStarted = Completer(); + test('test', () async { + waitStarted.future.then((_) { + Future(() => print("one")); + Future(() => print("two")); + Future(() => print("three")); + Future(() => print("four")); + Future(testDone.complete); + }); + }); + + test('wait', () { + waitStarted.complete(); + return testDone.future; + });''', ''' + ::group::✅ test + ::endgroup:: + one + two + three + four + ::group::✅ wait + ::endgroup:: + 🎉 2 tests passed.'''); + }); + + // todo: fix this + // test('interleaves prints and errors', () { + // return _expectReport(''' + // // This completer ensures that the test isolate isn't killed until all + // // prints have happened. + // var completer = Completer(); + // test('test', () { + // scheduleMicrotask(() { + // print("three"); + // print("four"); + // throw "second error"; + // }); + + // scheduleMicrotask(() { + // print("five"); + // print("six"); + // completer.complete(); + // }); + + // print("one"); + // print("two"); + // throw "first error"; + // }); + + // test('wait', () => completer.future);''', ''' + // +0: test + // one + // two + // +0 -1: test [E] + // first error + // test.dart 24:11 main. + + // three + // four + // second error + // test.dart 13:13 main.. + // ===== asynchronous gap =========================== + // dart:async scheduleMicrotask + // test.dart 10:11 main. + + // five + // six + // +0 -1: wait + // +1 -1: Some tests failed.'''); + // }); + }); + + group('skip:', () { + test('displays skipped tests', () { + return _expectReport(''' + test('skip 1', () {}, skip: true); + test('skip 2', () {}, skip: true); + test('skip 3', () {}, skip: true);''', ''' + ::group::❎ skip 1 (skipped) + ::endgroup:: + ::group::❎ skip 2 (skipped) + ::endgroup:: + ::group::❎ skip 3 (skipped) + ::endgroup:: + 🎉 0 tests passed, 3 skipped.'''); + }); + + test('displays a skipped group', () { + return _expectReport(''' + group('skip', () { + test('test 1', () {}); + test('test 2', () {}); + test('test 3', () {}); + }, skip: true);''', ''' + ::group::❎ skip test 1 (skipped) + ::endgroup:: + ::group::❎ skip test 2 (skipped) + ::endgroup:: + ::group::❎ skip test 3 (skipped) + ::endgroup:: + 🎉 0 tests passed, 3 skipped.'''); + }); + + test('runs skipped tests along with successful tests', () { + return _expectReport(''' + test('skip 1', () {}, skip: true); + test('success 1', () {}); + test('skip 2', () {}, skip: true); + test('success 2', () {});''', ''' + ::group::❎ skip 1 (skipped) + ::endgroup:: + ::group::✅ success 1 + ::endgroup:: + ::group::❎ skip 2 (skipped) + ::endgroup:: + ::group::✅ success 2 + ::endgroup:: + 🎉 2 tests passed, 2 skipped.'''); + }); + + test('runs skipped tests along with successful and failing tests', () { + return _expectReport(''' + test('failure 1', () => throw TestFailure('oh no')); + test('skip 1', () {}, skip: true); + test('success 1', () {}); + test('failure 2', () => throw TestFailure('oh no')); + test('skip 2', () {}, skip: true); + test('success 2', () {});''', ''' + ::group::❌ failure 1 (failed) + oh no + test.dart 6:35 main. + ::endgroup:: + ::group::❎ skip 1 (skipped) + ::endgroup:: + ::group::✅ success 1 + ::endgroup:: + ::group::❌ failure 2 (failed) + oh no + test.dart 9:35 main. + ::endgroup:: + ::group::❎ skip 2 (skipped) + ::endgroup:: + ::group::✅ success 2 + ::endgroup:: + ::error::2 tests passed, 2 failed, 2 skipped.'''); + }); + + test('displays the skip reason if available', () { + return _expectReport(''' + test('skip 1', () {}, skip: 'some reason'); + test('skip 2', () {}, skip: 'or another');''', ''' + ::group::❎ skip 1 (skipped) + Skip: some reason + ::endgroup:: + ::group::❎ skip 2 (skipped) + Skip: or another + ::endgroup:: + 🎉 0 tests passed, 2 skipped.'''); + }); + }); + + test('loadSuite, setUpAll, and tearDownAll hidden if no content', () { + return _expectReport(''' + group('one', () { + setUpAll(() {/* nothing to do here */}); + tearDownAll(() {/* nothing to do here */}); + test('test 1', () {}); + });''', ''' + ::group::✅ one test 1 + ::endgroup:: + 🎉 1 test passed.'''); + }); + + test('setUpAll and tearDownAll show when they have content', () { + return _expectReport(''' + group('one', () { + setUpAll(() { print('one'); }); + tearDownAll(() { print('two'); }); + test('test 1', () {}); + });''', ''' + ::group::⏺ one (setUpAll) + one + ::endgroup:: + ::group::✅ one test 1 + ::endgroup:: + ::group::⏺ one (tearDownAll) + two + ::endgroup:: + 🎉 1 test passed.'''); + }); +} + +Future _expectReport( + String tests, + String expected, { + List args = const [], + bool useContains = false, +}) async { + await d.file('test.dart', ''' + import 'dart:async'; + + import 'package:test/test.dart'; + + void main() { +$tests + } + ''').create(); + + var test = await runTest([ + 'test.dart', + ...args, + ], reporter: 'github'); + await test.shouldExit(); + + var stdoutLines = await test.stdoutStream().toList(); + var actual = stdoutLines + .map((line) => line.trim()) + .where((line) => line.isNotEmpty) + .join('\n'); + + // Un-indent the expected string. + var indentation = expected.indexOf(RegExp('[^ ]')); + expected = expected.split('\n').map((line) { + if (line.isEmpty) return line; + return line.substring(indentation); + }).join('\n'); + + expect(actual, useContains ? contains(expected) : equals(expected)); +} diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index c3938833e..8b0f546ec 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/pkgs/test/test/runner/runner_test.dart @@ -107,6 +107,7 @@ Output: [compact] A single line, updated continuously. [expanded] (default) A separate line for each update. + [github] A custom reporter for GitHub Actions output. [json] A machine-readable format (see https://dart.dev/go/test-docs/json_reporter.md). --file-reporter Enable an additional reporter writing test results to a file. diff --git a/pkgs/test_core/lib/src/runner/configuration/reporters.dart b/pkgs/test_core/lib/src/runner/configuration/reporters.dart index c72e7db17..a8c2528fb 100644 --- a/pkgs/test_core/lib/src/runner/configuration/reporters.dart +++ b/pkgs/test_core/lib/src/runner/configuration/reporters.dart @@ -45,7 +45,7 @@ final _allReporters = { Directory(config.paths.single.testPath).existsSync(), printPlatform: config.suiteDefaults.runtimes.length > 1)), 'github': ReporterDetails( - 'Custom output for Github Actions.', + 'A custom reporter for GitHub Actions output.', (config, engine, sink) => GithubReporter.watch(engine, sink, printPath: config.paths.length > 1 || Directory(config.paths.single.testPath).existsSync())), diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index 99a4912f3..0e1bbde65 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -41,6 +41,8 @@ class GithubReporter implements Reporter { final Map> _testMessages = {}; + final Set _completedTests = {}; + /// Watches the tests run by [engine] and prints their results as JSON. static GithubReporter watch( Engine engine, @@ -93,7 +95,13 @@ class GithubReporter implements Reporter { // Collect messages from tests as they are emitted. _subscriptions.add(liveTest.onMessage.listen((message) { - _testMessages.putIfAbsent(liveTest, () => []).add(message); + if (_completedTests.contains(liveTest)) { + // The test has already completed and it's previous messages were + // written out; ensure this post-completion output is not lost. + _sink.writeln(message.text); + } else { + _testMessages.putIfAbsent(liveTest, () => []).add(message); + } })); } @@ -108,7 +116,10 @@ class GithubReporter implements Reporter { test.individualName == setUpAllName || test.individualName == tearDownAllName; - // Don't emit any info for 'loadSuite', setUpAll, or tearDownAll tests + // Mark this test as having completed. + _completedTests.add(test); + + // Don't emit any info for loadSuite, setUpAll, or tearDownAll tests // unless they contain errors or other info. if (synthetic && (errors.isEmpty && messages.isEmpty)) { return; @@ -133,9 +144,10 @@ class GithubReporter implements Reporter { name = '${test.suite.path}: $name'; } } - // TODO: have a visual indication when passed tests still contain contents - // in the group? _sink.writeln(_helper.startGroup('$prefix $name$statusSuffix')); + // TODO: strip out any ::group:: and ::endgroup:: tokens? + // TODO: Note that printing messages and errors in this manner could display + // them in a different order than they were generated. for (var message in messages) { _sink.writeln(message.text); } @@ -167,33 +179,20 @@ class GithubReporter implements Reporter { : '${_GithubHelper.success} $message', ); } - - // TODO: Do we need to bake in awareness about tests that haven't completed - // yet? some reporters seem to, but not all - // ignore: unused_element - String _normalizeTestResult(LiveTest liveTest) { - // For backwards-compatibility, report skipped tests as successes. - if (liveTest.state.result == Result.skipped) return 'success'; - // if test is still active, it was probably cancelled - if (_engine.active.contains(liveTest)) return 'error'; - return liveTest.state.result.toString(); - } } class _GithubHelper { + // Char sets avilable at https://www.compart.com/en/unicode/. static const String passed = '✅'; static const String skipped = '❎'; static const String failed = '❌'; - - // Char sets avilable at https://www.compart.com/en/unicode/: - // ⛔, ⏹, ⏺, ⏩, ⏪, ∅, ❎, 🚫 static const String synthetic = '⏺'; - static const String success = '🎉'; _GithubHelper(); String startGroup(String title) => '::group::${title.replaceAll('\n', ' ')}'; + final String endGroup = '::endgroup::'; String error(String message) => '::error::$message'; From f0d7d23c7ee870c0466b6bc1d9ecf29ea900b00a Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 22 Mar 2022 14:12:12 -0700 Subject: [PATCH 09/14] fix a test in runner_test.dart --- pkgs/test/test/runner/runner_test.dart | 10 ++++++++-- pkgs/test_core/lib/src/runner/reporter/github.dart | 1 - 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index 8b0f546ec..54c434cde 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/pkgs/test/test/runner/runner_test.dart @@ -722,8 +722,14 @@ void main(List args) async { await test.runTests(args); test.completeShutdown(); }''').create(); - var test = await runDart(['runner.dart', '--no-color', '--', 'test.dart'], - description: 'dart runner.dart -- test.dart'); + var test = await runDart([ + 'runner.dart', + '--no-color', + '--reporter', + 'compact', + '--', + 'test.dart', + ], description: 'dart runner.dart -- test.dart'); expect( test.stdout, emitsThrough(containsInOrder([ diff --git a/pkgs/test_core/lib/src/runner/reporter/github.dart b/pkgs/test_core/lib/src/runner/reporter/github.dart index 0e1bbde65..0d6b084ca 100644 --- a/pkgs/test_core/lib/src/runner/reporter/github.dart +++ b/pkgs/test_core/lib/src/runner/reporter/github.dart @@ -145,7 +145,6 @@ class GithubReporter implements Reporter { } } _sink.writeln(_helper.startGroup('$prefix $name$statusSuffix')); - // TODO: strip out any ::group:: and ::endgroup:: tokens? // TODO: Note that printing messages and errors in this manner could display // them in a different order than they were generated. for (var message in messages) { From c75e2013c8762c926c9db7eb441a2d6eb0b80021 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Wed, 23 Mar 2022 15:46:30 -0700 Subject: [PATCH 10/14] update readme and changelog --- pkgs/test/CHANGELOG.md | 6 +++ pkgs/test/README.md | 16 +++++++ pkgs/test/pubspec.yaml | 2 +- .../test/runner/github_reporter_test.dart | 46 ------------------- pkgs/test/test/runner/runner_test.dart | 4 +- .../lib/src/runner/configuration/args.dart | 3 +- .../src/runner/configuration/reporters.dart | 2 +- .../lib/src/runner/reporter/github.dart | 12 ++--- 8 files changed, 33 insertions(+), 58 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 11faa50c9..0d3ecb965 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.21.0-dev + +* Add a `github` reporter option for use with GitHub Actions. +* Make the `github` test reporter the default when we detect we're running on + GitHub Actions. + ## 1.20.2 * Drop `dart2js-path` command line argument. diff --git a/pkgs/test/README.md b/pkgs/test/README.md index 8cc53e9b9..3d693ae6b 100644 --- a/pkgs/test/README.md +++ b/pkgs/test/README.md @@ -3,6 +3,8 @@ * [Writing Tests](#writing-tests) * [Running Tests](#running-tests) * [Sharding Tests](#sharding-tests) + * [Shuffling Tests](#shuffling-tests) + * [Selecting a Test Reporter](#selecting-a-test-reporter) * [Collecting Code Coverage](#collecting-code-coverage) * [Restricting Tests to Certain Platforms](#restricting-tests-to-certain-platforms) * [Platform Selectors](#platform-selectors) @@ -209,6 +211,7 @@ the invocation to the `test` function, and are considered a match if on platform implementations. ### Sharding Tests + Tests can also be sharded with the `--total-shards` and `--shard-index` arguments, allowing you to split up your test suites and run them separately. For example, if you wanted to run 3 shards of your test suite, you could run them as follows: @@ -233,6 +236,19 @@ dart test --test-randomize-ordering-seed=random Setting `--test-randomize-ordering-seed=0` will have the same effect as not specifying it at all, meaning the test order will remain as-is. +### Selecting a Test Reporter + +You can adjust the output format of the test results using the `--reporter=