Unverified Commit 742a1b49 authored by fzyzcjy's avatar fzyzcjy Committed by GitHub

Fix that `flutter test` does not understand `concurrency` (#125942)

Close https://github.com/flutter/flutter/issues/125940

I will add tests if this PR looks roughly OK :)

The fix mainly mimics https://github.com/flutter/flutter/pull/115160 - just remove the default argument.

p.s. I ran into this bug when wanting to set concurrency in my dart_test.yaml for one set of my tests which I need to be executed without parallalization.
parent 6b756ab5
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:math' as math;
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import '../asset.dart'; import '../asset.dart';
...@@ -146,7 +144,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -146,7 +144,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
) )
..addOption('concurrency', ..addOption('concurrency',
abbr: 'j', abbr: 'j',
defaultsTo: math.max<int>(1, globals.platform.numberOfProcessors - 2).toString(),
help: 'The number of concurrent test processes to run. This will be ignored ' help: 'The number of concurrent test processes to run. This will be ignored '
'when running integration tests.', 'when running integration tests.',
valueHelp: 'jobs', valueHelp: 'jobs',
...@@ -353,8 +350,9 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -353,8 +350,9 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
); );
} }
int? jobs = int.tryParse(stringArg('concurrency')!); final String? concurrencyString = stringArg('concurrency');
if (jobs == null || jobs <= 0 || !jobs.isFinite) { int? jobs = concurrencyString == null ? null : int.tryParse(concurrencyString);
if (jobs != null && (jobs <= 0 || !jobs.isFinite)) {
throwToolExit( throwToolExit(
'Could not parse -j/--concurrency argument. It must be an integer greater than zero.' 'Could not parse -j/--concurrency argument. It must be an integer greater than zero.'
); );
......
...@@ -109,7 +109,8 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner { ...@@ -109,7 +109,8 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
'--file-reporter=$fileReporter', '--file-reporter=$fileReporter',
if (timeout != null) if (timeout != null)
...<String>['--timeout', timeout], ...<String>['--timeout', timeout],
'--concurrency=$concurrency', if (concurrency != null)
'--concurrency=$concurrency',
for (final String name in names) for (final String name in names)
...<String>['--name', name], ...<String>['--name', name],
for (final String plainName in plainNames) for (final String plainName in plainNames)
......
...@@ -158,7 +158,7 @@ dev_dependencies: ...@@ -158,7 +158,7 @@ dev_dependencies:
}); });
testUsingContext( testUsingContext(
'Confirmation that the reporter and timeout args are not set by default', 'Confirmation that the reporter, timeout, and concurrency args are not set by default',
() async { () async {
final FakePackageTest fakePackageTest = FakePackageTest(); final FakePackageTest fakePackageTest = FakePackageTest();
...@@ -175,6 +175,7 @@ dev_dependencies: ...@@ -175,6 +175,7 @@ dev_dependencies:
expect(fakePackageTest.lastArgs, isNot(contains('compact'))); expect(fakePackageTest.lastArgs, isNot(contains('compact')));
expect(fakePackageTest.lastArgs, isNot(contains('--timeout'))); expect(fakePackageTest.lastArgs, isNot(contains('--timeout')));
expect(fakePackageTest.lastArgs, isNot(contains('30s'))); expect(fakePackageTest.lastArgs, isNot(contains('30s')));
expect(fakePackageTest.lastArgs, isNot(contains('--concurrency')));
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: () => fs, FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
......
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