From d877b830cb811930871a032d217f65e9df57aa91 Mon Sep 17 00:00:00 2001 From: nturgut Date: Fri, 25 Sep 2020 12:30:57 -0700 Subject: [PATCH] [web] enable ios safari screenshot tests (#21226) * enable ios safari screenshot tests * this is the value used for screenshots in the repo. * change revision * fix the error made in the gaps * do not try to fetch on LUCI * lunix luci migth break existing prs. only skip fetching for mac * add a placeholder tests for shadow golden test * try to use iPhone 11 Pro on LUCI * change the scale and the smoke test image * add unmerged goldens PR to tests all the screenshots. will be removed later * change the goldens lock back to flutter/goldens * change wrong comment on screenshot tests block * address reviewer comments * change the commit number for goldens file * skip canvas blend mode tests * debugging LUCI error * debugging LUCI error printing directory contents * skip one test, remove the debug logs * change the revision number to include the correct chrome files --- lib/web_ui/dev/browser_lock.yaml | 5 +++-- lib/web_ui/dev/goldens_lock.yaml | 2 +- lib/web_ui/dev/test_platform.dart | 14 ++++++++++++-- lib/web_ui/dev/test_runner.dart | 9 +++++---- .../golden_files/smoke_test.iOS_Safari.png | Bin 2517 -> 1839 bytes .../engine/canvas_image_blend_mode_test.dart | 4 +++- .../engine/shadow_golden_test.dart | 14 +++++++++++++- .../engine/text_placeholders_test.dart | 10 +++++++++- 8 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/web_ui/dev/browser_lock.yaml b/lib/web_ui/dev/browser_lock.yaml index 09f2f56ab56..b1a83fe5869 100644 --- a/lib/web_ui/dev/browser_lock.yaml +++ b/lib/web_ui/dev/browser_lock.yaml @@ -34,7 +34,7 @@ ios-safari: # `heightOfHeader` is the number of pixels taken by the phone's header menu # and the browsers address bar. # TODO: https://github.com/flutter/flutter/issues/65672 - heightOfHeader: 282 + heightOfHeader: 189 # `heightOfFooter` is the number of pixels taken by the phone's navigation # menu. heightOfFooter: 250 @@ -45,7 +45,8 @@ ios-safari: # phone screeen we enlarge or shrink the area by applying a linear # transformation by a scale_factor (a.k.a. we perform isotropic scaling). # This value will be differ depending on the phone. - scaleFactor: 1.15 + # For iOS 13.0 iPhone 11 Pro, this number is 1.15. + scaleFactor: 1.00 ## geckodriver is used for testing Firefox Browser. It works with multiple ## Firefox Browser versions. diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index 1c38c1ca3a3..067577a642f 100644 --- a/lib/web_ui/dev/goldens_lock.yaml +++ b/lib/web_ui/dev/goldens_lock.yaml @@ -1,2 +1,2 @@ repository: https://github.com/flutter/goldens.git -revision: b6efc75885c23f0b5c485d8bd659ed339feec9ec +revision: 1a4722227af42c3f51450266016b1a07ae459e73 diff --git a/lib/web_ui/dev/test_platform.dart b/lib/web_ui/dev/test_platform.dart index cc0a5d58fed..18dd92cf1c4 100644 --- a/lib/web_ui/dev/test_platform.dart +++ b/lib/web_ui/dev/test_platform.dart @@ -197,7 +197,17 @@ class BrowserPlatform extends PlatformPlugin { 'golden_files', ); } else { - await fetchGoldens(); + // On LUCI MacOS bots the goldens are fetched by the recipe code. + // Fetch the goldens if: + // - Tests are running on a local machine. + // - Tests are running on an OS other than macOS. + if (!isLuci || !Platform.isMacOS) { + await fetchGoldens(); + } else { + if (!env.environment.webUiGoldensRepositoryDirectory.existsSync()) { + throw Exception('The goldens directory must have been copied'); + } + } goldensDirectory = p.join( env.environment.webUiGoldensRepositoryDirectory.path, 'engine', @@ -212,7 +222,7 @@ class BrowserPlatform extends PlatformPlugin { )); if (!file.existsSync() && !write) { return ''' -Golden file $filename does not exist. +Golden file $filename does not exist on path ${file.absolute.path} To automatically create this file call matchGoldenFile('$filename', write: true). '''; diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index e459a0c0efc..5ad1b3b53df 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -397,12 +397,13 @@ class TestCommand extends Command with ArgUtils { 'test', )); - // Screenshot tests and smoke tests only run on: "Chrome/iOS Safari locally" - // or "Chrome on a Linux bot". We can remove the Linux bot restriction - // after solving the git issue faced on macOS and Windows bots: + // Screenshot tests and smoke tests only run on: "Chrome/iOS Safari" + // locally and on LUCI. They are not available on Windows bots: // TODO: https://github.com/flutter/flutter/issues/63710 if ((isChrome && isLuci && io.Platform.isLinux) || - ((isChrome || isSafariIOS) && !isLuci)) { + ((isChrome || isSafariIOS) && !isLuci) || + (isSafariIOS && isLuci)) { + print('INFO: Also running the screenshot tests.'); // Separate screenshot tests from unit-tests. Screenshot tests must run // one at a time. Otherwise, they will end up screenshotting each other. // This is not an issue for unit-tests. diff --git a/lib/web_ui/test/golden_files/smoke_test.iOS_Safari.png b/lib/web_ui/test/golden_files/smoke_test.iOS_Safari.png index bd156b44e38a51010202fd0d48726cc36b6e8786..7f2995f12a42c8a2a0664bb68447f123a560c6b9 100644 GIT binary patch literal 1839 zcmeH||5K6&0LEXPg>^M+ZIvbBEaxiIT~?ZksArk8q$7b`DjMe3l*kb#BD&_%MS5GM zeKXIy6^mCubV`b-O(#x8uk;m3@vTfv&=3(wzOH{_zxB&=&)w&@=RS8&`Kj2bjq7); z2LNDW^zld>062tw?0wE_KjvMhKo9`d8loe^$gWXpNH2eW}FmzO3O}p?(r(^yVn>pY}Mz{WNcTNF0y}4&&{;? z+hL19-R3f7tz!Jks@hu|7Ne4rK)$3!^~X&vA?ZrNoK^~skvg;P=WMm9#TtIW>I^rV z|D?^bY+8)pmat^(RIi4ZGN5i3qVmez0!)X8(V{Gn$Bhl4vM0`6*RF{?i^JFxXtQ_m zQOHBV#iB`EvzpIx@6vRDrZ?BAR=s>zvHcdR=ueYHx2j>To`*zm#rDhf*uZe0R^(gDYD>$3# zuyAIvlLdvE<0PgmsO}me@v`FYX_zU>8b*VLm)QxFrzy})?2x-|&>t`8OroiIcvMkf zud#_fDOC0E0||*VrzML{iW8H>)$<}Hf!6mm-X-L_UY;lk%H-gAf4x{ypqO#ZBeD32 z93o94!TU)L^3O^s9m$T=$6n(MjUwe;*~%cVhI$M-x(#1bcnKns@Hyff=ux$=Wi$y# z%1?pf)A82jfwJd2DQwrt9tT8RBK4oAMGHb+_x z$aiYcA27^wqv7FneEz5lTA^KR`73joRSDNHGHS0NHgi0NK=Tgi)`>uYci=U{Q#zwS@;F z@<&dR*G*`%k#Ff!t}$a~O}ZV~Cj>H}1U?zlFcd^)QO;fRc)_4& z3(#paf0xgLLM5p+VD~6hjqWOc_)NxDr%sk22ueD@CamFvt^l_Hb%XNy{7Kl1?IL3RVq+= zfp4_uc3LaRjI?oc`CLta?R`Tx*S9q_h%rg76k^8x`=@rcug-G!i`;}s$1$8*hCFfl z3VK?t$kI3%BsL$k&gsn!U`VukA$~0NF%&aGu0;BCLLv0*9qv~|H>0;Xlc_Ap|r1pGJ!B~Uigd%;|oxbvX8r0+3 z((5s{K=Y6J%dJ}rFk!dsiwBd_ift8ImcD7WS5S^x3YOM|`8YV8Tj#MU1}Hh>=-l%~ sm`4@hD0JFX=2iR&eOlxHbHl#o&h35gmbzvbACC}-J{lXzk4XOMKVM8MNB{r; literal 2517 zcmeHJ|2xx*0$wCHF2|m3N{Ws>a-I?u$E=0hv2m0y+c_@fYvE957#7ws-|ChnwnK_# zZ5+pOPq9&&!-muc%h)0Z*Ji$qed;pvZMHGn?ff78@;>kT)B8N{^S;mX-opg@TbLa( zgFqk_0pFs&gFtqrec}s$+w%!8^fu-|AbaTnsPm!3T?$DOIeNjmV78vJKd1jzu|b@z(EKQ)#_p&CiEw;)zZZ)Bt7I5rKnj~}m z+|jp#^cQM|jws3=(^HJ&`zc2bKq}l$B@R}7%(Z|778YL57!~pVM`CH}z z0#|mnV$lL#GJ#{0@4FONUH|R(8z0~EP3=*QoCx4Z zHfo;xpq8bk`Z$C|ot}Gb4x@N^%vHuZxOBS5aTN|@v4(s@_HMoFhLK)~^flGhS2_s2 zk#jY-{p?n3g7)a>gh?btHMTG9N@!p;bf6?rP7^a<44%nl8&}l^UVhbIii>}!Q!2Y* zS(4Lnh~oNIn_x9jd-R!f0<>-CCKrgj^4|s@Cja@$h zing|+K3muBYyX|6sXem2@UY1>iBN;drMY*H6_h{|HYu%2|74)UX(rHy;IJy8_@m*a!(gu$N|_;ZD( zd)N`&765Q=#Y8HS4sL@}`0gnReAe7IspyCUi+5&jwn%J-l+i_^o20H+Jax={k{!(7 zNt-0>aT}e=lq1#_>nrWFVcqA#x~>HUvUux~i_TZgzdZHhF`pU4Ga)evMKFEx! zG+NywUe{s4yi2)+L)0!Pa@yT~_;t1Oa-JM-7d!g9KarG`bJMcKO{Mvb zuM4s7j2Q7{TDk2O)JAw1cH?R^ahAhAd;1YLECbCIo=q{C>_y_g*%Xx!T1UFicI&su z?}Kr}L$(d=4qeL~0ZK>F&eW6zUtbVQTAiVGf~p~pNI(m=&4AoH5**{IGOv<1OOw&_ z5dFewIJ0_f+Of?`?t~?5U(r(lKNumn5ju_=^(H znS#~2in?Mt8a(bE&a~m`nc#nZiOsvzU;q(jZ$HShfT&^lIP9JjGhQBF#qPG-(iYiG zrY2ivQ&L65Zpmsn7q?|!tvr-r$=L37o7BFDr+oA}7F*-3zw}2T_D|c|KxgkaJI;7% zX~Lo;O(ar=EEzQLk@%qhO*V`nwx-d6nuDu*{Ai^D2zsyQKe0xZn2G}9e>l#3mY>u3 fT!z24pmH6L>U8Ko)jaU&u0aCO!6?=@S8n|uEz@*? diff --git a/lib/web_ui/test/golden_tests/engine/canvas_image_blend_mode_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_image_blend_mode_test.dart index 962d930d888..e5581be9668 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_image_blend_mode_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_image_blend_mode_test.dart @@ -108,7 +108,9 @@ void testMain() async { rc.restore(); await _checkScreenshot(rc, 'canvas_image_blend_group$blendGroup', maxDiffRatePercent: 8.0); - }); + }, + skip: browserEngine == BrowserEngine.webkit && + operatingSystem == OperatingSystem.iOs); } // Regression test for https://github.com/flutter/flutter/issues/56971 diff --git a/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart b/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart index 307cdb583ca..1822bc1c3e9 100644 --- a/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart @@ -75,7 +75,8 @@ void testMain() async { builder.pop(); // offset } - void _paintBitmapCanvasShadow(double elevation, Offset offset, bool transparentOccluder) { + void _paintBitmapCanvasShadow( + double elevation, Offset offset, bool transparentOccluder) { final SurfacePath path = SurfacePath() ..addRect(const Rect.fromLTRB(0, 0, 20, 20)); builder.pushOffset(offset.dx, offset.dy); @@ -168,4 +169,15 @@ void testMain() async { }, testOn: 'chrome', ); + + /// For dart testing having `no tests ran` in a file is considered an error + /// and result in exit code 1. + /// See: https://github.com/dart-lang/test/pull/1173 + /// + /// Since screenshot tests run one by one and exit code is checked immediately + /// after that a test file that only runs in chrome will break the other + /// browsers. This method is added as a bandaid solution. + test('dummy tests to pass on other browsers', () async { + expect(2 + 2, 4); + }); } diff --git a/lib/web_ui/test/golden_tests/engine/text_placeholders_test.dart b/lib/web_ui/test/golden_tests/engine/text_placeholders_test.dart index eece5fcd5e2..8b46adcb384 100644 --- a/lib/web_ui/test/golden_tests/engine/text_placeholders_test.dart +++ b/lib/web_ui/test/golden_tests/engine/text_placeholders_test.dart @@ -15,11 +15,17 @@ void main() { internalBootstrapBrowserTest(() => testMain); } +/// Whether we are running on iOS Safari. +// TODO: https://github.com/flutter/flutter/issues/66656 +bool get isIosSafari => browserEngine == BrowserEngine.webkit && + operatingSystem == OperatingSystem.iOs; + void testMain() async { final EngineScubaTester scuba = await EngineScubaTester.initialize( viewportSize: const Size(600, 600), ); + setUpStableTestFonts(); testEachCanvas('draws paragraphs with placeholders', (EngineCanvas canvas) { @@ -40,7 +46,9 @@ void testMain() async { } recordingCanvas.endRecording(); recordingCanvas.apply(canvas, screenRect); - return scuba.diffCanvasScreenshot(canvas, 'text_with_placeholders'); + if (!isIosSafari) { + return scuba.diffCanvasScreenshot(canvas, 'text_with_placeholders'); + } }); testEachCanvas('text alignment and placeholders', (EngineCanvas canvas) {