Moved to RMSE for image comparison to account for slight variations in golden image tests #19658

gaaclarke · 2020-07-10T20:40:36Z

Description

There appears to be slight variations in golden images between different people's machines despite running on the same simulator os version. This will run a tight comparison to make sure the root-square error is less than 0.5.

Related Issues

flutter/flutter#56388

Tests

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the contributor guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the C++, Objective-C, Java style guides for the engine.
I read the tree hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation.
All existing and new tests are passing.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

golden image production.

fluttergithubbot · 2020-07-10T20:40:38Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

blasten · 2020-07-10T21:13:54Z

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

@@ -6,6 +6,8 @@
 #import <XCTest/XCTest.h>
 #include <sys/sysctl.h>

+static const double kRmseThreshold = 0.5;


how did you determinate this value?

I ran the tests on my machine without changes and that was the smallest threshold that that contains all the errors, plus it is a reasonably low error.

what would be an example of a false positive?

If you change https://github.com/flutter/engine/blob/master/testing/scenario_app/lib/src/platform_view.dart#L523 to 151 or one of the translates by 1 pixel, does the test pass?

It doesn't pass when i change it to 151. In that case the RMSE goes from 0.49 to 0.94.

RMSE factors in how big of a diff there is and is relative to the size of the images. So small discrepancies on large images make less of a difference.

gaaclarke · 2020-07-13T21:47:56Z

@blasten I started ignoring transparent pixels like we discussed offline Friday. I also had to tweak one of the assertions you added to your test which was flakey for jenn and I locally. Can you please check that out to make sure the test is still good. I think you just wanted to assert that you have 1 overlay which is inside of the view. You don't really care what the exact size is.

gaaclarke · 2020-07-13T21:48:45Z

@jmagman This should also fix the flake we were both seeing, I doubt it will do anything for flutter/flutter#61267 but maybe.

blasten

Thanks for fixing this!

