Golden tests failed to run on the 3.23 beta release

[gaaclarke]

Previously work was done to make sure the golden tests run on the release branches. It was verified to work in a mock run at #146078. When the 3.23 beta branch was created golden images were not reaped like it did in the mock test.

This is likely a difference in the environment between the test and the real release workflow. The code that added the ability for cocoon to run the skia gold on release branches was added in flutter/cocoon#3659 and appears intact still.

The PR that disabled the golden image tests for the 3.23 beta branch was flutter/engine#53163. It appears that the release team ran into issues beyond just the failure of cocoon to reap the golden images too since there was a need to disable the tests. I believe that is their GOLDCTL env var issue which should be resolved by adding skia gold as a dependency in the builder.

We need to identify the difference between the mock run and the real workflow to understand why cocoon did not reap the golden images.

cc @matanlurey

[gaaclarke]

@christopherfujino when you have a chance can you help provide some clarification and logs since we don't have access:

1. We saw that the golden images weren't reaped, can you provide the cocoon log that should be generated from this line: log.fine('This change\'s destination, ${pr.base!.ref}, does not run Skia Gold checks, skipping.');
2. It appears you were running into an issue beyond the reaping problem since the tests had to be disabled. Is this the issue you were running into: https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/12733/infra ? How come there was a GOLDCTL issue there, but not on Update .engine-release.version engine#53148? Can you clarify that was the issue that caused you to have to revert the tests, since failing to reap the golden images should not have blocked your workflow?

[christopherfujino]

@christopherfujino when you have a chance can you help provide some clarification and logs since we don't have access:

1. We saw that the golden images weren't reaped, can you provide the cocoon log that should be generated from this line: log.fine('This change\'s destination, ${pr.base!.ref}, does not run Skia Gold checks, skipping.');

All I see at the google cloud bucket at the path that it claims is an empty noop.txt file. My guess is either we didn't write a log file, or we wrote it to the wrong path.

2. It appears you were running into an issue beyond the reaping problem since the tests had to be disabled. Is this the issue you were running into: https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/12733/infra ?

How come there was a GOLDCTL issue there, but not on Update .engine-release.version engine#53148?

I don't know.

Can you clarify that was the issue that caused you to have to revert the tests, since failing to reap the golden images should not have blocked your workflow?

Yes, I reverted the tests because run_tests.py was raising a RuntimeError: The GOLDCTL environment variable is not set, failing the step and thus the entire build, and so built engine artifacts were never uploaded, and so could not be rolled to the framework.

[christopherfujino]

@christopherfujino when you have a chance can you help provide some clarification and logs since we don't have access:

1. We saw that the golden images weren't reaped, can you provide the cocoon log that should be generated from this line: log.fine('This change\'s destination, ${pr.base!.ref}, does not run Skia Gold checks, skipping.');

All I see at the google cloud bucket at the path that it claims is an empty noop.txt file. My guess is either we didn't write a log file, or we wrote it to the wrong path.

I read this wrong, I was looking at the wrong types of logs.

[christopherfujino]

Here are the pantheon logs:

350: {
logMessage: "Last known Gold status for flutter/engine#53148 was with sha: , status: , description: "
severity: "DEBUG"
time: "2024-05-31T17:18:11.270Z"
}
351: {
logMessage: "This change's destination, flutter-3.23-candidate.0, does not run Skia Gold checks, skipping."
severity: "DEBUG"
time: "2024-05-31T17:18:11.271Z"
}

[gaaclarke]

@matanlurey flutter/cocoon#3761 fixes the reaping problem. Can you link to the instructions for getting GOLDCTL set in a bot? I can't find the thing you linked to me the other day. We can show that to chris and hopefully he'll know where to put that.

[matanlurey]

@matanlurey flutter/cocoon#3761 fixes the reaping problem. Can you link to the instructions for getting GOLDCTL set in a bot? I can't find the thing you linked to me the other day. We can show that to chris and hopefully he'll know where to put that.

https://github.com/flutter/engine/tree/main/testing/skia_gold_client#configuring-ci

[gaaclarke]

@christopherfujino do you know how to reconcile the steps Matan listed above for getting GOLDCTL set and how to setup the release bots? Adding that dependency should be doing it. I'm not sure where the right place to put that would be for the release bots though or if they even work the same. If you point us in the right direction we can try it out.

[christopherfujino]

@christopherfujino do you know how to reconcile the steps Matan listed above for getting GOLDCTL set and how to setup the release bots? Adding that dependency should be doing it. I'm not sure where the right place to put that would be for the release bots though or if they even work the same. If you point us in the right direction we can try it out.

Off the top of my head, I can see that we do have goldctl specified as a dependency in .ci.yaml: https://github.com/flutter/engine/blob/flutter-3.18-candidate.18/.ci.yaml#L364

However, if you notice, that target is using the engine_v2/engine_v2 recipe--meanwhile, in the build that failed the test, it ran with the engine_v2/builder recipe: https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/12733/infra.

I do see that it has goldctl listed as a dependency in the input properties. However, I don't see a step that explicitly installed goldctl via cipd (which I would expect to see).

In this recipe, we do call api.flutter_deps.required_deps(env, env_prefixes, test_deps), which I would expect to install, via cipd, everything listed in the dependencies hash map of the build input properties: https://flutter.googlesource.com/recipes/+/refs/heads/flutter-3.23-candidate.0/recipes/engine_v2/builder.py#138

I would start tracing around that code, and comparing it with the build steps. I would first verify whether or not we actually installed in goldctl. If we did, then I would check why we didn't set the GOLDCTL env var that we wrap the test invocation with (you can see the env vars here: https://logs.chromium.org/logs/dart-internal/buildbucket/cr-buildbucket/8746400829535528113/+/u/test:_Impeller-golden_for_host_release/execution_details).