Add golden test review process to release cycle #144835
Comments
gaaclarke
added
c: proposal
jonahwilliams
added
P2
zanderso
added
P1
|
From the Impeller Weekly: The next steps were to append the release name (branch?) to the test to create a new variant for each release. |
|
The proposal has steeped for a few weeks and doesn't seem to have strong objection, so I'll start working on this. |
…51739) Work towards flutter/flutter#144835. Doc (_sorry, internal only_): [go/flutter-engine-goldens-workflow](http://goto.google.com/flutter-engine-goldens-workflow). This implements the majority of the proposed workflow, that is, optionally having a plain-text version at the root of the directory, and using it to apply a unique suffix we can review in release branches. As it stands, this is a NO-OP outside of tests (it will have no impact, and can be ignored). What's missing before using this feature in release branches: - Optimization work with the infra team (not sure if blocking or not): flutter/flutter#145842 - A dry-run of this with the release team to make sure it works as intended @gaaclarke As implemented, I _think_ we don't need anything special for [`dir_contents_diff`](https://github.com/flutter/engine/blob/b0d3663f439fd97c32bf4d46b9004c97841c43b8/tools/dir_contents_diff), but maybe I'm wrong - I think only the _test_ names are being changed, not the names on disk. /cc @zanderso as well.
|
Filed #146078 to ask for infra support since I did not get a response on Discord. |
|
@gaaclarke Is this effectively closed? |
|
Yes, we have a way that we can do golden test reviews, we've done a trial run to prove that it works. The only outstanding work may be for any documentation surrounding releases to get updated, if they exist. It may be worth mentioning to Chris in case he knows of anything that should be updated. |
|
I hesitate to add a lot of extra things on Chris' place right now, but at the same time this is pretty important for us to land Impeller and keep the quality high. @christopherfujino or @itsjustkevin could we help update the release documentation, if any? Quick summary for the uninitiated: https://github.com/flutter/engine/tree/main/testing/skia_gold_client#release-testing |
|
Or just point us to the documentation and we can update it. |
Currently, when we create release builds of the engine, the golden image test results are not checked (automated or manually). Some of them are not even executed, only certain invocations of "run_tests.py" are used.
We'd like to have confidence that our cherry-picks are not causing some failure, so ideally the golden tests would be executed and checked.
The golden image results can not be naively uploaded to skia gold because skia gold has a consistent timeline that checks against one branch. Comparing a golden from a release branch to the main branch would confuse skia gold and lead to situations where there is no correct course forward.
A potential solution would be to create new tests related to the release branch. For example
aiks_foo_test.pngcould becomeaiks_foo_test.png.v12_34_5for release12.32.5. The tests would first be uploaded with zero cherry-picks and blanket approved. Those images will then serve as the basis from which cherry-pick golden results will be compared.cc @zanderso @matanlurey
The text was updated successfully, but these errors were encountered: