Unverified Commit 4439fd41 authored by Jackson Gardner's avatar Jackson Gardner Committed by GitHub

Always use `--concurrency=1` for web tests. (#126179)

This should fix https://github.com/flutter/flutter/issues/126178

When we don't pass a `--concurrency` flag to the test package, it uses a default based on the number of cores that are on the machine. However, the web test platform itself serializes all these requests anyway, which can lead to the test package timing out. This is because from the test package's perspective, it has already started the loading process on a number of suites which are simply waiting for other test suites to compile and run. The ones that wait the longest can run up against the test packages 12 minute timeout for loading a given suite, even though they haven't actually started to try to load.

Instead, we should always pass `--concurrency=1` to the test package so that it doesn't attempt to start loads concurrently in the first place.
parent 38cac910
......@@ -238,13 +238,15 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
final Set<Uri> _testFileUris = <Uri>{};
bool get isWeb => stringArg('platform') == 'chrome';
@override
Future<Set<DevelopmentArtifact>> get requiredArtifacts async {
final Set<DevelopmentArtifact> results = _isIntegrationTest
// Use [DeviceBasedDevelopmentArtifacts].
? await super.requiredArtifacts
: <DevelopmentArtifact>{};
if (stringArg('platform') == 'chrome') {
if (isWeb) {
results.add(DevelopmentArtifact.web);
}
return results;
......@@ -357,11 +359,12 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
'Could not parse -j/--concurrency argument. It must be an integer greater than zero.'
);
}
if (_isIntegrationTest) {
if (_isIntegrationTest || isWeb) {
if (argResults!.wasParsed('concurrency')) {
globals.printStatus(
'-j/--concurrency was parsed but will be ignored, this option is not '
'supported when running Integration Tests.',
'supported when running Integration Tests or web tests.',
);
}
// Running with concurrency will result in deploying multiple test apps
......@@ -471,7 +474,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
concurrency: jobs,
testAssetDirectory: testAssetDirectory,
flutterProject: flutterProject,
web: stringArg('platform') == 'chrome',
web: isWeb,
randomSeed: stringArg('test-randomize-ordering-seed'),
reporter: stringArg('reporter'),
fileReporter: stringArg('file-reporter'),
......
......@@ -600,6 +600,25 @@ dev_dependencies:
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('Overrides concurrency when running web tests', () 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',
'--concurrency=100',
'--platform=chrome',
]);
expect(testRunner.lastConcurrency, 1);
}, overrides: <Type, Generator>{
FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('when running integration tests', () async {
final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);
......@@ -852,6 +871,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
late DebuggingOptions lastDebuggingOptionsValue;
String? lastFileReporterValue;
String? lastReporterOption;
int? lastConcurrency;
@override
Future<int> runTests(
......@@ -890,6 +910,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
lastDebuggingOptionsValue = debuggingOptions;
lastFileReporterValue = fileReporter;
lastReporterOption = reporter;
lastConcurrency = concurrency;
if (leastRunTime != null) {
await Future<void>.delayed(leastRunTime!);
......
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