-
Notifications
You must be signed in to change notification settings - Fork 29.7k
flutter test --wasm support
#145347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
flutter test --wasm support
#145347
Conversation
| import 'bitfield.dart' as bitfield; | ||
|
|
||
| /// The dart:html implementation of [bitfield.kMaxUnsignedSMI]. | ||
| /// The web implementation of [bitfield.kMaxUnsignedSMI]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "JS implementation"? I imagine Wasm can support the regular VM code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of right now, wasm uses this implementation. You can see the conditional import is gated on dart.library.js_util which evaluates to true on the dart2wasm platform:
| if (dart.library.js_util) '_bitfield_web.dart' as bitfield; |
You're right that we could probably switch over to using the VM version for wasm, since we don't have floating point precision issues with int on dart2wasm. I don't want to add yet another change that could potentially add risk to this big PR, so I'm filing that as a follow-on issue and we can look into that separately. #145540
| @JS('window') | ||
| external JSObject get _window; | ||
|
|
||
| /// The web implementation of [registerWebServiceExtension]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a non-web implementation of the registerWebServiceExtension? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically yes, see packages/flutter_driver/lib/src/extension/_extension_io.dart. It's a no-op, but it's an implementation.
|
|
||
| /// An unsupported [WebGoldenComparator] that exists for API compatibility. | ||
| class DefaultWebGoldenComparator extends WebGoldenComparator { | ||
| /// This is provided to prevent warnings from the analyzer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: end the doc with a period
| if (needsCrossOriginIsolated) | ||
| ...<String, String>{ | ||
| 'Cross-Origin-Opener-Policy': 'same-origin', | ||
| 'Cross-Origin-Embedder-Policy': 'require-corp', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we always used these, in all modes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered this. However, that would be a breaking change for anyone who writes a test that relies on being in a non-crossOriginIsolated context. It seems like it would be weird to do a cross-origin fetch in a unit test, but I could see it in an integration test possibly. I tried to make this change as non-breaking as possible. If we want to re-evaluate that decision later and say "all flutter test runs should run in a crossOriginIsolated context and if that breaks your test you should change your test" I think I'd be okay with that, but I'd rather not overload that behavior change with this PR.
| compilationArgs, | ||
| ); | ||
|
|
||
| return WebMemoryFS(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we not populating memory-fs in the wasm case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WebMemoryFS is specific to DDC's outputs. To be honest, I'm not entirely sure why we do this at all in the DDC case. In the dart2wasm case, all the compiler outputs are just put into the output directory and they are served directly from there. I didn't want to mess too much with the DDC compilation process in this PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps @jonahwilliams has some insight here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
writing thousands of 10-100 line files is incredibly slow. So slow that it can easily add an order of magnitude onto the compilation time of the ddc applications. If you're producing a single output file then its irrelevant and I wouldnt use the memory fs
| /// This hard-codes the device pixel ratio to 3.0 and a 2400 x 1800 window | ||
| /// size for the purposes of testing. | ||
| ui_web.debugOverrideDevicePixelRatio(3.0); | ||
| ui.window.debugPhysicalSizeOverride = const ui.Size(2400, 1800); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fly-by comment: The window singleton is deprecated and we've cleaned up the code base to no longer depend on this singleton. Can you please not introduce new references to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a new reference, it is copied over from the previous generated test bootstrap file. I'm happy to switch over to a different API though, as long as it is guaranteed to have the exact same behavior. What should I invoke instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not find an equivalent API that is accessible from this package. We can look into figuring out a solution for this later. Especially since this isn't really a new reference to this API, I don't want to block this change on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've filed #145558 to keep track of this.
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| // ignore_for_file: public_member_api_docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, will what is defined in here show up on api.flutter.dev? It should have docs then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically the same situation as
| // This code is copied from `package:web` which still needs its own |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should not show up on api.flutter.dev. If they do, there's probably a bug somewhere.
flutter/flutter@18340ea...14774b9 2024-03-22 [email protected] Roll Flutter Engine from eba6e31498b8 to 09dadce76828 (1 revision) (flutter/flutter#145603) 2024-03-22 [email protected] Roll Flutter Engine from f9a34ae0b14f to eba6e31498b8 (1 revision) (flutter/flutter#145598) 2024-03-22 [email protected] Intensive `if` chain refactoring (flutter/flutter#145194) 2024-03-22 [email protected] Adds numpad navigation shortcuts for Linux (flutter/flutter#145464) 2024-03-22 [email protected] Roll Flutter Engine from 5a12de1beab7 to f9a34ae0b14f (1 revision) (flutter/flutter#145581) 2024-03-22 [email protected] Roll Flutter Engine from e2f324beac3b to 5a12de1beab7 (1 revision) (flutter/flutter#145578) 2024-03-22 [email protected] Replace `RenderBox.compute*` with `RenderBox.get*` and add `@visibleForOverriding` (flutter/flutter#145503) 2024-03-22 [email protected] Add some cross references in the docs, move an example to a dartpad example (flutter/flutter#145571) 2024-03-22 [email protected] Fix `BorderSide.none` requiring explicit transparent color for `UnderlineInputBorder` (flutter/flutter#145329) 2024-03-22 [email protected] Roll Flutter Engine from a46a7b273a5b to e2f324beac3b (1 revision) (flutter/flutter#145576) 2024-03-21 [email protected] Fix nullability of getFullHeightForCaret (flutter/flutter#145554) 2024-03-21 [email protected] Add a `--no-gradle-generation` mode to the `generate_gradle_lockfiles.dart` script (flutter/flutter#145568) 2024-03-21 [email protected] Roll Flutter Engine from 1b842ae58b3d to a46a7b273a5b (2 revisions) (flutter/flutter#145569) 2024-03-21 [email protected] Fixed race condition in PollingDeviceDiscovery. (flutter/flutter#145506) 2024-03-21 [email protected] Roll Flutter Engine from a2ed373fa70f to 1b842ae58b3d (1 revision) (flutter/flutter#145565) 2024-03-21 [email protected] Clarify AutomaticKeepAliveClientMixin semantics in build method (flutter/flutter#145297) 2024-03-21 [email protected] Eliminate more window singleton usages (flutter/flutter#145560) 2024-03-21 [email protected] `flutter test --wasm` support (flutter/flutter#145347) 2024-03-21 [email protected] Roll Flutter Engine from eb262e9c34db to a2ed373fa70f (2 revisions) (flutter/flutter#145556) 2024-03-21 [email protected] Roll Flutter Engine from bad4a30e1c75 to eb262e9c34db (1 revision) (flutter/flutter#145555) 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],[email protected],[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: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter test --wasm.dart:htmlfrom flutter/flutter.bringup: true, so we don't know what kind of failures they will result in. Any failures they have will not block the tree at all yet while we're still inbringup: true. Once this PR is merged, I plan on looking at any failures and either fixing them or disabling them so we can get these CI steps running on presubmit.This fixes #126692