Propagate environment variables when `flutter drive` is invoked. #161452

matanlurey · 2025-01-10T22:16:55Z

~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).

jmagman · 2025-01-10T22:23:45Z

packages/flutter_tools/lib/src/commands/drive.dart

        testFile,
        stringsArg('test-arguments'),
-        <String, String>{},
+        _platform.environment,


I don't think the caller should have to know to do this. I think the API would make more sense if:

startTest the environment variable is optional.

The startTest implementations inherit the environment if nothing is passed in, and add VM_SERVICE_URL and whatever else it needs.

DriverService (today) does not have a Platform. It could either change (require one in the constructor to create it) or use globals.platform. What do you prefer?

We try not to use globals where possible in the tool #47161, so inject the Platform

Done, see note.

matanlurey · 2025-01-11T00:31:08Z

@jmagman This is nearly completely redone - I like the approach you outlined a lot more.

One change: I ended up just removing the environment: ... parameter entirely, as it isn't actually used ever other than to provide the platform.environment, so it's just another case to handle that isn't instrumented in the actual CLI (i.e. outside of tests, which can be controlled by providing a FakePlatform(environment: {...}).

PTAL!

andrewkolos

LGTM

…ed. (flutter/flutter#161452)

Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 [email protected] Remove unused method (flutter/flutter#161572) 2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 [email protected] Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 [email protected] Use wildcards (flutter/flutter#161548) 2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...

…tter#161452) Closes flutter#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).

…ed. (flutter/flutter#161452)

Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 [email protected] Remove unused method (flutter/flutter#161572) 2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 [email protected] Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 [email protected] Use wildcards (flutter/flutter#161548) 2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...

Add integration test.

46daf57

matanlurey requested review from andrewkolos and jmagman January 10, 2025 22:16

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 10, 2025

jmagman reviewed Jan 10, 2025

View reviewed changes

matanlurey added 2 commits January 10, 2025 16:22

Merge branch 'main' into capture-env-driver-start

43f76c5

Instead, refactor DriverService to always provide the environment.

33ed86d

matanlurey requested a review from jmagman January 11, 2025 00:31

andrewkolos approved these changes Jan 13, 2025

View reviewed changes

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2025

auto-submit bot added this pull request to the merge queue Jan 13, 2025

matanlurey mentioned this pull request Jan 13, 2025

Refactor android_engine_test, make it easier to debug/deflake locally. #161534

Merged

Merged via the queue into flutter:master with commit 449079d Jan 13, 2025
164 of 165 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2025

engine-flutter-autoroll mentioned this pull request Jan 14, 2025

Roll Flutter from 72db8f69e339 to f2c15f9debb2 (27 revisions) flutter/packages#8424

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025

449079d Propagate environment variables when flutter drive is invok…

bd69a23

…ed. (flutter/flutter#161452)

engine-flutter-autoroll mentioned this pull request Jan 14, 2025

Roll Flutter from 72db8f69e339 to c9923ca609cc (28 revisions) flutter/packages#8425

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025

449079d Propagate environment variables when flutter drive is invok…

847a9f7

…ed. (flutter/flutter#161452)

engine-flutter-autoroll mentioned this pull request Jan 14, 2025

Roll Flutter from 72db8f69e339 to ed9f9206629b (29 revisions) flutter/packages#8426

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025

449079d Propagate environment variables when flutter drive is invok…

ae8a035

…ed. (flutter/flutter#161452)

engine-flutter-autoroll mentioned this pull request Jan 14, 2025

Roll Flutter from 72db8f69e339 to 40c2b86fdf65 (33 revisions) flutter/packages#8428

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025

449079d Propagate environment variables when flutter drive is invok…

eb63504

…ed. (flutter/flutter#161452)

harri35 mentioned this pull request Feb 5, 2025

Linux web_tool_tests is 2.11% flaky #162728

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

449079d Propagate environment variables when flutter drive is invok…

48097e0

…ed. (flutter/flutter#161452)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

449079d Propagate environment variables when flutter drive is invok…

5f09544

…ed. (flutter/flutter#161452)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

449079d Propagate environment variables when flutter drive is invok…

918ea1d

…ed. (flutter/flutter#161452)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

449079d Propagate environment variables when flutter drive is invok…

c31fc0c

…ed. (flutter/flutter#161452)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

449079d Propagate environment variables when flutter drive is invok…

650a217

…ed. (flutter/flutter#161452)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Propagate environment variables when `flutter drive` is invoked. #161452

Propagate environment variables when `flutter drive` is invoked. #161452

Uh oh!

matanlurey commented Jan 10, 2025

Uh oh!

jmagman Jan 10, 2025

Uh oh!

matanlurey Jan 10, 2025

Uh oh!

jmagman Jan 10, 2025

Uh oh!

matanlurey Jan 11, 2025

Uh oh!

matanlurey commented Jan 11, 2025

Uh oh!

andrewkolos left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Propagate environment variables when flutter drive is invoked. #161452

Propagate environment variables when flutter drive is invoked. #161452

Uh oh!

Conversation

matanlurey commented Jan 10, 2025

Uh oh!

jmagman Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

jmagman Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey commented Jan 11, 2025

Uh oh!

andrewkolos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Propagate environment variables when `flutter drive` is invoked. #161452

Propagate environment variables when `flutter drive` is invoked. #161452