Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reverts "Rename isAvailableForEnvironment to isForEnvironment (#143176)" #144855

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Reverts "Rename isAvailableForEnvironment to isForEnvironment (#143176)" #144855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions packages/flutter_goldens/lib/flutter_goldens.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,7 @@ export 'skia_client.dart';
// /packages/flutter/test/widgets/basic_test.dart

const String _kFlutterRootKey = 'FLUTTER_ROOT';

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

final RegExp _kMainBranch = RegExp(r'master|main');

/// Main method that can be used in a `flutter_test_config.dart` file to set
/// [goldenFileComparator] to an instance of [FlutterGoldenFileComparator] that
Expand All @@ -38,11 +33,11 @@ bool _isMainBranch(String? branch) {
/// 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.isForEnvironment(platform)) {
if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix);
} else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
} else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix);
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
} else if (FlutterSkippingFileComparator.isAvailableForEnvironment(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 @@ -264,13 +259,13 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {

/// Decides based on the current environment if goldens tests should be
/// executed through Skia Gold.
static bool isForEnvironment(Platform platform) {
static bool isAvailableForEnvironment(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.
&& _isMainBranch(platform.environment['GIT_BRANCH']);
&& _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');

return luciPostSubmit;
}
Expand Down Expand Up @@ -354,12 +349,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 isForEnvironment(Platform platform) {
static bool isAvailableForEnvironment(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
&& _isMainBranch(platform.environment['GIT_BRANCH']);
&& _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
return luciPreSubmit;
}
}
Expand Down Expand Up @@ -425,12 +420,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 isForEnvironment(Platform platform) {
static bool isAvailableForEnvironment(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.
&& !_isMainBranch(platform.environment['GIT_BRANCH']);
&& !_kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
}
}

Expand All @@ -440,7 +435,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 `isForEnvironment` method for each one listed below.
/// met, see the `isAvailableForEnvironment` 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: 3 additions & 10 deletions packages/flutter_goldens/test/comparator_selection_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ _Comparator _testRecommendations({
},
operatingSystem: os,
);
if (FlutterPostSubmitFileComparator.isForEnvironment(platform)) {
if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
return _Comparator.post;
}
if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
return _Comparator.pre;
}
if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
return _Comparator.skip;
}
return _Comparator.local;
Expand Down Expand Up @@ -165,11 +165,4 @@ 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -741,7 +741,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -757,7 +757,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -773,7 +773,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -787,7 +787,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -803,7 +803,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -820,7 +820,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand Down Expand Up @@ -893,7 +893,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -910,7 +910,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -927,7 +927,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -944,7 +944,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -957,7 +957,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -972,7 +972,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -987,7 +987,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPreSubmitFileComparator.isForEnvironment(platform),
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -1004,7 +1004,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterPostSubmitFileComparator.isForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand All @@ -1025,7 +1025,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isForEnvironment(platform),
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1041,7 +1041,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isForEnvironment(platform),
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1055,7 +1055,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isForEnvironment(platform),
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1069,7 +1069,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkippingFileComparator.isForEnvironment(platform),
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
Expand All @@ -1082,7 +1082,7 @@ void main() {
operatingSystem: 'macos',
);
expect(
FlutterSkippingFileComparator.isForEnvironment(platform),
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
Expand Down
Loading