Skip to content

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

Revert "Refactoring Gold to enable both Luci & Cirrus support (#49815)" (#52670) 

This reverts commit 7edcc92b.
parent acd51a72
Hide whitespace changes
Inline Side-by-side
Showing with 149 additions and 338 deletions
packages/flutter/lib/src/cupertino/action_sheet.dart
View file @ 48fc1350
......@@ -85,8 +85,7 @@ const double _kDividerThickness = 1.0;
/// sheet title and message text style.
///
/// To display action buttons that look like standard iOS action sheet buttons,
/// provide [CupertinoActionSheetAction]s for the [actions] given to this action
/// sheet.
/// provide [CupertinoActionSheetAction]s for the [actions] given to this action sheet.
///
/// To include a iOS-style cancel button separate from the other buttons,
/// provide an [CupertinoActionSheetAction] for the [cancelButton] given to this
......
packages/flutter_goldens/lib/flutter_goldens.dart
View file @ 48fc1350
......@@ -28,13 +28,13 @@ const String _kFlutterRootKey = 'FLUTTER_ROOT';
/// instantiated is based on the current testing environment.
Future<void> main(FutureOr<void> testMain()) async {
const Platform platform = LocalPlatform();
if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform);
if (FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterSkiaGoldFileComparator.fromDefaultComparator(platform);
} else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform);
} else if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
'Golden file testing is not executed on some Cirrus shards.'
} else if (FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingGoldenFileComparator.fromDefaultComparator(
'Golden file testing is unavailable on LUCI and some Cirrus shards.'
);
} else {
goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform);
......@@ -50,7 +50,7 @@ Future<void> main(FutureOr<void> testMain()) async {
/// different [FlutterGoldenFileComparator]s, depending on the current testing
/// environment.
///
/// * The [FlutterPostSubmitFileComparator] is utilized during post-submit
/// * The [FlutterSkiaGoldFileComparator] is utilized during post-submit
/// testing, after a pull request has landed on the master branch. This
/// comparator uses the [SkiaGoldClient] and the `goldctl` tool to upload
/// tests to the [Flutter Gold dashboard](https://flutter-gold.skia.org).
......@@ -58,34 +58,28 @@ Future<void> main(FutureOr<void> testMain()) async {
/// repository.
///
/// * The [FlutterPreSubmitFileComparator] is utilized in pre-submit testing,
/// before a pull request lands on the master branch. When authorized, this
/// comparator uses the [SkiaGoldClient] to execute tryjobs, allowing
/// contributors to view and check in visual differences before landing the
/// change.
/// before a pull request can land on the master branch. This comparator
/// uses the [SkiaGoldClient] to request the baseline images kept by the
/// [Flutter Gold dashboard](https://flutter-gold.skia.org). It then
/// compares the current test image to the baseline images using the
/// standard [GoldenFileComparator.compareLists] to detect any pixel
/// difference. The [SkiaGoldClient] is also used here to check the active
/// ignores from the dashboard, in order to allow intended changes to pass
/// tests.
///
/// * When unable to authenticate the `goldctl` tool, this comparator
/// uses the [SkiaGoldClient] to request the baseline images kept by the
/// [Flutter Gold dashboard](https://flutter-gold.skia.org). It then
/// compares the current test image to the baseline images using the
/// standard [GoldenFileComparator.compareLists] to detect any pixel
/// difference. The [SkiaGoldClient] is also used in this case to check
/// the active ignores from the dashboard, in order to allow intended
/// changes to pass tests.
///
/// * The [FlutterLocalFileComparator] is used for local development testing.
/// Similar to the unauthorized implementation of the
/// * The [FlutterLocalFileComparator] is used for any other tests run outside
/// of the above conditions. Similar to the
/// [FlutterPreSubmitFileComparator], this comparator will use the
/// [SkiaGoldClient] to request baseline images from
/// [Flutter Gold](https://flutter-gold.skia.org) and manually compare
/// pixels. If a difference is detected, this comparator will
/// [Flutter Gold](https://flutter-gold.skia.org) and compares for the
/// current test image. If a difference is detected, this comparator will
/// generate failure output illustrating the found difference. If a baseline
/// is not found for a given test image, it will consider it a new test and
/// output the new image for verification.
///
/// The [FlutterSkippingFileComparator] is utilized to skip tests outside
/// of the appropriate environments described above. Currently, some Cirrus
/// test shards do not execute golden file testing, and as such do not require
/// a comparator. This comparator is also used when an internet connection is
/// The [FlutterSkippingGoldenFileComparator] is utilized to skip tests outside
/// of the appropriate environments. Currently, tests executing in post-submit
/// on the LUCI build environment are skipped, as post-submit checks are done
/// on Cirrus. This comparator is also used when an internet connection is
/// unavailable.
abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// Creates a [FlutterGoldenFileComparator] that will resolve golden file
......@@ -137,7 +131,7 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// Calculate the appropriate basedir for the current test context.
///
/// The optional [suffix] argument is used by the
/// [FlutterPostSubmitFileComparator] and the [FlutterPreSubmitFileComparator].
/// [FlutterSkiaGoldFileComparator] and the [FlutterPreSubmitFileComparator].
/// These [FlutterGoldenFileComparators] randomize their base directories to
/// maintain thread safety while using the `goldctl` tool.
@protected
......@@ -154,7 +148,7 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
if (!local) {
comparisonRoot = fs.systemTempDirectory.childDirectory(
'skia_goldens_$suffix'
'skia_goldens$suffix'
);
} else {
comparisonRoot = flutterRoot.childDirectory(
......@@ -190,11 +184,12 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
}
}
/// A [FlutterGoldenFileComparator] for testing golden images with Skia Gold in
/// post-submit.
/// A [FlutterGoldenFileComparator] for testing golden images with Skia Gold.
///
/// For testing across all platforms, the [SkiaGoldClient] is used to upload
/// images for framework-related golden tests and process results.
/// images for framework-related golden tests and process results. Currently
/// these tests are designed to be run post-submit on Cirrus CI, informed by the
/// environment.
///
/// See also:
///
......@@ -206,13 +201,13 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterPostSubmitFileComparator] that will test golden file
class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterSkiaGoldFileComparator] that will test golden file
/// images against Skia Gold.
///
/// The [fs] and [platform] parameters are useful in tests, where the default
/// file system and platform can be replaced by mock instances.
FlutterPostSubmitFileComparator(
FlutterSkiaGoldFileComparator(
final Uri basedir,
final SkiaGoldClient skiaClient, {
final FileSystem fs = const LocalFileSystem(),
......@@ -224,12 +219,12 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
platform: platform,
);
/// Creates a new [FlutterPostSubmitFileComparator] that mirrors the relative
/// Creates a new [FlutterSkiaGoldFileComparator] that mirrors the relative
/// path resolution of the default [goldenFileComparator].
///
/// The [goldens] and [defaultComparator] parameters are visible for testing
/// purposes only.
static Future<FlutterPostSubmitFileComparator> fromDefaultComparator(
static Future<FlutterSkiaGoldFileComparator> fromDefaultComparator(
final Platform platform, {
SkiaGoldClient goldens,
LocalFileComparator defaultComparator,
......@@ -243,15 +238,10 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
);
baseDirectory.createSync(recursive: true);
goldens ??= SkiaGoldClient(
baseDirectory,
ci: platform.environment.containsKey('CIRRUS_CI')
? ContinuousIntegrationEnvironment.cirrus
: ContinuousIntegrationEnvironment.luci,
);
goldens ??= SkiaGoldClient(baseDirectory);
await goldens.auth();
await goldens.imgtestInit();
return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens);
return FlutterSkiaGoldFileComparator(baseDirectory.uri, goldens);
}
@override
......@@ -263,40 +253,32 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
return skiaClient.imgtestAdd(golden.path, goldenFile);
}
/// Decides based on the current environment if goldens tests should be
/// executed through Skia Gold.
/// Decides based on the current environment whether goldens tests should be
/// performed against Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
final String cirrusBranch = platform.environment['CIRRUS_BRANCH'] ?? '';
final bool cirrusPostSubmit = platform.environment.containsKey('CIRRUS_CI')
return platform.environment.containsKey('CIRRUS_CI')
&& cirrusPR.isEmpty
&& cirrusBranch == 'master'
&& platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
final bool luciPostSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
// Luci tryjob environments contain this value to inform the [FlutterPreSubmitComparator].
&& !platform.environment.containsKey('GOLD_TRYJOB');
return cirrusPostSubmit || luciPostSubmit;
}
}
/// A [FlutterGoldenFileComparator] for testing golden images before changes are
/// merged into the master branch.
///
/// When authorized (on luci and most cirrus testing conditions), the comparator
/// executes tryjobs using the [SkiaGoldClient].
///
/// When unauthorized, this comparator utilizes the [SkiaGoldClient] to request
/// baseline images for the given device under test for manual comparison.
/// This comparator utilizes the [SkiaGoldClient] to request baseline images for
/// the given device under test for comparison. This comparator is only
/// initialized during pre-submit testing on Cirrus CI.
///
/// See also:
///
/// * [GoldenFileComparator], the abstract class that
/// [FlutterGoldenFileComparator] implements.
/// * [FlutterPostSubmitFileComparator], another
/// * [FlutterSkiaGoldFileComparator], another
/// [FlutterGoldenFileComparator] that uploads tests to the Skia Gold
/// dashboard in post-submit.
/// dashboard.
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
......@@ -340,22 +322,10 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
if (!baseDirectory.existsSync())
baseDirectory.createSync(recursive: true);
goldens ??= SkiaGoldClient(
baseDirectory,
ci: platform.environment.containsKey('CIRRUS_CI')
? ContinuousIntegrationEnvironment.cirrus
: ContinuousIntegrationEnvironment.luci,
);
goldens ??= SkiaGoldClient(baseDirectory);
bool onCirrusWithPermission = false;
if (platform.environment.containsKey('GOLD_SERVICE_ACCOUNT')) {
// Some contributors may not have permission on Cirrus to decrypt the
// service account.
onCirrusWithPermission =
!platform.environment['GOLD_SERVICE_ACCOUNT'].startsWith('ENCRYPTED');
}
final bool onLuci = platform.environment.containsKey('SWARMING_TASK_ID');
if (onCirrusWithPermission || onLuci) {
final bool hasWritePermission = !platform.environment['GOLD_SERVICE_ACCOUNT'].startsWith('ENCRYPTED');
if (hasWritePermission) {
await goldens.auth();
await goldens.tryjobInit();
return _AuthorizedFlutterPreSubmitComparator(
......@@ -386,17 +356,13 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
return false;
}
/// Decides based on the current environment if goldens tests should be
/// executed as pre-submit tests with Skia Gold.
/// Decides based on the current environment whether goldens tests should be
/// performed as pre-submit tests with Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
final bool cirrusPreSubmit = platform.environment.containsKey('CIRRUS_CI')
return platform.environment.containsKey('CIRRUS_CI')
&& cirrusPR.isNotEmpty
&& platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
final bool luciPreSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
&& platform.environment.containsKey('GOLD_TRYJOB');
return cirrusPreSubmit || luciPreSubmit;
}
}
......@@ -476,15 +442,19 @@ class _UnauthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileCompar
}
}
/// A [FlutterGoldenFileComparator] for testing conditions that do not execute
/// golden file tests.
/// A [FlutterGoldenFileComparator] for controlling post-submit testing
/// conditions that do not execute golden file tests.
///
/// Currently, this comparator is used in some Cirrus test shards, and when an
/// internet connection is not available for contacting Gold.
/// Currently, this comparator is used in post-submit checks on LUCI and with
/// some Cirrus shards that do not run framework tests. This comparator is also
/// used when an internet connection is not available for contacting Gold.
///
/// See also:
///
/// * [FlutterPostSubmitFileComparator], another [FlutterGoldenFileComparator]
/// * [FlutterGoldensRepositoryFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images using the
/// flutter/goldens repository.
/// * [FlutterSkiaGoldFileComparator], another [FlutterGoldenFileComparator]
/// that tests golden images through Skia Gold.
/// * [FlutterPreSubmitFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images before changes are
......@@ -492,24 +462,24 @@ class _UnauthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileCompar
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterSkippingFileComparator] that will skip tests that
class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterSkippingGoldenFileComparator] that will skip tests that
/// are not in the right environment for golden file testing.
FlutterSkippingFileComparator(
FlutterSkippingGoldenFileComparator(
final Uri basedir,
final SkiaGoldClient skiaClient,
this.reason,
) : assert(reason != null),
super(basedir, skiaClient);
/// Describes the reason for using the [FlutterSkippingFileComparator].
/// Describes the reason for using the [FlutterSkippingGoldenFileComparator].
///
/// Cannot be null.
final String reason;
/// Creates a new [FlutterSkippingFileComparator] that mirrors the
/// Creates a new [FlutterSkippingGoldenFileComparator] that mirrors the
/// relative path resolution of the default [goldenFileComparator].
static FlutterSkippingFileComparator fromDefaultComparator(
static FlutterSkippingGoldenFileComparator fromDefaultComparator(
String reason, {
LocalFileComparator defaultComparator,
}) {
......@@ -517,7 +487,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
const FileSystem fs = LocalFileSystem();
final Uri basedir = defaultComparator.basedir;
final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir));
return FlutterSkippingFileComparator(basedir, skiaClient, reason);
return FlutterSkippingGoldenFileComparator(basedir, skiaClient, reason);
}
@override
......@@ -531,11 +501,8 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
@override
Future<void> update(Uri golden, Uint8List imageBytes) => null;
/// Decides, based on the current environment, if this comparator should be
/// Decides based on the current environment whether this comparator should be
/// used.
///
/// If we are in a CI environment, luci or Cirrus, but are not using the other
/// comparators, we skip.
static bool isAvailableForEnvironment(Platform platform) {
return platform.environment.containsKey('SWARMING_TASK_ID')
|| platform.environment.containsKey('CIRRUS_CI');
......@@ -559,13 +526,13 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
///
/// * [GoldenFileComparator], the abstract class that
/// [FlutterGoldenFileComparator] implements.
/// * [FlutterPostSubmitFileComparator], another
/// * [FlutterSkiaGoldFileComparator], another
/// [FlutterGoldenFileComparator] that uploads tests to the Skia Gold
/// dashboard.
/// * [FlutterPreSubmitFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images before changes are
/// merged into the master branch.
/// * [FlutterSkippingFileComparator], another
/// * [FlutterSkippingGoldenFileComparator], another
/// [FlutterGoldenFileComparator] that controls post-submit testing
/// conditions that do not execute golden file tests.
class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalComparisonOutput {
......@@ -613,14 +580,14 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
try {
await goldens.getExpectations();
} on io.OSError catch (_) {
return FlutterSkippingFileComparator(
return FlutterSkippingGoldenFileComparator(
baseDirectory.uri,
goldens,
'OSError occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.',
);
} on io.SocketException catch (_) {
return FlutterSkippingFileComparator(
return FlutterSkippingGoldenFileComparator(
baseDirectory.uri,
goldens,
'SocketException occurred, could not reach Gold. '
......
packages/flutter_goldens/test/flutter_goldens_test.dart
View file @ 48fc1350
......@@ -108,7 +108,6 @@ void main() {
process: process,
platform: platform,
httpClient: mockHttpClient,
ci: ContinuousIntegrationEnvironment.cirrus,
);
when(process.run(any))
......@@ -166,76 +165,6 @@ void main() {
);
});
test('correctly inits tryjob for luci', () async {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'GOLDCTL' : 'goldctl',
'SWARMING_TASK_ID' : '4ae997b50dfd4d11',
'LOGDOG_STREAM_PREFIX' : 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672',
'GOLD_TRYJOB' : 'refs/pull/49815/head',
},
operatingSystem: 'macos'
);
skiaClient = SkiaGoldClient(
workDirectory,
fs: fs,
process: process,
platform: platform,
httpClient: mockHttpClient,
ci: ContinuousIntegrationEnvironment.luci,
);
final List<String> ciArguments = skiaClient.getCIArguments();
expect(
ciArguments,
equals(
<String>[
'--changelist', '49815',
'--cis', 'buildbucket',
'--jobid', '8885996262141582672',
],
),
);
});
test('correctly inits tryjob for cirrus', () async {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'GOLDCTL' : 'goldctl',
'CIRRUS_CI' : 'true',
'CIRRUS_TASK_ID' : '8885996262141582672',
'CIRRUS_PR' : '49815',
},
operatingSystem: 'macos'
);
skiaClient = SkiaGoldClient(
workDirectory,
fs: fs,
process: process,
platform: platform,
httpClient: mockHttpClient,
ci: ContinuousIntegrationEnvironment.cirrus,
);
final List<String> ciArguments = skiaClient.getCIArguments();
expect(
ciArguments,
equals(
<String>[
'--changelist', '49815',
'--cis', 'cirrus',
'--jobid', '8885996262141582672',
],
),
);
});
group('Request Handling', () {
String testName;
String pullRequestNumber;
......@@ -527,12 +456,12 @@ void main() {
});
group('FlutterGoldenFileComparator', () {
FlutterPostSubmitFileComparator comparator;
FlutterSkiaGoldFileComparator comparator;
setUp(() {
final Directory basedir = fs.directory('flutter/test/library/')
..createSync(recursive: true);
comparator = FlutterPostSubmitFileComparator(
comparator = FlutterSkiaGoldFileComparator(
basedir.uri,
MockSkiaGoldClient(),
fs: fs,
......@@ -568,7 +497,7 @@ void main() {
setUp(() {
final Directory basedir = fs.directory('flutter/test/library/')
..createSync(recursive: true);
comparator = FlutterPostSubmitFileComparator(
comparator = FlutterSkiaGoldFileComparator(
basedir.uri,
mockSkiaClient,
fs: fs,
......@@ -577,21 +506,7 @@ void main() {
});
group('correctly determines testing environment', () {
test('returns true for Luci', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'SWARMING_TASK_ID' : '12345678990',
},
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns true for Cirrus', () {
test('returns true', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -603,7 +518,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
......@@ -620,7 +535,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -636,7 +551,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -651,7 +566,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -668,7 +583,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -680,7 +595,7 @@ void main() {
final MockSkiaGoldClient mockSkiaClient = MockSkiaGoldClient();
group('correctly determines testing environment', () {
test('returns true for Cirrus', () {
test('returns true', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -696,21 +611,6 @@ void main() {
);
});
test('returns true for Luci', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'SWARMING_TASK_ID' : '12345678990',
'GOLD_TRYJOB' : 'git/ref/12345/head'
},
operatingSystem: 'macos'
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns false - no PR', () {
platform = FakePlatform(
environment: <String, String>{
......@@ -742,7 +642,7 @@ void main() {
);
});
test('returns false - not on Cirrus or Luci', () {
test('returns false - not on Cirrus', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -865,20 +765,33 @@ void main() {
group('Skipping', () {
group('correctly determines testing environment', () {
test('returns true on Cirrus shards that don\'t run golden tests', () {
test('returns true on LUCI', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI' : 'yep',
'SWARMING_TASK_ID' : '1234567890',
},
operatingSystem: 'macos'
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns true on Cirrus', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI' : 'yep',
},
operatingSystem: 'macos'
);
expect(
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns false - no CI', () {
platform = FakePlatform(
environment: <String, String>{
......@@ -887,7 +800,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkippingFileComparator.isAvailableForEnvironment(
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(
platform),
isFalse,
);
......@@ -978,7 +891,7 @@ void main() {
goldens: mockSkiaClient,
baseDirectory: mockDirectory,
);
expect(comparator.runtimeType, FlutterSkippingFileComparator);
expect(comparator.runtimeType, FlutterSkippingGoldenFileComparator);
when(mockSkiaClient.getExpectations())
.thenAnswer((_) => throw const SocketException("Can't reach Gold"));
......@@ -987,7 +900,7 @@ void main() {
goldens: mockSkiaClient,
baseDirectory: mockDirectory,
);
expect(comparator.runtimeType, FlutterSkippingFileComparator);
expect(comparator.runtimeType, FlutterSkippingGoldenFileComparator);
});
});
});
......
packages/flutter_goldens_client/lib/skia_client.dart
View file @ 48fc1350
......@@ -21,12 +21,6 @@ const String _kGoldctlKey = 'GOLDCTL';
const String _kServiceAccountKey = 'GOLD_SERVICE_ACCOUNT';
const String _kTestBrowserKey = 'FLUTTER_TEST_BROWSER';
/// Enum representing the supported CI environments used by flutter/flutter.
enum ContinuousIntegrationEnvironment {
luci,
cirrus,
}
/// A client for uploading image tests and making baseline requests to the
/// Flutter Gold Dashboard.
class SkiaGoldClient {
......@@ -35,7 +29,6 @@ class SkiaGoldClient {
this.fs = const LocalFileSystem(),
this.process = const LocalProcessManager(),
this.platform = const LocalPlatform(),
this.ci,
io.HttpClient httpClient,
}) : assert(workDirectory != null),
assert(fs != null),
......@@ -45,26 +38,23 @@ class SkiaGoldClient {
/// The file system to use for storing the local clone of the repository.
///
/// This is useful in tests, where a local file system (the default) can be
/// replaced by a memory file system.
/// This is useful in tests, where a local file system (the default) can
/// be replaced by a memory file system.
final FileSystem fs;
/// A wrapper for the [dart:io.Platform] API.
///
/// This is useful in tests, where the system platform (the default) can be
/// replaced by a mock platform instance.
/// This is useful in tests, where the system platform (the default) can
/// be replaced by a mock platform instance.
final Platform platform;
/// A controller for launching sub-processes.
///
/// This is useful in tests, where the real process manager (the default) can
/// be replaced by a mock process manager that doesn't really create
/// This is useful in tests, where the real process manager (the default)
/// can be replaced by a mock process manager that doesn't really create
/// sub-processes.
final ProcessManager process;
/// What testing environment we may be in, like Cirrus or Luci.
final ContinuousIntegrationEnvironment ci;
/// A client for making Http requests to the Flutter Gold dashboard.
final io.HttpClient httpClient;
......@@ -79,9 +69,9 @@ class SkiaGoldClient {
/// A map of known golden file tests and their associated positive image
/// hashes.
///
/// This is set and used by the [FlutterLocalFileComparator] and the
/// [_UnauthorizedFlutterPreSubmitComparator] to test against golden masters
/// maintained in the Flutter Gold dashboard.
/// This is set and used by the [FlutterLocalFileComparator] and
/// [FlutterPreSubmitFileComparator] to test against golden masters maintained
/// in the Flutter Gold dashboard.
Map<String, List<String>> get expectations => _expectations;
Map<String, List<String>> _expectations;
......@@ -104,58 +94,32 @@ class SkiaGoldClient {
/// Prepares the local work space for golden file testing and calls the
/// goldctl `auth` command.
///
/// This ensures that the goldctl tool is authorized and ready for testing.
/// Used by the [FlutterPostSubmitFileComparator] and the
/// [_AuthorizedFlutterPreSubmitComparator].
///
/// Based on the current environment, the goldctl tool may be authorized by
/// a service account provided by Cirrus, or through the context provided by a
/// luci environment.
/// This ensures that the goldctl tool is authorized and ready for testing. It
/// will only be called once for each instance of
/// [FlutterSkiaGoldFileComparator].
Future<void> auth() async {
if (await clientIsAuthorized())
return;
List<String> authArguments;
String failureContext;
switch (ci) {
case ContinuousIntegrationEnvironment.luci:
authArguments = <String>[
'auth',
'--work-dir', workDirectory
.childDirectory('temp')
.path,
'--luci',
];
failureContext =
'Luci environments authenticate using the file provided '
'by LUCI_CONTEXT. There may be an error with this file or Gold '
'authentication.';
break;
case ContinuousIntegrationEnvironment.cirrus:
if (_serviceAccount.isEmpty) {
final StringBuffer buf = StringBuffer()
..writeln('The Gold service account is unavailable.')..writeln(
'Without a service account, Gold can not be authorized.')..writeln(
'Please check your user permissions and current comparator.');
throw Exception(buf.toString());
}
final File authorization = workDirectory.childFile('serviceAccount.json');
await authorization.writeAsString(_serviceAccount);
authArguments = <String>[
'auth',
'--service-account', authorization.path,
'--work-dir', workDirectory
.childDirectory('temp')
.path,
];
failureContext = 'This could be caused by incorrect user permissions on '
'Cirrus, if the debug information below contains ENCRYPTED, the wrong '
'comparator was chosen for the test case.';
break;
if (_serviceAccount.isEmpty) {
final StringBuffer buf = StringBuffer()
..writeln('The Gold service account is unavailable.')
..writeln('Without a service account, Gold can not be authorized.')
..writeln('Please check your user permissions and current comparator.');
throw Exception(buf.toString());
}
final File authorization = workDirectory.childFile('serviceAccount.json');
await authorization.writeAsString(_serviceAccount);
final List<String> authArguments = <String>[
'auth',
'--service-account', authorization.path,
'--work-dir', workDirectory
.childDirectory('temp')
.path,
];
final io.ProcessResult result = await io.Process.run(
_goldctl,
authArguments,
......@@ -164,7 +128,10 @@ class SkiaGoldClient {
if (result.exitCode != 0) {
final StringBuffer buf = StringBuffer()
..writeln('Skia Gold authorization failed.')
..writeln(failureContext)
..writeln('This could be caused by incorrect user permissions, if the ')
..writeln('debug information below contains ENCRYPTED, the wrong ')
..writeln('comparator was chosen for the test case.')
..writeln()
..writeln('Debug information for Gold:')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
......@@ -178,10 +145,6 @@ class SkiaGoldClient {
/// It will only be called once for each instance of an
/// [_UnauthorizedFlutterPreSubmitComparator].
Future<void> emptyAuth() async {
// We only use emptyAuth when the service account cannot be decrypted on
// Cirrus.
assert(ci == ContinuousIntegrationEnvironment.cirrus);
final List<String> authArguments = <String>[
'auth',
'--work-dir', workDirectory
......@@ -208,8 +171,7 @@ class SkiaGoldClient {
/// Executes the `imgtest init` command in the goldctl tool.
///
/// The `imgtest` command collects and uploads test results to the Skia Gold
/// backend, the `init` argument initializes the current test. Used by the
/// [FlutterPostSubmitFileComparator].
/// backend, the `init` argument initializes the current test.
Future<void> imgtestInit() async {
final File keys = workDirectory.childFile('keys.json');
final File failures = workDirectory.childFile('failures.json');
......@@ -265,7 +227,7 @@ class SkiaGoldClient {
/// result.
///
/// The [testName] and [goldenFile] parameters reference the current
/// comparison being evaluated by the [FlutterPostSubmitFileComparator].
/// comparison being evaluated by the [FlutterSkiaGoldFileComparator].
Future<bool> imgtestAdd(String testName, File goldenFile) async {
assert(testName != null);
assert(goldenFile != null);
......@@ -298,8 +260,7 @@ class SkiaGoldClient {
/// Executes the `imgtest init` command in the goldctl tool for tryjobs.
///
/// The `imgtest` command collects and uploads test results to the Skia Gold
/// backend, the `init` argument initializes the current tryjob. Used by the
/// [_AuthorizedFlutterPreSubmitComparator].
/// backend, the `init` argument initializes the current tryjob.
Future<void> tryjobInit() async {
final File keys = workDirectory.childFile('keys.json');
final File failures = workDirectory.childFile('failures.json');
......@@ -307,6 +268,9 @@ class SkiaGoldClient {
await keys.writeAsString(_getKeysJSON());
await failures.create();
final String commitHash = await _getCurrentCommit();
final String pullRequest = platform.environment['CIRRUS_PR'];
final String cirrusTaskID = platform.environment['CIRRUS_TASK_ID'];
final List<String> imgtestInitArguments = <String>[
'imgtest', 'init',
......@@ -319,11 +283,12 @@ class SkiaGoldClient {
'--failure-file', failures.path,
'--passfail',
'--crs', 'github',
'--changelist', pullRequest,
'--cis', 'cirrus',
'--jobid', cirrusTaskID,
'--patchset_id', commitHash,
];
imgtestInitArguments.addAll(getCIArguments());
if (imgtestInitArguments.contains(null)) {
final StringBuffer buf = StringBuffer()
..writeln('A null argument was provided for Skia Gold tryjob init.')
......@@ -487,12 +452,11 @@ class SkiaGoldClient {
/// Returns a boolean value for whether or not the given test and current pull
/// request are ignored on Flutter Gold.
///
/// This is only relevant when used by the
/// [_UnauthorizedFlutterPreSubmitComparator] when a golden file test fails.
/// In order to land a change to an existing golden file, an ignore must be
/// set up in Flutter Gold. This will serve as a flag to permit the change to
/// land, protect against any unwanted changes, and ensure that changes that
/// have landed are triaged.
/// This is only relevant when used by the [FlutterPreSubmitFileComparator]
/// when a golden file test fails. In order to land a change to an existing
/// golden file, an ignore must be set up in Flutter Gold. This will serve as
/// a flag to permit the change to land, protect against any unwanted changes,
/// and ensure that changes that have landed are triaged.
Future<bool> testIsIgnoredForPullRequest(String pullRequest, String testName) async {
bool ignoreIsActive = false;
testName = cleanTestName(testName);
......@@ -610,7 +574,6 @@ class SkiaGoldClient {
String _getKeysJSON() {
final Map<String, dynamic> keys = <String, dynamic>{
'Platform' : platform.operatingSystem,
'CI' : ci.toString().split('.').last,
};
if (platform.environment[_kTestBrowserKey] != null)
keys['Browser'] = platform.environment[_kTestBrowserKey];
......@@ -638,34 +601,6 @@ class SkiaGoldClient {
}
return false;
}
/// Returns a list of arguments for initializing a tryjob based on the testing
/// environment.
List<String> getCIArguments() {
String pullRequest;
String jobId;
String cis;
switch (ci) {
case ContinuousIntegrationEnvironment.luci:
jobId = platform.environment['LOGDOG_STREAM_PREFIX'].split('/').last;
final List<String> refs = platform.environment['GOLD_TRYJOB'].split('/');
pullRequest = refs[refs.length - 2];
cis = 'buildbucket';
break;
case ContinuousIntegrationEnvironment.cirrus:
pullRequest = platform.environment['CIRRUS_PR'];
jobId = platform.environment['CIRRUS_TASK_ID'];
cis = 'cirrus';
break;
}
return <String>[
'--changelist', pullRequest,
'--cis', cis,
'--jobid', jobId,
];
}
}
/// Used to make HttpRequests during testing.
......@@ -688,10 +623,7 @@ class SkiaGoldDigest {
return SkiaGoldDigest(
imageHash: json['digest'] as String,
paramSet: Map<String, dynamic>.from(json['paramset'] as Map<String, dynamic> ??
<String, List<String>>{
'Platform': <String>[],
'Browser' : <String>[],
}),
<String, List<String>>{'Platform': <String>[]}),
testName: json['test'] as String,
status: json['status'] as String,
);
......
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