diff --git a/dev/bots/test.dart b/dev/bots/test.dart index dc7e4759a07..39733764c01 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -278,6 +278,8 @@ Future _runGeneralToolTests() async { testPaths: [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 @@ Future _runWebToolTests() async { path.join(flutterRoot, 'packages', 'flutter_tools'), forceSingleCore: true, testPaths: [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 156cfa41d5a..e46de50f17b 100644 --- a/packages/flutter_tools/README.md +++ b/packages/flutter_tools/README.md @@ -39,17 +39,30 @@ $ flutter analyze 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 @@ To run all of the unit tests: $ 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 807c2988eed..f437c0fdb2c 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 3ea5c84d64c..08359d9be8f 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 @@ void main() { 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 @@ void main() { 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 @@ void main() { 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 @@ void main() { 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 8d191c3bac6..af48dc7d7db 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 @@ void main() { 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 3fe0f7a1e09..483d8eba964 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 @@ String unsafeString = null; 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 b75a00e5bc4..2dbb1b351e6 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 @@ int x = 'String'; 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 0ad60ee6681..0b3c65a75ad 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 @@ void main() { 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 52fd6eea9d0..a9922233eff 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 @@ void main() { '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 9e1ed8c95da..f0478145622 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 @@ void main() { ...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 36dc030946f..c2c015437c6 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(['no-shuffle']) import 'dart:async'; @@ -40,7 +37,7 @@ import 'package:pedantic/pedantic.dart'; 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 @@ Future runFlutter( List 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 @@ void main() { } 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 @@ void main() { '', '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 529fd9e874b..79c9192f398 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart @@ -148,7 +148,6 @@ Matcher containsIgnoringWhitespace(String toSearch) { @isTest void test(String description, FutureOr Function() body, { String? testOn, - Timeout? timeout, dynamic skip, List? tags, Map? onPlatform, @@ -162,12 +161,14 @@ void test(String description, FutureOr Function() body, { }); 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 @@ void test(String description, FutureOr Function() body, { @isTest void testWithoutContext(String description, FutureOr Function() body, { String? testOn, - Timeout? timeout, dynamic skip, List? tags, Map? onPlatform, @@ -194,12 +194,14 @@ void testWithoutContext(String description, FutureOr Function() body, { 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 c224a277d91..38abdae9d24 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/packages/flutter_tools/test/src/context.dart @@ -62,7 +62,6 @@ void testUsingContext( Map overrides = const {}, 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 @@ void testUsingContext( // 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 99% rename from packages/flutter_tools/test/general.shard/web/chrome_test.dart rename to packages/flutter_tools/test/web.shard/chrome_test.dart index b4d4d96a8b0..10b1c25010c 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/os.dart'; 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 kChromeArgs = [ '--disable-background-timer-throttling', @@ -547,7 +547,7 @@ void main() { contains('Unable to connect to Chrome debug port:'), ); expect(logger.errorText, contains('SocketException')); - }, timeout: const Timeout.factor(2)); + }); } Future _testLaunchChrome(String userDataDir, FakeProcessManager processManager, ChromiumLauncher chromeLauncher) {