Unverified Commit a7997f60 authored by Matan Lurey's avatar Matan Lurey Committed by GitHub

Update `dev/devicelab/**` to provide `--local-engine-host`. (#132342)

Partial work towards https://github.com/flutter/flutter/issues/132245.

I have to admit I don't totally understand what I've updated, or whether
there are more integration points needed.
parent 978461c5
...@@ -91,10 +91,12 @@ flags to `bin/run.dart`: ...@@ -91,10 +91,12 @@ flags to `bin/run.dart`:
```sh ```sh
../../bin/cache/dart-sdk/bin/dart bin/run.dart --task=[some_task] \ ../../bin/cache/dart-sdk/bin/dart bin/run.dart --task=[some_task] \
--local-engine-src-path=[path_to_local]/engine/src \ --local-engine-src-path=[path_to_local]/engine/src \
--local-engine=[local_engine_architecture] --local-engine=[local_engine_architecture] \
--local-engine-host=[local_engine_host_architecture]
``` ```
An example of a local engine architecture is `android_debug_unopt_x86`. An example of a local engine architecture is `android_debug_unopt_x86` and
an example of a local engine host architecture is `host_debug_unopt`.
### Running an A/B test for engine changes ### Running an A/B test for engine changes
...@@ -111,13 +113,16 @@ Example: ...@@ -111,13 +113,16 @@ Example:
```sh ```sh
../../bin/cache/dart-sdk/bin/dart bin/run.dart --ab=10 \ ../../bin/cache/dart-sdk/bin/dart bin/run.dart --ab=10 \
--local-engine=host_debug_unopt \ --local-engine=host_debug_unopt \
--local-engine-host=host_debug_unopt \
-t bin/tasks/web_benchmarks_canvaskit.dart -t bin/tasks/web_benchmarks_canvaskit.dart
``` ```
The `--ab=10` tells the runner to run an A/B test 10 times. The `--ab=10` tells the runner to run an A/B test 10 times.
`--local-engine=host_debug_unopt` tells the A/B test to use the `--local-engine=host_debug_unopt` tells the A/B test to use the
`host_debug_unopt` engine build. `--local-engine` is required for A/B test. `host_debug_unopt` engine build. `--local-engine-host=host_debug_unopt` uses
the same engine build to run the `frontend_server` (in this example).
`--local-engine` is required for A/B test.
`--ab-result-file=filename` can be used to provide an alternate location to `--ab-result-file=filename` can be used to provide an alternate location to
output the JSON results file (defaults to `ABresults#.json`). A single `#` output the JSON results file (defaults to `ABresults#.json`). A single `#`
......
...@@ -38,6 +38,11 @@ Future<void> main(List<String> rawArgs) async { ...@@ -38,6 +38,11 @@ Future<void> main(List<String> rawArgs) async {
/// Required for A/B test mode. /// Required for A/B test mode.
final String? localEngine = args['local-engine'] as String?; final String? localEngine = args['local-engine'] as String?;
/// The build of the local engine to use as the host platform.
///
/// Required if [localEngine] is set.
final String? localEngineHost = args['local-engine-host'] as String?;
/// The build of the local Web SDK to use. /// The build of the local Web SDK to use.
/// ///
/// Required for A/B test mode. /// Required for A/B test mode.
...@@ -95,10 +100,16 @@ Future<void> main(List<String> rawArgs) async { ...@@ -95,10 +100,16 @@ Future<void> main(List<String> rawArgs) async {
stderr.writeln(argParser.usage); stderr.writeln(argParser.usage);
exit(1); exit(1);
} }
if (localEngineHost == null) {
stderr.writeln('When running in A/B test mode --local-engine-host is required.\n');
stderr.writeln(argParser.usage);
exit(1);
}
await _runABTest( await _runABTest(
runsPerTest: runsPerTest, runsPerTest: runsPerTest,
silent: silent, silent: silent,
localEngine: localEngine, localEngine: localEngine,
localEngineHost: localEngineHost,
localWebSdk: localWebSdk, localWebSdk: localWebSdk,
localEngineSrcPath: localEngineSrcPath, localEngineSrcPath: localEngineSrcPath,
deviceId: deviceId, deviceId: deviceId,
...@@ -125,6 +136,7 @@ Future<void> _runABTest({ ...@@ -125,6 +136,7 @@ Future<void> _runABTest({
required int runsPerTest, required int runsPerTest,
required bool silent, required bool silent,
required String? localEngine, required String? localEngine,
required String localEngineHost,
required String? localWebSdk, required String? localWebSdk,
required String? localEngineSrcPath, required String? localEngineSrcPath,
required String? deviceId, required String? deviceId,
...@@ -135,7 +147,11 @@ Future<void> _runABTest({ ...@@ -135,7 +147,11 @@ Future<void> _runABTest({
assert(localEngine != null || localWebSdk != null); assert(localEngine != null || localWebSdk != null);
final ABTest abTest = ABTest((localEngine ?? localWebSdk)!, taskName); final ABTest abTest = ABTest(
localEngine: (localEngine ?? localWebSdk)!,
localEngineHost: localEngineHost,
taskName: taskName,
);
for (int i = 1; i <= runsPerTest; i++) { for (int i = 1; i <= runsPerTest; i++) {
section('Run #$i'); section('Run #$i');
...@@ -292,6 +308,15 @@ ArgParser createArgParser(List<String> taskNames) { ...@@ -292,6 +308,15 @@ ArgParser createArgParser(List<String> taskNames) {
'This path is relative to --local-engine-src-path/out. This option\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).', 'is required when running an A/B test (see the --ab option).',
) )
..addOption(
'local-engine-host',
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 to use as the host platform 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).',
)
..addOption( ..addOption(
'local-web-sdk', 'local-web-sdk',
help: 'Name of a build output within the engine out directory, if you\n' help: 'Name of a build output within the engine out directory, if you\n'
......
...@@ -37,6 +37,15 @@ class TestCommand extends Command<void> { ...@@ -37,6 +37,15 @@ class TestCommand extends Command<void> {
'This path is relative to --local-engine-src-path/out. This option\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).', 'is required when running an A/B test (see the --ab option).',
); );
argParser.addOption(
'local-engine-host',
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 to use as the host platform 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).',
);
argParser.addOption( argParser.addOption(
'local-engine-src-path', 'local-engine-src-path',
help: 'Path to your engine src directory, if you are building Flutter\n' help: 'Path to your engine src directory, if you are building Flutter\n'
...@@ -74,6 +83,7 @@ class TestCommand extends Command<void> { ...@@ -74,6 +83,7 @@ class TestCommand extends Command<void> {
deviceId: argResults!['device-id'] as String?, deviceId: argResults!['device-id'] as String?,
gitBranch: argResults!['git-branch'] as String?, gitBranch: argResults!['git-branch'] as String?,
localEngine: argResults!['local-engine'] as String?, localEngine: argResults!['local-engine'] as String?,
localEngineHost: argResults!['local-engine-host'] as String?,
localEngineSrcPath: argResults!['local-engine-src-path'] as String?, localEngineSrcPath: argResults!['local-engine-src-path'] as String?,
luciBuilder: argResults!['luci-builder'] as String?, luciBuilder: argResults!['luci-builder'] as String?,
resultsPath: argResults!['results-file'] as String?, resultsPath: argResults!['results-file'] as String?,
......
...@@ -9,6 +9,7 @@ import 'task_result.dart'; ...@@ -9,6 +9,7 @@ import 'task_result.dart';
const String kBenchmarkTypeKeyName = 'benchmark_type'; const String kBenchmarkTypeKeyName = 'benchmark_type';
const String kBenchmarkVersionKeyName = 'version'; const String kBenchmarkVersionKeyName = 'version';
const String kLocalEngineKeyName = 'local_engine'; const String kLocalEngineKeyName = 'local_engine';
const String kLocalEngineHostKeyName = 'local_engine_host';
const String kTaskNameKeyName = 'task_name'; const String kTaskNameKeyName = 'task_name';
const String kRunStartKeyName = 'run_start'; const String kRunStartKeyName = 'run_start';
const String kRunEndKeyName = 'run_end'; const String kRunEndKeyName = 'run_end';
...@@ -24,13 +25,14 @@ enum FieldJustification { LEFT, RIGHT, CENTER } ...@@ -24,13 +25,14 @@ enum FieldJustification { LEFT, RIGHT, CENTER }
/// ///
/// See [printSummary] for more. /// See [printSummary] for more.
class ABTest { class ABTest {
ABTest(this.localEngine, this.taskName) ABTest({required this.localEngine, required this.localEngineHost, required this.taskName})
: runStart = DateTime.now(), : runStart = DateTime.now(),
_aResults = <String, List<double>>{}, _aResults = <String, List<double>>{},
_bResults = <String, List<double>>{}; _bResults = <String, List<double>>{};
ABTest.fromJsonMap(Map<String, dynamic> jsonResults) ABTest.fromJsonMap(Map<String, dynamic> jsonResults)
: localEngine = jsonResults[kLocalEngineKeyName] as String, : localEngine = jsonResults[kLocalEngineKeyName] as String,
localEngineHost = jsonResults[kLocalEngineHostKeyName] as String,
taskName = jsonResults[kTaskNameKeyName] as String, taskName = jsonResults[kTaskNameKeyName] as String,
runStart = DateTime.parse(jsonResults[kRunStartKeyName] as String), runStart = DateTime.parse(jsonResults[kRunStartKeyName] as String),
_runEnd = DateTime.parse(jsonResults[kRunEndKeyName] as String), _runEnd = DateTime.parse(jsonResults[kRunEndKeyName] as String),
...@@ -38,6 +40,7 @@ class ABTest { ...@@ -38,6 +40,7 @@ class ABTest {
_bResults = _convertFrom(jsonResults[kBResultsKeyName] as Map<String, dynamic>); _bResults = _convertFrom(jsonResults[kBResultsKeyName] as Map<String, dynamic>);
final String localEngine; final String localEngine;
final String localEngineHost;
final String taskName; final String taskName;
final DateTime runStart; final DateTime runStart;
DateTime? _runEnd; DateTime? _runEnd;
...@@ -86,6 +89,7 @@ class ABTest { ...@@ -86,6 +89,7 @@ class ABTest {
kBenchmarkTypeKeyName: kBenchmarkResultsType, kBenchmarkTypeKeyName: kBenchmarkResultsType,
kBenchmarkVersionKeyName: kBenchmarkABVersion, kBenchmarkVersionKeyName: kBenchmarkABVersion,
kLocalEngineKeyName: localEngine, kLocalEngineKeyName: localEngine,
kLocalEngineHostKeyName: localEngineHost,
kTaskNameKeyName: taskName, kTaskNameKeyName: taskName,
kRunStartKeyName: runStart.toIso8601String(), kRunStartKeyName: runStart.toIso8601String(),
kRunEndKeyName: runEnd!.toIso8601String(), kRunEndKeyName: runEnd!.toIso8601String(),
......
...@@ -74,11 +74,13 @@ class _TaskRunner { ...@@ -74,11 +74,13 @@ class _TaskRunner {
final bool runFlutterConfig = parameters['runFlutterConfig'] != 'false'; // used by tests to avoid changing the configuration final bool runFlutterConfig = parameters['runFlutterConfig'] != 'false'; // used by tests to avoid changing the configuration
final bool runProcessCleanup = parameters['runProcessCleanup'] != 'false'; final bool runProcessCleanup = parameters['runProcessCleanup'] != 'false';
final String? localEngine = parameters['localEngine']; final String? localEngine = parameters['localEngine'];
final String? localEngineHost = parameters['localEngineHost'];
final TaskResult result = await run( final TaskResult result = await run(
taskTimeout, taskTimeout,
runProcessCleanup: runProcessCleanup, runProcessCleanup: runProcessCleanup,
runFlutterConfig: runFlutterConfig, runFlutterConfig: runFlutterConfig,
localEngine: localEngine, localEngine: localEngine,
localEngineHost: localEngineHost,
); );
return ServiceExtensionResponse.result(json.encode(result.toJson())); return ServiceExtensionResponse.result(json.encode(result.toJson()));
}); });
...@@ -115,6 +117,7 @@ class _TaskRunner { ...@@ -115,6 +117,7 @@ class _TaskRunner {
bool runFlutterConfig = true, bool runFlutterConfig = true,
bool runProcessCleanup = true, bool runProcessCleanup = true,
required String? localEngine, required String? localEngine,
required String? localEngineHost,
}) async { }) async {
try { try {
_taskStarted = true; _taskStarted = true;
...@@ -144,6 +147,7 @@ class _TaskRunner { ...@@ -144,6 +147,7 @@ class _TaskRunner {
'--enable-macos-desktop', '--enable-macos-desktop',
'--enable-linux-desktop', '--enable-linux-desktop',
if (localEngine != null) ...<String>['--local-engine', localEngine], if (localEngine != null) ...<String>['--local-engine', localEngine],
if (localEngineHost != null) ...<String>['--local-engine-host', localEngineHost],
], canFail: true); ], canFail: true);
if (configResult != 0) { if (configResult != 0) {
print('Failed to enable configuration, tasks may not run.'); print('Failed to enable configuration, tasks may not run.');
......
...@@ -35,6 +35,7 @@ Future<void> runTasks( ...@@ -35,6 +35,7 @@ Future<void> runTasks(
String? deviceId, String? deviceId,
String? gitBranch, String? gitBranch,
String? localEngine, String? localEngine,
String? localEngineHost,
String? localEngineSrcPath, String? localEngineSrcPath,
String? luciBuilder, String? luciBuilder,
String? resultsPath, String? resultsPath,
...@@ -52,6 +53,7 @@ Future<void> runTasks( ...@@ -52,6 +53,7 @@ Future<void> runTasks(
taskName, taskName,
deviceId: deviceId, deviceId: deviceId,
localEngine: localEngine, localEngine: localEngine,
localEngineHost: localEngineHost,
localEngineSrcPath: localEngineSrcPath, localEngineSrcPath: localEngineSrcPath,
terminateStrayDartProcesses: terminateStrayDartProcesses, terminateStrayDartProcesses: terminateStrayDartProcesses,
silent: silent, silent: silent,
...@@ -99,6 +101,7 @@ Future<TaskResult> rerunTask( ...@@ -99,6 +101,7 @@ Future<TaskResult> rerunTask(
String taskName, { String taskName, {
String? deviceId, String? deviceId,
String? localEngine, String? localEngine,
String? localEngineHost,
String? localEngineSrcPath, String? localEngineSrcPath,
bool terminateStrayDartProcesses = false, bool terminateStrayDartProcesses = false,
bool silent = false, bool silent = false,
...@@ -114,6 +117,7 @@ Future<TaskResult> rerunTask( ...@@ -114,6 +117,7 @@ Future<TaskResult> rerunTask(
taskName, taskName,
deviceId: deviceId, deviceId: deviceId,
localEngine: localEngine, localEngine: localEngine,
localEngineHost: localEngineHost,
localEngineSrcPath: localEngineSrcPath, localEngineSrcPath: localEngineSrcPath,
terminateStrayDartProcesses: terminateStrayDartProcesses, terminateStrayDartProcesses: terminateStrayDartProcesses,
silent: silent, silent: silent,
...@@ -153,6 +157,7 @@ Future<TaskResult> runTask( ...@@ -153,6 +157,7 @@ Future<TaskResult> runTask(
bool terminateStrayDartProcesses = false, bool terminateStrayDartProcesses = false,
bool silent = false, bool silent = false,
String? localEngine, String? localEngine,
String? localEngineHost,
String? localWebSdk, String? localWebSdk,
String? localEngineSrcPath, String? localEngineSrcPath,
String? deviceId, String? deviceId,
...@@ -182,6 +187,7 @@ Future<TaskResult> runTask( ...@@ -182,6 +187,7 @@ Future<TaskResult> runTask(
'--enable-vm-service=0', // zero causes the system to choose a free port '--enable-vm-service=0', // zero causes the system to choose a free port
'--no-pause-isolates-on-exit', '--no-pause-isolates-on-exit',
if (localEngine != null) '-DlocalEngine=$localEngine', if (localEngine != null) '-DlocalEngine=$localEngine',
if (localEngineHost != null) '-DlocalEngineHost=$localEngineHost',
if (localWebSdk != null) '-DlocalWebSdk=$localWebSdk', if (localWebSdk != null) '-DlocalWebSdk=$localWebSdk',
if (localEngineSrcPath != null) '-DlocalEngineSrcPath=$localEngineSrcPath', if (localEngineSrcPath != null) '-DlocalEngineSrcPath=$localEngineSrcPath',
taskExecutable, taskExecutable,
......
...@@ -26,6 +26,14 @@ String? get localEngineFromEnv { ...@@ -26,6 +26,14 @@ String? get localEngineFromEnv {
return isDefined ? const String.fromEnvironment('localEngine') : null; return isDefined ? const String.fromEnvironment('localEngine') : null;
} }
/// The local engine host 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 localEngineHostFromEnv {
const bool isDefined = bool.hasEnvironment('localEngineHost');
return isDefined ? const String.fromEnvironment('localEngineHost') : null;
}
/// The local engine source path to use if a local engine is used for [flutter] /// The local engine source path to use if a local engine is used for [flutter]
/// and [evalFlutter]. /// and [evalFlutter].
/// ///
...@@ -453,6 +461,7 @@ List<String> _flutterCommandArgs(String command, List<String> options) { ...@@ -453,6 +461,7 @@ List<String> _flutterCommandArgs(String command, List<String> options) {
'screenshot', 'screenshot',
}; };
final String? localEngine = localEngineFromEnv; final String? localEngine = localEngineFromEnv;
final String? localEngineHost = localEngineHostFromEnv;
final String? localEngineSrcPath = localEngineSrcPathFromEnv; final String? localEngineSrcPath = localEngineSrcPathFromEnv;
final String? localWebSdk = localWebSdkFromEnv; final String? localWebSdk = localWebSdkFromEnv;
return <String>[ return <String>[
...@@ -468,6 +477,7 @@ List<String> _flutterCommandArgs(String command, List<String> options) { ...@@ -468,6 +477,7 @@ List<String> _flutterCommandArgs(String command, List<String> options) {
hostAgent.dumpDirectory!.path, hostAgent.dumpDirectory!.path,
], ],
if (localEngine != null) ...<String>['--local-engine', localEngine], if (localEngine != null) ...<String>['--local-engine', localEngine],
if (localEngineHost != null) ...<String>['--local-engine-host', localEngineHost],
if (localEngineSrcPath != null) ...<String>['--local-engine-src-path', localEngineSrcPath], if (localEngineSrcPath != null) ...<String>['--local-engine-src-path', localEngineSrcPath],
if (localWebSdk != null) ...<String>['--local-web-sdk', localWebSdk], if (localWebSdk != null) ...<String>['--local-web-sdk', localWebSdk],
...options, ...options,
......
...@@ -1169,6 +1169,7 @@ class PerfTest { ...@@ -1169,6 +1169,7 @@ class PerfTest {
await selectedDevice.unlock(); await selectedDevice.unlock();
final String deviceId = selectedDevice.deviceId; final String deviceId = selectedDevice.deviceId;
final String? localEngine = localEngineFromEnv; final String? localEngine = localEngineFromEnv;
final String? localEngineHost = localEngineHostFromEnv;
final String? localEngineSrcPath = localEngineSrcPathFromEnv; final String? localEngineSrcPath = localEngineSrcPathFromEnv;
Future<void> Function()? manifestReset; Future<void> Function()? manifestReset;
...@@ -1181,6 +1182,10 @@ class PerfTest { ...@@ -1181,6 +1182,10 @@ class PerfTest {
try { try {
final List<String> options = <String>[ final List<String> options = <String>[
if (localEngine != null) ...<String>['--local-engine', localEngine], if (localEngine != null) ...<String>['--local-engine', localEngine],
if (localEngineHost != null) ...<String>[
'--local-engine-host',
localEngineHost
],
if (localEngineSrcPath != null) ...<String>[ if (localEngineSrcPath != null) ...<String>[
'--local-engine-src-path', '--local-engine-src-path',
localEngineSrcPath localEngineSrcPath
......
...@@ -9,7 +9,7 @@ import 'common.dart'; ...@@ -9,7 +9,7 @@ import 'common.dart';
void main() { void main() {
test('ABTest', () { test('ABTest', () {
final ABTest ab = ABTest('engine', 'test'); final ABTest ab = ABTest(localEngine: 'engine', localEngineHost: 'engine', taskName: 'test');
for (int i = 0; i < 5; i++) { for (int i = 0; i < 5; i++) {
final TaskResult aResult = TaskResult.fromJson(<String, dynamic>{ final TaskResult aResult = TaskResult.fromJson(<String, dynamic>{
......
...@@ -96,7 +96,7 @@ void main() { ...@@ -96,7 +96,7 @@ void main() {
final ProcessResult result = await runScript( final ProcessResult result = await runScript(
<String>['smoke_test_success'], <String>['smoke_test_success'],
<String>['--ab=2', '--local-engine=host_debug_unopt', '--ab-result-file', abResultsFile.path], <String>['--ab=2', '--local-engine=host_debug_unopt', '--local-engine-host=host_debug_unopt', '--ab-result-file', abResultsFile.path],
); );
expect(result.exitCode, 0); expect(result.exitCode, 0);
......
...@@ -37,6 +37,7 @@ void main() { ...@@ -37,6 +37,7 @@ void main() {
group('engine environment declarations', () { group('engine environment declarations', () {
test('localEngine', () { test('localEngine', () {
expect(localEngineFromEnv, null); expect(localEngineFromEnv, null);
expect(localEngineHostFromEnv, null);
expect(localEngineSrcPathFromEnv, null); expect(localEngineSrcPathFromEnv, null);
}); });
}); });
......
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