Switching to Gold Status Check by Piinks · Pull Request #51968 · flutter/flutter

Piinks · 2020-03-04T21:54:13Z

Description

This PR will change framework tests to rely on the Gold status check introduced in
flutter/cocoon#651

This also adds an "inconsequential golden test" to more easily validate changes to Gold integration and detect if the dashboard is not working properly.

Related Issues

Fixes #48744

Tests

Inconsequential golden file test.

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 Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
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.

skia-gold · 2020-03-04T22:00:06Z

Gold has detected one or more untriaged digests on patchset 1.
Please triage them at https://flutter-gold.skia.org/search?issue=51968.

fluttergithubbot · 2020-03-05T22:31:09Z

Golden image changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

Check Handling Breaking Changes. While there are exceptions to this rule, if this patch modifies an existing golden file, it is probably not an exception. Only new golden files are not considered breaking changes.

Writing a golden file test for package:flutter may also provide guidance for this change.

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

Changes reported for pull request 51968 at sha 0075363

skia-gold · 2020-03-05T23:12:05Z

Gold has detected one or more untriaged digests on patchset 2.
Please triage them at https://flutter-gold.skia.org/search?issue=51968.

fluttergithubbot · 2020-03-05T23:42:53Z

Golden image changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

Check Handling Breaking Changes. While there are exceptions to this rule, if this patch modifies an existing golden file, it is probably not an exception. Only new golden files are not considered breaking changes.

Writing a golden file test for package:flutter may also provide guidance for this change.

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

Changes reported for pull request 51968 at sha a9fa8dd

Piinks · 2020-03-06T17:59:52Z

I am going to close this for now, will return.

fluttergithubbot · 2020-03-10T17:55:28Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

Changes to golden files are considered breaking changes, so consult Handling Breaking Changes to proceed. While there are exceptions to this rule, if this patch modifies an existing golden file, it is probably not an exception. Only new golden file tests, or downstream changes like those from skia updates are considered non-breaking.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #51968 at sha a9fa8dd

skia-gold · 2020-03-10T18:07:46Z

Gold has detected one or more untriaged digests on patchset 3.
Please triage them at https://flutter-gold.skia.org/search?issue=51968.

fluttergithubbot · 2020-03-10T18:55:27Z

Golden file changes remain available for triage from new commit, Click here to view.

Changes reported for pull request #51968 at sha 0906b43

packages/flutter/test/widgets/basic_test.dart

skia-gold · 2020-03-11T18:51:21Z

Gold has detected one or more untriaged digests on patchset 4.
Please triage them at https://flutter-gold.skia.org/search?issue=51968.

fluttergithubbot · 2020-03-11T19:46:27Z

Golden file changes remain available for triage from new commit, Click here to view.

Changes reported for pull request #51968 at sha dc2fe1b

Piinks · 2020-03-11T20:49:56Z

Triaged at 1:49 pm pt, flutter-gold should go green in 3..2..1..

Piinks · 2020-03-11T20:52:23Z

Going to push a commit now that alters an existing golden file, validating a previous green state for flutter-gold will be updated to reflect the new commit.

skia-gold · 2020-03-11T21:05:21Z

Gold has detected one or more untriaged digests on patchset 6.
Please triage them at https://flutter-gold.skia.org/search?issue=51968.

fluttergithubbot · 2020-03-11T21:51:17Z

Golden file changes remain available for triage from new commit, Click here to view.

Changes reported for pull request #51968 at sha 09c495e

Piinks · 2020-03-11T21:52:25Z

Now I will revert the unintended change and make sure it can go green again.

packages/flutter/test/widgets/color_filter_test.dart

Piinks · 2020-03-12T16:34:52Z

@cbracken @yjbanov this is the change I mentioned to alleviate the engine roll process. This fixes #48744, where it there was a lot of discussion around solving this. Feel free to tag other interested parties. :)

goderbauer · 2020-03-12T21:56:39Z

packages/flutter/test/widgets/basic_test.dart

    });
  });
