Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
May 8, 2023
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
115d42b
Always use `--concurrency=1` for web tests.
eyebrowsoffire May 5, 2023
41cdde3
Still block using the pool.
eyebrowsoffire May 5, 2023
eb93d18
Rework the test.
eyebrowsoffire May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 7 additions & 4 deletions packages/flutter_tools/lib/src/commands/test.dart
View file
Open in desktop
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
17 changes: 9 additions & 8 deletions packages/flutter_tools/lib/src/test/flutter_web_platform.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'dart:typed_data';
import 'package:async/async.dart';
import 'package:http_multi_server/http_multi_server.dart';
import 'package:package_config/package_config.dart';
import 'package:pool/pool.dart';
import 'package:process/process.dart';
import 'package:shelf/shelf.dart' as shelf;
import 'package:shelf/shelf_io.dart' as shelf_io;
Expand Down Expand Up @@ -97,10 +96,7 @@ class FlutterWebPlatform extends PlatformPlugin {
final AsyncMemoizer<void> _closeMemo = AsyncMemoizer<void>();
final String _root;

/// Allows only one test suite (typically one test file) to be loaded and run
/// at any given point in time. Loading more than one file at a time is known
/// to lead to flaky tests.
final Pool _suiteLock = Pool(1);
bool _isLoadingSuite = false;

BrowserManager? _browserManager;
late TestGoldenComparator _testGoldenComparator;
Expand Down Expand Up @@ -441,13 +437,18 @@ class FlutterWebPlatform extends PlatformPlugin {
if (_closed) {
throw StateError('Load called on a closed FlutterWebPlatform');
}
final PoolResource lockResource = await _suiteLock.request();

/// We only allow one test suite (typically one test file) to be loaded and
/// run at any given point in time. Loading more than one file at a time is
/// known to lead to flaky tests.
assert(_isLoadingSuite == false);
_isLoadingSuite = true;
eyebrowsoffire marked this conversation as resolved.
Show resolved Hide resolved

final Runtime browser = platform.runtime;
try {
_browserManager = await _launchBrowser(browser);
} on Error catch (_) {
await _suiteLock.close();
_isLoadingSuite = false;
rethrow;
}

Expand All @@ -461,7 +462,7 @@ class FlutterWebPlatform extends PlatformPlugin {
final RunnerSuite suite = await _browserManager!.load(relativePath, suiteUrl, suiteConfig, message, onDone: () async {
await _browserManager!.close();
_browserManager = null;
lockResource.release();
_isLoadingSuite = false;
});
if (_closed) {
throw StateError('Load called on a closed FlutterWebPlatform');
Expand Down
22 changes: 22 additions & 0 deletions packages/flutter_tools/test/commands.shard/hermetic/test_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,28 @@ dev_dependencies:
]),
});

testUsingContext('Overrides concurrency when running web tests', () 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',
'--concurrency=100',
'--platform=chrome',
]);

expect(fakePackageTest.lastArgs, contains('--concurrency=1'));
}, overrides: <Type, Generator>{
FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(),
DeviceManager: () => _FakeDeviceManager(<Device>[
FakeDevice('ephemeral', 'ephemeral', type: PlatformType.android),
]),
});

group('Detecting Integration Tests', () {
testUsingContext('when integration_test is not passed', () async {
final FakePackageTest fakePackageTest = FakePackageTest();
Expand Down