From 6e2d1c357c26ff9302d349285ad3e55fe0d46efd Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 10 Dec 2025 01:54:06 +0000 Subject: [PATCH] Hide skip reasons in compact and failure reporters Closes #2546, Fixes #2542 Skipped tests with messages can cause noisy output in otherwise very short test run logs. In most cases the skip messages are not useful, so suppress them by default. They can still be seen in the expanded reporter. Expand the final output when there are skipped tests since the otherwise obvious yellow lines are no longer shown. Add support for testing against disallowed output in the compact reporter tests. Existing tests ignore unexpected output that comes between any expected lines. --- pkgs/test/CHANGELOG.md | 1 + .../test/runner/compact_reporter_test.dart | 16 +++++++------- .../runner/failures_only_reporter_test.dart | 16 ++++++++++++-- pkgs/test_core/CHANGELOG.md | 1 + .../lib/src/runner/reporter/compact.dart | 22 +++++++++++++------ .../src/runner/reporter/failures_only.dart | 19 +++++++++++----- 6 files changed, 52 insertions(+), 23 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 91df125ea..ce0028854 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -4,6 +4,7 @@ packages using RegExps. * Require a function definition named `main` directly in a test suite and provide a more direct error message than a failing compiler output. +* Suppress skip reason messages in the compact and failures-only reporters. ## 1.28.0 diff --git a/pkgs/test/test/runner/compact_reporter_test.dart b/pkgs/test/test/runner/compact_reporter_test.dart index 842b65531..d0a646424 100644 --- a/pkgs/test/test/runner/compact_reporter_test.dart +++ b/pkgs/test/test/runner/compact_reporter_test.dart @@ -392,7 +392,8 @@ void main() { +1 ~2: skip 2 +1 ~2: success 2 +2 ~2: success 2 - +2 ~2: All tests passed!''', + +2 ~2: 2 skipped tests. + +2 ~2: All other tests passed!''', ); }); @@ -431,22 +432,17 @@ void main() { ); }); - test('displays the skip reason if available', () { + test('hides skip reasons', () { return _expectReport( ''' test('skip 1', () {}, skip: 'some reason'); test('skip 2', () {}, skip: 'or another');''', ''' +0: loading test.dart - +0: skip 1 - Skip: some reason - +0 ~1: skip 1 - +0 ~1: skip 2 - Skip: or another - +0 ~2: skip 2 +0 ~2: All tests skipped.''', + disallowedLines: ['some reason', 'or another'], ); }); @@ -573,6 +569,7 @@ Future _expectReport( String expected, { List args = const [], bool chainStackTraces = true, + Iterable? disallowedLines, }) async { await d.file('test.dart', ''' import 'dart:async'; @@ -622,4 +619,7 @@ $tests }); expect(actual, containsAllInOrder(expectedLines)); + for (final disallowed in disallowedLines ?? const []) { + expect(actual, everyElement(isNot(contains(disallowed)))); + } } diff --git a/pkgs/test/test/runner/failures_only_reporter_test.dart b/pkgs/test/test/runner/failures_only_reporter_test.dart index 2b56af47d..badb817a5 100644 --- a/pkgs/test/test/runner/failures_only_reporter_test.dart +++ b/pkgs/test/test/runner/failures_only_reporter_test.dart @@ -196,17 +196,29 @@ void main() { }); group('skip:', () { - test('does not emit for skips', () { + test('reports when all tests are skipped', () { return _expectReport( ''' test('skip 1', () {}, skip: true); test('skip 2', () {}, skip: true); - test('skip 3', () {}, skip: true);''', + test('skip 3', () {}, skip: 'some reason');''', ''' +0 ~3: All tests skipped.''', ); }); + test('reports count of skipped tests', () { + return _expectReport( + ''' + test('skip 1', () {}); + test('skip 2', () {}, skip: true); + test('skip 3', () {}, skip: 'some reason');''', + ''' + +1 ~2: 2 skipped tests. + +1 ~2: All other tests passed!''', + ); + }); + test('runs skipped tests along with successful and failing tests', () { return _expectReport( ''' diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index e36d0ec41..6ba2579a8 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -4,6 +4,7 @@ packages using RegExps. * Require a function definition named `main` directly in a test suite and provide a more direct error message than a failing compiler output. +* Suppress skip reason messages in the compact and failures-only reporters. ## 0.6.14 diff --git a/pkgs/test_core/lib/src/runner/reporter/compact.dart b/pkgs/test_core/lib/src/runner/reporter/compact.dart index 3095ede2b..40fa1dbb6 100644 --- a/pkgs/test_core/lib/src/runner/reporter/compact.dart +++ b/pkgs/test_core/lib/src/runner/reporter/compact.dart @@ -7,8 +7,8 @@ import 'dart:io'; import 'dart:isolate'; import 'package:test_api/src/backend/live_test.dart'; // ignore: implementation_imports -import 'package:test_api/src/backend/message.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/state.dart'; // ignore: implementation_imports +import 'package:test_api/src/backend/util/pretty_print.dart'; // ignore: implementation_imports import '../../util/io.dart'; import '../../util/pretty_print.dart'; @@ -228,13 +228,11 @@ class CompactReporter implements Reporter { _subscriptions.add( liveTest.onMessage.listen((message) { + if (liveTest.test.metadata.skip) return; _progressLine(_description(liveTest), truncate: false); if (!_printedNewline) _sink.writeln(''); _printedNewline = true; - - var text = message.text; - if (message.type == MessageType.skip) text = ' $_yellow$text$_noColor'; - _sink.writeln(text); + _sink.writeln(message.text); }), ); @@ -345,11 +343,21 @@ class CompactReporter implements Reporter { _progressLine('Some tests failed.', color: _red); _sink.writeln(''); } else if (_engine.passed.isEmpty) { - _progressLine('All tests skipped.'); + _progressLine('${_yellow}All tests skipped.$_noColor'); _sink.writeln(''); - } else { + } else if (_engine.skipped.isEmpty) { _progressLine('All tests passed!'); _sink.writeln(''); + } else { + final skippedCount = _engine.skipped.length; + _progressLine( + '$_yellow' + '$skippedCount skipped ${pluralize('test', skippedCount)}.' + '$_noColor', + ); + _sink.writeln(''); + _progressLine('All other tests passed!'); + _sink.writeln(''); } if (_shouldPrintStackTraceChainingNotice) { diff --git a/pkgs/test_core/lib/src/runner/reporter/failures_only.dart b/pkgs/test_core/lib/src/runner/reporter/failures_only.dart index 1549da52b..e5ece6b96 100644 --- a/pkgs/test_core/lib/src/runner/reporter/failures_only.dart +++ b/pkgs/test_core/lib/src/runner/reporter/failures_only.dart @@ -5,8 +5,8 @@ import 'dart:async'; import 'package:test_api/src/backend/live_test.dart'; // ignore: implementation_imports -import 'package:test_api/src/backend/message.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/state.dart'; // ignore: implementation_imports +import 'package:test_api/src/backend/util/pretty_print.dart'; // ignore: implementation_imports import '../../util/pretty_print.dart'; import '../engine.dart'; @@ -161,11 +161,10 @@ class FailuresOnlyReporter implements Reporter { _subscriptions.add( liveTest.onMessage.listen((message) { + if (liveTest.test.metadata.skip) return; // TODO - Should this suppress output? Behave like printOnFailure? _progressLine(_description(liveTest)); - var text = message.text; - if (message.type == MessageType.skip) text = ' $_yellow$text$_noColor'; - _sink.writeln(text); + _sink.writeln(message.text); }), ); } @@ -219,9 +218,17 @@ class FailuresOnlyReporter implements Reporter { } _progressLine('Some tests failed.', color: _red); } else if (_engine.passed.isEmpty) { - _progressLine('All tests skipped.'); - } else { + _progressLine('${_yellow}All tests skipped.$_noColor'); + } else if (_engine.skipped.isEmpty) { _progressLine('All tests passed!'); + } else { + final skippedCount = _engine.skipped.length; + _progressLine( + '$_yellow' + '$skippedCount skipped ${pluralize('test', skippedCount)}.' + '$_noColor', + ); + _progressLine('All other tests passed!'); } if (_shouldPrintStackTraceChainingNotice) {