Unverified Commit daaba8a4 authored by Harry Terkelsen's avatar Harry Terkelsen Committed by GitHub

Add --local-web-sdk in devicelab runner to make --ab testing work for web (#123825)

This allows us to check for performance differences in local Web SDKs.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
parent 54dbbd76
......@@ -38,6 +38,11 @@ Future<void> main(List<String> rawArgs) async {
/// Required for A/B test mode.
final String? localEngine = args['local-engine'] as String?;
/// The build of the local Web SDK to use.
///
/// Required for A/B test mode.
final String? localWebSdk = args['local-web-sdk'] as String?;
/// The path to the engine "src/" directory.
final String? localEngineSrcPath = args['local-engine-src-path'] as String?;
......@@ -85,8 +90,8 @@ Future<void> main(List<String> rawArgs) async {
stderr.writeln(argParser.usage);
exit(1);
}
if (localEngine == null) {
stderr.writeln('When running in A/B test mode --local-engine is required.\n');
if (localEngine == null && localWebSdk == null) {
stderr.writeln('When running in A/B test mode --local-engine or --local-web-sdk is required.\n');
stderr.writeln(argParser.usage);
exit(1);
}
......@@ -94,6 +99,7 @@ Future<void> main(List<String> rawArgs) async {
runsPerTest: runsPerTest,
silent: silent,
localEngine: localEngine,
localWebSdk: localWebSdk,
localEngineSrcPath: localEngineSrcPath,
deviceId: deviceId,
resultsFile: resultsFile,
......@@ -118,7 +124,8 @@ Future<void> main(List<String> rawArgs) async {
Future<void> _runABTest({
required int runsPerTest,
required bool silent,
required String localEngine,
required String? localEngine,
required String? localWebSdk,
required String? localEngineSrcPath,
required String? deviceId,
required String resultsFile,
......@@ -126,7 +133,9 @@ Future<void> _runABTest({
}) async {
print('$taskName A/B test. Will run $runsPerTest times.');
final ABTest abTest = ABTest(localEngine, taskName);
assert(localEngine != null || localWebSdk != null);
final ABTest abTest = ABTest((localEngine ?? localWebSdk)!, taskName);
for (int i = 1; i <= runsPerTest; i++) {
section('Run #$i');
......@@ -152,6 +161,7 @@ Future<void> _runABTest({
taskName,
silent: silent,
localEngine: localEngine,
localWebSdk: localWebSdk,
localEngineSrcPath: localEngineSrcPath,
deviceId: deviceId,
);
......@@ -282,6 +292,14 @@ ArgParser createArgParser(List<String> taskNames) {
'This path is relative to --local-engine-src-path/out. This option\n'
'is required when running an A/B test (see the --ab option).',
)
..addOption(
'local-web-sdk',
help: 'Name of a build output within the engine out directory, if you\n'
'are building Flutter locally. Use this to select a specific\n'
'version of the engine if you have built multiple engine targets.\n'
'This path is relative to --local-engine-src-path/out. This option\n'
'is required when running an A/B test (see the --ab option).',
)
..addFlag(
'list',
abbr: 'l',
......
......@@ -153,6 +153,7 @@ Future<TaskResult> runTask(
bool terminateStrayDartProcesses = false,
bool silent = false,
String? localEngine,
String? localWebSdk,
String? localEngineSrcPath,
String? deviceId,
List<String>? taskArgs,
......@@ -181,6 +182,7 @@ Future<TaskResult> runTask(
'--enable-vm-service=0', // zero causes the system to choose a free port
'--no-pause-isolates-on-exit',
if (localEngine != null) '-DlocalEngine=$localEngine',
if (localWebSdk != null) '-DlocalWebSdk=$localWebSdk',
if (localEngineSrcPath != null) '-DlocalEngineSrcPath=$localEngineSrcPath',
taskExecutable,
...?taskArgs,
......
......@@ -35,6 +35,14 @@ String? get localEngineSrcPathFromEnv {
return isDefined ? const String.fromEnvironment('localEngineSrcPath') : null;
}
/// The local Web SDK to use for [flutter] and [evalFlutter], if any.
///
/// This is set as an environment variable when running the task, see runTask in runner.dart.
String? get localWebSdkFromEnv {
const bool isDefined = bool.hasEnvironment('localWebSdk');
return isDefined ? const String.fromEnvironment('localWebSdk') : null;
}
List<ProcessInfo> _runningProcesses = <ProcessInfo>[];
ProcessManager _processManager = const LocalProcessManager();
......@@ -446,6 +454,7 @@ List<String> _flutterCommandArgs(String command, List<String> options) {
};
final String? localEngine = localEngineFromEnv;
final String? localEngineSrcPath = localEngineSrcPathFromEnv;
final String? localWebSdk = localWebSdkFromEnv;
return <String>[
command,
if (deviceOperatingSystem == DeviceOperatingSystem.ios && supportedDeviceTimeoutCommands.contains(command))
......@@ -460,6 +469,7 @@ List<String> _flutterCommandArgs(String command, List<String> options) {
],
if (localEngine != null) ...<String>['--local-engine', localEngine],
if (localEngineSrcPath != null) ...<String>['--local-engine-src-path', localEngineSrcPath],
if (localWebSdk != null) ...<String>['--local-web-sdk', localWebSdk],
...options,
];
}
......
......@@ -1192,6 +1192,7 @@ class WebCompileTest {
return inDirectory<Map<String, int>>(directory, () async {
final Map<String, int> metrics = <String, int>{};
await flutter('clean');
await flutter('packages', options: <String>['get']);
final Stopwatch? watch = measureBuildTime ? Stopwatch() : null;
watch?.start();
......@@ -1200,6 +1201,7 @@ class WebCompileTest {
'-v',
'--release',
'--no-pub',
'--no-web-resources-cdn',
]);
watch?.stop();
final String buildDir = path.join(directory, 'build', 'web');
......
......@@ -25,11 +25,13 @@ Future<TaskResult> runWebBenchmark({ required bool useCanvasKit }) async {
Logger.root.level = Level.INFO;
final String macrobenchmarksDirectory = path.join(flutterDirectory.path, 'dev', 'benchmarks', 'macrobenchmarks');
return inDirectory(macrobenchmarksDirectory, () async {
await flutter('clean');
await evalFlutter('build', options: <String>[
'web',
'--dart-define=FLUTTER_WEB_ENABLE_PROFILING=true',
'--web-renderer=${useCanvasKit ? 'canvaskit' : 'html'}',
'--profile',
'--no-web-resources-cdn',
'-t',
'lib/web_benchmarks.dart',
]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment