Skip to content

Commit 7042079

Browse files
[ci] Add a web version of Dart unit tests (flutter#4352)
Adds new LUCI targets in bringup mode to run all possible Dart unit tests in Chrome. This is a new test (see linked issue), not a port of a Cirrus test, so it involves changes to tooling and packages: - The tooling now accepts an explicit platform. The default behavior if none is provided is the previous behavior of running in VM for everything but web plugin implementations (since that's convenient locally). - The tooling now has a basic understanding of `dart_test.yaml` `test_on` directives, to know when to skip. - Packages that don't support web have opt-out files. - Packages that do support web but have a few tests that fail on web have those tests opted out. - Packages that do support web but have a lot of failures on web have temporary opt-out files with TODOs + issue links. Most of flutter/flutter#128979
1 parent b310246 commit 7042079

File tree

21 files changed

+407
-37
lines changed

21 files changed

+407
-37
lines changed

.ci.yaml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,50 @@ targets:
136136
cores: "32"
137137
package_sharding: "--shardIndex 1 --shardCount 2"
138138

139+
- name: Linux_web web_dart_unit_test_shard_1 master
140+
bringup: true # New target
141+
recipe: packages/packages
142+
timeout: 60
143+
properties:
144+
target_file: web_dart_unit_tests.yaml
145+
channel: master
146+
version_file: flutter_master.version
147+
cores: "32"
148+
package_sharding: "--shardIndex 0 --shardCount 2"
149+
150+
- name: Linux_web web_dart_unit_test_shard_2 master
151+
bringup: true # New target
152+
recipe: packages/packages
153+
timeout: 60
154+
properties:
155+
target_file: web_dart_unit_tests.yaml
156+
channel: master
157+
version_file: flutter_master.version
158+
cores: "32"
159+
package_sharding: "--shardIndex 1 --shardCount 2"
160+
161+
- name: Linux_web web_dart_unit_test_shard_1 stable
162+
bringup: true # New target
163+
recipe: packages/packages
164+
timeout: 60
165+
properties:
166+
target_file: web_dart_unit_tests.yaml
167+
channel: stable
168+
version_file: flutter_stable.version
169+
cores: "32"
170+
package_sharding: "--shardIndex 0 --shardCount 2"
171+
172+
- name: Linux_web web_dart_unit_test_shard_2 stable
173+
bringup: true # New target
174+
recipe: packages/packages
175+
timeout: 60
176+
properties:
177+
target_file: web_dart_unit_tests.yaml
178+
channel: stable
179+
version_file: flutter_stable.version
180+
cores: "32"
181+
package_sharding: "--shardIndex 1 --shardCount 2"
182+
139183
- name: Linux analyze master
140184
recipe: packages/packages
141185
timeout: 30

.ci/targets/dart_unit_tests.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ tasks:
33
script: .ci/scripts/prepare_tool.sh
44
- name: Dart unit tests
55
script: script/tool_runner.sh
6-
args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml"]
6+
args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml", "--platform=vm"]
77
# Re-run tests with path-based dependencies to ensure that publishing
88
# the changes won't break tests of other packages in the respository
99
# that depend on it.

.ci/targets/web_dart_unit_tests.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
tasks:
2+
- name: prepare tool
3+
script: .ci/scripts/prepare_tool.sh
4+
- name: Dart unit tests - web
5+
script: script/tool_runner.sh
6+
args: ["dart-test", "--exclude=script/configs/dart_unit_tests_exceptions.yaml", "--platform=chrome"]

.cirrus.yml

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -158,22 +158,6 @@ task:
158158
memory: 16G
159159
matrix:
160160
### Platform-agnostic tasks ###
161-
- name: dart_unit_tests
162-
env:
163-
matrix:
164-
PACKAGE_SHARDING: "--shardIndex 0 --shardCount 2"
165-
PACKAGE_SHARDING: "--shardIndex 1 --shardCount 2"
166-
matrix:
167-
CHANNEL: "master"
168-
CHANNEL: "stable"
169-
unit_test_script:
170-
- ./script/tool_runner.sh dart-test --exclude=script/configs/dart_unit_tests_exceptions.yaml
171-
pathified_unit_test_script:
172-
# Run tests with path-based dependencies to ensure that publishing
173-
# the changes won't break tests of other packages in the repository
174-
# that depend on it.
175-
- ./script/tool_runner.sh make-deps-path-based --target-dependencies-with-non-breaking-updates
176-
- $PLUGIN_TOOL_COMMAND dart-test --run-on-dirty-packages --exclude=script/configs/dart_unit_tests_exceptions.yaml
177161
- name: linux-custom_package_tests
178162
env:
179163
PATH: $PATH:/usr/local/bin

packages/flutter_markdown/test/image_test.dart

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:io' as io;
66

7+
import 'package:flutter/foundation.dart';
78
import 'package:flutter/gestures.dart';
89
import 'package:flutter/material.dart';
910
import 'package:flutter_markdown/flutter_markdown.dart';
@@ -90,7 +91,7 @@ void defineTests() {
9091
);
9192

9293
testWidgets(
93-
'local files should be files',
94+
'local files should be files on non-web',
9495
(WidgetTester tester) async {
9596
const String data = '![alt](http.png)';
9697
await tester.pumpWidget(
@@ -105,6 +106,26 @@ void defineTests() {
105106

106107
expect(image.image is FileImage, isTrue);
107108
},
109+
skip: kIsWeb,
110+
);
111+
112+
testWidgets(
113+
'local files should be network on web',
114+
(WidgetTester tester) async {
115+
const String data = '![alt](http.png)';
116+
await tester.pumpWidget(
117+
boilerplate(
118+
const Markdown(data: data),
119+
),
120+
);
121+
122+
final Iterable<Widget> widgets = tester.allWidgets;
123+
final Image image =
124+
widgets.firstWhere((Widget widget) => widget is Image) as Image;
125+
126+
expect(image.image is NetworkImage, isTrue);
127+
},
128+
skip: !kIsWeb,
108129
);
109130

110131
testWidgets(
@@ -150,6 +171,7 @@ void defineTests() {
150171
matchesGoldenFile(
151172
'assets/images/golden/image_test/resource_asset_logo.png'));
152173
},
174+
skip: kIsWeb, // Goldens are platform-specific.
153175
);
154176

155177
testWidgets(
@@ -168,6 +190,7 @@ void defineTests() {
168190
expect(image.width, 50);
169191
expect(image.height, 50);
170192
},
193+
skip: kIsWeb,
171194
);
172195

173196
testWidgets(
@@ -360,6 +383,7 @@ void defineTests() {
360383
matchesGoldenFile(
361384
'assets/images/golden/image_test/custom_builder_asset_logo.png'));
362385
},
386+
skip: kIsWeb, // Goldens are platform-specific.
363387
);
364388
});
365389
}

