Cupertino icons golden tests #7421

LongCatIsLooong · 2024-08-15T17:59:22Z

This is for flutter/flutter#148075, we want to make sure the glyphs don't look different when the font generation script is migrated from python.
Also it will be nice to be able to inspect glyph changes whenever the ttf file gets updated. The flutter test command takes 6s to complete on my computer.

Unfortunately this test:

is sensitive to how flutter renders text on Linux, and how the Icon widget positions the glyph.
will require contributors to have access to a Linux box in order to generate new goldens.

The tests are disabled on the web since it needs the dart:io import.

The icons_list.dart file is automatically generated. Not sure if there's a way to incorporate the code gen step on CI?

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

LongCatIsLooong · 2024-08-15T20:54:33Z

third_party/packages/cupertino_icons/test/icons_list.dart

+/// The full list of [IconData] constants defined in the [CupertinoIcons] class,
+/// sorted by their code points in ascending order.
+///
+/// This list cannot be mutated.


This automatically generated. Is there a way to incorporate the code gen step on CI?

This automatically generated.

By what? There should be a prominent comment in this file explain that it is generated, and how to regenerate it.

Is there a way to incorporate the code gen step on CI?

Since this is included from another file, not currently; there is not currently a mechanism to generate files before analyze is run.

Now that I think about it, it's probably not a good idea to try to automate the codegen for now.

The codegen script generates the List of IconData from the CupertinoICons class which currently lives in flutter/flutter, so it updates with a delay after a new cupertino_icons version is available.

This list should be updated manually for now, after changing the ttf file. I'll add a comment to this file (or in a more visible place? since it's tied to the font).

victorsanni

LGTM

stuartmorgan-g

is sensitive to how flutter renders text on Linux, and how the Icon widget positions the glyph.

Any test in flutter/packages that is sensitive to framework rendering must be set up to run only on main (example), otherwise the first time there is a framework change that affects this test, it will become impossible to roll.

will require contributors to have access to a Linux box in order to generate new goldens.

FYI, in practice this means that the test will periodically be disabled by the gardener, with an issue filed against the package's CODEOWNER to re-enable it, because not all gardeners have access to a machine that can regenerate goldens.

stuartmorgan-g · 2024-08-18T12:02:09Z

third_party/packages/cupertino_icons/test/icons_list.dart

+/// The full list of [IconData] constants defined in the [CupertinoIcons] class,
+/// sorted by their code points in ascending order.
+///
+/// This list cannot be mutated.


This automatically generated.

By what? There should be a prominent comment in this file explain that it is generated, and how to regenerate it.

Is there a way to incorporate the code gen step on CI?

Since this is included from another file, not currently; there is not currently a mechanism to generate files before analyze is run.

LongCatIsLooong · 2024-08-20T19:03:29Z

is sensitive to how flutter renders text on Linux, and how the Icon widget positions the glyph.

Any test in flutter/packages that is sensitive to framework rendering must be set up to run only on main (example), otherwise the first time there is a framework change that affects this test, it will become impossible to roll.

will require contributors to have access to a Linux box in order to generate new goldens.

FYI, in practice this means that the test will periodically be disabled by the gardener, with an issue filed against the package's CODEOWNER to re-enable it, because not all gardeners have access to a machine that can regenerate goldens.

Added myself to the codeowner list of the golden files.

stuartmorgan-g

One question, but otherwise my CI concerns are addressed, thanks!

stuartmorgan-g · 2024-08-27T15:09:58Z

third_party/packages/cupertino_icons/pubspec.yaml

  flutter:
    sdk: flutter
  flutter_test:
    sdk: flutter
+  path: 1.9.0


Why is this pinned to an exact version? path is allowed as an unpinned dependency in the repo.

I made the change after reading https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#dependencies

If you add a dev_dependency on an external package, pin it to a specific version if at all possible.

But it looks like that only applies to external packages. I'll change it back to any.

It shouldn't be any, it should be a standard ^ dependency.

stuartmorgan-g · 2024-08-27T15:14:12Z

Version/CHANGELOG override: dev_dependency changes are exempt per policy.

Obsolete

stuartmorgan-g · 2024-08-27T15:31:32Z

third_party/packages/cupertino_icons/pubspec.yaml

@@ -14,7 +14,7 @@ dev_dependencies:
    sdk: flutter
  flutter_test:
    sdk: flutter
-  path: 1.9.0
+  path: any


Please don't ever use any anywhere in a package; this is a recipe for OOB tree breakage. It literally means "I want to get intentionally breaking changes without warning".

