From fc8865efb10025d04b4bd858e8ed084faec2a665 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Fri, 30 Jun 2023 07:10:47 -0400 Subject: [PATCH 01/21] [ci] Switch Dart unit tests to LUCI Enables the new LUCI Dart unit tests, and removes the Cirrus version. Part of https://github.com/flutter/flutter/issues/114373 --- .ci.yaml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.ci.yaml b/.ci.yaml index 5183c8800b3..6524401b9c1 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -93,10 +93,10 @@ targets: version_file: flutter_master.version - name: Linux dart_unit_test_shard_1 master - bringup: true # New target recipe: packages/packages timeout: 60 properties: + add_recipes_cq: "true" target_file: dart_unit_tests.yaml channel: master version_file: flutter_master.version @@ -104,10 +104,10 @@ targets: package_sharding: "--shardIndex 0 --shardCount 2" - name: Linux dart_unit_test_shard_2 master - bringup: true # New target recipe: packages/packages timeout: 60 properties: + add_recipes_cq: "true" target_file: dart_unit_tests.yaml channel: master version_file: flutter_master.version @@ -115,7 +115,6 @@ targets: package_sharding: "--shardIndex 1 --shardCount 2" - name: Linux dart_unit_test_shard_1 stable - bringup: true # New target recipe: packages/packages timeout: 60 properties: @@ -126,7 +125,6 @@ targets: package_sharding: "--shardIndex 0 --shardCount 2" - name: Linux dart_unit_test_shard_2 stable - bringup: true # New target recipe: packages/packages timeout: 60 properties: From f8daed8d49cb202bc8cd169c28a50bb3a8d43678 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Fri, 30 Jun 2023 07:11:52 -0400 Subject: [PATCH 02/21] Remove Cirrus version --- .cirrus.yml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 77b9903ed04..97490d59375 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -158,22 +158,6 @@ task: memory: 16G matrix: ### Platform-agnostic tasks ### - - name: dart_unit_tests - env: - matrix: - PACKAGE_SHARDING: "--shardIndex 0 --shardCount 2" - PACKAGE_SHARDING: "--shardIndex 1 --shardCount 2" - matrix: - CHANNEL: "master" - CHANNEL: "stable" - unit_test_script: - - ./script/tool_runner.sh dart-test --exclude=script/configs/dart_unit_tests_exceptions.yaml - pathified_unit_test_script: - # Run tests with path-based dependencies to ensure that publishing - # the changes won't break tests of other packages in the repository - # that depend on it. - - ./script/tool_runner.sh make-deps-path-based --target-dependencies-with-non-breaking-updates - - $PLUGIN_TOOL_COMMAND dart-test --run-on-dirty-packages --exclude=script/configs/dart_unit_tests_exceptions.yaml - name: linux-custom_package_tests env: PATH: $PATH:/usr/local/bin From 2254dd7729ce8f4571e93f970d36d1d579b6ab30 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 10:20:02 -0400 Subject: [PATCH 03/21] Add new target --- .ci/targets/web_dart_unit_tests.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .ci/targets/web_dart_unit_tests.yaml diff --git a/.ci/targets/web_dart_unit_tests.yaml b/.ci/targets/web_dart_unit_tests.yaml new file mode 100644 index 00000000000..deb8fa54e38 --- /dev/null +++ b/.ci/targets/web_dart_unit_tests.yaml @@ -0,0 +1,6 @@ +tasks: + - name: prepare tool + script: .ci/scripts/prepare_tool.sh + - name: Dart unit tests - web + script: script/tool_runner.sh + args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml", "--browser=chrome"] From 26326e475943b999cb74e18f77f5416368ff5a58 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 10:22:20 -0400 Subject: [PATCH 04/21] Temporarily repurpose the main unit tests for CI run --- .ci.yaml | 34 ++++++-------------- script/tool/lib/src/dart_test_command.dart | 17 +++++++--- script/tool/test/dart_test_command_test.dart | 22 ++++++------- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/.ci.yaml b/.ci.yaml index 6524401b9c1..e102a48302b 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -96,43 +96,29 @@ targets: recipe: packages/packages timeout: 60 properties: - add_recipes_cq: "true" - target_file: dart_unit_tests.yaml + target_file: web_dart_unit_tests.yaml channel: master version_file: flutter_master.version cores: "32" package_sharding: "--shardIndex 0 --shardCount 2" + dependencies: >- + [ + {"dependency": "chrome_and_driver", "version": "version:114.0"} + ] - name: Linux dart_unit_test_shard_2 master recipe: packages/packages timeout: 60 properties: - add_recipes_cq: "true" - target_file: dart_unit_tests.yaml + target_file: web_dart_unit_tests.yaml channel: master version_file: flutter_master.version cores: "32" package_sharding: "--shardIndex 1 --shardCount 2" - - - name: Linux dart_unit_test_shard_1 stable - recipe: packages/packages - timeout: 60 - properties: - target_file: dart_unit_tests.yaml - channel: stable - version_file: flutter_stable.version - cores: "32" - package_sharding: "--shardIndex 0 --shardCount 2" - - - name: Linux dart_unit_test_shard_2 stable - recipe: packages/packages - timeout: 60 - properties: - target_file: dart_unit_tests.yaml - channel: stable - version_file: flutter_stable.version - cores: "32" - package_sharding: "--shardIndex 1 --shardCount 2" + dependencies: >- + [ + {"dependency": "chrome_and_driver", "version": "version:114.0"} + ] - name: Linux analyze master recipe: packages/packages diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index 9a93d2d9a2a..904b04a8d7e 100644 --- a/script/tool/lib/src/dart_test_command.dart +++ b/script/tool/lib/src/dart_test_command.dart @@ -7,7 +7,6 @@ import 'package:platform/platform.dart'; import 'common/core.dart'; import 'common/package_looping_command.dart'; -import 'common/plugin_utils.dart'; import 'common/process_runner.dart'; import 'common/repository_package.dart'; @@ -27,8 +26,16 @@ class DartTestCommand extends PackageLoopingCommand { 'See https://github.com/dart-lang/sdk/blob/main/docs/process/experimental-flags.md ' 'for details.', ); + argParser.addOption( + _platformFlag, + defaultsTo: '', + help: + 'Runs Dart unit tests on the given platform instead of the Dart VM.', + ); } + static const String _platformFlag = 'platform'; + @override final String name = 'dart-test'; @@ -64,6 +71,7 @@ class DartTestCommand extends PackageLoopingCommand { /// Runs the Dart tests for a Flutter package, returning true on success. Future _runFlutterTests(RepositoryPackage package) async { final String experiment = getStringArg(kEnableExperiment); + final String platform = getStringArg(_platformFlag); final int exitCode = await processRunner.runAndStream( flutterCommand, @@ -71,10 +79,7 @@ class DartTestCommand extends PackageLoopingCommand { 'test', '--color', if (experiment.isNotEmpty) '--enable-experiment=$experiment', - // TODO(ditman): Remove this once all plugins are migrated to 'drive'. - if (pluginSupportsPlatform(platformWeb, package, - requiredMode: PlatformSupport.inline)) - '--platform=chrome', + if (platform.isNotEmpty) '--platform=$platform', ], workingDir: package.directory, ); @@ -96,6 +101,7 @@ class DartTestCommand extends PackageLoopingCommand { } final String experiment = getStringArg(kEnableExperiment); + final String platform = getStringArg(_platformFlag); exitCode = await processRunner.runAndStream( 'dart', @@ -103,6 +109,7 @@ class DartTestCommand extends PackageLoopingCommand { 'run', if (experiment.isNotEmpty) '--enable-experiment=$experiment', 'test', + if (platform.isNotEmpty) '--platform=$platform', ], workingDir: package.directory, ); diff --git a/script/tool/test/dart_test_command_test.dart b/script/tool/test/dart_test_command_test.dart index 7f52fcb176c..82ae0907c13 100644 --- a/script/tool/test/dart_test_command_test.dart +++ b/script/tool/test/dart_test_command_test.dart @@ -231,7 +231,7 @@ void main() { ])); }); - test('runs on Chrome for web plugins', () async { + test('runs in Chrome when requested for Flutter package', () async { final RepositoryPackage plugin = createFakePlugin( 'plugin', packagesDir, @@ -241,7 +241,8 @@ void main() { }, ); - await runCapturingPrint(runner, ['dart-test']); + await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); expect( processRunner.recordedCalls, @@ -254,23 +255,22 @@ void main() { ); }); - test('Does not run on Chrome for web endorsements', () async { - final RepositoryPackage plugin = createFakePlugin( - 'plugin', + test('runs in Chrome when requested for Dart package', () async { + final RepositoryPackage package = createFakePackage( + 'package', packagesDir, extraFiles: ['test/empty_test.dart'], - platformSupport: { - platformWeb: const PlatformDetails(PlatformSupport.federated), - }, ); - await runCapturingPrint(runner, ['dart-test']); + await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); expect( processRunner.recordedCalls, orderedEquals([ - ProcessCall(getFlutterCommand(mockPlatform), - const ['test', '--color'], plugin.path), + ProcessCall('dart', const ['pub', 'get'], package.path), + ProcessCall('dart', + const ['run', 'test', '--platform=chrome'], package.path), ]), ); }); From 89968886223d7350bea8e0545d3af1ab97bfb5af Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 10:51:13 -0400 Subject: [PATCH 05/21] Fix flag --- .ci/targets/web_dart_unit_tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/targets/web_dart_unit_tests.yaml b/.ci/targets/web_dart_unit_tests.yaml index deb8fa54e38..ad56af5144f 100644 --- a/.ci/targets/web_dart_unit_tests.yaml +++ b/.ci/targets/web_dart_unit_tests.yaml @@ -3,4 +3,4 @@ tasks: script: .ci/scripts/prepare_tool.sh - name: Dart unit tests - web script: script/tool_runner.sh - args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml", "--browser=chrome"] + args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml", "--platform=chrome"] From 906fb7bfa3256bfd76cb9409d9e9efbfe21a6c85 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 11:38:45 -0400 Subject: [PATCH 06/21] Skip non-web plugins on web --- script/tool/lib/src/dart_test_command.dart | 12 ++++ script/tool/test/dart_test_command_test.dart | 69 ++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index 904b04a8d7e..222850113be 100644 --- a/script/tool/lib/src/dart_test_command.dart +++ b/script/tool/lib/src/dart_test_command.dart @@ -7,6 +7,7 @@ import 'package:platform/platform.dart'; import 'common/core.dart'; import 'common/package_looping_command.dart'; +import 'common/plugin_utils.dart'; import 'common/process_runner.dart'; import 'common/repository_package.dart'; @@ -59,6 +60,17 @@ class DartTestCommand extends PackageLoopingCommand { return PackageResult.skip('No test/ directory.'); } + // Skip running plugin tests for non-web-supporting plugins (or non-web + // federated plugin implementations) on web, since there's no reason to + // expect them to work. + final bool webPlatform = getStringArg(_platformFlag).isNotEmpty && + getStringArg(_platformFlag) != 'vm'; + if (webPlatform && + isFlutterPlugin(package) && + !pluginSupportsPlatform(platformWeb, package)) { + return PackageResult.skip("Non-web plugin tests don't need web testing."); + } + bool passed; if (package.requiresFlutter()) { passed = await _runFlutterTests(package); diff --git a/script/tool/test/dart_test_command_test.dart b/script/tool/test/dart_test_command_test.dart index 82ae0907c13..8a6f0db0251 100644 --- a/script/tool/test/dart_test_command_test.dart +++ b/script/tool/test/dart_test_command_test.dart @@ -232,6 +232,29 @@ void main() { }); test('runs in Chrome when requested for Flutter package', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + extraFiles: ['test/empty_test.dart'], + ); + + await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const ['test', '--color', '--platform=chrome'], + package.path), + ]), + ); + }); + + test('runs in Chrome when requested for Flutter plugin that implement web', + () async { final RepositoryPackage plugin = createFakePlugin( 'plugin', packagesDir, @@ -255,6 +278,52 @@ void main() { ); }); + test('runs in Chrome when requested for Flutter plugin that endorse web', + () async { + final RepositoryPackage plugin = createFakePlugin( + 'plugin', + packagesDir, + extraFiles: ['test/empty_test.dart'], + platformSupport: { + platformWeb: const PlatformDetails(PlatformSupport.federated), + }, + ); + + await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const ['test', '--color', '--platform=chrome'], + plugin.path), + ]), + ); + }); + + test('skips running non-web-plugins in browser mode', () async { + createFakePlugin( + 'non_web_plugin', + packagesDir, + extraFiles: ['test/empty_test.dart'], + ); + + final List output = await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); + + expect( + output, + containsAllInOrder([ + contains("Non-web plugin tests don't need web testing."), + ])); + expect( + processRunner.recordedCalls, + orderedEquals([]), + ); + }); + test('runs in Chrome when requested for Dart package', () async { final RepositoryPackage package = createFakePackage( 'package', From 9960a21e6d0fd29a7acfde8776a3827e6e4633e1 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 11:53:25 -0400 Subject: [PATCH 07/21] Support opting out of web tests --- script/tool/lib/src/dart_test_command.dart | 27 +++++++++++++++++--- script/tool/test/dart_test_command_test.dart | 24 +++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index 222850113be..292a0d4ca75 100644 --- a/script/tool/lib/src/dart_test_command.dart +++ b/script/tool/lib/src/dart_test_command.dart @@ -65,10 +65,15 @@ class DartTestCommand extends PackageLoopingCommand { // expect them to work. final bool webPlatform = getStringArg(_platformFlag).isNotEmpty && getStringArg(_platformFlag) != 'vm'; - if (webPlatform && - isFlutterPlugin(package) && - !pluginSupportsPlatform(platformWeb, package)) { - return PackageResult.skip("Non-web plugin tests don't need web testing."); + if (webPlatform) { + if (isFlutterPlugin(package) && + !pluginSupportsPlatform(platformWeb, package)) { + return PackageResult.skip( + "Non-web plugin tests don't need web testing."); + } + if (_requiresVM(package)) { + return PackageResult.skip('Package has opted out of non-vm testing.'); + } } bool passed; @@ -128,4 +133,18 @@ class DartTestCommand extends PackageLoopingCommand { return exitCode == 0; } + + bool _requiresVM(RepositoryPackage package) { + final File testConfig = package.directory.childFile('dart_test.yaml'); + if (!testConfig.existsSync()) { + return false; + } + // test_on lines can be very complex, but in pratice the packages in this + // repo currently only need the ability to require vm or not, so that + // simple directive is all that is supported. + final RegExp vmRequrimentRegex = RegExp(r'^test_on:\s*vm$'); + return testConfig + .readAsLinesSync() + .any((String line) => vmRequrimentRegex.hasMatch(line)); + } } diff --git a/script/tool/test/dart_test_command_test.dart b/script/tool/test/dart_test_command_test.dart index 8a6f0db0251..e1aa5b04576 100644 --- a/script/tool/test/dart_test_command_test.dart +++ b/script/tool/test/dart_test_command_test.dart @@ -344,6 +344,30 @@ void main() { ); }); + test('skips running in browser mode if package opts out', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + extraFiles: ['test/empty_test.dart'], + ); + package.directory.childFile('dart_test.yaml').writeAsStringSync(''' +test_on: vm +'''); + + final List output = await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); + + expect( + output, + containsAllInOrder([ + contains('Package has opted out of non-vm testing.'), + ])); + expect( + processRunner.recordedCalls, + orderedEquals([]), + ); + }); + test('enable-experiment flag', () async { final RepositoryPackage plugin = createFakePlugin('a', packagesDir, extraFiles: ['test/empty_test.dart']); From 2bc0801f90992858d4372848cc3f33000a47c38a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 11:53:36 -0400 Subject: [PATCH 08/21] Opt out flutter_migrate --- packages/flutter_migrate/dart_test.yaml | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/flutter_migrate/dart_test.yaml diff --git a/packages/flutter_migrate/dart_test.yaml b/packages/flutter_migrate/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/flutter_migrate/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm From 6608914cfbcb7e77ea2b96b19ba6964152c6da84 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 12:08:08 -0400 Subject: [PATCH 09/21] flutter_markdown adjustments --- .../flutter_markdown/test/image_test.dart | 26 ++++++++++++++++++- packages/flutter_markdown/test/link_test.dart | 10 +++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/flutter_markdown/test/image_test.dart b/packages/flutter_markdown/test/image_test.dart index f70785dd8c4..65d9aba71da 100644 --- a/packages/flutter_markdown/test/image_test.dart +++ b/packages/flutter_markdown/test/image_test.dart @@ -4,6 +4,7 @@ import 'dart:io' as io; +import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter_markdown/flutter_markdown.dart'; @@ -90,7 +91,7 @@ void defineTests() { ); testWidgets( - 'local files should be files', + 'local files should be files on non-web', (WidgetTester tester) async { const String data = '![alt](http.png)'; await tester.pumpWidget( @@ -105,6 +106,26 @@ void defineTests() { expect(image.image is FileImage, isTrue); }, + skip: kIsWeb, + ); + + testWidgets( + 'local files should be network on web', + (WidgetTester tester) async { + const String data = '![alt](http.png)'; + await tester.pumpWidget( + boilerplate( + const Markdown(data: data), + ), + ); + + final Iterable widgets = tester.allWidgets; + final Image image = + widgets.firstWhere((Widget widget) => widget is Image) as Image; + + expect(image.image is NetworkImage, isTrue); + }, + skip: !kIsWeb, ); testWidgets( @@ -150,6 +171,7 @@ void defineTests() { matchesGoldenFile( 'assets/images/golden/image_test/resource_asset_logo.png')); }, + skip: kIsWeb, // Goldens are platform-specific. ); testWidgets( @@ -168,6 +190,7 @@ void defineTests() { expect(image.width, 50); expect(image.height, 50); }, + skip: kIsWeb, ); testWidgets( @@ -360,6 +383,7 @@ void defineTests() { matchesGoldenFile( 'assets/images/golden/image_test/custom_builder_asset_logo.png')); }, + skip: kIsWeb, // Goldens are platform-specific. ); }); } diff --git a/packages/flutter_markdown/test/link_test.dart b/packages/flutter_markdown/test/link_test.dart index d7b7be9cae1..bbe4c15c6d4 100644 --- a/packages/flutter_markdown/test/link_test.dart +++ b/packages/flutter_markdown/test/link_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_markdown/flutter_markdown.dart'; @@ -1147,8 +1148,13 @@ void defineTests() { final Finder imageFinder = find.byType(Image); expect(imageFinder, findsOneWidget); final Image image = imageFinder.evaluate().first.widget as Image; - final FileImage fi = image.image as FileImage; - expect(fi.file.path, equals('uri3')); + if (kIsWeb) { + final NetworkImage fi = image.image as NetworkImage; + expect(fi.url.endsWith('uri3'), true); + } else { + final FileImage fi = image.image as FileImage; + expect(fi.file.path, equals('uri3')); + } expect(linkTapResults, isNull); }, ); From e259bef8b5152d2db3bc4c622fe1f105ff9b0f3a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 12:09:32 -0400 Subject: [PATCH 10/21] Opt go_router_build out --- packages/go_router_builder/dart_test.yaml | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/go_router_builder/dart_test.yaml diff --git a/packages/go_router_builder/dart_test.yaml b/packages/go_router_builder/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/go_router_builder/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm From e7c098fce44372d92737695280a6c1bcb8368749 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 12:18:58 -0400 Subject: [PATCH 11/21] Skip one test on web --- .../test/types/bitmap_test.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_platform_interface/test/types/bitmap_test.dart b/packages/google_maps_flutter/google_maps_flutter_platform_interface/test/types/bitmap_test.dart index 469f9820efa..94132f66aa5 100644 --- a/packages/google_maps_flutter/google_maps_flutter_platform_interface/test/types/bitmap_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_platform_interface/test/types/bitmap_test.dart @@ -185,7 +185,9 @@ void main() { expect((mip.toJson() as List)[2], 1); expect((scaled.toJson() as List)[2], 3); - }); + }, + // TODO(stuartmorgan): Investigate timeout on web. + skip: kIsWeb); test('name cannot be null or empty', () { expect(() { From 05e31446dde5c4e9ef896e0a4c464d0d6f46a977 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 12:20:01 -0400 Subject: [PATCH 12/21] Opt out metrics center --- packages/metrics_center/dart_test.yaml | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/metrics_center/dart_test.yaml diff --git a/packages/metrics_center/dart_test.yaml b/packages/metrics_center/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/metrics_center/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm From 2442d54473c470529f04468e3a102af1677fc652 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 12:27:53 -0400 Subject: [PATCH 13/21] Temporarily opt palette_generator out --- packages/palette_generator/dart_test.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 packages/palette_generator/dart_test.yaml diff --git a/packages/palette_generator/dart_test.yaml b/packages/palette_generator/dart_test.yaml new file mode 100644 index 00000000000..ded7e017b79 --- /dev/null +++ b/packages/palette_generator/dart_test.yaml @@ -0,0 +1,3 @@ +# TODO(stuartmorgan): Adjust the tests not to require dart:io +# See https://github.com/flutter/flutter/issues/129839 +test_on: vm From b53b5acd923fb14e88e9edaf361f642f47c71678 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 12:57:26 -0400 Subject: [PATCH 14/21] Opt out several more packages that don't support web --- packages/multicast_dns/dart_test.yaml | 1 + packages/pigeon/dart_test.yaml | 1 + packages/xdg_directories/dart_test.yaml | 1 + 3 files changed, 3 insertions(+) create mode 100644 packages/multicast_dns/dart_test.yaml create mode 100644 packages/pigeon/dart_test.yaml create mode 100644 packages/xdg_directories/dart_test.yaml diff --git a/packages/multicast_dns/dart_test.yaml b/packages/multicast_dns/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/multicast_dns/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm diff --git a/packages/pigeon/dart_test.yaml b/packages/pigeon/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/pigeon/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm diff --git a/packages/xdg_directories/dart_test.yaml b/packages/xdg_directories/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/xdg_directories/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm From c591b91a4aa46f5ab6e90b73f1dc69bc11975961 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 13:00:45 -0400 Subject: [PATCH 15/21] Opt out of unsupported tests in standard_message_codec --- .../test/standard_message_codec_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/standard_message_codec/test/standard_message_codec_test.dart b/packages/standard_message_codec/test/standard_message_codec_test.dart index 6feb5fc170f..299bfa295d8 100644 --- a/packages/standard_message_codec/test/standard_message_codec_test.dart +++ b/packages/standard_message_codec/test/standard_message_codec_test.dart @@ -66,7 +66,7 @@ void main() { expect(written.lengthInBytes, equals(8)); final ReadBuffer read = ReadBuffer(written); expect(read.getInt64(), equals(-9000000000000)); - }); + }, testOn: 'vm' /* Int64 isn't supported on web */); test('of 64-bit integer in big endian', () { final WriteBuffer write = WriteBuffer(); @@ -75,7 +75,7 @@ void main() { expect(written.lengthInBytes, equals(8)); final ReadBuffer read = ReadBuffer(written); expect(read.getInt64(endian: Endian.big), equals(-9000000000000)); - }); + }, testOn: 'vm' /* Int64 isn't supported on web */); test('of double', () { final WriteBuffer write = WriteBuffer(); @@ -115,7 +115,7 @@ void main() { final ReadBuffer read = ReadBuffer(written); read.getUint8(); expect(read.getInt64List(3), equals(integers)); - }); + }, testOn: 'vm' /* Int64 isn't supported on web */); test('of float list when unaligned', () { final Float32List floats = From da4b6f0a7944f19c6a21e204cee2a7c8bde970a1 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 13:11:15 -0400 Subject: [PATCH 16/21] Temporarily disable rfw --- packages/rfw/dart_test.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 packages/rfw/dart_test.yaml diff --git a/packages/rfw/dart_test.yaml b/packages/rfw/dart_test.yaml new file mode 100644 index 00000000000..ae7e4513525 --- /dev/null +++ b/packages/rfw/dart_test.yaml @@ -0,0 +1,3 @@ +# TODO(stuartmorgan): Fix the web failures, and enable. See +# https://github.com/flutter/flutter/issues/129843 +test_on: vm From d8e01416ab8d8cbfecf7502457793d7bb4b4577b Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 14:41:18 -0400 Subject: [PATCH 17/21] dart_test.yaml is a test-only change --- script/tool/lib/src/common/package_state_utils.dart | 1 + script/tool/test/common/package_state_utils_test.dart | 1 + 2 files changed, 2 insertions(+) diff --git a/script/tool/lib/src/common/package_state_utils.dart b/script/tool/lib/src/common/package_state_utils.dart index 3885cde8e7a..51b76957cc5 100644 --- a/script/tool/lib/src/common/package_state_utils.dart +++ b/script/tool/lib/src/common/package_state_utils.dart @@ -111,6 +111,7 @@ bool _isTestChange(List pathComponents) { pathComponents.contains('androidTest') || pathComponents.contains('RunnerTests') || pathComponents.contains('RunnerUITests') || + pathComponents.last == 'dart_test.yaml' || // Pigeon's custom platform tests. pathComponents.first == 'platform_tests'; } diff --git a/script/tool/test/common/package_state_utils_test.dart b/script/tool/test/common/package_state_utils_test.dart index 9c8c2c04731..0f7a83afa1d 100644 --- a/script/tool/test/common/package_state_utils_test.dart +++ b/script/tool/test/common/package_state_utils_test.dart @@ -72,6 +72,7 @@ void main() { 'packages/a_plugin/pigeons/messages.dart', // Test scripts. 'packages/a_plugin/run_tests.sh', + 'packages/a_plugin/dart_test.yaml', // Tools. 'packages/a_plugin/tool/a_development_tool.dart', // Example build files. From 4e62e4781d7730f242627569349694d9e98169ab Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 14:42:28 -0400 Subject: [PATCH 18/21] Opt out go_router_builder example --- packages/go_router_builder/example/dart_test.yaml | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/go_router_builder/example/dart_test.yaml diff --git a/packages/go_router_builder/example/dart_test.yaml b/packages/go_router_builder/example/dart_test.yaml new file mode 100644 index 00000000000..91ec220b8e2 --- /dev/null +++ b/packages/go_router_builder/example/dart_test.yaml @@ -0,0 +1 @@ +test_on: vm From b93cb1266d7f54b2fc838d046b26a473abdbf319 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 14:45:12 -0400 Subject: [PATCH 19/21] Remove temporary testing hack; add new targets --- .ci.yaml | 78 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/.ci.yaml b/.ci.yaml index e102a48302b..cf6b714d90d 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -93,20 +93,62 @@ targets: version_file: flutter_master.version - name: Linux dart_unit_test_shard_1 master + bringup: true # New target recipe: packages/packages timeout: 60 properties: - target_file: web_dart_unit_tests.yaml + target_file: dart_unit_tests.yaml channel: master version_file: flutter_master.version cores: "32" package_sharding: "--shardIndex 0 --shardCount 2" - dependencies: >- - [ - {"dependency": "chrome_and_driver", "version": "version:114.0"} - ] - name: Linux dart_unit_test_shard_2 master + bringup: true # New target + recipe: packages/packages + timeout: 60 + properties: + target_file: dart_unit_tests.yaml + channel: master + version_file: flutter_master.version + cores: "32" + package_sharding: "--shardIndex 1 --shardCount 2" + + - name: Linux dart_unit_test_shard_1 stable + bringup: true # New target + recipe: packages/packages + timeout: 60 + properties: + target_file: dart_unit_tests.yaml + channel: stable + version_file: flutter_stable.version + cores: "32" + package_sharding: "--shardIndex 0 --shardCount 2" + + - name: Linux dart_unit_test_shard_2 stable + bringup: true # New target + recipe: packages/packages + timeout: 60 + properties: + target_file: dart_unit_tests.yaml + channel: stable + version_file: flutter_stable.version + cores: "32" + package_sharding: "--shardIndex 1 --shardCount 2" + + - name: Linux_web web_dart_unit_test_shard_1 master + bringup: true # New target + recipe: packages/packages + timeout: 60 + properties: + target_file: web_dart_unit_tests.yaml + channel: master + version_file: flutter_master.version + cores: "32" + package_sharding: "--shardIndex 0 --shardCount 2" + + - name: Linux_web web_dart_unit_test_shard_2 master + bringup: true # New target recipe: packages/packages timeout: 60 properties: @@ -115,10 +157,28 @@ targets: version_file: flutter_master.version cores: "32" package_sharding: "--shardIndex 1 --shardCount 2" - dependencies: >- - [ - {"dependency": "chrome_and_driver", "version": "version:114.0"} - ] + + - name: Linux_web web_dart_unit_test_shard_1 stable + bringup: true # New target + recipe: packages/packages + timeout: 60 + properties: + target_file: web_dart_unit_tests.yaml + channel: stable + version_file: flutter_stable.version + cores: "32" + package_sharding: "--shardIndex 0 --shardCount 2" + + - name: Linux_web web_dart_unit_test_shard_2 stable + bringup: true # New target + recipe: packages/packages + timeout: 60 + properties: + target_file: web_dart_unit_tests.yaml + channel: stable + version_file: flutter_stable.version + cores: "32" + package_sharding: "--shardIndex 1 --shardCount 2" - name: Linux analyze master recipe: packages/packages From aac156f81689c5c18931e6d955f367f49f031630 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 15:51:57 -0400 Subject: [PATCH 20/21] Restore old default behavior, add explicit vm mode --- script/tool/lib/src/dart_test_command.dart | 67 +++++++--- script/tool/test/dart_test_command_test.dart | 125 ++++++++++++++++++- 2 files changed, 175 insertions(+), 17 deletions(-) diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index 292a0d4ca75..5c997b39a91 100644 --- a/script/tool/lib/src/dart_test_command.dart +++ b/script/tool/lib/src/dart_test_command.dart @@ -29,9 +29,8 @@ class DartTestCommand extends PackageLoopingCommand { ); argParser.addOption( _platformFlag, - defaultsTo: '', - help: - 'Runs Dart unit tests on the given platform instead of the Dart VM.', + help: 'Runs tests on the given platform instead of the default platform ' + '("vm" in most cases, "chrome" for web plugin implementations).', ); } @@ -60,11 +59,17 @@ class DartTestCommand extends PackageLoopingCommand { return PackageResult.skip('No test/ directory.'); } + String? platform = getNullableStringArg(_platformFlag); + // Skip running plugin tests for non-web-supporting plugins (or non-web // federated plugin implementations) on web, since there's no reason to // expect them to work. - final bool webPlatform = getStringArg(_platformFlag).isNotEmpty && - getStringArg(_platformFlag) != 'vm'; + final bool webPlatform = platform != null && platform != 'vm'; + final bool explicitVMPlatform = platform == 'vm'; + final bool isWebOnlyPluginImplementation = pluginSupportsPlatform( + platformWeb, package, + requiredMode: PlatformSupport.inline) && + package.directory.basename.endsWith('_web'); if (webPlatform) { if (isFlutterPlugin(package) && !pluginSupportsPlatform(platformWeb, package)) { @@ -72,23 +77,40 @@ class DartTestCommand extends PackageLoopingCommand { "Non-web plugin tests don't need web testing."); } if (_requiresVM(package)) { + // This explict skip is necessary because trying to run tests in a mode + // that the package has opted out of returns a non-zero exit code. return PackageResult.skip('Package has opted out of non-vm testing.'); } + } else if (explicitVMPlatform) { + if (isWebOnlyPluginImplementation) { + return PackageResult.skip("Web plugin tests don't need vm testing."); + } + if (_requiresNonVM(package)) { + // This explict skip is necessary because trying to run tests in a mode + // that the package has opted out of returns a non-zero exit code. + return PackageResult.skip('Package has opted out of vm testing.'); + } + } else if (platform == null && isWebOnlyPluginImplementation) { + // If no explicit mode is requested, run web plugin implementations in + // Chrome since their tests are not expected to work in vm mode. This + // allows easily running all unit tests locally, without having to run + // both modes. + platform = 'chrome'; } bool passed; if (package.requiresFlutter()) { - passed = await _runFlutterTests(package); + passed = await _runFlutterTests(package, platform: platform); } else { - passed = await _runDartTests(package); + passed = await _runDartTests(package, platform: platform); } return passed ? PackageResult.success() : PackageResult.fail(); } /// Runs the Dart tests for a Flutter package, returning true on success. - Future _runFlutterTests(RepositoryPackage package) async { + Future _runFlutterTests(RepositoryPackage package, + {String? platform}) async { final String experiment = getStringArg(kEnableExperiment); - final String platform = getStringArg(_platformFlag); final int exitCode = await processRunner.runAndStream( flutterCommand, @@ -96,7 +118,7 @@ class DartTestCommand extends PackageLoopingCommand { 'test', '--color', if (experiment.isNotEmpty) '--enable-experiment=$experiment', - if (platform.isNotEmpty) '--platform=$platform', + if (platform != null) '--platform=$platform', ], workingDir: package.directory, ); @@ -104,7 +126,8 @@ class DartTestCommand extends PackageLoopingCommand { } /// Runs the Dart tests for a non-Flutter package, returning true on success. - Future _runDartTests(RepositoryPackage package) async { + Future _runDartTests(RepositoryPackage package, + {String? platform}) async { // Unlike `flutter test`, `pub run test` does not automatically get // packages int exitCode = await processRunner.runAndStream( @@ -118,7 +141,6 @@ class DartTestCommand extends PackageLoopingCommand { } final String experiment = getStringArg(kEnableExperiment); - final String platform = getStringArg(_platformFlag); exitCode = await processRunner.runAndStream( 'dart', @@ -126,7 +148,7 @@ class DartTestCommand extends PackageLoopingCommand { 'run', if (experiment.isNotEmpty) '--enable-experiment=$experiment', 'test', - if (platform.isNotEmpty) '--platform=$platform', + if (platform != null) '--platform=$platform', ], workingDir: package.directory, ); @@ -141,10 +163,27 @@ class DartTestCommand extends PackageLoopingCommand { } // test_on lines can be very complex, but in pratice the packages in this // repo currently only need the ability to require vm or not, so that - // simple directive is all that is supported. + // simple directive is all that is currently supported. final RegExp vmRequrimentRegex = RegExp(r'^test_on:\s*vm$'); return testConfig .readAsLinesSync() .any((String line) => vmRequrimentRegex.hasMatch(line)); } + + bool _requiresNonVM(RepositoryPackage package) { + final File testConfig = package.directory.childFile('dart_test.yaml'); + if (!testConfig.existsSync()) { + return false; + } + // test_on lines can be very complex, but in pratice the packages in this + // repo currently only need the ability to require vm or not, so a simple + // one-target directive is all that's supported currently. Making it + // deliberately strict avoids the possibility of accidentally skipping vm + // coverage due to a complex expression that's not handled correctly. + final RegExp testOnRegex = RegExp(r'^test_on:\s*([a-z])*\s*$'); + return testConfig.readAsLinesSync().any((String line) { + final RegExpMatch? match = testOnRegex.firstMatch(line); + return match != null && match.group(1) != 'vm'; + }); + } } diff --git a/script/tool/test/dart_test_command_test.dart b/script/tool/test/dart_test_command_test.dart index e1aa5b04576..e8643f34823 100644 --- a/script/tool/test/dart_test_command_test.dart +++ b/script/tool/test/dart_test_command_test.dart @@ -253,10 +253,34 @@ void main() { ); }); - test('runs in Chrome when requested for Flutter plugin that implement web', + test('runs in Chrome by default for Flutter plugins that implement web', () async { final RepositoryPackage plugin = createFakePlugin( - 'plugin', + 'some_plugin_web', + packagesDir, + extraFiles: ['test/empty_test.dart'], + platformSupport: { + platformWeb: const PlatformDetails(PlatformSupport.inline), + }, + ); + + await runCapturingPrint(runner, ['dart-test']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const ['test', '--color', '--platform=chrome'], + plugin.path), + ]), + ); + }); + + test('runs in Chrome when requested for Flutter plugins that implement web', + () async { + final RepositoryPackage plugin = createFakePlugin( + 'some_plugin_web', packagesDir, extraFiles: ['test/empty_test.dart'], platformSupport: { @@ -303,7 +327,7 @@ void main() { ); }); - test('skips running non-web-plugins in browser mode', () async { + test('skips running non-web plugins in browser mode', () async { createFakePlugin( 'non_web_plugin', packagesDir, @@ -324,6 +348,55 @@ void main() { ); }); + test('skips running web plugins in explicit vm mode', () async { + createFakePlugin( + 'some_plugin_web', + packagesDir, + extraFiles: ['test/empty_test.dart'], + platformSupport: { + platformWeb: const PlatformDetails(PlatformSupport.inline), + }, + ); + + final List output = await runCapturingPrint( + runner, ['dart-test', '--platform=vm']); + + expect( + output, + containsAllInOrder([ + contains("Web plugin tests don't need vm testing."), + ])); + expect( + processRunner.recordedCalls, + orderedEquals([]), + ); + }); + + test('does not skip for plugins that endorse web', () async { + final RepositoryPackage plugin = createFakePlugin( + 'some_plugin', + packagesDir, + extraFiles: ['test/empty_test.dart'], + platformSupport: { + platformWeb: const PlatformDetails(PlatformSupport.federated), + platformAndroid: const PlatformDetails(PlatformSupport.federated), + }, + ); + + await runCapturingPrint( + runner, ['dart-test', '--platform=chrome']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const ['test', '--color', '--platform=chrome'], + plugin.path), + ]), + ); + }); + test('runs in Chrome when requested for Dart package', () async { final RepositoryPackage package = createFakePackage( 'package', @@ -368,6 +441,52 @@ test_on: vm ); }); + test('skips running in vm mode if package opts out', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + extraFiles: ['test/empty_test.dart'], + ); + package.directory.childFile('dart_test.yaml').writeAsStringSync(''' +test_on: browser +'''); + + final List output = await runCapturingPrint( + runner, ['dart-test', '--platform=vm']); + + expect( + output, + containsAllInOrder([ + contains('Package has opted out of vm testing.'), + ])); + expect( + processRunner.recordedCalls, + orderedEquals([]), + ); + }); + + test('tries to run for a test_on that the tool does not recognize', + () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + extraFiles: ['test/empty_test.dart'], + ); + package.directory.childFile('dart_test.yaml').writeAsStringSync(''' +test_on: !vm && firefox +'''); + + await runCapturingPrint(runner, ['dart-test']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('dart', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['run', 'test'], package.path), + ]), + ); + }); + test('enable-experiment flag', () async { final RepositoryPackage plugin = createFakePlugin('a', packagesDir, extraFiles: ['test/empty_test.dart']); From 7fca7ebf9f98fd062aae93629737b18983630f25 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 30 Jun 2023 15:52:50 -0400 Subject: [PATCH 21/21] Switch non-web LUCI to explicit vm mode --- .ci/targets/dart_unit_tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/targets/dart_unit_tests.yaml b/.ci/targets/dart_unit_tests.yaml index 8f86e0d1b49..e64bba8fa1d 100644 --- a/.ci/targets/dart_unit_tests.yaml +++ b/.ci/targets/dart_unit_tests.yaml @@ -3,7 +3,7 @@ tasks: script: .ci/scripts/prepare_tool.sh - name: Dart unit tests script: script/tool_runner.sh - args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml"] + args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml", "--platform=vm"] # Re-run tests with path-based dependencies to ensure that publishing # the changes won't break tests of other packages in the respository # that depend on it.