add option for bulk-updating screenshots; update screenshots #12797

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

@@ -47,7 +54,7 @@ class TestsCommand extends Command<bool> {
  Future<bool> run() async {
    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)

git@github.com:flutter/engine.git/compare/1d62160fdb2f...4a849e0 git log 1d62160..4a849e0 --no-merges --oneline 2019-10-08 dnfield@google.com Color matrix doc (flutter/engine#12982) 2019-10-07 jason-simmons@users.noreply.github.com Use the standard gen_snapshot target unless the platform requires host_targeting_host (flutter/engine#12988) 2019-10-07 bkonyi@google.com Roll src/third_party/dart 8413a0db0d..8ba6f7e2eb (39 commits) (flutter/engine#12981) 2019-10-07 liyuqian@google.com Unblock Fuchsia roll (flutter/engine#12977) 2019-10-06 chinmaygarde@google.com Update buildroot to pull in ubsan updates. (flutter/engine#12821) 2019-10-05 skia-flutter-autoroll@skia.org Roll src/third_party/skia 95edac1c9a4a..4c82a9fc83a5 (13 commits) (flutter/engine#12818) 2019-10-05 chinmaygarde@google.com Enable sanitizer build variants. (flutter/engine#12816) 2019-10-05 katelovett@google.com Revert "Adding Link SemanticsFlag (#12453)" (flutter/engine#12815) 2019-10-04 jason-simmons@users.noreply.github.com Use the x64 host toolchain for x86 target gen_snapshot only on Linux (flutter/engine#12809) 2019-10-04 yjbanov@google.com add option for bulk-updating screenshots; update screenshots (flutter/engine#12797) 2019-10-04 dnfield@google.com unbreak unopt fuchsia (flutter/engine#12805) 2019-10-04 jason-simmons@users.noreply.github.com Build gen_snapshot with a 64-bit host toolchain even if the target platform is 32-bit (flutter/engine#12802) 2019-10-04 dnfield@google.com [flutter_runner] a11y updates, tests! (flutter/engine#12380) 2019-10-04 WATER1350@gmail.com Fix crash in copypixelbuffer (flutter/engine#10326) 2019-10-04 bkonyi@google.com Roll src/third_party/dart d6c6d12ebf..8413a0db0d (2 commits) 2019-10-04 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from JyZWz... to kwa2O... (flutter/engine#12803) 2019-10-04 katelovett@google.com 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 liyuqian@google.com 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

git@github.com:flutter/engine.git/compare/1d62160fdb2f...4a849e0 git log 1d62160..4a849e0 --no-merges --oneline 2019-10-08 dnfield@google.com Color matrix doc (flutter/engine#12982) 2019-10-07 jason-simmons@users.noreply.github.com Use the standard gen_snapshot target unless the platform requires host_targeting_host (flutter/engine#12988) 2019-10-07 bkonyi@google.com Roll src/third_party/dart 8413a0db0d..8ba6f7e2eb (39 commits) (flutter/engine#12981) 2019-10-07 liyuqian@google.com Unblock Fuchsia roll (flutter/engine#12977) 2019-10-06 chinmaygarde@google.com Update buildroot to pull in ubsan updates. (flutter/engine#12821) 2019-10-05 skia-flutter-autoroll@skia.org Roll src/third_party/skia 95edac1c9a4a..4c82a9fc83a5 (13 commits) (flutter/engine#12818) 2019-10-05 chinmaygarde@google.com Enable sanitizer build variants. (flutter/engine#12816) 2019-10-05 katelovett@google.com Revert "Adding Link SemanticsFlag (flutter#12453)" (flutter/engine#12815) 2019-10-04 jason-simmons@users.noreply.github.com Use the x64 host toolchain for x86 target gen_snapshot only on Linux (flutter/engine#12809) 2019-10-04 yjbanov@google.com add option for bulk-updating screenshots; update screenshots (flutter/engine#12797) 2019-10-04 dnfield@google.com unbreak unopt fuchsia (flutter/engine#12805) 2019-10-04 jason-simmons@users.noreply.github.com Build gen_snapshot with a 64-bit host toolchain even if the target platform is 32-bit (flutter/engine#12802) 2019-10-04 dnfield@google.com [flutter_runner] a11y updates, tests! (flutter/engine#12380) 2019-10-04 WATER1350@gmail.com Fix crash in copypixelbuffer (flutter/engine#10326) 2019-10-04 bkonyi@google.com Roll src/third_party/dart d6c6d12ebf..8413a0db0d (2 commits) 2019-10-04 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from JyZWz... to kwa2O... (flutter/engine#12803) 2019-10-04 katelovett@google.com 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 liyuqian@google.com 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, mdebbar and ferhatb 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

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

add option for bulk-updating screenshots; update screenshots #12797

add option for bulk-updating screenshots; update screenshots #12797

Uh oh!

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!

Uh oh!