add option for bulk-updating screenshots; update screenshots by yjbanov · Pull Request #12797 · flutter/engine

yjbanov · 2019-10-04T17:55:44Z

You can now run felt test --update-screenshot-goldens to update screenshots. This option composes with other felt test options.

yjbanov · 2019-10-04T17:56:00Z

Marking as WIP because it depends on flutter/goldens#51

ditman

Thanks for implementing this, very useful!

ditman · 2019-10-04T18:10:13Z

lib/web_ui/dev/test_platform.dart

+      p.fromUri(await Isolate.resolvePackageUri(
+          Uri.parse('package:test/src/runner/browser/static/favicon.ico'))),
+      root: root,
+      isUpdateScreenshotGoldens: isUpdateScreenshotGoldens,


I know about the styleguide but... shouldUpdateScreenshotGoldens?

should could mean there's possibility that it won't update anything. How about doUpdateScreenshotGoldens?

I was trying to limit to [is, has, can, should], but yeah, doUpdateScreenshotGoldens or even just updateScreenshotGoldens sounds a little bit better (to me)

+1 to bool updateScreenshotGoldens

updateScreenshotGoldens sounds too much like a method name. I'll go with do. I think anything that sounds like a statement that could be true or false is OK. is, has, can, should, did, was, must, all sound fine to me.

I draw the line on archaisms though. shallWeUpdateScreenshotGoldens is a no-go :)

I know my comment isn't helping, it's only adding to the bike-shedding. But doUpdateScreenshotGoldens still sounds like a method name to me.

wouldYouBeSoKindToUpdateScreenshotGoldens

lib/web_ui/dev/test_platform.dart

lib/web_ui/dev/test_runner.dart

lib/web_ui/test/golden_tests/golden_success_smoke_test.dart

ditman · 2019-10-04T19:37:14Z

lib/web_ui/dev/test_runner.dart

-  final String destinationAhemTtfPath =
-      path.join(environment.webUiRootDir.path, 'lib', 'assets', 'ahem.ttf');
-  sourceAhemTtf.copySync(destinationAhemTtfPath);
+const List<String> _kTestFonts = <String>['ahem.ttf', 'Roboto-Light.ttf'];


