Skip to content

F
Front-End
Unverified Commit 4820e0cd authored by Kate Lovett's avatar Kate Lovett Committed by GitHub
Browse files
Options

Fix skipping golden comparator for CI environments (#145619) 

Fixes https://github.com/flutter/flutter/issues/145618

The local file comparator was being used in post submit for the Linux coverage shard. This corrects it to choose the skipping comparator.
parent de68c938
Hide whitespace changes
Inline Side-by-side
Showing with 35 additions and 34 deletions
packages/flutter_goldens/lib/flutter_goldens.dart
View file @ 4820e0cd
...@@ -44,7 +44,9 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre ...@@ -44,7 +44,9 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix, log: print); goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix, log: print);
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) { } else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator( goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
'Golden file testing is not executed on Cirrus, or LUCI environments outside of flutter/flutter.', '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.',
namePrefix: namePrefix, namePrefix: namePrefix,
log: print log: print
); );
...@@ -432,13 +434,12 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { ...@@ -432,13 +434,12 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
/// used. /// used.
/// ///
/// If we are in a CI environment, LUCI or Cirrus, but are not using the other /// If we are in a CI environment, LUCI or Cirrus, but are not using the other
/// comparators, we skip. /// 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) { static bool isForEnvironment(Platform platform) {
return (platform.environment.containsKey('SWARMING_TASK_ID') return platform.environment.containsKey('SWARMING_TASK_ID')
// Some builds are still being run on Cirrus, we should skip these. // Some builds are still being run on Cirrus, we should skip these.
|| platform.environment.containsKey('CIRRUS_CI')) || platform.environment.containsKey('CIRRUS_CI');
// If we are in CI, skip on branches that are not main.
&& !_isMainBranch(platform.environment['GIT_BRANCH']);
} }
} }
......
packages/flutter_goldens/test/comparator_selection_test.dart
View file @ 4820e0cd
...@@ -60,18 +60,18 @@ void main() { ...@@ -60,18 +60,18 @@ void main() {
expect(_testRecommendations(hasFlutterRoot: true, hasGold: true), _Comparator.local); expect(_testRecommendations(hasFlutterRoot: true, hasGold: true), _Comparator.local);
// If we don't have gold but are on CI, we skip regardless. // If we don't have gold but are on CI, we skip regardless.
expect(_testRecommendations(hasLuci: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasLuci: true), _Comparator.skip);
expect(_testRecommendations(hasLuci: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasLuci: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasLuci: true, hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasLuci: true, hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be 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 Cirrus variables should have no effect.
expect(_testRecommendations(hasGold: true, hasLuci: true), _Comparator.post); expect(_testRecommendations(hasGold: true, hasLuci: true), _Comparator.post);
...@@ -86,8 +86,8 @@ void main() { ...@@ -86,8 +86,8 @@ void main() {
expect(_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: 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. // On Cirrus (with Gold and not on Luci), we skip regardless.
expect(_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true), _Comparator.skip);
expect(_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.skip);
}); });
test('Comparator recommendations - release branch', () { test('Comparator recommendations - release branch', () {
...@@ -136,18 +136,18 @@ void main() { ...@@ -136,18 +136,18 @@ void main() {
expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasGold: true), _Comparator.local); expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasGold: true), _Comparator.local);
// If we don't have gold but are on CI, we skip regardless. // If we don't have gold but are on CI, we skip regardless.
expect(_testRecommendations(os: 'linux', hasLuci: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasLuci: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasLuci: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasLuci: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.skip);
expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasCirrus: true), _Comparator.local); // TODO(ianh): this should be 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.local); // TODO(ianh): this should be 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. // 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), _Comparator.post);
...@@ -162,8 +162,8 @@ void main() { ...@@ -162,8 +162,8 @@ void main() {
expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _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. // On Cirrus (with Gold and not on Luci), we skip regardless.
expect(_testRecommendations(os: 'linux', hasCirrus: true, hasGold: true, hasFlutterRoot: true), _Comparator.local); // TODO(ianh): this should be skip 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.local); // TODO(ianh): this should be skip expect(_testRecommendations(os: 'linux', hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.skip);
}); });
test('Branch names', () { test('Branch names', () {
......
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

HIAST - GitLab