packages/flutter_markdown/test/link_test.dart

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'package:flutter/foundation.dart';
56
import 'package:flutter/gestures.dart';
67
import 'package:flutter/widgets.dart';
78
import 'package:flutter_markdown/flutter_markdown.dart';
@@ -1147,8 +1148,13 @@ void defineTests() {
11471148
final Finder imageFinder = find.byType(Image);
11481149
expect(imageFinder, findsOneWidget);
11491150
final Image image = imageFinder.evaluate().first.widget as Image;
1150-
final FileImage fi = image.image as FileImage;
1151-
expect(fi.file.path, equals('uri3'));
1151+
if (kIsWeb) {
1152+
final NetworkImage fi = image.image as NetworkImage;
1153+
expect(fi.url.endsWith('uri3'), true);
1154+
} else {
1155+
final FileImage fi = image.image as FileImage;
1156+
expect(fi.file.path, equals('uri3'));
1157+
}
11521158
expect(linkTapResults, isNull);
11531159
},
11541160
);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm

packages/google_maps_flutter/google_maps_flutter_platform_interface/test/types/bitmap_test.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,9 @@ void main() {
185185

186186
expect((mip.toJson() as List<dynamic>)[2], 1);
187187
expect((scaled.toJson() as List<dynamic>)[2], 3);
188-
});
188+
},
189+
// TODO(stuartmorgan): Investigate timeout on web.
190+
skip: kIsWeb);
189191

