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

Re-land Luci Support for Gold (#52760)

parent 2d9902d9
......@@ -85,7 +85,8 @@ 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
......
......@@ -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 (FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterSkiaGoldFileComparator.fromDefaultComparator(platform);
if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform);
} else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform);
} else if (FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingGoldenFileComparator.fromDefaultComparator(
'Golden file testing is unavailable on LUCI and some Cirrus shards.'
} else if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
'Golden file testing is not executed on some Cirrus & Luci environments.'
);
} 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 [FlutterSkiaGoldFileComparator] is utilized during post-submit
/// * The [FlutterPostSubmitFileComparator] 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,29 +58,35 @@ Future<void> main(FutureOr<void> testMain()) async {
/// repository.
///
/// * The [FlutterPreSubmitFileComparator] is utilized in pre-submit testing,
/// 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.
/// 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.
///
/// * The [FlutterLocalFileComparator] is used for any other tests run outside
/// of the above conditions. Similar to the
/// * 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
/// [FlutterPreSubmitFileComparator], this comparator will use the
/// [SkiaGoldClient] to request baseline images from
/// [Flutter Gold](https://flutter-gold.skia.org) and compares for the
/// current test image. If a difference is detected, this comparator will
/// [Flutter Gold](https://flutter-gold.skia.org) and manually compare
/// pixels. 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 [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.
///
/// The [FlutterSkippingFileComparator] is utilized to skip tests outside
/// of the appropriate environments described above. Currently, some Cirrus
/// test shards and Luci environments do not execute golden file testing, and
/// as such do not require a comparator. This comparator is also used when an
/// internet connection is unavailable.
abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// Creates a [FlutterGoldenFileComparator] that will resolve golden file
/// URIs relative to the specified [basedir], and retrieve golden baselines
......@@ -131,7 +137,7 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// Calculate the appropriate basedir for the current test context.
///
/// The optional [suffix] argument is used by the
/// [FlutterSkiaGoldFileComparator] and the [FlutterPreSubmitFileComparator].
/// [FlutterPostSubmitFileComparator] and the [FlutterPreSubmitFileComparator].
/// These [FlutterGoldenFileComparators] randomize their base directories to
/// maintain thread safety while using the `goldctl` tool.
@protected
......@@ -148,7 +154,7 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
if (!local) {
comparisonRoot = fs.systemTempDirectory.childDirectory(
'skia_goldens$suffix'
'skia_goldens_$suffix'
);
} else {
comparisonRoot = flutterRoot.childDirectory(
......@@ -184,12 +190,11 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
}
}
/// A [FlutterGoldenFileComparator] for testing golden images with Skia Gold.
/// A [FlutterGoldenFileComparator] for testing golden images with Skia Gold in
/// post-submit.
///
/// For testing across all platforms, the [SkiaGoldClient] is used to upload
/// 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.
/// images for framework-related golden tests and process results.
///
/// See also:
///
......@@ -201,13 +206,13 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterSkiaGoldFileComparator] that will test golden file
class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterPostSubmitFileComparator] 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.
FlutterSkiaGoldFileComparator(
FlutterPostSubmitFileComparator(
final Uri basedir,
final SkiaGoldClient skiaClient, {
final FileSystem fs = const LocalFileSystem(),
......@@ -219,12 +224,12 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
platform: platform,
);
/// Creates a new [FlutterSkiaGoldFileComparator] that mirrors the relative
/// Creates a new [FlutterPostSubmitFileComparator] that mirrors the relative
/// path resolution of the default [goldenFileComparator].
///
/// The [goldens] and [defaultComparator] parameters are visible for testing
/// purposes only.
static Future<FlutterSkiaGoldFileComparator> fromDefaultComparator(
static Future<FlutterPostSubmitFileComparator> fromDefaultComparator(
final Platform platform, {
SkiaGoldClient goldens,
LocalFileComparator defaultComparator,
......@@ -238,10 +243,15 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
);
baseDirectory.createSync(recursive: true);
goldens ??= SkiaGoldClient(baseDirectory);
goldens ??= SkiaGoldClient(
baseDirectory,
ci: platform.environment.containsKey('CIRRUS_CI')
? ContinuousIntegrationEnvironment.cirrus
: ContinuousIntegrationEnvironment.luci,
);
await goldens.auth();
await goldens.imgtestInit();
return FlutterSkiaGoldFileComparator(baseDirectory.uri, goldens);
return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens);
}
@override
......@@ -253,32 +263,41 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
return skiaClient.imgtestAdd(golden.path, goldenFile);
}
/// Decides based on the current environment whether goldens tests should be
/// performed against Skia Gold.
/// Decides based on the current environment if goldens tests should be
/// executed through Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
final String cirrusBranch = platform.environment['CIRRUS_BRANCH'] ?? '';
return platform.environment.containsKey('CIRRUS_CI')
final bool cirrusPostSubmit = platform.environment.containsKey('CIRRUS_CI')
&& cirrusPR.isEmpty
&& cirrusBranch == 'master'
&& platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
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');
return cirrusPostSubmit || luciPostSubmit;
}
}
/// A [FlutterGoldenFileComparator] for testing golden images before changes are
/// merged into the master branch.
///
/// 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.
/// 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.
///
/// See also:
///
/// * [GoldenFileComparator], the abstract class that
/// [FlutterGoldenFileComparator] implements.
/// * [FlutterSkiaGoldFileComparator], another
/// * [FlutterPostSubmitFileComparator], another
/// [FlutterGoldenFileComparator] that uploads tests to the Skia Gold
/// dashboard.
/// dashboard in post-submit.
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
......@@ -322,10 +341,22 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
if (!baseDirectory.existsSync())
baseDirectory.createSync(recursive: true);
goldens ??= SkiaGoldClient(baseDirectory);
goldens ??= SkiaGoldClient(
baseDirectory,
ci: platform.environment.containsKey('CIRRUS_CI')
? ContinuousIntegrationEnvironment.cirrus
: ContinuousIntegrationEnvironment.luci,
);
final bool hasWritePermission = !platform.environment['GOLD_SERVICE_ACCOUNT'].startsWith('ENCRYPTED');
if (hasWritePermission) {
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) {
await goldens.auth();
await goldens.tryjobInit();
return _AuthorizedFlutterPreSubmitComparator(
......@@ -356,13 +387,18 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
return false;
}
/// Decides based on the current environment whether goldens tests should be
/// performed as pre-submit tests with Skia Gold.
/// Decides based on the current environment if goldens tests should be
/// executed as pre-submit tests with Skia Gold.
static bool isAvailableForEnvironment(Platform platform) {
final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
return platform.environment.containsKey('CIRRUS_CI')
final bool cirrusPreSubmit = 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('GOLDCTL')
&& platform.environment.containsKey('GOLD_TRYJOB');
return cirrusPreSubmit || luciPreSubmit;
}
}
......@@ -442,19 +478,16 @@ class _UnauthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileCompar
}
}
/// A [FlutterGoldenFileComparator] for controlling post-submit testing
/// conditions that do not execute golden file tests.
/// A [FlutterGoldenFileComparator] for testing conditions that do not execute
/// golden file tests.
///
/// 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.
/// Currently, this comparator is used in some Cirrus test shards and Luci
/// environments, as well as when an internet connection is not available for
/// contacting Gold.
///
/// See also:
///
/// * [FlutterGoldensRepositoryFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images using the
/// flutter/goldens repository.
/// * [FlutterSkiaGoldFileComparator], another [FlutterGoldenFileComparator]
/// * [FlutterPostSubmitFileComparator], another [FlutterGoldenFileComparator]
/// that tests golden images through Skia Gold.
/// * [FlutterPreSubmitFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images before changes are
......@@ -462,24 +495,24 @@ class _UnauthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileCompar
/// * [FlutterLocalFileComparator], another
/// [FlutterGoldenFileComparator] that tests golden images locally on your
/// current machine.
class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterSkippingGoldenFileComparator] that will skip tests that
class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterSkippingFileComparator] that will skip tests that
/// are not in the right environment for golden file testing.
FlutterSkippingGoldenFileComparator(
FlutterSkippingFileComparator(
final Uri basedir,
final SkiaGoldClient skiaClient,
this.reason,
) : assert(reason != null),
super(basedir, skiaClient);
/// Describes the reason for using the [FlutterSkippingGoldenFileComparator].
/// Describes the reason for using the [FlutterSkippingFileComparator].
///
/// Cannot be null.
final String reason;
/// Creates a new [FlutterSkippingGoldenFileComparator] that mirrors the
/// Creates a new [FlutterSkippingFileComparator] that mirrors the
/// relative path resolution of the default [goldenFileComparator].
static FlutterSkippingGoldenFileComparator fromDefaultComparator(
static FlutterSkippingFileComparator fromDefaultComparator(
String reason, {
LocalFileComparator defaultComparator,
}) {
......@@ -487,7 +520,7 @@ class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
const FileSystem fs = LocalFileSystem();
final Uri basedir = defaultComparator.basedir;
final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir));
return FlutterSkippingGoldenFileComparator(basedir, skiaClient, reason);
return FlutterSkippingFileComparator(basedir, skiaClient, reason);
}
@override
......@@ -501,8 +534,11 @@ class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
@override
Future<void> update(Uri golden, Uint8List imageBytes) => null;
/// Decides based on the current environment whether this comparator should be
/// Decides, based on the current environment, if 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');
......@@ -526,13 +562,13 @@ class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
///
/// * [GoldenFileComparator], the abstract class that
/// [FlutterGoldenFileComparator] implements.
/// * [FlutterSkiaGoldFileComparator], another
/// * [FlutterPostSubmitFileComparator], 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.
/// * [FlutterSkippingGoldenFileComparator], another
/// * [FlutterSkippingFileComparator], another
/// [FlutterGoldenFileComparator] that controls post-submit testing
/// conditions that do not execute golden file tests.
class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalComparisonOutput {
......@@ -580,14 +616,14 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
try {
await goldens.getExpectations();
} on io.OSError catch (_) {
return FlutterSkippingGoldenFileComparator(
return FlutterSkippingFileComparator(
baseDirectory.uri,
goldens,
'OSError occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.',
);
} on io.SocketException catch (_) {
return FlutterSkippingGoldenFileComparator(
return FlutterSkippingFileComparator(
baseDirectory.uri,
goldens,
'SocketException occurred, could not reach Gold. '
......
......@@ -108,6 +108,7 @@ void main() {
process: process,
platform: platform,
httpClient: mockHttpClient,
ci: ContinuousIntegrationEnvironment.cirrus,
);
when(process.run(any))
......@@ -165,6 +166,76 @@ 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;
......@@ -456,12 +527,12 @@ void main() {
});
group('FlutterGoldenFileComparator', () {
FlutterSkiaGoldFileComparator comparator;
FlutterPostSubmitFileComparator comparator;
setUp(() {
final Directory basedir = fs.directory('flutter/test/library/')
..createSync(recursive: true);
comparator = FlutterSkiaGoldFileComparator(
comparator = FlutterPostSubmitFileComparator(
basedir.uri,
MockSkiaGoldClient(),
fs: fs,
......@@ -497,7 +568,7 @@ void main() {
setUp(() {
final Directory basedir = fs.directory('flutter/test/library/')
..createSync(recursive: true);
comparator = FlutterSkiaGoldFileComparator(
comparator = FlutterPostSubmitFileComparator(
basedir.uri,
mockSkiaClient,
fs: fs,
......@@ -506,7 +577,22 @@ void main() {
});
group('correctly determines testing environment', () {
test('returns true', () {
test('returns true for Luci', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'SWARMING_TASK_ID' : '12345678990',
'GOLDCTL' : 'goldctl',
},
operatingSystem: 'macos'
);
expect(
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns true for Cirrus', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -518,7 +604,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
......@@ -535,7 +621,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -551,7 +637,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -566,7 +652,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -583,7 +669,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkiaGoldFileComparator.isAvailableForEnvironment(platform),
FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
isFalse,
);
});
......@@ -595,7 +681,7 @@ void main() {
final MockSkiaGoldClient mockSkiaClient = MockSkiaGoldClient();
group('correctly determines testing environment', () {
test('returns true', () {
test('returns true for Cirrus', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -611,6 +697,22 @@ void main() {
);
});
test('returns true for Luci', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'SWARMING_TASK_ID' : '12345678990',
'GOLDCTL' : 'goldctl',
'GOLD_TRYJOB' : 'git/ref/12345/head'
},
operatingSystem: 'macos'
);
expect(
FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns false - no PR', () {
platform = FakePlatform(
environment: <String, String>{
......@@ -642,7 +744,7 @@ void main() {
);
});
test('returns false - not on Cirrus', () {
test('returns false - not on Cirrus or Luci', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -765,21 +867,7 @@ void main() {
group('Skipping', () {
group('correctly determines testing environment', () {
test('returns true on LUCI', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'SWARMING_TASK_ID' : '1234567890',
},
operatingSystem: 'macos'
);
expect(
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns true on Cirrus', () {
test('returns true on Cirrus shards that don\'t run golden tests', () {
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
......@@ -788,10 +876,11 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform),
FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
isTrue,
);
});
test('returns false - no CI', () {
platform = FakePlatform(
environment: <String, String>{
......@@ -800,7 +889,7 @@ void main() {
operatingSystem: 'macos'
);
expect(
FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(
FlutterSkippingFileComparator.isAvailableForEnvironment(
platform),
isFalse,
);
......@@ -891,7 +980,7 @@ void main() {
goldens: mockSkiaClient,
baseDirectory: mockDirectory,
);
expect(comparator.runtimeType, FlutterSkippingGoldenFileComparator);
expect(comparator.runtimeType, FlutterSkippingFileComparator);
when(mockSkiaClient.getExpectations())
.thenAnswer((_) => throw const SocketException("Can't reach Gold"));
......@@ -900,7 +989,7 @@ void main() {
goldens: mockSkiaClient,
baseDirectory: mockDirectory,
);
expect(comparator.runtimeType, FlutterSkippingGoldenFileComparator);
expect(comparator.runtimeType, FlutterSkippingFileComparator);
});
});
});
......
......@@ -21,6 +21,12 @@ 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 {
......@@ -29,6 +35,7 @@ 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),
......@@ -38,23 +45,26 @@ 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;
......@@ -69,9 +79,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
/// [FlutterPreSubmitFileComparator] to test against golden masters maintained
/// in the Flutter Gold dashboard.
/// This is set and used by the [FlutterLocalFileComparator] and the
/// [_UnauthorizedFlutterPreSubmitComparator] to test against golden masters
/// maintained in the Flutter Gold dashboard.
Map<String, List<String>> get expectations => _expectations;
Map<String, List<String>> _expectations;
......@@ -94,31 +104,57 @@ 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. It
/// will only be called once for each instance of
/// [FlutterSkiaGoldFileComparator].
/// 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.
Future<void> auth() async {
if (await clientIsAuthorized())
return;
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);
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 List<String> authArguments = <String>[
'auth',
'--service-account', authorization.path,
'--work-dir', workDirectory
.childDirectory('temp')
.path,
];
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;
}
final io.ProcessResult result = await io.Process.run(
_goldctl,
......@@ -128,10 +164,7 @@ class SkiaGoldClient {
if (result.exitCode != 0) {
final StringBuffer buf = StringBuffer()
..writeln('Skia Gold authorization failed.')
..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(failureContext)
..writeln('Debug information for Gold:')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
......@@ -145,6 +178,10 @@ 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
......@@ -171,7 +208,8 @@ 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.
/// backend, the `init` argument initializes the current test. Used by the
/// [FlutterPostSubmitFileComparator].
Future<void> imgtestInit() async {
final File keys = workDirectory.childFile('keys.json');
final File failures = workDirectory.childFile('failures.json');
......@@ -227,7 +265,7 @@ class SkiaGoldClient {
/// result.
///
/// The [testName] and [goldenFile] parameters reference the current
/// comparison being evaluated by the [FlutterSkiaGoldFileComparator].
/// comparison being evaluated by the [FlutterPostSubmitFileComparator].
Future<bool> imgtestAdd(String testName, File goldenFile) async {
assert(testName != null);
assert(goldenFile != null);
......@@ -260,7 +298,8 @@ 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.
/// backend, the `init` argument initializes the current tryjob. Used by the
/// [_AuthorizedFlutterPreSubmitComparator].
Future<void> tryjobInit() async {
final File keys = workDirectory.childFile('keys.json');
final File failures = workDirectory.childFile('failures.json');
......@@ -268,9 +307,6 @@ 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',
......@@ -283,12 +319,11 @@ 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.')
......@@ -452,11 +487,12 @@ 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 [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.
/// 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.
Future<bool> testIsIgnoredForPullRequest(String pullRequest, String testName) async {
bool ignoreIsActive = false;
testName = cleanTestName(testName);
......@@ -574,6 +610,7 @@ 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];
......@@ -601,6 +638,34 @@ 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.
......@@ -623,7 +688,10 @@ 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>[]}),
<String, List<String>>{
'Platform': <String>[],
'Browser' : <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