[web] enable CanvasKit tests using a local bundle fetched from CIPD by yjbanov · Pull Request #92134 · flutter/flutter

yjbanov · 2021-10-19T21:57:51Z

Fetch CanvasKit from CIPD to the Flutter SDK cache.
Copy CanvasKit files to build/web/canvaskit during flutter build web.
Use the local CanvasKit copy (instead of unpkg.com) when running unit-tests.
Add test shards that run currently passing tests in CanvasKit mode (which is the vast majority).
Golden tests use the ("WebRenderer": "canvaskit") key/value pair to distinguish the goldens from the HTML ones.

Fixes #71604
Fixes #70101

christopherfujino · 2021-10-20T20:09:00Z

nit: update comment

CaseyHillers

.ci.yaml lgtm

CaseyHillers · 2021-10-20T20:14:44Z

What's the reasoning for these being strings instead of numbers?

Technically all subshard names are strings, but some shards have a custom convention. Web shards use the numeric convention that uses subshard number ("0", "1", etc) for all subshards except the last one, which uses "_last" suffix (e.g. 7_last).

Having said that I totally botched the convention and now I fixed it. Unfortunately, because .ci.yaml changes do not take effect immediately it's easy to make mistakes like that..

christopherfujino · 2021-10-20T20:16:53Z

christopherfujino · 2021-10-20T20:27:40Z

Avoid adding new calls to globals in tests. I suggest doing something like:

for (final FileSystemStyle style in <FileSystemStyle>[FileSystemStyle.posix, FileSystemStyle.windows]) { test('WebServiceWorker generates a service_worker for a web resource folder for ${style == FileSystemStyle.posix ? 'posix' : 'windows'}', () { final FileSystem fileSystem = MemoryFileSystem(style: style); ... }); }

After an offline discussion, reverted to globals.fs. The test is using TestBed and Environment which are setup to use globals.fs. This file overall needs a separate clean-ups to stop depending on globals.

christopherfujino · 2021-10-20T20:30:41Z

avoid adding globals to tests

christopherfujino

Some nits about the tests, but otherwise the tools change LGTM

keyonghan

LGTM with test ownership.

Piinks

Should the test shards that are being added show up in CI here? They do not appear to be listed as checks, and without running them, Gold is not seeing this new info.

Piinks

According to https://flutter-gold.skia.org/search?crs=github&issue=92134&patchsets=1&positive=true
these are the only builds that are sending info to Gold:

Windows framework_tests_widgets
Windows framework_tests_libraries
Linux web_tests_5
Linux web_tests_4
Linux web_tests_3
Linux web_tests_2
Mac framework_tests_widgets
Linux web_tests_1
Linux web_tests_7_last
Linux web_tests_6
Linux framework_tests_widgets
Linux web_tests_0
Linux framework_tests_libraries
Mac framework_tests_libraries

yjbanov · 2021-10-20T22:45:15Z

@Piinks good question. Unfortunately, the new shards do not show up right away. More details here, but the tl;dr is that new test shards are added as bringup: true and submitted. They take time to propagate to LUCI and start running without failing the build. Once we confirm that everything is working (perhaps with follow-up fixes), then we can make them build blocking.

yjbanov · 2021-10-20T22:45:52Z

@Piinks good question. Unfortunately, the new shards do not show up right away. More details here, but the tl;dr is that new test shards are added as bringup: true and submitted. They take time to propagate to LUCI and start running without failing the build. Once we confirm that everything is working (perhaps with follow-up fixes), then we can make them build blocking.

yjbanov · 2021-10-20T22:46:45Z

(sorry for duplicate posts; Github is giving me errors when I post but is actually posting my comments)

Piinks · 2021-10-20T23:07:26Z

Ah I see, ok! In that case it sounds like we would validate later that Gold is receiving the data correctly. Would that testing happen in post submit then? Currently the Gold flow checks images in during presubmit. It may cause some issues temporarily if these are run in post submit first, not sure though since we haven't done that before. 🙃

yjbanov · 2021-10-20T23:26:31Z

Ah I see, ok! In that case it sounds like we would validate later that Gold is receiving the data correctly. Would that testing happen in post submit then? Currently the Gold flow checks images in during presubmit. It may cause some issues temporarily if these are run in post submit first, not sure though since we haven't done that before. upside_down_face

Oh no! Catch-22. @CaseyHillers save us!

CaseyHillers · 2021-10-21T04:34:12Z

Oh no! Catch-22. @CaseyHillers save us!

@Piinks if we're not running in presubmit, I don't think there would be a way for gold to block PRs. This should be safe to land. If there's a breakage, that's a bug and we'll need to fix the process to adhere with the tree's flaky test policy.

mdebbar · 2021-10-21T13:13:34Z

Suggestion: There's no need for async/await here.

Suggested change

Future<void> _runWebHtmlUnitTests() async {

await _runWebUnitTests('html');

}

Future<void> _runWebCanvasKitUnitTests() async {

await _runWebUnitTests('canvaskit');

}

Future<void> _runWebHtmlUnitTests() {

return _runWebUnitTests('html');

}

Future<void> _runWebCanvasKitUnitTests() {

return _runWebUnitTests('canvaskit');

}

