[flutter_tools] [dap] Add support for passing env variables to spawned processes #107415
Conversation
flutter-dashboard
bot
added
the
tool
| @@ -316,12 +316,13 @@ class FlutterDebugAdapter extends DartDebugAdapter<FlutterLaunchRequestArguments | |||
| } | |||
|
|
|||
| @visibleForOverriding | |||
| Future<void> launchAsProcess(String executable, List<String> processArgs) async { | |||
| Future<void> launchAsProcess(String executable, List<String> processArgs, Map<String, String>? env) async { | |||
There was a problem hiding this comment.
nit. prefer named arguments with 3 or more
| final MockFlutterDebugAdapter adapter = MockFlutterDebugAdapter(fileSystem: globals.fs, platform: globals.platform); | ||
| group('launchRequest', () { | ||
| test('runs "flutter run" with --machine', () async { | ||
| final MockFlutterDebugAdapter adapter = MockFlutterDebugAdapter(fileSystem: globals.fs, platform: globals.platform); |
There was a problem hiding this comment.
Does the MockFlutterDebugAdapter actually use the real filesystem? If so, these tests should be probably be under flutter_tools/test/integration.shard/dap/flutter_adapter_test.dart. If not, can we pass a memory file system and FakePlatform to them instead?
There was a problem hiding this comment.
They don't use them, these ones are just simple unit tests. I've changed them to use MemoryFileSystem.test() and FakePlatform, but I made the platform match the real one so that the Windows bots will test with Windows paths etc.
(I haven't verified on Windows so if I didn't do this right the bots may fail, I'll check back on this later since I gotta be away for a little)
There was a problem hiding this comment.
This does fail on windows:
00:48 +1256 ~3 -2: test\general.shard\dap\flutter_test_adapter_test.dart: flutter test adapter includes toolArgs [E]
Expected: 'C:\\fake\\flutter\\bin\\flutter.bat'
Actual: 'C:\\fake\\flutter/bin/flutter.bat'
Which: is different.
Expected: ... e\\flutter\\bin\\flu ...
Actual: ... e\\flutter/bin/flutt ...
^
Differ at offset 17
package:test_api expect
test\general.shard\dap\flutter_test_adapter_test.dart 49:7 main.<fn>.<fn>
I think when constructing the MemoryFileSystem, you have to also specify the FileSystemStyle in the constructor
|
@christopherfujino Windows framework_tests_widgets says it's an infra failure. Is there a way to re-run that one without re-running everything? |
Done |
|
Thanks! |
…o spawned processes (flutter/flutter#107415)
…o spawned processes (flutter/flutter#107415)
…d processes (flutter#107415) * [flutter_tools] [dap] Add support for passing env variables to spawned processes * Use named args * Use in-memory fs and FakePlatform * Pass filesystem style to MemoryFileSystem
This wires up the
envfield that may be passed in the DAP's launch args to the Flutter processes. These may be set by the user (although for non-test that's less useful since they only apply to the tool and not the process running on the device), but also allows editors to add vars that may be used for analytics (such asPUB_ENVIRONMENT).Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.