190192
test('name cannot be null or empty', () {
191193
expect(() {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm

packages/multicast_dns/dart_test.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# TODO(stuartmorgan): Adjust the tests not to require dart:io
2+
# See https://github.com/flutter/flutter/issues/129839
3+
test_on: vm

packages/pigeon/dart_test.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm

packages/rfw/dart_test.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# TODO(stuartmorgan): Fix the web failures, and enable. See
2+
# https://github.com/flutter/flutter/issues/129843
3+
test_on: vm

packages/standard_message_codec/test/standard_message_codec_test.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ void main() {
6666
expect(written.lengthInBytes, equals(8));
6767
final ReadBuffer read = ReadBuffer(written);
6868
expect(read.getInt64(), equals(-9000000000000));
69-
});
69+
}, testOn: 'vm' /* Int64 isn't supported on web */);
7070

7171
test('of 64-bit integer in big endian', () {
7272
final WriteBuffer write = WriteBuffer();
@@ -75,7 +75,7 @@ void main() {
7575
expect(written.lengthInBytes, equals(8));
7676
final ReadBuffer read = ReadBuffer(written);
7777
expect(read.getInt64(endian: Endian.big), equals(-9000000000000));
78-
});
78+
}, testOn: 'vm' /* Int64 isn't supported on web */);
7979

8080
test('of double', () {
8181
final WriteBuffer write = WriteBuffer();
@@ -115,7 +115,7 @@ void main() {
115115
final ReadBuffer read = ReadBuffer(written);
116116
read.getUint8();
117117
expect(read.getInt64List(3), equals(integers));
118-
});
118+
}, testOn: 'vm' /* Int64 isn't supported on web */);
119119

120120
test('of float list when unaligned', () {
121121
final Float32List floats =
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test_on: vm

script/tool/lib/src/common/package_state_utils.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ bool _isTestChange(List<String> pathComponents) {
111111
pathComponents.contains('androidTest') ||
112112
pathComponents.contains('RunnerTests') ||
113113
pathComponents.contains('RunnerUITests') ||
114+
pathComponents.last == 'dart_test.yaml' ||
114115
// Pigeon's custom platform tests.
115116
pathComponents.first == 'platform_tests';
116117
}

script/tool/lib/src/dart_test_command.dart

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,15 @@ class DartTestCommand extends PackageLoopingCommand {
2727
'See https://github.com/dart-lang/sdk/blob/main/docs/process/experimental-flags.md '
2828
'for details.',
2929
);
30+
argParser.addOption(
31+
_platformFlag,
32+
help: 'Runs tests on the given platform instead of the default platform '
33+
'("vm" in most cases, "chrome" for web plugin implementations).',
34+
);
3035
}
3136

37+
static const String _platformFlag = 'platform';
38+
3239
@override
3340
final String name = 'dart-test';
3441

@@ -52,17 +59,57 @@ class DartTestCommand extends PackageLoopingCommand {
5259
return PackageResult.skip('No test/ directory.');
5360
}
5461

62+
String? platform = getNullableStringArg(_platformFlag);
63+
64+
// Skip running plugin tests for non-web-supporting plugins (or non-web
65+
// federated plugin implementations) on web, since there's no reason to
66+
// expect them to work.
67+
final bool webPlatform = platform != null && platform != 'vm';
68+
final bool explicitVMPlatform = platform == 'vm';
69+
final bool isWebOnlyPluginImplementation = pluginSupportsPlatform(
70+
platformWeb, package,
71+
requiredMode: PlatformSupport.inline) &&
72+
package.directory.basename.endsWith('_web');
73+
if (webPlatform) {
74+
if (isFlutterPlugin(package) &&
75+
!pluginSupportsPlatform(platformWeb, package)) {
76+
return PackageResult.skip(
77+
"Non-web plugin tests don't need web testing.");
78+
}
79+
if (_requiresVM(package)) {
80+
// This explict skip is necessary because trying to run tests in a mode
81+
// that the package has opted out of returns a non-zero exit code.
82+
return PackageResult.skip('Package has opted out of non-vm testing.');
83+
}
84+
} else if (explicitVMPlatform) {
85+
if (isWebOnlyPluginImplementation) {
86+
return PackageResult.skip("Web plugin tests don't need vm testing.");
87+
}
88+
if (_requiresNonVM(package)) {
89+
// This explict skip is necessary because trying to run tests in a mode
90+
// that the package has opted out of returns a non-zero exit code.
91+
return PackageResult.skip('Package has opted out of vm testing.');
92+
}
93+
} else if (platform == null && isWebOnlyPluginImplementation) {
94+
// If no explicit mode is requested, run web plugin implementations in
95+
// Chrome since their tests are not expected to work in vm mode. This
96+
// allows easily running all unit tests locally, without having to run
97+
// both modes.
98+
platform = 'chrome';
99+
}
100+
55101
bool passed;
56102
if (package.requiresFlutter()) {
57-
passed = await _runFlutterTests(package);
103+
passed = await _runFlutterTests(package, platform: platform);
58104
} else {
59-
passed = await _runDartTests(package);
105+
passed = await _runDartTests(package, platform: platform);
60106
}
61107
return passed ? PackageResult.success() : PackageResult.fail();
62108
}
63109