mdebbar · 2021-10-21T13:43:27Z

Just curious, why not:

final File target = environment.outputDir.childDirectory('canvaskit').childFile(relativePath); await file.copy(target.path);

childFile takes a basename, not a sub-path.

mdebbar · 2021-10-21T13:48:28Z

s/directory directory/directory

Piinks · 2021-10-21T15:43:23Z

@Piinks if we're not running in presubmit, I don't think there would be a way for gold to block PRs. This should be safe to land. If there's a breakage, that's a bug and we'll need to fix the process to adhere with the tree's flaky test policy.

SGTM!

Piinks

LGTM - Gold changes look fine, but will need to confirm and triage all of the images with the new key set once these shards are actually running.

yjbanov · 2021-10-21T18:13:10Z

LGTM - Gold changes look fine, but will need to confirm and triage all of the images with the new key set once these shards are actually running.

Thanks! I might need some handholding to see it through. Might need help from you, @Piinks, and from @CaseyHillers.

…lutter#92134)

abelokon0711 · 2022-03-09T11:10:39Z

Could you explain me please how exactly does this solve #70101?

P-B1101 · 2022-07-02T06:07:25Z

I think this command will use bundled canvaskit from your build folder.
flutter build web --dart-define=FLUTTER_WEB_CANVASKIT_URL=/canvaskit/
I test it using IIS from my localhost and it's worked. I think it will work on any other host too.

Part of flutter/flutter#92134 It seems the issue is already closed and fixed, so I'm attempting to remove this TODO ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

google-cla Bot added the cla: yes label Oct 19, 2021

yjbanov force-pushed the ck-tests branch 3 times, most recently from e5a82ca to 371987b Compare October 19, 2021 23:49

yjbanov mentioned this pull request Oct 20, 2021

TESTOWNERS instructions unclear #92190

Closed

yjbanov force-pushed the ck-tests branch from 371987b to 786e83a Compare October 20, 2021 18:38

yjbanov marked this pull request as ready for review October 20, 2021 19:52

yjbanov requested review from CaseyHillers, Piinks and keyonghan as code owners October 20, 2021 19:52

yjbanov requested review from christopherfujino and harryterkelsen October 20, 2021 19:52

christopherfujino reviewed Oct 20, 2021

View reviewed changes

CaseyHillers approved these changes Oct 20, 2021

View reviewed changes

christopherfujino reviewed Oct 20, 2021

View reviewed changes

christopherfujino approved these changes Oct 20, 2021

View reviewed changes

keyonghan approved these changes Oct 20, 2021

View reviewed changes

Piinks reviewed Oct 20, 2021

View reviewed changes

yjbanov force-pushed the ck-tests branch from 786e83a to 60d8776 Compare October 21, 2021 00:01

mdebbar approved these changes Oct 21, 2021

View reviewed changes

[web] enable CanvasKit tests using a local bundle fetched from CIPD

098965c

yjbanov force-pushed the ck-tests branch from 60d8776 to 098965c Compare October 21, 2021 17:18

yjbanov requested a review from Piinks October 21, 2021 17:31

Piinks approved these changes Oct 21, 2021

View reviewed changes

yjbanov merged commit ad7727a into flutter:master Oct 21, 2021

clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021

[web] enable CanvasKit tests using a local bundle fetched from CIPD (f…

a2ce9df

…lutter#92134)

TahaTesser mentioned this pull request Jan 12, 2022

[web] flutter build web --web-renderer html also generates a canvaskit folder #96509

Closed

jonas-martinez mentioned this pull request Mar 28, 2022

Add the canvas file to Devtools on compile lenra-io/devtool#51

Closed

rob-mur mentioned this pull request Jan 19, 2023

Prefer require-corp over credentialless for Cross-Origin-Embedder-Policy fzyzcjy/flutter_rust_bridge#985

Closed

ValentinVignal mentioned this pull request Aug 22, 2026

[material_ui] Remove !chrome in text_field_test.dart flutter/packages#12548

Merged

11 tasks done

-Future<void> _runWebHtmlUnitTests() async {
-  await _runWebUnitTests('html');
-}
-Future<void> _runWebCanvasKitUnitTests() async {
-  await _runWebUnitTests('canvaskit');
-}
+Future<void> _runWebHtmlUnitTests() {
+  return _runWebUnitTests('html');
+}
+Future<void> _runWebCanvasKitUnitTests() {
+  return _runWebUnitTests('canvaskit');
+}

Conversation

yjbanov commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CaseyHillers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

keyonghan left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

yjbanov commented Oct 20, 2021

Uh oh!

yjbanov commented Oct 20, 2021

Uh oh!

yjbanov commented Oct 20, 2021

Uh oh!

Piinks commented Oct 20, 2021

Uh oh!

yjbanov commented Oct 20, 2021

Uh oh!

CaseyHillers commented Oct 21, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Piinks commented Oct 21, 2021

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

yjbanov commented Oct 21, 2021

Uh oh!

abelokon0711 commented Mar 9, 2022

Uh oh!

P-B1101 commented Jul 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

yjbanov commented Oct 19, 2021 •

edited

Loading

christopherfujino Oct 20, 2021 •

edited

Loading