Skip to content

Commit

Permalink
Rename isAvailableForEnvironment to isForEnvironment (#143176)
Browse files Browse the repository at this point in the history 
This is part 4 of a broken down version of the #140101 refactor.

This PR renames isAvailableForEnvironment to isForEnvironment and replaces a regular expression with a simple function. (The latter will change the behaviour for people with branch names like `mainly_refactors` or `chess_master_experiment` or whatever, but I'm pretty sure the old behaviour was not intended.)
  • Loading branch information
@Hixie
Hixie authored Mar 8, 2024
1 parent fde345a commit 5bd34ef
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 34 deletions.
27 changes: 16 additions & 11 deletions packages/flutter_goldens/lib/flutter_goldens.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ export 'skia_client.dart';
// /packages/flutter/test/widgets/basic_test.dart

const String _kFlutterRootKey = 'FLUTTER_ROOT';
final RegExp _kMainBranch = RegExp(r'master|main');

bool _isMainBranch(String? branch) {
return branch == 'main'
|| branch == 'master';
}


/// Main method that can be used in a `flutter_test_config.dart` file to set
/// [goldenFileComparator] to an instance of [FlutterGoldenFileComparator] that
Expand All @@ -33,11 +38,11 @@ final RegExp _kMainBranch = RegExp(r'master|main');
/// When set, the `namePrefix` is prepended to the names of all gold images.
Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePrefix}) async {
const Platform platform = LocalPlatform();
if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
if (FlutterPostSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix);
} else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
} else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix);
} else if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
'Golden file testing is not executed on Cirrus, or LUCI environments outside of flutter/flutter.',
namePrefix: namePrefix
Expand Down Expand Up @@ -259,13 +264,13 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {

/// Decides based on the current environment if goldens tests should be
/// executed through Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
static bool isForEnvironment(Platform platform) {
final bool luciPostSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
&& platform.environment.containsKey('GOLDCTL')
// Luci tryjob environments contain this value to inform the [FlutterPreSubmitComparator].
&& !platform.environment.containsKey('GOLD_TRYJOB')
// Only run on main branch.
&& _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
&& _isMainBranch(platform.environment['GIT_BRANCH']);

return luciPostSubmit;
}
Expand Down Expand Up @@ -349,12 +354,12 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {

/// Decides based on the current environment if goldens tests should be
/// executed as pre-submit tests with Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
static bool isForEnvironment(Platform platform) {
final bool luciPreSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
&& platform.environment.containsKey('GOLDCTL')
&& platform.environment.containsKey('GOLD_TRYJOB')
// Only run on the main branch
&& _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
&& _isMainBranch(platform.environment['GIT_BRANCH']);
return luciPreSubmit;
}
}
Expand Down Expand Up @@ -420,12 +425,12 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
///
/// If we are in a CI environment, LUCI or Cirrus, but are not using the other
/// comparators, we skip.
static bool isAvailableForEnvironment(Platform platform) {
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'))
// If we are in CI, skip on branches that are not main.
&& !_kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
&& !_isMainBranch(platform.environment['GIT_BRANCH']);
}
}

Expand All @@ -435,7 +440,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
/// This comparator utilizes the [SkiaGoldClient] to request baseline images for
/// the given device under test for comparison. This comparator is initialized
/// when conditions for all other [FlutterGoldenFileComparators] have not been
/// met, see the `isAvailableForEnvironment` method for each one listed below.
/// met, see the `isForEnvironment` method for each one listed below.
///
/// The [FlutterLocalFileComparator] is intended to run on local machines and
/// serve as a smoke test during development. As such, it will not be able to
Expand Down
13 changes: 10 additions & 3 deletions packages/flutter_goldens/test/comparator_selection_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ _Comparator _testRecommendations({
},
operatingSystem: os,
);
if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
if (FlutterPostSubmitFileComparator.isForEnvironment(platform)) {
return _Comparator.post;
}
if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
return _Comparator.pre;
}
if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
return _Comparator.skip;
}
return _Comparator.local;
Expand Down Expand Up @@ -165,4 +165,11 @@ void main() {
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, hasTryJob: true), _Comparator.local); // TODO(ianh): this should be skip
});

test('Branch names', () {
expect(_testRecommendations(hasLuci: true, hasGold: true, hasFlutterRoot: true), _Comparator.post);
expect(_testRecommendations(branch: 'master', hasLuci: true, hasGold: true, hasFlutterRoot: true), _Comparator.post);
expect(_testRecommendations(branch: 'the_master_of_justice', hasLuci: true, hasGold: true, hasFlutterRoot: true), _Comparator.skip);
expect(_testRecommendations(branch: 'maintain_accuracy', hasLuci: true, hasGold: true, hasFlutterRoot: true), _Comparator.skip);
});
}
40 changes: 20 additions & 20 deletions packages/flutter_goldens/test/flutter_goldens_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -741,7 +741,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -757,7 +757,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -773,7 +773,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -787,7 +787,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -803,7 +803,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -820,7 +820,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand Down Expand Up @@ -893,7 +893,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -910,7 +910,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -927,7 +927,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -944,7 +944,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -957,7 +957,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -972,7 +972,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -987,7 +987,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPreSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -1004,7 +1004,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand All @@ -1025,7 +1025,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1041,7 +1041,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1055,7 +1055,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1069,7 +1069,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingFileComparator.isForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1082,7 +1082,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingFileComparator.isForEnvironment(platform),
isFalse,
);
});
Expand Down

0 comments on commit 5bd34ef

Please sign in to comment.