Unverified Commit ceab1248 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Use FlutterLocalFileComparator when user permission denied on Cirrus (#46688)

parent 36e599eb
......@@ -268,6 +268,10 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
// TODO(Piinks): Better handling for first-time contributors that cannot decrypt
// the service account is needed. Gold has a new feature `goldctl imgtest check`
// that could work. There is also the previous implementation that did not use
// goldctl for this edge case. https://github.com/flutter/flutter/issues/46687
class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterPreSubmitFileComparator] that will test golden file
/// images against baselines requested from Flutter Gold.
......@@ -324,9 +328,12 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
/// performed as pre-submit tests with Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
final bool hasWritePermission = platform.environment['CIRRUS_USER_PERMISSION'] == 'admin'
|| platform.environment['CIRRUS_USER_PERMISSION'] == 'write';
return platform.environment.containsKey('CIRRUS_CI')
&& cirrusPR.isNotEmpty
&& platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
&& platform.environment.containsKey('GOLD_SERVICE_ACCOUNT')
&& hasWritePermission;
}
}
......@@ -392,8 +399,12 @@ class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
/// Decides based on the current environment whether this comparator should be
/// used.
static bool isAvailableForEnvironment(Platform platform) {
return platform.environment.containsKey('SWARMING_TASK_ID')
|| platform.environment.containsKey('CIRRUS_CI');
return (platform.environment.containsKey('SWARMING_TASK_ID')
|| platform.environment.containsKey('CIRRUS_CI'))
// A service account means that this is a Gold shard. At this point, it
// means we don't have permission to use the account, so we will pass
// through to the [FlutterLocalFileComparator].
&& !platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
}
}
......
......@@ -399,6 +399,7 @@ void main() {
'CIRRUS_CI': 'true',
'CIRRUS_PR': '1234',
'GOLD_SERVICE_ACCOUNT' : 'service account...',
'CIRRUS_USER_PERMISSION' : 'write',
},
operatingSystem: 'macos'
);
......@@ -451,6 +452,74 @@ void main() {
isFalse,
);
});
test('returns true - admin privileges', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI': 'true',
'CIRRUS_PR': '1234',
'GOLD_SERVICE_ACCOUNT' : 'service account...',
'CIRRUS_USER_PERMISSION' : 'admin',
},
operatingSystem: 'macos'
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns true - write privileges', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI': 'true',
'CIRRUS_PR': '1234',
'GOLD_SERVICE_ACCOUNT' : 'service account...',
'CIRRUS_USER_PERMISSION' : 'write',
},
operatingSystem: 'macos'
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns false - read privileges', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI': 'true',
'CIRRUS_PR': '1234',
'GOLD_SERVICE_ACCOUNT' : 'service account...',
'CIRRUS_USER_PERMISSION' : 'read',
},
operatingSystem: 'macos'
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
test('returns false - no privileges', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI': 'true',
'CIRRUS_PR': '1234',
'GOLD_SERVICE_ACCOUNT' : 'service account...',
'CIRRUS_USER_PERMISSION' : 'none',
},
operatingSystem: 'macos'
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
});
});
......@@ -483,10 +552,26 @@ void main() {
isTrue,
);
});
test('returns false', () {
test('returns false - no CI', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
},
operatingSystem: 'macos'
);
expect(
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(
platform),
isFalse,
);
});
test('returns false - permission pass through', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI' : 'yep',
'GOLD_SERVICE_ACCOUNT' : 'This is a Gold shard!',
},
operatingSystem: 'macos'
);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment