firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove Cirrus CI from Flutter goldens. (#161396)

Browse Source 
Towards https://github.com/flutter/flutter/issues/161387.

I believe this effectively a NOP as the `CIRRUS_`-environment variables
are never set.
This commit is contained in:
Matan Lurey 2025-01-09 14:27:24 -08:00 committed by GitHub
parent 900eab1601
commit 59f630b7b5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 10 additions and 263 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
packages/flutter_goldens/lib/flutter_goldens.dart
View File

@ -89,9 +89,8 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromLocalFileComparator(
localFileComparator: goldenFileComparator as LocalFileComparator,
'Golden file testing is not executed on Cirrus, or LUCI environments '
'outside of flutter/flutter, or in test shards that are not configured '
'for using goldctl.',
'Golden file testing is not executed on LUCI environments outside of '
'flutter, or in test shards that are not configured for using goldctl.',
platform: platform,
namePrefix: namePrefix,
log: print,
@ -448,7 +447,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
/// A [FlutterGoldenFileComparator] for testing conditions that do not execute
/// golden file tests.
///
/// Currently, this comparator is used on Cirrus, or in Luci environments when executing tests
/// Currently, this comparator is used on Luci environments when executing tests
/// outside of the flutter/flutter repository.
///
/// See also:
@ -521,14 +520,11 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
/// Decides, based on the current environment, if this comparator should be
/// used.
///
/// If we are in a CI environment, LUCI or Cirrus, but are not using the other
/// If we are in a CI environment, i.e. LUCI, but are not using the other
/// comparators, we skip. Otherwise we would fallback to the local comparator,
/// for which failures cannot be resolved in a CI environment.
static bool isForEnvironment(Platform platform) {
return platform.environment.containsKey('SWARMING_TASK_ID')
// Some builds are still being run on Cirrus, we should skip these.
||
platform.environment.containsKey('CIRRUS_CI');
return platform.environment.containsKey('SWARMING_TASK_ID');
}
}

259
packages/flutter_goldens/test/comparator_selection_test.dart
View File

@ -11,7 +11,6 @@ enum _Comparator { post, pre, skip, local }
_Comparator _testRecommendations({
bool hasFlutterRoot = false,
bool hasLuci = false,
bool hasCirrus = false,
bool hasGold = false,
bool hasTryJob = false,
String branch = 'main',
@ -21,11 +20,7 @@ _Comparator _testRecommendations({
environment: <String, String>{
if (hasFlutterRoot) 'FLUTTER_ROOT': '/flutter',
if (hasLuci) 'SWARMING_TASK_ID': '8675309',
if (hasCirrus) 'CIRRUS_CI': 'true',
if (hasCirrus) 'CIRRUS_PR': '',
if (hasCirrus) 'CIRRUS_BRANCH': branch,
if (hasGold) 'GOLDCTL': 'goldctl',
if (hasGold && hasCirrus) 'GOLD_SERVICE_ACCOUNT': 'service account...',
if (hasTryJob) 'GOLD_TRYJOB': 'git/ref/12345/head',
'GIT_BRANCH': branch,
},
@ -54,71 +49,25 @@ void main() {
// If we don't have gold but are on CI, we skip regardless.
expect(_testRecommendations(hasLuci: true), _Comparator.skip);
expect(_testRecommendations(hasLuci: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasLuci: true, hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true), _Comparator.skip);
expect(
_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasTryJob: true),
_Comparator.skip,
);
expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true), _Comparator.skip);
expect(
_testRecommendations(hasFlutterRoot: true, hasCirrus: true, hasTryJob: true),
_Comparator.skip,
);
expect(
_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true),
_Comparator.skip,
);
expect(
_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true, hasTryJob: true),
_Comparator.skip,
);
// On Luci, with Gold, post-submit. Flutter root and Cirrus variables should have no effect.
// On Luci, with Gold, post-submit. Flutter root and LUCI variables should have no effect.
expect(_testRecommendations(hasGold: true, hasLuci: true), _Comparator.post);
expect(_testRecommendations(hasGold: true, hasLuci: true, hasCirrus: true), _Comparator.post);
expect(
_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true),
_Comparator.post,
);
expect(
_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true),
_Comparator.post,
);
// On Luci, with Gold, pre-submit. Flutter root and Cirrus variables should have no effect.
// On Luci, with Gold, pre-submit. Flutter root and LUCI variables should have no effect.
expect(_testRecommendations(hasGold: true, hasLuci: true, hasTryJob: true), _Comparator.pre);
expect(
_testRecommendations(hasGold: true, hasLuci: true, hasCirrus: true, hasTryJob: true),
_Comparator.pre,
);
expect(
_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true, hasTryJob: true),
_Comparator.pre,
);
expect(
_testRecommendations(
hasGold: true,
hasLuci: true,
hasFlutterRoot: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.pre,
);
// On Cirrus (with Gold and not on Luci), we skip regardless.
expect(
_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true),
_Comparator.skip,
);
expect(
_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true),
_Comparator.skip,
);
});
test('Comparator recommendations - release branch', () {
@ -146,27 +95,6 @@ void main() {
_testRecommendations(branch: 'flutter-3.16-candidate.0', hasLuci: true, hasTryJob: true),
_Comparator.skip,
);
expect(
_testRecommendations(branch: 'flutter-3.16-candidate.0', hasCirrus: true),
_Comparator.skip,
);
expect(
_testRecommendations(branch: 'flutter-3.16-candidate.0', hasCirrus: true, hasTryJob: true),
_Comparator.skip,
);
expect(
_testRecommendations(branch: 'flutter-3.16-candidate.0', hasLuci: true, hasCirrus: true),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasLuci: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasLuci: true),
_Comparator.skip,
@ -180,57 +108,12 @@ void main() {
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasFlutterRoot: true,
hasCirrus: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasFlutterRoot: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasFlutterRoot: true,
hasLuci: true,
hasCirrus: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasFlutterRoot: true,
hasLuci: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.skip,
);
// On Luci, with Gold, post-submit. Flutter root and Cirrus variables should have no effect. Branch should make us skip.
// On Luci, with Gold, post-submit. Flutter root and LUCI variables should have no effect. Branch should make us skip.
expect(
_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasGold: true,
hasLuci: true,
hasCirrus: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
@ -240,18 +123,8 @@ void main() {
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasGold: true,
hasLuci: true,
hasFlutterRoot: true,
hasCirrus: true,
),
_Comparator.skip,
);
// On Luci, with Gold, pre-submit. Flutter root and Cirrus variables should have no effect. Branch should make us skip.
// On Luci, with Gold, pre-submit. Flutter root and LUCI variables should have no effect. Branch should make us skip.
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
@ -266,48 +139,6 @@ void main() {
branch: 'flutter-3.16-candidate.0',
hasGold: true,
hasLuci: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasGold: true,
hasLuci: true,
hasFlutterRoot: true,
hasTryJob: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasGold: true,
hasLuci: true,
hasFlutterRoot: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.skip,
);
// On Cirrus (with Gold and not on Luci), we skip regardless.
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasCirrus: true,
hasGold: true,
hasFlutterRoot: true,
),
_Comparator.skip,
);
expect(
_testRecommendations(
branch: 'flutter-3.16-candidate.0',
hasCirrus: true,
hasGold: true,
hasFlutterRoot: true,
hasTryJob: true,
),
@ -328,13 +159,6 @@ void main() {
// If we don't have gold but are on CI, we skip regardless.
expect(_testRecommendations(os: 'linux', hasLuci: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasLuci: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true), _Comparator.skip);
expect(
_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true, hasTryJob: true),
_Comparator.skip,
);
expect(
_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true),
_Comparator.skip,
@ -343,65 +167,19 @@ void main() {
_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasTryJob: true),
_Comparator.skip,
);
expect(
_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true),
_Comparator.skip,
);
expect(
_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true, hasTryJob: true),
_Comparator.skip,
);
expect(
_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasCirrus: true),
_Comparator.skip,
);
expect(
_testRecommendations(
os: 'linux',
hasFlutterRoot: true,
hasLuci: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.skip,
);
// On Luci, with Gold, post-submit. Flutter root and Cirrus variables should have no effect.
expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true), _Comparator.post);
expect(
_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasCirrus: true),
_Comparator.post,
);
expect(
_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasFlutterRoot: true),
_Comparator.post,
);
expect(
_testRecommendations(
os: 'linux',
hasGold: true,
hasLuci: true,
hasFlutterRoot: true,
hasCirrus: true,
),
_Comparator.post,
);
// On Luci, with Gold, pre-submit. Flutter root and Cirrus variables should have no effect.
// On Luci, with Gold, pre-submit. Flutter root should have no effect.
expect(
_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasTryJob: true),
_Comparator.pre,
);
expect(
_testRecommendations(
os: 'linux',
hasGold: true,
hasLuci: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.pre,
);
expect(
_testRecommendations(
os: 'linux',
@ -412,33 +190,6 @@ void main() {
),
_Comparator.pre,
);
expect(
_testRecommendations(
os: 'linux',
hasGold: true,
hasLuci: true,
hasFlutterRoot: true,
hasCirrus: true,
hasTryJob: true,
),
_Comparator.pre,
);
// On Cirrus (with Gold and not on Luci), we skip regardless.
expect(
_testRecommendations(os: 'linux', hasCirrus: true, hasGold: true, hasFlutterRoot: true),
_Comparator.skip,
);
expect(
_testRecommendations(
os: 'linux',
hasCirrus: true,
hasGold: true,
hasFlutterRoot: true,
hasTryJob: true,
),
_Comparator.skip,
);
});
test('Branch names', () {