+  testWidgets('Inconsequential golden test', (WidgetTester tester) async {
+    // This test does not matter, it can be approved at any time if rendering


I was confused by "this test does not matter" - if it does not matter we should just delete it :)

Maybe rephrase to something like: The test validates foobar. Any changes to the golden file can be approved at any time.

goderbauer · 2020-03-12T21:58:19Z

packages/flutter_goldens_client/lib/skia_client.dart

    }

-    return result.exitCode == 0;
+    return true;


Why does this method have a return value if it is always true? Should we change it to return Future<void>.

goderbauer · 2020-03-12T22:00:26Z

packages/flutter_goldens/lib/flutter_goldens.dart

-    return skiaClient.tryjobAdd(golden.path, goldenFile);
+    await skiaClient.tryjobAdd(golden.path, goldenFile);
+
+    return true;


This probably deserves a comment. Why does the comparison always succeed?

…o testGoldCheck

goderbauer

LGTM

lock · 2020-04-02T16:50:11Z

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

Adding check for gold status test

0075363

googlebot added the cla: yes label Mar 4, 2020

fluttergithubbot added the c: API break Backwards-incompatible API changes label Mar 5, 2020

Nit

a9fa8dd

Piinks mentioned this pull request Mar 5, 2020

Performance Improvement for Gold Check flutter/cocoon#672

Merged

Piinks closed this Mar 6, 2020

Piinks reopened this Mar 10, 2020

Updating fail conditions for tryjobs

0906b43

Piinks commented Mar 11, 2020

View reviewed changes

packages/flutter/test/widgets/basic_test.dart Outdated Show resolved Hide resolved

Update packages/flutter/test/widgets/basic_test.dart

dc2fe1b

Merge branch 'master' of github.com:flutter/flutter into testGoldCheck

aff1a0a

Altered existing golden file

09c495e

Piinks commented Mar 11, 2020

View reviewed changes

packages/flutter/test/widgets/color_filter_test.dart Outdated Show resolved Hide resolved

Update packages/flutter/test/widgets/color_filter_test.dart

f7c11e2

Piinks changed the title ~~[WIP] Switching to Gold Status Check~~ Switching to Gold Status Check Mar 12, 2020

Piinks requested a review from goderbauer March 12, 2020 00:53

goderbauer reviewed Mar 12, 2020

View reviewed changes

digiter added the passed secondary triage label Mar 12, 2020

Piinks added 2 commits March 12, 2020 16:31

Review feedback

3e6120a

Merge branch 'testGoldCheck' of https://github.com/Piinks/flutter int…

5e1a916

…o testGoldCheck

goderbauer approved these changes Mar 12, 2020

View reviewed changes

Piinks added waiting for tree to go green and removed work in progress; do not review labels Mar 13, 2020

fluttergithubbot merged commit aa622fb into flutter:master Mar 13, 2020

lock bot locked and limited conversation to collaborators Apr 2, 2020

Piinks deleted the testGoldCheck branch March 14, 2025 17:14

Conversation

Piinks commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

skia-gold commented Mar 4, 2020

Uh oh!

fluttergithubbot commented Mar 5, 2020

Uh oh!

skia-gold commented Mar 5, 2020

Uh oh!

fluttergithubbot commented Mar 5, 2020

Uh oh!

Piinks commented Mar 6, 2020

Uh oh!

fluttergithubbot commented Mar 10, 2020

Uh oh!

skia-gold commented Mar 10, 2020

Uh oh!

fluttergithubbot commented Mar 10, 2020

Uh oh!

Uh oh!

skia-gold commented Mar 11, 2020

Uh oh!

fluttergithubbot commented Mar 11, 2020

Uh oh!

Piinks commented Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piinks commented Mar 11, 2020

Uh oh!

skia-gold commented Mar 11, 2020

Uh oh!

fluttergithubbot commented Mar 11, 2020

Uh oh!

Piinks commented Mar 11, 2020

Uh oh!

Uh oh!

Piinks commented Mar 12, 2020

Uh oh!

goderbauer Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

Piinks Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

goderbauer Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

Piinks Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

goderbauer Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

Piinks Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

lock bot commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Piinks commented Mar 4, 2020 •

edited

Loading

Piinks commented Mar 11, 2020 •

edited

Loading