…ations in golden image tests (flutter/engine#19658)

* 160b268 Set locale in Linux shell (flutter/engine#19470) * c479b93 Fix documentation of unset platform view ID (flutter/engine#19320) * f3ab78d Fix clipboard paste functionality not working (flutter/engine#19489) * 9345347 Added the ability to set properties in interface builder for FlutterViewController. (flutter/engine#19458) * 69fdf6d Use identical in hashValues instead of operator== (flutter/engine#19615) * 3dc8163 [android] Pass synthesized eventType to VirtualDisplay platform views and fix memory leak (flutter/engine#19620) * b16c47d using text capitalization value in web (flutter/engine#19564) * 49c0161 Roll Skia from 16bf7d31c819 to 4d48bb35972f (32 revisions) (flutter/engine#19635) * 2d6afa7 Roll Skia from 4d48bb35972f to 9f821489c9f3 (4 revisions) (flutter/engine#19638) * 35f59b9 Roll Fuchsia Linux SDK from NC9pp... to 5R9a0... (flutter/engine#19639) * 53038e2 Roll Fuchsia Mac SDK from iwQCA... to Wym-S... (flutter/engine#19640) * d9e68f4 Roll Skia from 9f821489c9f3 to f8a6b5b4b0d0 (1 revision) (flutter/engine#19643) * d329617 Changes to fix build errors in google3. (flutter/engine#19616) * dd0a6c4 Roll Skia from f8a6b5b4b0d0 to 5160e8caa226 (9 revisions) (flutter/engine#19647) * d98d539 Manual Roll of Dart dfdc7e45c5...06cb010247 (flutter/engine#19649) * dfd0727 don't throw error for the new autofill request (flutter/engine#19633) * 39e98d2 Manual Skia roll to c91db040ad18b9cc3236e342e9acca020eaafd10 (flutter/engine#19650) * f9acd08 Avoid a copy in EncodeImage (flutter/engine#19504) * 7e101f1 Roll Fuchsia Linux SDK from 5R9a0... to GOf1K... (flutter/engine#19656) * f3be9f1 Incorporate compat info changes into flutter engine (flutter/engine#19606) * 9188ff6 Manual Roll of Dart 707c6404f9...dfdc7e45c5 (flutter/engine#19657) * cbdd3e2 Roll Fuchsia Mac SDK from Wym-S... to -v4bL... (flutter/engine#19661) * 6efb152 Roll Dart SDK from 707c6404f969 to 0e25306d3f78 (4 revisions) (flutter/engine#19666) * 22a440c Roll Dart SDK from 0e25306d3f78 to 6d06476bae6b (1 revision) (flutter/engine#19669) * 8063923 Roll Fuchsia Linux SDK from GOf1K... to QWI76... (flutter/engine#19671) * c99deb0 CkPaint uses SkPaint (flutter/engine#19562) * 015f2ea Use the main bundle if the App bundle is not found (flutter/engine#18749) * f82d30b Roll Dart SDK from 6d06476bae6b to a9e67d81941a (3 revisions) (flutter/engine#19677) * 61b2fd2 Roll Fuchsia Mac SDK from -v4bL... to phQJt... (flutter/engine#19681) * 74b541d Roll Fuchsia Linux SDK from QWI76... to mrlGt... (flutter/engine#19682) * 25ce3db Roll Skia from c91db040ad18 to e4f36d7ac8f5 (20 revisions) (flutter/engine#19686) * 162dba1 Roll Skia from e4f36d7ac8f5 to ed15b1c39b8b (1 revision) (flutter/engine#19687) * f97c38b Roll Fuchsia Mac SDK from phQJt... to kEVc6... (flutter/engine#19688) * 40c4ee8 Roll Fuchsia Linux SDK from mrlGt... to YfVT0... (flutter/engine#19690) * ec944b0 Roll Skia from ed15b1c39b8b to 1434ce1aa94d (1 revision) (flutter/engine#19692) * 7ce988a Roll Skia from 1434ce1aa94d to f64be13cbf84 (2 revisions) (flutter/engine#19695) * 5f7ca41 Roll Fuchsia Mac SDK from kEVc6... to hRul_... (flutter/engine#19696) * d24549c Roll Skia from f64be13cbf84 to 041796e60364 (3 revisions) (flutter/engine#19697) * 617bd88 Roll Dart SDK from a9e67d81941a to fecc8163afc7 (3 revisions) (flutter/engine#19703) * 8d241e4 [Android] Prevent FlutterRenderer listener from calling JNI after detach (flutter/engine#19558) * 309e514 Roll Skia from 041796e60364 to d1ce4cb2beb8 (2 revisions) (flutter/engine#19704) * 211f18e Roll Skia from d1ce4cb2beb8 to 7c1967700b44 (6 revisions) (flutter/engine#19707) * 0532227 Roll Skia from 7c1967700b44 to 439709a97dfd (8 revisions) (flutter/engine#19712) * 1e02bfd Support decimal information on the TextInputType (flutter/engine#19664) * 7a95e32 Linux: Use a hash table to map cursors (flutter/engine#19561) * df23044 [web] Implement ulps for path ops (flutter/engine#19711) * 4a3aa4d Fixes typo in android_context_gl.h (flutter/engine#19700) * 5b966eb Roll Skia from 439709a97dfd to 2604a89d3353 (3 revisions) (flutter/engine#19715) * d024ae4 [fuchsia] Use memory_requirements_2 extension. (flutter/engine#19678) * 4392fbf Roll Fuchsia Linux SDK from YfVT0... to 2ct5j... (flutter/engine#19720) * ab05c79 Roll Fuchsia Mac SDK from hRul_... to ZXLvD... (flutter/engine#19721) * c05ca7e Roll Dart SDK from fecc8163afc7 to f997d62a6d29 (13 revisions) (flutter/engine#19722) * ae37971 Remove xcpretty from unit tests to see full output on test failures (flutter/engine#19667) * 91f80ef Moved to RMSE for image comparison to account for slight variations in golden image tests (flutter/engine#19658) * 1e23309 Wait for platform view to appear in iOS UI tests (flutter/engine#19725)

…n golden image tests (flutter#19658) Moved to RMSE for image comparison to account for slight variations in golden image production. (also fixed a flakey test)

* Update 1.20.2 engine to use Dart 2.9.1 * Use a single mask view to clip iOS platform view (#20050) * Moved to RMSE for image comparison to account for slight variations in golden image tests (#19658) Moved to RMSE for image comparison to account for slight variations in golden image production. (also fixed a flakey test) Co-authored-by: Chris Yang <ychris@google.com> Co-authored-by: gaaclarke <30870216+gaaclarke@users.noreply.github.com>

* 160b268 Set locale in Linux shell (flutter/engine#19470) * c479b93 Fix documentation of unset platform view ID (flutter/engine#19320) * f3ab78d Fix clipboard paste functionality not working (flutter/engine#19489) * 9345347 Added the ability to set properties in interface builder for FlutterViewController. (flutter/engine#19458) * 69fdf6d Use identical in hashValues instead of operator== (flutter/engine#19615) * 3dc8163 [android] Pass synthesized eventType to VirtualDisplay platform views and fix memory leak (flutter/engine#19620) * b16c47d using text capitalization value in web (flutter/engine#19564) * 49c0161 Roll Skia from 16bf7d31c819 to 4d48bb35972f (32 revisions) (flutter/engine#19635) * 2d6afa7 Roll Skia from 4d48bb35972f to 9f821489c9f3 (4 revisions) (flutter/engine#19638) * 35f59b9 Roll Fuchsia Linux SDK from NC9pp... to 5R9a0... (flutter/engine#19639) * 53038e2 Roll Fuchsia Mac SDK from iwQCA... to Wym-S... (flutter/engine#19640) * d9e68f4 Roll Skia from 9f821489c9f3 to f8a6b5b4b0d0 (1 revision) (flutter/engine#19643) * d329617 Changes to fix build errors in google3. (flutter/engine#19616) * dd0a6c4 Roll Skia from f8a6b5b4b0d0 to 5160e8caa226 (9 revisions) (flutter/engine#19647) * d98d539 Manual Roll of Dart dfdc7e45c5...06cb010247 (flutter/engine#19649) * dfd0727 don't throw error for the new autofill request (flutter/engine#19633) * 39e98d2 Manual Skia roll to c91db040ad18b9cc3236e342e9acca020eaafd10 (flutter/engine#19650) * f9acd08 Avoid a copy in EncodeImage (flutter/engine#19504) * 7e101f1 Roll Fuchsia Linux SDK from 5R9a0... to GOf1K... (flutter/engine#19656) * f3be9f1 Incorporate compat info changes into flutter engine (flutter/engine#19606) * 9188ff6 Manual Roll of Dart 707c6404f9...dfdc7e45c5 (flutter/engine#19657) * cbdd3e2 Roll Fuchsia Mac SDK from Wym-S... to -v4bL... (flutter/engine#19661) * 6efb152 Roll Dart SDK from 707c6404f969 to 0e25306d3f78 (4 revisions) (flutter/engine#19666) * 22a440c Roll Dart SDK from 0e25306d3f78 to 6d06476bae6b (1 revision) (flutter/engine#19669) * 8063923 Roll Fuchsia Linux SDK from GOf1K... to QWI76... (flutter/engine#19671) * c99deb0 CkPaint uses SkPaint (flutter/engine#19562) * 015f2ea Use the main bundle if the App bundle is not found (flutter/engine#18749) * f82d30b Roll Dart SDK from 6d06476bae6b to a9e67d81941a (3 revisions) (flutter/engine#19677) * 61b2fd2 Roll Fuchsia Mac SDK from -v4bL... to phQJt... (flutter/engine#19681) * 74b541d Roll Fuchsia Linux SDK from QWI76... to mrlGt... (flutter/engine#19682) * 25ce3db Roll Skia from c91db040ad18 to e4f36d7ac8f5 (20 revisions) (flutter/engine#19686) * 162dba1 Roll Skia from e4f36d7ac8f5 to ed15b1c39b8b (1 revision) (flutter/engine#19687) * f97c38b Roll Fuchsia Mac SDK from phQJt... to kEVc6... (flutter/engine#19688) * 40c4ee8 Roll Fuchsia Linux SDK from mrlGt... to YfVT0... (flutter/engine#19690) * ec944b0 Roll Skia from ed15b1c39b8b to 1434ce1aa94d (1 revision) (flutter/engine#19692) * 7ce988a Roll Skia from 1434ce1aa94d to f64be13cbf84 (2 revisions) (flutter/engine#19695) * 5f7ca41 Roll Fuchsia Mac SDK from kEVc6... to hRul_... (flutter/engine#19696) * d24549c Roll Skia from f64be13cbf84 to 041796e60364 (3 revisions) (flutter/engine#19697) * 617bd88 Roll Dart SDK from a9e67d81941a to fecc8163afc7 (3 revisions) (flutter/engine#19703) * 8d241e4 [Android] Prevent FlutterRenderer listener from calling JNI after detach (flutter/engine#19558) * 309e514 Roll Skia from 041796e60364 to d1ce4cb2beb8 (2 revisions) (flutter/engine#19704) * 211f18e Roll Skia from d1ce4cb2beb8 to 7c1967700b44 (6 revisions) (flutter/engine#19707) * 0532227 Roll Skia from 7c1967700b44 to 439709a97dfd (8 revisions) (flutter/engine#19712) * 1e02bfd Support decimal information on the TextInputType (flutter/engine#19664) * 7a95e32 Linux: Use a hash table to map cursors (flutter/engine#19561) * df23044 [web] Implement ulps for path ops (flutter/engine#19711) * 4a3aa4d Fixes typo in android_context_gl.h (flutter/engine#19700) * 5b966eb Roll Skia from 439709a97dfd to 2604a89d3353 (3 revisions) (flutter/engine#19715) * d024ae4 [fuchsia] Use memory_requirements_2 extension. (flutter/engine#19678) * 4392fbf Roll Fuchsia Linux SDK from YfVT0... to 2ct5j... (flutter/engine#19720) * ab05c79 Roll Fuchsia Mac SDK from hRul_... to ZXLvD... (flutter/engine#19721) * c05ca7e Roll Dart SDK from fecc8163afc7 to f997d62a6d29 (13 revisions) (flutter/engine#19722) * ae37971 Remove xcpretty from unit tests to see full output on test failures (flutter/engine#19667) * 91f80ef Moved to RMSE for image comparison to account for slight variations in golden image tests (flutter/engine#19658) * 1e23309 Wait for platform view to appear in iOS UI tests (flutter/engine#19725)

Moved to RMSE for image comparison to account for slight variations in

5aab325

golden image production.

googlebot added the cla: yes label Jul 10, 2020

auto-assign bot requested a review from gw280 July 10, 2020 20:43

gaaclarke requested a review from liyuqian July 10, 2020 20:44

gaaclarke added 2 commits July 10, 2020 13:45

ran formatter

5eb302f

reverted accidental commit

d15ad8d

gaaclarke requested a review from blasten July 10, 2020 20:50

blasten reviewed Jul 10, 2020

View reviewed changes

gaaclarke added 3 commits July 13, 2020 14:39

started ignoring transparent pixels and fixed a flakey test

98797e4

stopped ignoring transparent pixels when calculating the rmse

71a0d0a

removed print statement

1e3c8ac

gaaclarke requested a review from blasten July 13, 2020 21:48

tweaked the test for transparency

7811b9b

blasten approved these changes Jul 13, 2020

View reviewed changes

gaaclarke merged commit 91f80ef into flutter:master Jul 13, 2020

engine-flutter-autoroll mentioned this pull request Jul 14, 2020

Roll Engine from f10f17257300 to 1e23309a386e (55 revisions) flutter/flutter#61410

Closed

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

91f80ef Moved to RMSE for image comparison to account for slight vari…

407656c

…ations in golden image tests (flutter/engine#19658)

jmagman mentioned this pull request Jul 14, 2020

[ios] scenario_app flakey: timeout finding platform view flutter/flutter#61267

Closed

engine-flutter-autoroll mentioned this pull request Jul 14, 2020

Roll Engine from f10f17257300 to 1e23309a386e (55 revisions) flutter/flutter#61435

Merged

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

91f80ef Moved to RMSE for image comparison to account for slight vari…

78624c1

…ations in golden image tests (flutter/engine#19658)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved to RMSE for image comparison to account for slight variations in golden image tests #19658

Moved to RMSE for image comparison to account for slight variations in golden image tests #19658

gaaclarke commented Jul 10, 2020

fluttergithubbot commented Jul 10, 2020

blasten Jul 10, 2020

gaaclarke Jul 10, 2020

blasten Jul 13, 2020

gaaclarke Jul 13, 2020

gaaclarke Jul 13, 2020

blasten Jul 13, 2020

gaaclarke commented Jul 13, 2020

gaaclarke commented Jul 13, 2020

blasten left a comment

Moved to RMSE for image comparison to account for slight variations in golden image tests #19658

Moved to RMSE for image comparison to account for slight variations in golden image tests #19658

Conversation

gaaclarke commented Jul 10, 2020

Description

Related Issues

Tests

Checklist

Breaking Change

fluttergithubbot commented Jul 10, 2020

blasten Jul 10, 2020

Choose a reason for hiding this comment

gaaclarke Jul 10, 2020

Choose a reason for hiding this comment

blasten Jul 13, 2020

Choose a reason for hiding this comment

gaaclarke Jul 13, 2020

Choose a reason for hiding this comment

gaaclarke Jul 13, 2020

Choose a reason for hiding this comment

blasten Jul 13, 2020

Choose a reason for hiding this comment

gaaclarke commented Jul 13, 2020

gaaclarke commented Jul 13, 2020

blasten left a comment

Choose a reason for hiding this comment