Add --preview-dart-2 to 'flutter test' #14135
Conversation
| ? await compile( | ||
| sdkRoot: artifacts.getArtifactPath(Artifact.flutterPatchedSdkPath), | ||
| incrementalCompilerByteStorePath: '' /* not null is enough */, | ||
| mainPath: listenerFile.path, |
There was a problem hiding this comment.
also add parameter strongMode: true
There was a problem hiding this comment.
The test fails in strongMode, we won't be able to confirm the flow until flutter and frontend is strong mode clean. Given that I would prefer to add strong mode separately in follow up PR.
There was a problem hiding this comment.
Ok as discussed off line you will land this PR, follow it up with a PR to make it work on linux too and another PR to add the strong mode option.
It is important to get the strong mode option in as we want to get to the errors quickly.
| // bundlePath needs to point to a folder with `platform.dill` file. | ||
| final String bundlePath = previewDart2 ? artifacts.getArtifactPath(Artifact.flutterPatchedSdkPath) : null; | ||
|
|
||
| final Process process = await _startProcess( |
There was a problem hiding this comment.
The --strong option needs to be passed to the engine being invoked also as we will want to also test the runtime strong mode checks.
| if (host.type == InternetAddressType.IP_V6) | ||
| command.add('--ipv6'); | ||
| if (bundlePath != null) { | ||
| command.add('--flutter-assets-dir=$bundlePath'); |
There was a problem hiding this comment.
@aam Do you recall why this was needed?
It is a bit odd that we are passing the SDK path as the asset dir.
There was a problem hiding this comment.
I think that option is the way how bundle path(that should have platform.dill) is passed to flutter tester(per accompanying flutter/engine#4564).
There was a problem hiding this comment.
This implies all assets to be used by tester needs to be put in the sdk path. That's quite odd. In platform packaging the dill files are not found in the asset directory. Would it make sense to separate these concerns and change the way the dill files are located in the tester? [probably a new flag]?
There was a problem hiding this comment.
@tvolkert FYI I will attempt this if you don't have any objection. The request is to change tester to locate dill files w/o needing the asset flag so we can safely remove this particular line of code in flutter_platform.dart. When we add proper asset support [a separate param to installHook?], we can add it back. If you disagree with the methodology, let me know.
There was a problem hiding this comment.
No objection. I agree that a new flag to separate these things for flutter_tester would make sense.
git@github.com:flutter/engine.git/compare/fdaa7cf12175...ee4c2a5 git log fdaa7cf..ee4c2a5 --first-parent --oneline 2019-12-05 skia-flutter-autoroll@skia.org Roll src/third_party/skia 6344c2937997..0af32fdf5fea (12 commits) (flutter#14139) 2019-12-05 dworsham@google.com Wire up Opacity on Fuchsia, round 2 (flutter#14024) 2019-12-05 gw280@google.com Disable fml_tests until they're fixed on Fuchsia (flutter#14137) 2019-12-05 30870216+gaaclarke@users.noreply.github.com Started specifying the OS version for running the tests. (flutter#14094) 2019-12-04 skia-flutter-autoroll@skia.org Roll src/third_party/skia ccca30aad770..6344c2937997 (13 commits) (flutter#14133) 2019-12-04 30870216+gaaclarke@users.noreply.github.com Expanded our scenario_app docs. (flutter#14136) 2019-12-04 yjbanov@google.com [web][felt] fix source map path (flutter#14134) 2019-12-04 chinmaygarde@google.com Fix platform view offsets incorrectly taking into account device pixel ratios. (flutter#14135)
git@github.com:flutter/engine.git/compare/fdaa7cf12175...ee4c2a5 git log fdaa7cf..ee4c2a5 --first-parent --oneline 2019-12-05 skia-flutter-autoroll@skia.org Roll src/third_party/skia 6344c2937997..0af32fdf5fea (12 commits) (#14139) 2019-12-05 dworsham@google.com Wire up Opacity on Fuchsia, round 2 (#14024) 2019-12-05 gw280@google.com Disable fml_tests until they're fixed on Fuchsia (#14137) 2019-12-05 30870216+gaaclarke@users.noreply.github.com Started specifying the OS version for running the tests. (#14094) 2019-12-04 skia-flutter-autoroll@skia.org Roll src/third_party/skia ccca30aad770..6344c2937997 (13 commits) (#14133) 2019-12-04 30870216+gaaclarke@users.noreply.github.com Expanded our scenario_app docs. (#14136) 2019-12-04 yjbanov@google.com [web][felt] fix source map path (#14134) 2019-12-04 chinmaygarde@google.com Fix platform view offsets incorrectly taking into account device pixel ratios. (#14135)
git@github.com:flutter/engine.git/compare/fdaa7cf12175...ee4c2a5 git log fdaa7cf..ee4c2a5 --first-parent --oneline 2019-12-05 skia-flutter-autoroll@skia.org Roll src/third_party/skia 6344c2937997..0af32fdf5fea (12 commits) (#14139) 2019-12-05 dworsham@google.com Wire up Opacity on Fuchsia, round 2 (#14024) 2019-12-05 gw280@google.com Disable fml_tests until they're fixed on Fuchsia (#14137) 2019-12-05 30870216+gaaclarke@users.noreply.github.com Started specifying the OS version for running the tests. (#14094) 2019-12-04 skia-flutter-autoroll@skia.org Roll src/third_party/skia ccca30aad770..6344c2937997 (13 commits) (#14133) 2019-12-04 30870216+gaaclarke@users.noreply.github.com Expanded our scenario_app docs. (#14136) 2019-12-04 yjbanov@google.com [web][felt] fix source map path (#14134) 2019-12-04 chinmaygarde@google.com Fix platform view offsets incorrectly taking into account device pixel ratios. (#14135) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
5 participants
This is toward fixing #14105.
This relies on flutter/engine#4559.