Hide skip reasons in compact and failure reporters (#2578)
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.
diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md
index 91df125..ce00288 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 842b655..d0a6464 100644
--- a/pkgs/test/test/runner/compact_reporter_test.dart
+++ b/pkgs/test/test/runner/compact_reporter_test.dart
@@ -392,7 +392,8 @@
+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 @@
);
});
- 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 @@
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 @@
});
expect(actual, containsAllInOrder(expectedLines));
+ for (final disallowed in disallowedLines ?? const <String>[]) {
+ 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 2b56af4..badb817 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 @@
});
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 bb35860..885647d 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.
* Fix default coverage filter when running in a workspace package. Default
filter now includes all the workspace's package.
diff --git a/pkgs/test_core/lib/src/runner/reporter/compact.dart b/pkgs/test_core/lib/src/runner/reporter/compact.dart
index 3095ede..40fa1db 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: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 @@
_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,10 +343,20 @@
_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 if (_engine.skipped.isEmpty) {
+ _progressLine('All tests passed!');
_sink.writeln('');
} else {
- _progressLine('All tests passed!');
+ final skippedCount = _engine.skipped.length;
+ _progressLine(
+ '$_yellow'
+ '$skippedCount skipped ${pluralize('test', skippedCount)}.'
+ '$_noColor',
+ );
+ _sink.writeln('');
+ _progressLine('All other tests passed!');
_sink.writeln('');
}
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 1549da5..e5ece6b 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 @@
_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 @@
}
_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) {
