Unverified Commit d7454d55 authored by Elias Yishak's avatar Elias Yishak Committed by GitHub

removing default values for [reporter] and [timeout] in flutter test (#115160)

* removing default values for [reporter] and [timeout]]

* passing reporter arg to see tests pass

* added test to confirm TestCommand is not passing defaults

* add'l helper message for [reporter] arg

* default behavior for github actions + fixed tests

* removing github conditional for reporter + related test

* removing unused import
parent 243a8301
...@@ -199,7 +199,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -199,7 +199,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
) )
..addOption('reporter', ..addOption('reporter',
abbr: 'r', abbr: 'r',
help: 'Set how to print test results.', help: 'Set how to print test results. If unset, value will default to either compact or expanded.',
allowed: <String>['compact', 'expanded', 'github', 'json'], allowed: <String>['compact', 'expanded', 'github', 'json'],
allowedHelp: <String, String>{ allowedHelp: <String, String>{
'compact': 'A single line that updates dynamically (The default reporter).', 'compact': 'A single line that updates dynamically (The default reporter).',
...@@ -213,7 +213,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -213,7 +213,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
'in seconds (e.g. "60s"), ' 'in seconds (e.g. "60s"), '
'as a multiplier of the default timeout (e.g. "2x"), ' 'as a multiplier of the default timeout (e.g. "2x"), '
'or as the string "none" to disable the timeout entirely.', 'or as the string "none" to disable the timeout entirely.',
defaultsTo: '30s',
); );
addDdsOptions(verboseHelp: verboseHelp); addDdsOptions(verboseHelp: verboseHelp);
usesFatalWarningsOption(verboseHelp: verboseHelp); usesFatalWarningsOption(verboseHelp: verboseHelp);
...@@ -254,18 +253,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -254,18 +253,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
@override @override
String get category => FlutterCommandCategory.project; String get category => FlutterCommandCategory.project;
// Lookup the default reporter if one was not specified.
String _getReporter() {
final String? reporter = stringArgDeprecated('reporter');
if (reporter != null) {
return reporter;
}
if (globals.platform.environment['GITHUB_ACTIONS']?.toLowerCase() == 'true') {
return 'github';
}
return 'compact';
}
@override @override
Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) { Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) {
_testFiles = argResults!.rest.map<String>(globals.fs.path.absolute).toList(); _testFiles = argResults!.rest.map<String>(globals.fs.path.absolute).toList();
...@@ -473,7 +460,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -473,7 +460,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
flutterProject: flutterProject, flutterProject: flutterProject,
web: stringArgDeprecated('platform') == 'chrome', web: stringArgDeprecated('platform') == 'chrome',
randomSeed: stringArgDeprecated('test-randomize-ordering-seed'), randomSeed: stringArgDeprecated('test-randomize-ordering-seed'),
reporter: _getReporter(), reporter: stringArgDeprecated('reporter'),
timeout: stringArgDeprecated('timeout'), timeout: stringArgDeprecated('timeout'),
runSkipped: boolArgDeprecated('run-skipped'), runSkipped: boolArgDeprecated('run-skipped'),
shardIndex: shardIndex, shardIndex: shardIndex,
......
...@@ -102,8 +102,8 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner { ...@@ -102,8 +102,8 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
'--pause-after-load', '--pause-after-load',
if (machine) if (machine)
...<String>['-r', 'json'] ...<String>['-r', 'json']
else else if (reporter != null)
...<String>['-r', reporter ?? 'compact'], ...<String>['-r', reporter],
if (timeout != null) if (timeout != null)
...<String>['--timeout', timeout], ...<String>['--timeout', timeout],
'--concurrency=$concurrency', '--concurrency=$concurrency',
......
...@@ -10,7 +10,6 @@ import 'package:file/memory.dart'; ...@@ -10,7 +10,6 @@ import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/commands/test.dart'; import 'package:flutter_tools/src/commands/test.dart';
import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/device.dart';
...@@ -156,6 +155,30 @@ dev_dependencies: ...@@ -156,6 +155,30 @@ dev_dependencies:
Cache: () => Cache.test(processManager: FakeProcessManager.any()), Cache: () => Cache.test(processManager: FakeProcessManager.any()),
}); });
testUsingContext(
'Confirmation that the reporter and timeout args are not set by default',
() async {
final FakePackageTest fakePackageTest = FakePackageTest();
final TestCommand testCommand = TestCommand(testWrapper: fakePackageTest);
final CommandRunner<void> commandRunner =
createTestCommandRunner(testCommand);
await commandRunner.run(const <String>[
'test',
'--no-pub',
]);
expect(fakePackageTest.lastArgs, isNot(contains('-r')));
expect(fakePackageTest.lastArgs, isNot(contains('compact')));
expect(fakePackageTest.lastArgs, isNot(contains('--timeout')));
expect(fakePackageTest.lastArgs, isNot(contains('30s')));
}, overrides: <Type, Generator>{
FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(),
Cache: () => Cache.test(processManager: FakeProcessManager.any()),
});
group('shard-index and total-shards', () { group('shard-index and total-shards', () {
testUsingContext('with the params they are Piped to package:test', testUsingContext('with the params they are Piped to package:test',
() async { () async {
...@@ -661,60 +684,6 @@ dev_dependencies: ...@@ -661,60 +684,6 @@ dev_dependencies:
]), ]),
}); });
testUsingContext('Tests on github actions default to github reporter', () async {
final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);
final TestCommand testCommand = TestCommand(testRunner: testRunner);
final CommandRunner<void> commandRunner = createTestCommandRunner(testCommand);
await commandRunner.run(const <String>[
'test',
'--no-pub',
]);
expect(
testRunner.lastReporterOption,
'github',
);
}, overrides: <Type, Generator>{
FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(
environment: <String, String>{
'GITHUB_ACTIONS': 'true',
},
),
DeviceManager: () => _FakeDeviceManager(<Device>[
FakeDevice('ephemeral', 'ephemeral', type: PlatformType.android),
]),
});
testUsingContext('Tests default to compact reporter if not specified and not on Github actions', () async {
final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);
final TestCommand testCommand = TestCommand(testRunner: testRunner);
final CommandRunner<void> commandRunner = createTestCommandRunner(testCommand);
await commandRunner.run(const <String>[
'test',
'--no-pub',
]);
expect(
testRunner.lastReporterOption,
'compact',
);
}, overrides: <Type, Generator>{
FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(
environment: <String, String>{}
),
DeviceManager: () => _FakeDeviceManager(<Device>[
FakeDevice('ephemeral', 'ephemeral', type: PlatformType.android),
]),
});
testUsingContext('Integration tests given flavor', () async { testUsingContext('Integration tests given flavor', () async {
final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0); final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);
......
...@@ -313,6 +313,8 @@ Future<ProcessResult> _runFlutterTest( ...@@ -313,6 +313,8 @@ Future<ProcessResult> _runFlutterTest(
'--no-color', '--no-color',
'--no-version-check', '--no-version-check',
'--no-pub', '--no-pub',
'--reporter',
'compact',
...extraArguments, ...extraArguments,
testPath, testPath,
]; ];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment