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

Author: eliasyishak

packages/flutter_tools/lib/src/commands/test.dart

@@ -199,7 +199,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
       )
       ..addOption('reporter',
         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'],
         allowedHelp: <String, String>{
           'compact':  'A single line that updates dynamically (The default reporter).',
@@ -213,7 +213,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
               'in seconds (e.g. "60s"), '
               'as a multiplier of the default timeout (e.g. "2x"), '
               'or as the string "none" to disable the timeout entirely.',
-        defaultsTo: '30s',
       );
     addDdsOptions(verboseHelp: verboseHelp);
     usesFatalWarningsOption(verboseHelp: verboseHelp);
@@ -254,18 +253,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
   @override
   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
   Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) {
     _testFiles = argResults!.rest.map<String>(globals.fs.path.absolute).toList();
@@ -473,7 +460,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
       flutterProject: flutterProject,
       web: stringArgDeprecated('platform') == 'chrome',
       randomSeed: stringArgDeprecated('test-randomize-ordering-seed'),
-      reporter: _getReporter(),
+      reporter: stringArgDeprecated('reporter'),
       timeout: stringArgDeprecated('timeout'),
       runSkipped: boolArgDeprecated('run-skipped'),
       shardIndex: shardIndex,

packages/flutter_tools/lib/src/test/runner.dart

@@ -102,8 +102,8 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
         '--pause-after-load',
       if (machine)
         ...<String>['-r', 'json']
-      else
-        ...<String>['-r', reporter ?? 'compact'],
+      else if (reporter != null)
+        ...<String>['-r', reporter],
       if (timeout != null)
         ...<String>['--timeout', timeout],
       '--concurrency=$concurrency',

packages/flutter_tools/test/commands.shard/hermetic/test_test.dart

@@ -10,7 +10,6 @@ import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/base/file_system.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/commands/test.dart';
 import 'package:flutter_tools/src/device.dart';
@@ -156,6 +155,30 @@ dev_dependencies:
     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', () {
     testUsingContext('with the params they are Piped to package:test',
         () async {
@@ -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 {
     final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);
 

packages/flutter_tools/test/integration.shard/test_test.dart

@@ -313,6 +313,8 @@ Future<ProcessResult> _runFlutterTest(
     '--no-color',
     '--no-version-check',
     '--no-pub',
+    '--reporter',
+    'compact',
     ...extraArguments,
     testPath,
   ];