Skip to content

Commit

Permalink
Always use --concurrency=1 for web tests. (#126179)
Browse files Browse the repository at this point in the history
This should fix #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.
  • Loading branch information
eyebrowsoffire committed May 8, 2023
1 parent 38cac91 commit 4439fd4
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
11 changes: 7 additions & 4 deletions packages/flutter_tools/lib/src/commands/test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'),
Expand Down
21 changes: 21 additions & 0 deletions packages/flutter_tools/test/commands.shard/hermetic/test_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -852,6 +871,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
late DebuggingOptions lastDebuggingOptionsValue;
String? lastFileReporterValue;
String? lastReporterOption;
int? lastConcurrency;

@override
Future<int> runTests(
Expand Down Expand Up @@ -890,6 +910,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
lastDebuggingOptionsValue = debuggingOptions;
lastFileReporterValue = fileReporter;
lastReporterOption = reporter;
lastConcurrency = concurrency;

if (leastRunTime != null) {
await Future<void>.delayed(leastRunTime!);
Expand Down

0 comments on commit 4439fd4

Please sign in to comment.