Skip to content

Conversation

@eyebrowsoffire
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire commented Mar 18, 2024

  • Adds support for flutter test --wasm.
    • The test compilation flow is a bit different now, so that it supports compilers other than DDC. Specifically, when we run a set of unit tests, we generate a "switchboard" main function that imports each unit test and runs the main function for a specific one based off of a value set by the JS bootstrapping code. This way, there is one compile step and the same compile output is invoked for each unit test file.
  • Also, removes all references to dart:html from flutter/flutter.
  • Adds CI steps for running the framework unit tests with dart2wasm+skwasm
    • These steps are marked as 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 in bringup: 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

@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: integration_test The flutter/packages/integration_test plugin labels Mar 18, 2024
@github-actions github-actions bot added a: internationalization Supporting other languages or locales. (aka i18n) platform-web Web applications specifically labels Mar 18, 2024
@eyebrowsoffire eyebrowsoffire marked this pull request as ready for review March 20, 2024 20:57
@eyebrowsoffire eyebrowsoffire requested a review from ditman March 20, 2024 21:28
import 'bitfield.dart' as bitfield;

/// The dart:html implementation of [bitfield.kMaxUnsignedSMI].
/// The web implementation of [bitfield.kMaxUnsignedSMI].
Copy link
Contributor

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.

Copy link
Contributor Author

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].
Copy link
Contributor

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? :)

Copy link
Contributor Author

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
Copy link
Contributor

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',
Copy link
Contributor

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?

Copy link
Contributor Author

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();
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

Copy link
Contributor

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);
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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
. I'll go ahead and add the same comment to the ignore.

Copy link
Contributor

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.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 21, 2024
@auto-submit auto-submit bot merged commit 31209d0 into flutter:master Mar 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 22, 2024
camsim99 pushed a commit to flutter/packages that referenced this pull request Mar 25, 2024
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
bleroux added a commit to NevercodeHQ/flutter that referenced this pull request Mar 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@eyebrowsoffire eyebrowsoffire deleted the flutter_test_wasm branch December 12, 2025 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: integration_test The flutter/packages/integration_test plugin f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get framework tests running with dart2wasm

4 participants