64110
/// Runs the Dart tests for a Flutter package, returning true on success.
65-
Future<bool> _runFlutterTests(RepositoryPackage package) async {
111+
Future<bool> _runFlutterTests(RepositoryPackage package,
112+
{String? platform}) async {
66113
final String experiment = getStringArg(kEnableExperiment);
67114

68115
final int exitCode = await processRunner.runAndStream(
@@ -71,18 +118,16 @@ class DartTestCommand extends PackageLoopingCommand {
71118
'test',
72119
'--color',
73120
if (experiment.isNotEmpty) '--enable-experiment=$experiment',
74-
// TODO(ditman): Remove this once all plugins are migrated to 'drive'.
75-
if (pluginSupportsPlatform(platformWeb, package,
76-
requiredMode: PlatformSupport.inline))
77-
'--platform=chrome',
121+
if (platform != null) '--platform=$platform',
78122
],
79123
workingDir: package.directory,
80124
);
81125
return exitCode == 0;
82126
}
83127

84128
/// Runs the Dart tests for a non-Flutter package, returning true on success.
85-
Future<bool> _runDartTests(RepositoryPackage package) async {
129+
Future<bool> _runDartTests(RepositoryPackage package,
130+
{String? platform}) async {
86131
// Unlike `flutter test`, `pub run test` does not automatically get
87132
// packages
88133
int exitCode = await processRunner.runAndStream(
@@ -103,10 +148,42 @@ class DartTestCommand extends PackageLoopingCommand {
103148
'run',
104149
if (experiment.isNotEmpty) '--enable-experiment=$experiment',
105150
'test',
151+
if (platform != null) '--platform=$platform',
106152
],
107153
workingDir: package.directory,
108154
);
109155

110156
return exitCode == 0;
111157
}
158+
159+
bool _requiresVM(RepositoryPackage package) {
160+
final File testConfig = package.directory.childFile('dart_test.yaml');
161+
if (!testConfig.existsSync()) {
162+
return false;
163+
}
164+
// test_on lines can be very complex, but in pratice the packages in this
165+
// repo currently only need the ability to require vm or not, so that
166+
// simple directive is all that is currently supported.
167+
final RegExp vmRequrimentRegex = RegExp(r'^test_on:\s*vm$');
168+
return testConfig
169+
.readAsLinesSync()
170+
.any((String line) => vmRequrimentRegex.hasMatch(line));
171+
}
172+
173+
bool _requiresNonVM(RepositoryPackage package) {
174+
final File testConfig = package.directory.childFile('dart_test.yaml');
175+
if (!testConfig.existsSync()) {
176+
return false;
177+
}
178+
// test_on lines can be very complex, but in pratice the packages in this
179+
// repo currently only need the ability to require vm or not, so a simple
180+
// one-target directive is all that's supported currently. Making it
181+
// deliberately strict avoids the possibility of accidentally skipping vm
182+
// coverage due to a complex expression that's not handled correctly.
183+
final RegExp testOnRegex = RegExp(r'^test_on:\s*([a-z])*\s*$');
184+
return testConfig.readAsLinesSync().any((String line) {
185+
final RegExpMatch? match = testOnRegex.firstMatch(line);
186+
return match != null && match.group(1) != 'vm';
187+
});
188+
}
112189
}

script/tool/test/common/package_state_utils_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ void main() {
7272
'packages/a_plugin/pigeons/messages.dart',
7373
// Test scripts.
7474
'packages/a_plugin/run_tests.sh',
75+
'packages/a_plugin/dart_test.yaml',
7576
// Tools.
7677
'packages/a_plugin/tool/a_development_tool.dart',
7778
// Example build files.

0 commit comments

Comments
 (0)