Skip to content

[ci] Add a web version of Dart unit tests #4352

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

Merged
merged 21 commits into from
Jul 5, 2023

Conversation

stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Jun 30, 2023

Adds new LUCI targets in bringup mode to run all possible Dart unit tests in Chrome.

This is a new test (see linked issue), not a port of a Cirrus test, so it involves changes to tooling and packages:

  • The tooling now accepts an explicit platform. The default behavior if none is provided is the previous behavior of running in VM for everything but web plugin implementations (since that's convenient locally).
  • The tooling now has a basic understanding of dart_test.yaml test_on directives, to know when to skip.
  • Packages that don't support web have opt-out files.
  • Packages that do support web but have a few tests that fail on web have those tests opted out.
  • Packages that do support web but have a lot of failures on web have temporary opt-out files with TODOs + issue links.

Most of flutter/flutter#128979

.ci.yaml Outdated
@@ -93,48 +93,32 @@ targets:
version_file: flutter_master.version

- name: Linux dart_unit_test_shard_1 master
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to change the name to reflect web: Linux web_dart_unit_test_shard_1 master

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Jun 30, 2023

Choose a reason for hiding this comment

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

This was me hacking the web version into the existing config so I could run in CI without mucking around with led. The non-draft version once I have things passing will be new, bringup configs.

.ci.yaml Outdated
@@ -93,48 +93,32 @@ targets:
version_file: flutter_master.version

- name: Linux dart_unit_test_shard_1 master
bringup: true # New target
Copy link
Contributor

Choose a reason for hiding this comment

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

As this seems a new target, shall we keep the bringup?

@stuartmorgan-g stuartmorgan-g marked this pull request as ready for review June 30, 2023 20:03
@stuartmorgan-g
Copy link
Contributor Author

This is now ready for review. (Those of you who are incidentally pulled in because of minor changes to the package test configs are welcome to ignore this.)

@stuartmorgan-g stuartmorgan-g changed the title [ci] WIP: Add a web version of Dart unit tests [ci] Add a web version of Dart unit tests Jun 30, 2023
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

go_router_builder lgtm

@@ -185,7 +185,9 @@ void main() {

expect((mip.toJson() as List<dynamic>)[2], 1);
expect((scaled.toJson() as List<dynamic>)[2], 3);
});
},
// TODO(stuartmorgan): Investigate timeout on web.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on a comment from David later I think we just need to change this to inline image data. I'll do a fast-follow on this.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2023
@auto-submit auto-submit bot merged commit 7042079 into flutter:main Jul 5, 2023
@stuartmorgan-g stuartmorgan-g deleted the luci-dart-unit-web branch July 5, 2023 00:28
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 5, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Jul 5, 2023
flutter/packages@a03b900...7042079

2023-07-05 [email protected] [ci] Add a web version of Dart unit tests (flutter/packages#4352)
2023-07-04 [email protected] Roll Flutter from aa5f4a2 to 590ef2d (5 revisions) (flutter/packages#4373)

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-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

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/+doc/main/autoroll/README.md
stuartmorgan-g added a commit that referenced this pull request Jul 6, 2023
Enables the new LUCI Dart unit tests.

There is no corresponding Cirrus removal because that test was
accidentally removed early in
#4352.

Part of flutter/flutter#114373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants