Add a golden scenario test for fallback font rendering on iOS take 2 #22033

xster · 2020-10-21T17:02:53Z

Didn't have time to revise #20687 and merge it before. This would have actually helped catch flutter/flutter#68399 too I believe.

Original PR relates to flutter/flutter#60013. If user specifies a font that doesn't exist, it should properly fallback to the SF font and render it correctly.

Also removing the duplicate iPhone SE images as discussed with @blasten and refactor a modular golden test part out of the platform view stuff.

Also addresses flutter/flutter#68625.

xster · 2020-10-21T17:07:30Z

I think the previous pre-submit failures were due to our LUCI bots re-running the golden generation on iOS 13 whereas I made it on iOS 14. Might have to download an Xcode 11 copy again.

blasten · 2020-10-21T17:36:05Z

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

+    XCTAttachment* screenshotAttachment;
+    screenshotAttachment = [XCTAttachment attachmentWithImage:screenshot.image];


nit: these two can be in a single line

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

blasten · 2020-10-21T17:37:53Z

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

+  if (![_goldenImage compareGoldenToImage:screenshot.image]) {
+    XCTAttachment* screenshotAttachment;
+    screenshotAttachment = [XCTAttachment attachmentWithImage:screenshot.image];
+    screenshotAttachment.name = _goldenImage.goldenName;


maybe format this as make it _goldenImage.goldenName + "_current"

called it "actual" to match testing terminology

blasten · 2020-10-21T17:38:11Z

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

+  XCUIScreenshot* screenshot = [[XCUIScreen mainScreen] screenshot];
+  if (!_goldenImage.image) {
+    XCTAttachment* attachment = [XCTAttachment attachmentWithScreenshot:screenshot];
+    attachment.name = @"new_golden";


similar: maybe format this as make it _goldenImage.goldenName + "_new"

sure, good idea

blasten · 2020-10-21T17:39:24Z

...ing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/xcshareddata/xcschemes/Scenarios.xcscheme

-            BlueprintName = "Scenarios"
-            ReferencedContainer = "container:Scenarios.xcodeproj">
-         </BuildableReference>
-      </MacroExpansion>


what does this do?

I think I stopped using a build var in the argument scheme somewhere. This whole thing is just for facilitating local test runs inside Xcode for debug and doesn't affect anything.

blasten

LGTM + nits

xster

I have no idea how but I weaksauced the last PR which wasn't passing because the golden screenshot I saved was in the wrong dpi.

xster · 2020-10-21T18:23:21Z

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

+  XCUIScreenshot* screenshot = [[XCUIScreen mainScreen] screenshot];
+  if (!_goldenImage.image) {
+    XCTAttachment* attachment = [XCTAttachment attachmentWithScreenshot:screenshot];
+    attachment.name = @"new_golden";


sure, good idea

xster · 2020-10-21T18:26:14Z

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

+  if (![_goldenImage compareGoldenToImage:screenshot.image]) {
+    XCTAttachment* screenshotAttachment;
+    screenshotAttachment = [XCTAttachment attachmentWithImage:screenshot.image];
+    screenshotAttachment.name = _goldenImage.goldenName;


called it "actual" to match testing terminology

xster · 2020-10-21T20:24:30Z

oops, forgot the tree is closed

xster · 2020-10-22T18:02:05Z

Tests are crapping out probably due to some transient nnbd issue. Rebasing

chinmaygarde · 2020-10-22T22:11:21Z

There are conflicts to the auto-merge bot won't land this. @xster can you please rebase? The tree is open now.

… take 2 (flutter/engine#22033)

* 000bf4b Roll Skia from 2d2f82c00aeb to 5c7bb326a7b3 (33 revisions) (flutter/engine#22059) * ae92dbf Roll Fuchsia Linux SDK from lPMs_KwnU... to gqS_DIjN4... (flutter/engine#22057) * 92cd74e Roll Fuchsia Mac SDK from pZ9FgVZTK... to WLxBkBnZa... (flutter/engine#22055) * e51c710 Roll Dart SDK from a3d902d8598e to 9f907e198970 (2 revisions) (flutter/engine#22058) * 326b202 Reland fuchsia external view embedder will be shared with platform view (flutter/engine#22008) * a9a9a2f Roll Skia from 5c7bb326a7b3 to 65674e4c2e56 (3 revisions) (flutter/engine#22060) * 1233fe4 Revert "Revert "Explicitly make the X connection for EGL. (#21831)" (#21851)" (flutter/engine#21871) * aed8e01 Fixes Edge trigger route change announcement (flutter/engine#21975) * 6bc70e4 Reland: Migration to PlatformDispatcher and multi-window (flutter/engine#21932) * 5ca5e26 Add FlEventChannel (flutter/engine#21316) * 77b0052 Roll Skia from 65674e4c2e56 to 01b05e5b830b (3 revisions) (flutter/engine#22062) * 3d27fd5 Support loading assets from Android dynamic feature modules (flutter/engine#21504) * 742dfbe support uri intent launcher in android (flutter/engine#21275) * cde1e3f Auto detect mode to determine which rendering backend to use. (flutter/engine#21852) * 329ccf7 Roll Skia from 01b05e5b830b to 53281c712159 (1 revision) (flutter/engine#22065) * cde78c1 Add a golden scenario test for fallback font rendering on iOS take 2 (flutter/engine#22033) * 4f4599b Roll Dart SDK from 9f907e198970 to 37ccceacad41 (3 revisions) (flutter/engine#22069) * f0b10c5 [web] Prevent using DOM nodes for canvas with large number of draws (flutter/engine#22064) * a86ba57 Roll Fuchsia Mac SDK from WLxBkBnZa... to zDfaxkqlv... (flutter/engine#22073) * 645198a Roll Fuchsia Linux SDK from gqS_DIjN4... to vuKxZmSVj... (flutter/engine#22074) * 0b26570 Revert dart rolls (flutter/engine#22078)

… take 2 (#22033)" This reverts commit cde78c1.

… take 2 (#22033)" (#22095) This reverts commit cde78c1.

…lutter#22033)

… take 2 (flutter#22033)" (flutter#22095) This reverts commit cde78c1.

googlebot added the cla: yes label Oct 21, 2020

xster changed the title ~~Add a golden scenario test for fallback font rendering on iOS~~ Add a golden scenario test for fallback font rendering on iOS take 2 Oct 21, 2020

xster requested review from blasten and iskakaushik October 21, 2020 17:04

blasten reviewed Oct 21, 2020

View reviewed changes

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m Outdated Show resolved Hide resolved

blasten reviewed Oct 21, 2020

View reviewed changes

blasten approved these changes Oct 21, 2020

View reviewed changes

xster commented Oct 21, 2020

View reviewed changes

xster added waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. and removed waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. labels Oct 21, 2020

xster force-pushed the font-fallback-golden branch from 2080e89 to dd9c63b Compare October 22, 2020 18:03

xster added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Oct 22, 2020

xster added 4 commits October 22, 2020 15:27

Add a golden scenario test for fallback font rendering on iOS

fb80ad7

autoformat

9a8d741

review and fix broken test

301eba3

autoformat

f18ef54

xster force-pushed the font-fallback-golden branch from dd9c63b to f18ef54 Compare October 22, 2020 22:27

merge some issues with the multi-window refactor

4229866

fluttergithubbot merged commit cde78c1 into flutter:master Oct 23, 2020

xster deleted the font-fallback-golden branch October 23, 2020 03:38

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to 4f4599b269c5 (17 revisions) flutter/flutter#68852

Closed

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

cde78c1 Add a golden scenario test for fallback font rendering on iOS…

2f9bf5e

… take 2 (flutter/engine#22033)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to f0b10c54bf68 (18 revisions) flutter/flutter#68859

Closed

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

cde78c1 Add a golden scenario test for fallback font rendering on iOS…

e97f532

… take 2 (flutter/engine#22033)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to 645198aa72c1 (20 revisions) flutter/flutter#68863

Closed

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

cde78c1 Add a golden scenario test for fallback font rendering on iOS…

6794a41

… take 2 (flutter/engine#22033)

engine-flutter-autoroll mentioned this pull request Oct 23, 2020

Roll Engine from defa8be2b106 to 0b26570e933f (21 revisions) flutter/flutter#68897

Merged

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

cde78c1 Add a golden scenario test for fallback font rendering on iOS…

fb59864

… take 2 (flutter/engine#22033)

dnfield added a commit that referenced this pull request Oct 24, 2020

Revert "Add a golden scenario test for fallback font rendering on iOS…

0ea807f

… take 2 (#22033)" This reverts commit cde78c1.

dnfield mentioned this pull request Oct 24, 2020

Revert "Add a golden scenario test for fallback font rendering on iOS take 2" #22095

Merged

dnfield added a commit that referenced this pull request Oct 24, 2020

Revert "Add a golden scenario test for fallback font rendering on iOS…

daa0f7a

… take 2 (#22033)" (#22095) This reverts commit cde78c1.

chaselatta pushed a commit to chaselatta/engine that referenced this pull request Nov 30, 2020

Add a golden scenario test for fallback font rendering on iOS take 2 (f…

dd0ccbe

…lutter#22033)

chaselatta pushed a commit to chaselatta/engine that referenced this pull request Nov 30, 2020

Revert "Add a golden scenario test for fallback font rendering on iOS…

1084177

… take 2 (flutter#22033)" (flutter#22095) This reverts commit cde78c1.

jmagman mentioned this pull request Oct 11, 2022

[Impeller] testFontRenderingWhenSuppliedWithBogusFont scenario test fails with impeller turned on, does not render bogus font as system font flutter/flutter#113250

Closed

		XCTAttachment* screenshotAttachment;
		screenshotAttachment = [XCTAttachment attachmentWithImage:screenshot.image];

Add a golden scenario test for fallback font rendering on iOS take 2 #22033

Add a golden scenario test for fallback font rendering on iOS take 2 #22033

Uh oh!

Conversation

xster commented Oct 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xster commented Oct 21, 2020 • 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!

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!

blasten left a comment

Choose a reason for hiding this comment

Uh oh!

xster left a comment • 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!

xster commented Oct 21, 2020

Uh oh!

xster commented Oct 22, 2020

Uh oh!

chinmaygarde commented Oct 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xster commented Oct 21, 2020 •

edited

Loading

xster commented Oct 21, 2020 •

edited

Loading

xster left a comment •

edited

Loading