From 449079d4f16f1e85ed1b015e08e6d6072c965d75 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 13 Jan 2025 10:48:58 -0800 Subject: [PATCH] Propagate environment variables when `flutter drive` is invoked. (#161452) Closes https://github.com/flutter/flutter/issues/161449. ~3 LOC, with 203 lines of tests (including an e2e integration test that it actually works). Feedback welcome! (The reason I'm working on this is the ability to pass environment variables makes it much easier and less hacky to make `android_engine_test` configurable, i.e. have different expected outputs for OpenGLES/Vulkan, compare screenshots locally for deflaking, etc). --- .../flutter_tools/lib/src/commands/drive.dart | 4 +- .../lib/src/drive/drive_service.dart | 13 +++- .../lib/src/drive/web_driver_service.dart | 7 +- .../commands.shard/hermetic/drive_test.dart | 2 - .../drive/drive_service_test.dart | 26 ++++--- .../drive/web_driver_service_test.dart | 2 + .../driver_environment_test.dart | 67 +++++++++++++++++++ .../web.shard/web_driver_service_test.dart | 3 +- 8 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 packages/flutter_tools/test/integration.shard/driver_environment_test.dart diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 8b969cad908..52fcd654aa0 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -65,6 +65,7 @@ class DriveCommand extends RunCommandBase { }) : _flutterDriverFactory = flutterDriverFactory, _fileSystem = fileSystem, _logger = logger, + _platform = platform, _fsUtils = FileSystemUtils(fileSystem: fileSystem, platform: platform), super(verboseHelp: verboseHelp) { requiresPubspecYaml(); @@ -205,6 +206,7 @@ class DriveCommand extends RunCommandBase { FlutterDriverFactory? _flutterDriverFactory; final FileSystem _fileSystem; final Logger _logger; + final Platform _platform; final FileSystemUtils _fsUtils; Timer? timeoutTimer; Map? screenshotTokens; @@ -292,6 +294,7 @@ class DriveCommand extends RunCommandBase { _flutterDriverFactory ??= FlutterDriverFactory( applicationPackageFactory: ApplicationPackageFactory.instance!, logger: _logger, + platform: _platform, processUtils: globals.processUtils, dartSdkPath: globals.artifacts!.getArtifactPath(Artifact.engineDartBinary), devtoolsLauncher: DevtoolsLauncher.instance!, @@ -336,7 +339,6 @@ class DriveCommand extends RunCommandBase { final Future testResultFuture = driverService.startTest( testFile, stringsArg('test-arguments'), - {}, packageConfig, chromeBinary: stringArg('chrome-binary'), headless: boolArg('headless'), diff --git a/packages/flutter_tools/lib/src/drive/drive_service.dart b/packages/flutter_tools/lib/src/drive/drive_service.dart index af44bc5e658..fce9f86fbae 100644 --- a/packages/flutter_tools/lib/src/drive/drive_service.dart +++ b/packages/flutter_tools/lib/src/drive/drive_service.dart @@ -13,6 +13,7 @@ import '../application_package.dart'; import '../base/common.dart'; import '../base/dds.dart'; import '../base/logger.dart'; +import '../base/platform.dart'; import '../base/process.dart'; import '../build_info.dart'; import '../device.dart'; @@ -24,17 +25,20 @@ import 'web_driver_service.dart'; class FlutterDriverFactory { FlutterDriverFactory({ required ApplicationPackageFactory applicationPackageFactory, + required Platform platform, required Logger logger, required ProcessUtils processUtils, required String dartSdkPath, required DevtoolsLauncher devtoolsLauncher, }) : _applicationPackageFactory = applicationPackageFactory, + _platform = platform, _logger = logger, _processUtils = processUtils, _dartSdkPath = dartSdkPath, _devtoolsLauncher = devtoolsLauncher; final ApplicationPackageFactory _applicationPackageFactory; + final Platform _platform; final Logger _logger; final ProcessUtils _processUtils; final String _dartSdkPath; @@ -45,12 +49,14 @@ class FlutterDriverFactory { if (web) { return WebDriverService( logger: _logger, + platform: _platform, processUtils: _processUtils, dartSdkPath: _dartSdkPath, ); } return FlutterDriverService( logger: _logger, + platform: _platform, processUtils: _processUtils, dartSdkPath: _dartSdkPath, applicationPackageFactory: _applicationPackageFactory, @@ -84,7 +90,6 @@ abstract class DriverService { Future startTest( String testFile, List arguments, - Map environment, PackageConfig packageConfig, { bool? headless, String? chromeBinary, @@ -110,12 +115,14 @@ class FlutterDriverService extends DriverService { FlutterDriverService({ required ApplicationPackageFactory applicationPackageFactory, required Logger logger, + required Platform platform, required ProcessUtils processUtils, required String dartSdkPath, required DevtoolsLauncher devtoolsLauncher, @visibleForTesting VMServiceConnector vmServiceConnector = connectToVmService, }) : _applicationPackageFactory = applicationPackageFactory, _logger = logger, + _platform = platform, _processUtils = processUtils, _dartSdkPath = dartSdkPath, _vmServiceConnector = vmServiceConnector, @@ -125,6 +132,7 @@ class FlutterDriverService extends DriverService { final ApplicationPackageFactory _applicationPackageFactory; final Logger _logger; + final Platform _platform; final ProcessUtils _processUtils; final String _dartSdkPath; final VMServiceConnector _vmServiceConnector; @@ -227,7 +235,6 @@ class FlutterDriverService extends DriverService { Future startTest( String testFile, List arguments, - Map environment, PackageConfig packageConfig, { bool? headless, String? chromeBinary, @@ -251,7 +258,7 @@ class FlutterDriverService extends DriverService { try { final int result = await _processUtils.stream( [_dartSdkPath, ...arguments, testFile], - environment: {'VM_SERVICE_URL': _vmServiceUri, ...environment}, + environment: {..._platform.environment, 'VM_SERVICE_URL': _vmServiceUri}, ); return result; } finally { diff --git a/packages/flutter_tools/lib/src/drive/web_driver_service.dart b/packages/flutter_tools/lib/src/drive/web_driver_service.dart index a3f8b5a1e5d..2ae8fd40380 100644 --- a/packages/flutter_tools/lib/src/drive/web_driver_service.dart +++ b/packages/flutter_tools/lib/src/drive/web_driver_service.dart @@ -13,6 +13,7 @@ import 'package:webdriver/async_io.dart' as async_io; import '../base/common.dart'; import '../base/io.dart'; import '../base/logger.dart'; +import '../base/platform.dart'; import '../base/process.dart'; import '../base/utils.dart'; import '../build_info.dart'; @@ -29,13 +30,16 @@ class WebDriverService extends DriverService { WebDriverService({ required ProcessUtils processUtils, required String dartSdkPath, + required Platform platform, required Logger logger, }) : _processUtils = processUtils, _dartSdkPath = dartSdkPath, + _platform = platform, _logger = logger; final ProcessUtils _processUtils; final String _dartSdkPath; + final Platform _platform; final Logger _logger; late ResidentRunner _residentRunner; @@ -143,7 +147,6 @@ class WebDriverService extends DriverService { Future startTest( String testFile, List arguments, - Map environment, PackageConfig packageConfig, { bool? headless, String? chromeBinary, @@ -195,9 +198,9 @@ class WebDriverService extends DriverService { final int result = await _processUtils.stream( [_dartSdkPath, ...arguments, testFile], environment: { + ..._platform.environment, 'VM_SERVICE_URL': _webUri.toString(), ..._additionalDriverEnvironment(webDriver, browserName, androidEmulator), - ...environment, }, ); await webDriver.quit(); diff --git a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart index 95dd5a7a2ef..b5dd43c6ad4 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart @@ -702,7 +702,6 @@ class NeverEndingDriverService extends Fake implements DriverService { Future startTest( String testFile, List arguments, - Map environment, PackageConfig packageConfig, { bool? headless, String? chromeBinary, @@ -736,7 +735,6 @@ class FailingFakeDriverService extends Fake implements DriverService { Future startTest( String testFile, List arguments, - Map environment, PackageConfig packageConfig, { bool? headless, String? chromeBinary, diff --git a/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart b/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart index 354f7842111..c2e5cb5e07f 100644 --- a/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart +++ b/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart @@ -10,6 +10,7 @@ import 'package:flutter_tools/src/application_package.dart'; import 'package:flutter_tools/src/base/dds.dart'; import 'package:flutter_tools/src/base/io.dart' as io; import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/process.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/convert.dart'; @@ -139,6 +140,7 @@ void main() { final DriverService driverService = setUpDriverService( processManager: processManager, vmService: fakeVmServiceHost.vmService, + platform: FakePlatform(environment: {'FOO': 'BAR'}), ); final Device device = FakeDevice( LaunchResult.succeeded(vmServiceUri: Uri.parse('http://127.0.0.1:63426/1UasC_ihpXY=/')), @@ -149,12 +151,9 @@ void main() { device, DebuggingOptions.enabled(BuildInfo.profile, ipv6: true), ); - final int testResult = await driverService.startTest( - 'foo.test', - ['--enable-experiment=non-nullable'], - {'FOO': 'BAR'}, - PackageConfig([Package('test', Uri.base)]), - ); + final int testResult = await driverService.startTest('foo.test', [ + '--enable-experiment=non-nullable', + ], PackageConfig([Package('test', Uri.base)])); expect(testResult, 23); }); @@ -180,6 +179,7 @@ void main() { processManager: processManager, vmService: fakeVmServiceHost.vmService, devtoolsLauncher: launcher, + platform: FakePlatform(environment: {'FOO': 'BAR'}), ); final Device device = FakeDevice( LaunchResult.succeeded(vmServiceUri: Uri.parse('http://127.0.0.1:63426/1UasC_ihpXY=/')), @@ -193,7 +193,6 @@ void main() { final int testResult = await driverService.startTest( 'foo.test', ['--enable-experiment=non-nullable'], - {'FOO': 'BAR'}, PackageConfig([Package('test', Uri.base)]), profileMemory: 'devtools_memory.json', ); @@ -222,6 +221,7 @@ void main() { final DriverService driverService = setUpDriverService( processManager: processManager, vmService: fakeVmServiceHost.vmService, + platform: FakePlatform(environment: {'FOO': 'BAR'}), ); final Device device = FakeDevice( LaunchResult.succeeded(vmServiceUri: Uri.parse('http://127.0.0.1:63426/1UasC_ihpXY=/')), @@ -232,12 +232,9 @@ void main() { device, DebuggingOptions.enabled(BuildInfo.profile, ipv6: true), ); - final int testResult = await driverService.startTest( - 'foo.test', - ['--enable-experiment=non-nullable'], - {'FOO': 'BAR'}, - PackageConfig.empty, - ); + final int testResult = await driverService.startTest('foo.test', [ + '--enable-experiment=non-nullable', + ], PackageConfig.empty); expect(testResult, 23); }, @@ -276,7 +273,6 @@ void main() { final int testResult = await driverService.startTest( 'foo.test', [], - {}, PackageConfig([Package('test', Uri.base)]), ); @@ -483,6 +479,7 @@ void main() { FlutterDriverService setUpDriverService({ Logger? logger, + Platform? platform, ProcessManager? processManager, FlutterVmService? vmService, DevtoolsLauncher? devtoolsLauncher, @@ -491,6 +488,7 @@ FlutterDriverService setUpDriverService({ return FlutterDriverService( applicationPackageFactory: FakeApplicationPackageFactory(FakeApplicationPackage()), logger: logger, + platform: platform ?? FakePlatform(), processUtils: ProcessUtils( logger: logger, processManager: processManager ?? FakeProcessManager.any(), diff --git a/packages/flutter_tools/test/general.shard/drive/web_driver_service_test.dart b/packages/flutter_tools/test/general.shard/drive/web_driver_service_test.dart index 467a4c54d2b..9a398c38c00 100644 --- a/packages/flutter_tools/test/general.shard/drive/web_driver_service_test.dart +++ b/packages/flutter_tools/test/general.shard/drive/web_driver_service_test.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:file/file.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/net.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/process.dart'; import 'package:flutter_tools/src/base/time.dart'; import 'package:flutter_tools/src/build_info.dart'; @@ -411,6 +412,7 @@ WebDriverService setUpDriverService() { logger: logger, processUtils: ProcessUtils(logger: logger, processManager: FakeProcessManager.any()), dartSdkPath: 'dart', + platform: FakePlatform(), ); } diff --git a/packages/flutter_tools/test/integration.shard/driver_environment_test.dart b/packages/flutter_tools/test/integration.shard/driver_environment_test.dart new file mode 100644 index 00000000000..81996cb0b67 --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/driver_environment_test.dart @@ -0,0 +1,67 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:flutter_tools/src/base/io.dart'; + +import '../src/common.dart'; +import 'test_data/project.dart'; +import 'test_utils.dart'; + +void main() { + late Directory tempDir; + + setUp(() async { + tempDir = fileSystem.systemTempDirectory.createTempSync('driver_environment_test.'); + }); + + tearDown(() async { + tryToDelete(tempDir); + }); + + testWithoutContext('environment variables are passed to the drive script', () async { + final Project project = _PrintEnvironmentVariablesInTestDriverProject(); + await project.setUpIn(tempDir); + + final ProcessResult result = await processManager.run( + [flutterBin, 'drive', '-d', 'flutter-tester'], + workingDirectory: tempDir.path, + environment: {'FOO': 'BAR'}, + ); + + printOnFailure('stdout: ${result.stdout}'); + printOnFailure('stderr: ${result.stderr}'); + expect(result.exitCode, 0); + + expect(result.stdout.toString(), contains('FOO=BAR')); + }); +} + +final class _PrintEnvironmentVariablesInTestDriverProject extends Project { + @override + final String pubspec = ''' + name: test + environment: + sdk: ^3.7.0-0 + + dependencies: + flutter: + sdk: flutter + '''; + + @override + final String main = r'void main() {}'; + + @override + Future setUpIn(Directory dir) async { + await super.setUpIn(dir); + writeFile(fileSystem.path.join(dir.path, 'test_driver', 'main_test.dart'), r''' +import 'dart:io' as io; + +void main() { + print('FOO=${io.Platform.environment['FOO']}'); +} +'''); + } +} diff --git a/packages/flutter_tools/test/web.shard/web_driver_service_test.dart b/packages/flutter_tools/test/web.shard/web_driver_service_test.dart index 8585ac65ef1..d5b7bd14f60 100644 --- a/packages/flutter_tools/test/web.shard/web_driver_service_test.dart +++ b/packages/flutter_tools/test/web.shard/web_driver_service_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/process.dart'; import 'package:flutter_tools/src/drive/web_driver_service.dart'; import 'package:package_config/package_config_types.dart'; @@ -20,13 +21,13 @@ void main() { logger: logger, processUtils: ProcessUtils(logger: logger, processManager: FakeProcessManager.empty()), dartSdkPath: 'dart', + platform: FakePlatform(), ); const String link = 'https://flutter.dev/to/integration-test-on-web'; try { await service.startTest( 'foo.test', [], - {}, PackageConfig([Package('test', Uri.base)]), driverPort: 1, headless: true,