Skip to content

Hide skip reasons in compact and failure reporters #2578

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.

Sign up for GitHub

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

Open
natebosch wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Hide skip reasons in compact and failure reporters #2578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions pkgs/test/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 8 additions & 8 deletions pkgs/test/test/runner/compact_reporter_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!''',
);
});

Expand Down Expand Up @@ -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'],
);
});

Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -622,4 +619,7 @@ $tests
});

expect(actual, containsAllInOrder(expectedLines));
for (final disallowed in disallowedLines ?? const <String>[]) {
expect(actual, everyElement(isNot(contains(disallowed))));
}
}
16 changes: 14 additions & 2 deletions pkgs/test/test/runner/failures_only_reporter_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
'''
Expand Down
1 change: 1 addition & 0 deletions pkgs/test_core/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 15 additions & 7 deletions pkgs/test_core/lib/src/runner/reporter/compact.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}),
);

Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 13 additions & 6 deletions pkgs/test_core/lib/src/runner/reporter/failures_only.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}),
);
}
Expand Down Expand Up @@ -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) {
Expand Down