…oong/packages into cupertino-icons-golden-tests

flutter/packages@d862279...2a0f254 2024-08-28 [email protected] [interactive_media_ads] Adds internal wrapper for remaining methods of the Android native `AdsManager` (flutter/packages#7437) 2024-08-28 [email protected] [shared_preferences] Fix typo in changelog (flutter/packages#7523) 2024-08-27 [email protected] Remove matan from codeowners (flutter/packages#7511) 2024-08-27 [email protected] [shared_preferences] Add test to enforce mutable lists (flutter/packages#7369) 2024-08-27 [email protected] [google_sign_in_ios] Fix "callee requires a non-null parameter" analyzer warning (flutter/packages#7513) 2024-08-27 [email protected] [in_app_purchase_storekit] Allows 'localizedDescription' to be nullable to handle occasional nulls in Storekit objects (flutter/packages#7515) 2024-08-27 [email protected] Cupertino icons golden tests (flutter/packages#7421) 2024-08-26 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 2.0.10 to 2.0.20 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7500) 2024-08-26 [email protected] [video_player] Updates minimum supported SDK (flutter/packages#7498) 2024-08-26 [email protected] [url_launcher] Ignore new `unreachable_switch_default` warning. (flutter/packages#7487) 2024-08-26 [email protected] Revert "[video_player] Relands #6456: Uses SurfaceProducer, this time with setCallback for suspend/resume lifecycles" (flutter/packages#7497) 2024-08-24 [email protected] [video_player] Relands #6456: Uses `SurfaceProducer`, this time with `setCallback` for suspend/resume lifecycles. (flutter/packages#6989) 2024-08-23 [email protected] [google_maps_flutter_android] Convert `PlatformTileOverlay` to Pigeon (flutter/packages#7467) 2024-08-23 [email protected] [go_router] Fixes issue so that the parseRouteInformationWithContext â�¦ (flutter/packages#7337) 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://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/packages@d862279...2a0f254 2024-08-28 [email protected] [interactive_media_ads] Adds internal wrapper for remaining methods of the Android native `AdsManager` (flutter/packages#7437) 2024-08-28 [email protected] [shared_preferences] Fix typo in changelog (flutter/packages#7523) 2024-08-27 [email protected] Remove matan from codeowners (flutter/packages#7511) 2024-08-27 [email protected] [shared_preferences] Add test to enforce mutable lists (flutter/packages#7369) 2024-08-27 [email protected] [google_sign_in_ios] Fix "callee requires a non-null parameter" analyzer warning (flutter/packages#7513) 2024-08-27 [email protected] [in_app_purchase_storekit] Allows 'localizedDescription' to be nullable to handle occasional nulls in Storekit objects (flutter/packages#7515) 2024-08-27 [email protected] Cupertino icons golden tests (flutter/packages#7421) 2024-08-26 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 2.0.10 to 2.0.20 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7500) 2024-08-26 [email protected] [video_player] Updates minimum supported SDK (flutter/packages#7498) 2024-08-26 [email protected] [url_launcher] Ignore new `unreachable_switch_default` warning. (flutter/packages#7487) 2024-08-26 [email protected] Revert "[video_player] Relands flutter#6456: Uses SurfaceProducer, this time with setCallback for suspend/resume lifecycles" (flutter/packages#7497) 2024-08-24 [email protected] [video_player] Relands flutter#6456: Uses `SurfaceProducer`, this time with `setCallback` for suspend/resume lifecycles. (flutter/packages#6989) 2024-08-23 [email protected] [google_maps_flutter_android] Convert `PlatformTileOverlay` to Pigeon (flutter/packages#7467) 2024-08-23 [email protected] [go_router] Fixes issue so that the parseRouteInformationWithContext â�¦ (flutter/packages#7337) 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://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

LongCatIsLooong added 2 commits August 15, 2024 10:43

add golden tests for cupertino_icons

822ec82

fix goldens

a474995

github-actions bot added the p: cupertino_icons label Aug 15, 2024

LongCatIsLooong added 3 commits August 15, 2024 12:43

hooray

718a9a6

unmodifiable

9721434

dart:io

c5ba512

LongCatIsLooong commented Aug 15, 2024

View reviewed changes

LongCatIsLooong marked this pull request as ready for review August 15, 2024 21:29

LongCatIsLooong requested a review from MitchellGoodwin as a code owner August 15, 2024 21:29

LongCatIsLooong requested review from stuartmorgan-g and victorsanni August 15, 2024 21:31

Linux only

76e4d40

LongCatIsLooong force-pushed the cupertino-icons-golden-tests branch 2 times, most recently from 3fbb667 to ff7cd7b Compare August 16, 2024 00:47

pin dev deps version

7a33212

LongCatIsLooong force-pushed the cupertino-icons-golden-tests branch from ff7cd7b to 7a33212 Compare August 16, 2024 01:15

victorsanni approved these changes Aug 16, 2024

View reviewed changes

stuartmorgan-g previously requested changes Aug 18, 2024

View reviewed changes

LongCatIsLooong added 2 commits August 20, 2024 11:30

Update cupertino_icons_golden_test.dart

f021829

Update icons_list.dart

8aa7409

Update cupertino_icons_golden_test.dart

b7e2ff8

LongCatIsLooong requested a review from stuartmorgan-g August 20, 2024 19:49

Update CODEOWNERS

894af19

stuartmorgan-g reviewed Aug 27, 2024

View reviewed changes

stuartmorgan-g added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Aug 27, 2024

unpin path

8f0fe45

stuartmorgan-g requested changes Aug 27, 2024

View reviewed changes

Merge branch 'cupertino-icons-golden-tests' of github.com:LongCatIsLo…

fb4c4bb

…oong/packages into cupertino-icons-golden-tests

LongCatIsLooong force-pushed the cupertino-icons-golden-tests branch from ff3a577 to fb4c4bb Compare August 27, 2024 15:32

pin path to ^1.9.0

be49a15

stuartmorgan-g approved these changes Aug 27, 2024

View reviewed changes

LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 27, 2024

auto-submit bot merged commit ef81058 into flutter:main Aug 27, 2024
76 checks passed

engine-flutter-autoroll mentioned this pull request Aug 27, 2024

Roll Packages from d862279f291f to ef81058368d6 (8 revisions) flutter/flutter#154191

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 27, 2024

ef81058 Cupertino icons golden tests (flutter/packages#7421)

e409de5

engine-flutter-autoroll mentioned this pull request Aug 28, 2024

Roll Packages from d862279f291f to cc7b5fc79cd3 (12 revisions) flutter/flutter#154259

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 28, 2024

ef81058 Cupertino icons golden tests (flutter/packages#7421)

c8cdc3f

engine-flutter-autoroll mentioned this pull request Aug 28, 2024

Roll Packages from d862279f291f to cc7b5fc79cd3 (12 revisions) flutter/flutter#154277

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 28, 2024

ef81058 Cupertino icons golden tests (flutter/packages#7421)

08097d0

engine-flutter-autoroll mentioned this pull request Aug 29, 2024

Roll Packages from d862279f291f to 2a0f25419c52 (14 revisions) flutter/flutter#154341

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 29, 2024

ef81058 Cupertino icons golden tests (flutter/packages#7421)

ffe896e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Cupertino icons golden tests #7421

Cupertino icons golden tests #7421

Uh oh!

LongCatIsLooong commented Aug 15, 2024 •

edited

Loading

Uh oh!

LongCatIsLooong Aug 15, 2024

Uh oh!

stuartmorgan-g Aug 18, 2024

Uh oh!

LongCatIsLooong Aug 20, 2024

Uh oh!

victorsanni left a comment

Uh oh!

stuartmorgan-g left a comment

Uh oh!

stuartmorgan-g Aug 18, 2024

Uh oh!

LongCatIsLooong commented Aug 20, 2024 •

edited

Loading

Uh oh!

stuartmorgan-g left a comment

Uh oh!

stuartmorgan-g Aug 27, 2024

Uh oh!

LongCatIsLooong Aug 27, 2024

Uh oh!

stuartmorgan-g Aug 27, 2024

Uh oh!

stuartmorgan-g commented Aug 27, 2024

Uh oh!

stuartmorgan-g Aug 27, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Cupertino icons golden tests #7421

Cupertino icons golden tests #7421

Uh oh!

Conversation

LongCatIsLooong commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

LongCatIsLooong Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Aug 18, 2024

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Aug 20, 2024

Choose a reason for hiding this comment

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Aug 18, 2024

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Aug 27, 2024

Uh oh!

stuartmorgan-g Aug 27, 2024 • 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!

Uh oh!

Uh oh!

LongCatIsLooong commented Aug 15, 2024 •

edited

Loading

LongCatIsLooong commented Aug 20, 2024 •

edited

Loading

stuartmorgan-g Aug 27, 2024 •

edited

Loading