Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Sep 12, 2022

The write parameter is a remnant from the old way we used to do golden tests. It is used to selectively "update" a golden.

We are removing it in favor of the (already existent) update-screenshot-goldens flag that updates all goldens at once.

The flag and the write parameter are currently no-ops because we moved to Skia Gold which stores and updates goldens in post-submit. That said, we will likely re-introduce local golden testing, and for that we need these flags.

@flutter-dashboard
Copy link

This pull request was opened against a branch other than main. Since Flutter pull requests should not normally be opened against branches other than main, I have changed the base to main. If this was intended, you may modify the base back to master. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than main, unless this is an intentional hotfix/cherrypick.

@flutter-dashboard flutter-dashboard bot changed the base branch from master to main September 12, 2022 14:36
@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Sep 12, 2022
@mdebbar mdebbar force-pushed the remove_golden_write branch from ac6fecf to ac6a711 Compare September 12, 2022 18:39
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Future<String> _diffScreenshot(
String filename,
bool write,
double maxDiffRateFailure,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still use the "max diff rate" arguments for anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several things I want to clean up in our golden tests. I'll add max diff rate to the list.

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 13, 2022
@auto-submit auto-submit bot merged commit a71c096 into flutter:main Sep 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 13, 2022
cfontas pushed a commit to cfontas/engine that referenced this pull request Sep 14, 2022
Oleh-Sv pushed a commit to Oleh-Sv/engine that referenced this pull request Sep 28, 2022
@mdebbar mdebbar deleted the remove_golden_write branch January 17, 2023 18:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants