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.

Author: natebosch

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
 

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<void> _expectReport(
   String expected, {
   List<String> args = const [],
   bool chainStackTraces = true,
+  Iterable<String>? 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 <String>[]) {
+    expect(actual, everyElement(isNot(contains(disallowed))));
+  }
 }

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(
         '''

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
 

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) {

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) {