From 4439fd41d916572ee0d54830c314b83c12dd011e Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Mon, 8 May 2023 09:33:15 -0700 Subject: [PATCH] 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. --- .../flutter_tools/lib/src/commands/test.dart | 11 ++++++---- .../commands.shard/hermetic/test_test.dart | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart index fbd85c346f..2d0919bbad 100644 --- a/packages/flutter_tools/lib/src/commands/test.dart +++ b/packages/flutter_tools/lib/src/commands/test.dart @@ -238,13 +238,15 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { final Set _testFileUris = {}; + bool get isWeb => stringArg('platform') == 'chrome'; + @override Future> get requiredArtifacts async { final Set results = _isIntegrationTest // Use [DeviceBasedDevelopmentArtifacts]. ? await super.requiredArtifacts : {}; - 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'), diff --git a/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart index ab57a98311..03f8ba075a 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart @@ -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 commandRunner = createTestCommandRunner(testCommand); + + await commandRunner.run(const [ + 'test', + '--no-pub', + '--concurrency=100', + '--platform=chrome', + ]); + + expect(testRunner.lastConcurrency, 1); + }, overrides: { + 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 runTests( @@ -890,6 +910,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner { lastDebuggingOptionsValue = debuggingOptions; lastFileReporterValue = fileReporter; lastReporterOption = reporter; + lastConcurrency = concurrency; if (leastRunTime != null) { await Future.delayed(leastRunTime!);