Some test cleanup for flutter_tools. (#90227)
diff --git a/dev/bots/test.dart b/dev/bots/test.dart
index dc7e475..3973376 100644
--- a/dev/bots/test.dart
+++ b/dev/bots/test.dart
@@ -278,6 +278,8 @@
testPaths: <String>[path.join('test', 'general.shard')],
enableFlutterToolAsserts: false,
// Detect unit test time regressions (poor time delay handling, etc).
+ // This overrides the 15 minute default for tools tests.
+ // See the README.md and dart_test.yaml files in the flutter_tools package.
perTestTimeout: const Duration(seconds: 2),
);
}
@@ -295,8 +297,6 @@
path.join(flutterRoot, 'packages', 'flutter_tools'),
forceSingleCore: true,
testPaths: <String>[path.join('test', 'web.shard')],
- enableFlutterToolAsserts: true,
- perTestTimeout: const Duration(minutes: 3),
includeLocalEngineEnv: true,
);
}
diff --git a/packages/flutter_tools/README.md b/packages/flutter_tools/README.md
index 156cfa4..e46de50 100644
--- a/packages/flutter_tools/README.md
+++ b/packages/flutter_tools/README.md
@@ -39,17 +39,30 @@
As with other parts of the Flutter repository, all changes in behavior [must be
tested](https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#write-test-find-bug).
Tests live under the `test/` subdirectory.
-- Hermetic unit tests of tool internals go under `test/general.shard`.
-- Tests of tool commands go under `test/commands.shard`. Hermetic tests go under
- its `hermetic/` subdirectory. Non-hermetic tests go under its `permeable`
- sub-directory. Avoid adding tests here and prefer writing either a unit test or a full
- integration test.
-- Integration tests (e.g. tests that run the tool in a subprocess) go under
- `test/integration.shard`.
-In general, the tests for the code in a file called `file.dart` should go in a
-file called `file_test.dart` in the subdirectory that matches the behavior of
-the test.
+- Hermetic unit tests of tool internals go under `test/general.shard`
+ and must run in significantly less than two seconds.
+
+- Tests of tool commands go under `test/commands.shard`. Hermetic
+ tests go under its `hermetic/` subdirectory. Non-hermetic tests go
+ under its `permeable` sub-directory. Avoid adding tests here and
+ prefer writing either a unit test or a full integration test.
+
+- Integration tests (e.g. tests that run the tool in a subprocess) go
+ under `test/integration.shard`.
+
+- Slow web-related tests go in the `test/web.shard` directory.
+
+In general, the tests for the code in a file called `file.dart` should
+go in a file called `file_test.dart` in the subdirectory that matches
+the behavior of the test.
+
+The `dart_test.yaml` file configures the timeout for these tests to be
+15 minutes. The `test.dart` script that is used in CI overrides this
+to two seconds for the `test/general.shard` directory, to catch
+behaviour that is unexpectedly slow.
+
+Please avoid setting any other timeouts.
#### Using local engine builds in integration tests
@@ -73,29 +86,26 @@
$ flutter test test/general.shard
```
-The tests in `test/integration.shard` are slower to run than the tests in
-`test/general.shard`. Depending on your development computer, you might
-want to increase timeouts and limit concurrency. Generally it is easier to run
-these on CI, or to manually verify the behavior you are changing instead of running
-the test.
+The tests in `test/integration.shard` are slower to run than the tests
+in `test/general.shard`. Depending on your development computer, you
+might want to limit concurrency. Generally it is easier to run these
+on CI, or to manually verify the behavior you are changing instead of
+running the test.
-The integration tests also require the `FLUTTER_ROOT` environment variable
-to be set. The full invocation to run everything might therefore look something like:
+The integration tests also require the `FLUTTER_ROOT` environment
+variable to be set. The full invocation to run everything might
+therefore look something like:
```shell
$ FLUTTER_ROOT=~/path/to/flutter-sdk
-$ flutter test --timeout 2x --concurrency 1
+$ flutter test --concurrency 1
```
-This will take about an hour to complete.
+This may take some time (on the order of an hour). The unit tests
+alone take much less time (on the order of a minute).
-To run only the tests in `test/general.shard` (which takes about a minute),
-in this directory run:
-```shell
-$ flutter test test/general.shard
-```
+You can run the tests in a specific file, e.g.:
-To run the tests in a specific file, run:
```shell
$ flutter test test/general.shard/utils_test.dart
```
diff --git a/packages/flutter_tools/dart_test.yaml b/packages/flutter_tools/dart_test.yaml
index 807c298..f437c0f 100644
--- a/packages/flutter_tools/dart_test.yaml
+++ b/packages/flutter_tools/dart_test.yaml
@@ -6,6 +6,9 @@
# overloaded. For this reason, we set the test timeout to
# significantly more than it would be by default, and we never set the
# timeouts in the tests themselves.
+#
+# For the `test/general.shard` specifically, the `dev/bots/test.dart` script
+# overrides this, reducing it to only 2000ms. Unit tests must run fast!
timeout: 15m
tags:
diff --git a/packages/flutter_tools/test/integration.shard/deferred_components_test.dart b/packages/flutter_tools/test/integration.shard/deferred_components_test.dart
index 3ea5c84..08359d9 100644
--- a/packages/flutter_tools/test/integration.shard/deferred_components_test.dart
+++ b/packages/flutter_tools/test/integration.shard/deferred_components_test.dart
@@ -62,7 +62,7 @@
expect(archive.findFile('base/assets/flutter_assets/test_assets/asset1.txt') != null, true);
expect(result.exitCode, 0);
- }, timeout: const Timeout(Duration(minutes: 5)));
+ });
testWithoutContext('simple build appbundle all targets succeeds', () async {
final DeferredComponentsProject project = DeferredComponentsProject(BasicDeferredComponentsConfig());
@@ -105,7 +105,7 @@
expect(archive.findFile('base/assets/flutter_assets/test_assets/asset1.txt') != null, true);
expect(result.exitCode, 0);
- }, timeout: const Timeout(Duration(minutes: 5)));
+ });
testWithoutContext('simple build appbundle no-deferred-components succeeds', () async {
final DeferredComponentsProject project = DeferredComponentsProject(BasicDeferredComponentsConfig());
@@ -151,7 +151,7 @@
expect(archive.findFile('base/assets/flutter_assets/test_assets/asset1.txt') != null, true);
expect(result.exitCode, 0);
- }, timeout: const Timeout(Duration(minutes: 5)));
+ });
testWithoutContext('simple build appbundle mismatched golden no-validate-deferred-components succeeds', () async {
final DeferredComponentsProject project = DeferredComponentsProject(MismatchedGoldenDeferredComponentsConfig());
@@ -198,7 +198,7 @@
expect(archive.findFile('base/assets/flutter_assets/test_assets/asset1.txt') != null, true);
expect(result.exitCode, 0);
- }, timeout: const Timeout(Duration(minutes: 5)));
+ });
testWithoutContext('simple build appbundle missing android dynamic feature module fails', () async {
final DeferredComponentsProject project = DeferredComponentsProject(NoAndroidDynamicFeatureModuleDeferredComponentsConfig());
diff --git a/packages/flutter_tools/test/integration.shard/flutter_attach_test.dart b/packages/flutter_tools/test/integration.shard/flutter_attach_test.dart
index 8d191c3..af48dc7 100644
--- a/packages/flutter_tools/test/integration.shard/flutter_attach_test.dart
+++ b/packages/flutter_tools/test/integration.shard/flutter_attach_test.dart
@@ -104,5 +104,5 @@
continuePollingValue: '',
matches: equals(vmServiceUri),
);
- }, timeout: const Timeout.factor(4));
+ });
}
diff --git a/packages/flutter_tools/test/integration.shard/flutter_build_null_unsafe_test.dart b/packages/flutter_tools/test/integration.shard/flutter_build_null_unsafe_test.dart
index 3fe0f7a..483d8eb 100644
--- a/packages/flutter_tools/test/integration.shard/flutter_build_null_unsafe_test.dart
+++ b/packages/flutter_tools/test/integration.shard/flutter_build_null_unsafe_test.dart
@@ -78,6 +78,6 @@
if (targetPlatform == 'ios') '--no-codesign',
], workingDirectory: projectRoot.path);
expect(result.exitCode, 0);
- }, timeout: const Timeout(Duration(minutes: 3)));
+ });
}
}
diff --git a/packages/flutter_tools/test/integration.shard/flutter_build_with_compilation_error_test.dart b/packages/flutter_tools/test/integration.shard/flutter_build_with_compilation_error_test.dart
index b75a00e..2dbb1b3 100644
--- a/packages/flutter_tools/test/integration.shard/flutter_build_with_compilation_error_test.dart
+++ b/packages/flutter_tools/test/integration.shard/flutter_build_with_compilation_error_test.dart
@@ -71,6 +71,6 @@
contains("A value of type 'String' can't be assigned to a variable of type 'int'."),
);
expect(result.exitCode, 1);
- }, timeout: const Timeout(Duration(minutes: 3)));
+ });
}
}
diff --git a/packages/flutter_tools/test/integration.shard/flutter_run_test.dart b/packages/flutter_tools/test/integration.shard/flutter_run_test.dart
index 0ad60ee..0b3c65a 100644
--- a/packages/flutter_tools/test/integration.shard/flutter_run_test.dart
+++ b/packages/flutter_tools/test/integration.shard/flutter_run_test.dart
@@ -69,5 +69,5 @@
continuePollingValue: '',
matches: isNotEmpty,
);
- }, timeout: const Timeout.factor(4));
+ });
}
diff --git a/packages/flutter_tools/test/integration.shard/gradle_non_android_plugin_test.dart b/packages/flutter_tools/test/integration.shard/gradle_non_android_plugin_test.dart
index 52fd6ee..a992223 100644
--- a/packages/flutter_tools/test/integration.shard/gradle_non_android_plugin_test.dart
+++ b/packages/flutter_tools/test/integration.shard/gradle_non_android_plugin_test.dart
@@ -90,5 +90,5 @@
'app-release.apk',
);
expect(fileSystem.file(exampleAppApk), exists);
- }, timeout: const Timeout(Duration(minutes: 5)));
+ });
}
diff --git a/packages/flutter_tools/test/integration.shard/macos_content_validation_test.dart b/packages/flutter_tools/test/integration.shard/macos_content_validation_test.dart
index 9e1ed8c..f047814 100644
--- a/packages/flutter_tools/test/integration.shard/macos_content_validation_test.dart
+++ b/packages/flutter_tools/test/integration.shard/macos_content_validation_test.dart
@@ -145,8 +145,6 @@
...getLocalEngineArguments(),
'clean',
], workingDirectory: workingDirectory);
- }, skip: !platform.isMacOS, // [intended] only makes sense for macos platform.
- timeout: const Timeout(Duration(minutes: 5)),
- );
+ }, skip: !platform.isMacOS); // [intended] only makes sense for macos platform.
}
}
diff --git a/packages/flutter_tools/test/integration.shard/overall_experience_test.dart b/packages/flutter_tools/test/integration.shard/overall_experience_test.dart
index 36dc030..c2c0154 100644
--- a/packages/flutter_tools/test/integration.shard/overall_experience_test.dart
+++ b/packages/flutter_tools/test/integration.shard/overall_experience_test.dart
@@ -25,10 +25,7 @@
// @dart = 2.8
// This file is ready to transition, just uncomment /*?*/, /*!*/, and /*late*/.
-// TODO(gspencergoog): Remove this tag once this test's state leaks/test
-// dependencies have been fixed.
-// https://github.com/flutter/flutter/issues/85160
-// Fails with "flutter test --test-randomize-ordering-seed=1000"
+// This file intentionally assumes the tests run in order.
@Tags(<String>['no-shuffle'])
import 'dart:async';
@@ -40,7 +37,7 @@
import 'package:process/process.dart';
import '../src/common.dart';
-import 'test_utils.dart' show fileSystem, platform;
+import 'test_utils.dart' show fileSystem;
const ProcessManager processManager = LocalProcessManager();
final String flutterRoot = getFlutterRoot();
@@ -189,7 +186,7 @@
List<Transition> transitions, {
bool debug = false,
bool logging = true,
- Duration expectedMaxDuration = const Duration(minutes: 10), // must be less than test timeout of 15 minutes!
+ Duration expectedMaxDuration = const Duration(minutes: 10), // must be less than test timeout of 15 minutes! See ../../dart_test.yaml.
}) async {
final Stopwatch clock = Stopwatch()..start();
final Process process = await processManager.start(
@@ -358,7 +355,7 @@
} finally {
tryToDelete(fileSystem.directory(tempDirectory));
}
- }, skip: platform.isWindows); // https://github.com/flutter/flutter/issues/87924
+ }, skip: Platform.isWindows); // [intended] Windows doesn't support sending signals so we don't care if it can store the PID.
testWithoutContext('flutter run handle SIGUSR1/2', () async {
final String tempDirectory = fileSystem.systemTempDirectory.createTempSync('flutter_overall_experience_test.').resolveSymbolicLinksSync();
@@ -622,5 +619,5 @@
'',
'Application finished.',
]);
- }, skip: Platform.isWindows); // TODO(zanderso): Re-enable when this test is reliable on device lab, https://github.com/flutter/flutter/issues/81556
+ });
}
diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart
index 529fd9e..79c9192 100644
--- a/packages/flutter_tools/test/src/common.dart
+++ b/packages/flutter_tools/test/src/common.dart
@@ -148,7 +148,6 @@
@isTest
void test(String description, FutureOr<void> Function() body, {
String? testOn,
- Timeout? timeout,
dynamic skip,
List<String>? tags,
Map<String, dynamic>? onPlatform,
@@ -162,12 +161,14 @@
});
return body();
},
- timeout: timeout,
skip: skip,
tags: tags,
onPlatform: onPlatform,
retry: retry,
testOn: testOn,
+ // We don't support "timeout"; see ../../dart_test.yaml which
+ // configures all tests to have a 15 minute timeout which should
+ // definitely be enough.
);
}
@@ -182,7 +183,6 @@
@isTest
void testWithoutContext(String description, FutureOr<void> Function() body, {
String? testOn,
- Timeout? timeout,
dynamic skip,
List<String>? tags,
Map<String, dynamic>? onPlatform,
@@ -194,12 +194,14 @@
contextKey: const _NoContext(),
});
},
- timeout: timeout,
skip: skip,
tags: tags,
onPlatform: onPlatform,
retry: retry,
testOn: testOn,
+ // We don't support "timeout"; see ../../dart_test.yaml which
+ // configures all tests to have a 15 minute timeout which should
+ // definitely be enough.
);
}
diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart
index c224a27..38abdae 100644
--- a/packages/flutter_tools/test/src/context.dart
+++ b/packages/flutter_tools/test/src/context.dart
@@ -62,7 +62,6 @@
Map<Type, Generator> overrides = const <Type, Generator>{},
bool initializeFlutterRoot = true,
String testOn,
- Timeout timeout,
bool skip, // should default to `false`, but https://github.com/dart-lang/test/issues/545 doesn't allow this
}) {
if (overrides[FileSystem] != null && overrides[ProcessManager] == null) {
@@ -167,7 +166,10 @@
// BotDetector implementation in the overrides.
BotDetector: overrides[BotDetector] ?? () => const FakeBotDetector(true),
});
- }, testOn: testOn, skip: skip, timeout: timeout);
+ }, testOn: testOn, skip: skip);
+ // We don't support "timeout"; see ../../dart_test.yaml which
+ // configures all tests to have a 15 minute timeout which should
+ // definitely be enough.
}
void _printBufferedErrors(AppContext testContext) {
diff --git a/packages/flutter_tools/test/general.shard/web/chrome_test.dart b/packages/flutter_tools/test/web.shard/chrome_test.dart
similarity index 98%
rename from packages/flutter_tools/test/general.shard/web/chrome_test.dart
rename to packages/flutter_tools/test/web.shard/chrome_test.dart
index b4d4d96..10b1c25 100644
--- a/packages/flutter_tools/test/general.shard/web/chrome_test.dart
+++ b/packages/flutter_tools/test/web.shard/chrome_test.dart
@@ -12,9 +12,9 @@
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/web/chrome.dart';
-import '../../src/common.dart';
-import '../../src/fake_process_manager.dart';
-import '../../src/fakes.dart';
+import '../src/common.dart';
+import '../src/fake_process_manager.dart';
+import '../src/fakes.dart';
const List<String> kChromeArgs = <String>[
'--disable-background-timer-throttling',
@@ -547,7 +547,7 @@
contains('Unable to connect to Chrome debug port:'),
);
expect(logger.errorText, contains('SocketException'));
- }, timeout: const Timeout.factor(2));
+ });
}
Future<Chromium> _testLaunchChrome(String userDataDir, FakeProcessManager processManager, ChromiumLauncher chromeLauncher) {