Roboto Light is quite thin, so this has a small risk of having extra antialias problems (but it'll be a cool test for the fuzzy comparison!)

For some reason I thought light was the default. Strangely the tests passed locally. I would expect them to fail if I picked a wrong font. Trying Roboto-Regular.

Of course, it would help if the smoke test used the Flutter engine at all! LOL. Fixing.

yjbanov · 2019-10-04T20:42:01Z

I switched to a bundled Roboto and to flutter/golden master. Will land on green unless told otherwise.

mdebbar · 2019-10-04T20:49:25Z

lib/web_ui/dev/test_runner.dart

    Chrome.version = chromeVersion;

-    _copyAhemFontIntoWebUi();
+    _copyTestFontsIntoWebUi();


ditman · 2019-10-04T21:09:50Z

lib/web_ui/test/golden_tests/golden_success_smoke_test.dart

-void main() {
+void main() async {
+  debugEmulateFlutterTesterEnvironment = true;
+  await webOnlyInitializePlatform(assetManager: WebOnlyMockAssetManager());


(I guess there's some styles in the platform init that causes the color of the font to become red)

Other than the above, LGTM!

yjbanov · 2019-10-04T22:06:55Z

Landing on red LUCI. The LUCI failure is Android AOT on Windows, which is unrelated to this change.

…lutter/engine#12797)

[email protected]:flutter/engine.git/compare/1d62160fdb2f...4a849e0 git log 1d62160..4a849e0 --no-merges --oneline 2019-10-08 [email protected] Color matrix doc (flutter/engine#12982) 2019-10-07 [email protected] Use the standard gen_snapshot target unless the platform requires host_targeting_host (flutter/engine#12988) 2019-10-07 [email protected] Roll src/third_party/dart 8413a0db0d..8ba6f7e2eb (39 commits) (flutter/engine#12981) 2019-10-07 [email protected] Unblock Fuchsia roll (flutter/engine#12977) 2019-10-06 [email protected] Update buildroot to pull in ubsan updates. (flutter/engine#12821) 2019-10-05 [email protected] Roll src/third_party/skia 95edac1c9a4a..4c82a9fc83a5 (13 commits) (flutter/engine#12818) 2019-10-05 [email protected] Enable sanitizer build variants. (flutter/engine#12816) 2019-10-05 [email protected] Revert "Adding Link SemanticsFlag (#12453)" (flutter/engine#12815) 2019-10-04 [email protected] Use the x64 host toolchain for x86 target gen_snapshot only on Linux (flutter/engine#12809) 2019-10-04 [email protected] add option for bulk-updating screenshots; update screenshots (flutter/engine#12797) 2019-10-04 [email protected] unbreak unopt fuchsia (flutter/engine#12805) 2019-10-04 [email protected] Build gen_snapshot with a 64-bit host toolchain even if the target platform is 32-bit (flutter/engine#12802) 2019-10-04 [email protected] [flutter_runner] a11y updates, tests! (flutter/engine#12380) 2019-10-04 [email protected] Fix crash in copypixelbuffer (flutter/engine#10326) 2019-10-04 [email protected] Roll src/third_party/dart d6c6d12ebf..8413a0db0d (2 commits) 2019-10-04 [email protected] Roll fuchsia/sdk/core/mac-amd64 from JyZWz... to kwa2O... (flutter/engine#12803) 2019-10-04 [email protected] Adding Link SemanticsFlag (flutter/engine#12453) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md

[email protected]:flutter/engine.git/compare/1d62160fdb2f...4a849e0 git log 1d62160..4a849e0 --no-merges --oneline 2019-10-08 [email protected] Color matrix doc (flutter/engine#12982) 2019-10-07 [email protected] Use the standard gen_snapshot target unless the platform requires host_targeting_host (flutter/engine#12988) 2019-10-07 [email protected] Roll src/third_party/dart 8413a0db0d..8ba6f7e2eb (39 commits) (flutter/engine#12981) 2019-10-07 [email protected] Unblock Fuchsia roll (flutter/engine#12977) 2019-10-06 [email protected] Update buildroot to pull in ubsan updates. (flutter/engine#12821) 2019-10-05 [email protected] Roll src/third_party/skia 95edac1c9a4a..4c82a9fc83a5 (13 commits) (flutter/engine#12818) 2019-10-05 [email protected] Enable sanitizer build variants. (flutter/engine#12816) 2019-10-05 [email protected] Revert "Adding Link SemanticsFlag (flutter#12453)" (flutter/engine#12815) 2019-10-04 [email protected] Use the x64 host toolchain for x86 target gen_snapshot only on Linux (flutter/engine#12809) 2019-10-04 [email protected] add option for bulk-updating screenshots; update screenshots (flutter/engine#12797) 2019-10-04 [email protected] unbreak unopt fuchsia (flutter/engine#12805) 2019-10-04 [email protected] Build gen_snapshot with a 64-bit host toolchain even if the target platform is 32-bit (flutter/engine#12802) 2019-10-04 [email protected] [flutter_runner] a11y updates, tests! (flutter/engine#12380) 2019-10-04 [email protected] Fix crash in copypixelbuffer (flutter/engine#10326) 2019-10-04 [email protected] Roll src/third_party/dart d6c6d12ebf..8413a0db0d (2 commits) 2019-10-04 [email protected] Roll fuchsia/sdk/core/mac-amd64 from JyZWz... to kwa2O... (flutter/engine#12803) 2019-10-04 [email protected] Adding Link SemanticsFlag (flutter/engine#12453) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md

add option for bulk-updating screenshots; update screenshots

ef08427

yjbanov requested review from ditman, ferhatb and mdebbar October 4, 2019 17:55

googlebot added the cla: yes label Oct 4, 2019

yjbanov added the Work in progress (WIP) Not ready (yet) for review! label Oct 4, 2019

ditman approved these changes Oct 4, 2019

View reviewed changes

use bundled Roboto font

e2da9ae

ditman reviewed Oct 4, 2019

View reviewed changes

yjbanov added 3 commits October 4, 2019 12:46

switch to Roboto-Regular

39ac372

make golden smoke test use the engine

dc5c289

use the real SHA

6a99791

doUpdateScreenshotGoldens

427a000

mdebbar approved these changes Oct 4, 2019

View reviewed changes

lib/web_ui/dev/test_runner.dart

Chrome.version = chromeVersion;

_copyAhemFontIntoWebUi();

_copyTestFontsIntoWebUi();

Copy link

Contributor

mdebbar Oct 4, 2019

Choose a reason for hiding this comment

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

👍

ditman reviewed Oct 4, 2019

View reviewed changes

yjbanov merged commit bba0065 into flutter:master Oct 4, 2019

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

74cf864

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42114

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

b5bdc20

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42116

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

642ecdb

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42117

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

e50d944

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42119

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

d0e9e5a

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42120

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

c93cf9f

…lutter/engine#12797)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

21a65f1

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42171

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

fff27f5

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42173

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

3af3073

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42177

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

6986fb9

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42180

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 7, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

bd165fe

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 7, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42181

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

dd4e7c3

…lutter/engine#12797)

This was referenced Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42184

Closed

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42185

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

d7d1b02

…lutter/engine#12797)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

a5c2f97

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42188

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

b8234d0

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42191

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

e615794

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42193

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

f0fc6a9

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42195

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

70e2072

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42196

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

e4a88df

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..b785ab902a5e (13 commits) flutter/flutter#42199

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 8, 2019

bba0065 add option for bulk-updating screenshots; update screenshots (f…

6f5ba90

…lutter/engine#12797)

engine-flutter-autoroll mentioned this pull request Oct 8, 2019

Roll engine 1d62160fdb2f..4a849e0f8dd5 (17 commits) flutter/flutter#42200

Merged

Conversation

yjbanov commented Oct 4, 2019

Uh oh!

yjbanov commented Oct 4, 2019

Uh oh!

ditman 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!

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!

Uh oh!

Uh oh!

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!

yjbanov commented Oct 4, 2019

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!

yjbanov commented Oct 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants