[Engine] Make SkiaGoldClient a NOP when the branch is not main or master. (#161187)

Unblocks https://github.com/flutter/flutter/pull/160556.

Currently the merge queue runs in post-submit mode, which looks for (and
fails to find) digests for PRs that have not yet been submitted,
blocking the engine goldens from being enabled/checked in
https://github.com/flutter/flutter/pull/160556. This PR is a proposal to
fix that by skipping the tests.

We might decide instead that tests should not be running in the merge
queue at all, in which case we will _not_ merge this PR and will change
how the merge queue works instead. Otherwise this PR is a proof of
concept of [aligning implementations with the
framework](a9b3f6c042/packages/flutter_goldens/lib/flutter_goldens.dart (L338)).
This commit is contained in:
Matan Lurey 2025-01-06 17:38:52 -08:00 committed by GitHub
parent bb805fad1e
commit 95c1bc1c3e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 91 additions and 0 deletions

View File

@ -88,6 +88,12 @@ interface class SkiaGoldClient {
path.join(_engineRoot.flutterDir.path, '.engine-release.version'),
);
if (!_isMainBranch) {
_stderr.writeln(
'Current git branch (${_environment['GIT_BRANCH']}) is not "main" or "master", so golden files are not checked.',
);
}
// If the file is not found or cannot be read, we are in an invalid state.
try {
_releaseVersion = ReleaseVersion.parse(releaseVersionFile.readAsStringSync());
@ -113,6 +119,14 @@ interface class SkiaGoldClient {
return (environment ?? io.Platform.environment).containsKey(_kLuciEnvName);
}
/// Whether the client is currently running on the main branch.
bool get _isMainBranch {
return switch (_environment['GIT_BRANCH']) {
'main' || 'master' => true,
_ => false,
};
}
/// Whether the current environment is a presubmit job.
bool get _isPresubmit {
return isLuciEnv(environment: _environment) &&
@ -319,6 +333,13 @@ interface class SkiaGoldClient {
int pixelColorDelta = 0,
required int screenshotSize,
}) async {
if (!_isMainBranch) {
if (verbose) {
_stderr.writeln('Skipping $testName (not on git main branch)');
}
return;
}
assert(_isPresubmit || _isPostsubmit);
// Clean the test name to remove the file extension.

View File

@ -20,6 +20,7 @@ void main() {
/// Simulating what a presubmit environment would look like.
const Map<String, String> presubmitEnv = <String, String>{
'GIT_BRANCH': 'master',
'GOLDCTL': 'python tools/goldctl.py',
'GOLD_TRYJOB': 'flutter/engine/1234567890',
'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/1234567890/+/logdog',
@ -28,6 +29,7 @@ void main() {
/// Simulating what a postsubmit environment would look like.
const Map<String, String> postsubmitEnv = <String, String>{
'GIT_BRANCH': 'master',
'GOLDCTL': 'python tools/goldctl.py',
'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/1234567890/+/logdog',
'LUCI_CONTEXT': '{}',
@ -87,6 +89,74 @@ void main() {
}
});
test('prints a warning and skips when the git branch is not master or main', () async {
final _TestFixture fixture = _TestFixture();
try {
final SkiaGoldClient client = createClient(
fixture,
environment: {...presubmitEnv, 'GIT_BRANCH': 'merge-queue-foo'},
onRun: (List<String> command) {
expect(command, <String>[
'python tools/goldctl.py',
'auth',
'--work-dir',
p.join(fixture.workDirectory.path, 'temp'),
'--luci',
]);
createAuthOptDotJson(fixture.workDirectory.path);
return io.ProcessResult(0, 0, '', '');
},
);
// In case we change our mind, auth is still expected to work.
await client.auth();
expect(
fixture.outputSink.toString(),
stringContainsInOrder([
'Current git branch',
'merge-queue-foo',
'is not "main" or "master"',
]),
);
} finally {
fixture.dispose();
}
});
test('always a success when the git branch is not master or main', () async {
final _TestFixture fixture = _TestFixture();
try {
final SkiaGoldClient client = createClient(
fixture,
environment: {...presubmitEnv, 'GIT_BRANCH': 'merge-queue-foo'},
onRun: (List<String> command) {
expect(command, <String>[
'python tools/goldctl.py',
'auth',
'--work-dir',
p.join(fixture.workDirectory.path, 'temp'),
'--luci',
]);
createAuthOptDotJson(fixture.workDirectory.path);
return io.ProcessResult(0, 0, '', '');
},
);
// In case we change our mind, auth is still expected to work.
await client.auth();
// Always completes OK.
await client.addImg(
'test-name.foo',
io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')),
screenshotSize: 1000,
);
} finally {
fixture.dispose();
}
});
test('auth executes successfully', () async {
final _TestFixture fixture = _TestFixture();
try {