[flutter_tools] refactor drive launch into separate service, split by mobile+desktop and web #68451
Conversation
flutter-dashboard
bot
added
tool
jonahwilliams
changed the title
|
@yjbanov can you double check the web changes (or find someone else for them) - I ran through the web test on cirrus and it works with this change. |
| dartSdkPath: globals.artifacts.getArtifactPath(Artifact.engineDartBinary), | ||
| ); | ||
| final DriverService driverService = _flutterDriverFactory.createDriverService(web); | ||
| // if (web) { |
| if (debuggingOptions.debuggingEnabled) { | ||
| observatoryDiscovery = ProtocolDiscovery.observatory( | ||
| await getLogReader(), | ||
| await AdbLogReader.createLogReader( |
There was a problem hiding this comment.
Add a comment why not to use getLogReader?
| dartSdkPath: globals.artifacts.getArtifactPath(Artifact.engineDartBinary), | ||
| ); | ||
| final DriverService driverService = _flutterDriverFactory.createDriverService(web); | ||
| // if (web) { |
| if (await _fileSystem.type(testFile) != FileSystemEntityType.file) { | ||
| throwToolExit('Test file not found: $testFile'); | ||
| } | ||
| final Device device = await findTargetDevice(includeUnsupportedDevices: stringArg('use-application-binary') == null); |
There was a problem hiding this comment.
Where is deviceDiscoveryTimeout now?
There was a problem hiding this comment.
Shared with the existing code in flutter_command.dart: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/runner/flutter_command.dart#L1059
| fileSystem: globals.fs, | ||
| logger: globals.logger, |
There was a problem hiding this comment.
Aren't these null at this point?
There was a problem hiding this comment.
No, the only interfaces that can't be injected at this point are those that depend on Cache/Artifacts/Version since those are still constructed when the first FlutterCommand is executed
|
Updated to include device disposal from #68655 |
|
G3Fix is CL/338167561 |
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
Thanks for doing this and making it testable. I was always scared to touch drive code, it was spaghetti
3 participants
Description
Overhaul of flutter drive in order to deliver a better experience, namely:
flutter runandflutter drivenow share more flags, so code paths that were previously only testable on run are now testable on drive.Web changes
Passes on the one test in the repo, otherwise the webdriver code has been isolated as much as possible
Additional NNBD related bug fixes:
No longer passes --enable-experiment to the test script. (FYI @blasten ). earlier we might have assumed that the flutter gallery benchmarks would be migrated along side the app and flutter driver, but only the app under test needs to be migrated. The test scripts should never be run with the experiment.