From 5b126127d3655a89cf571247782da3cb6454e7b6 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Mon, 11 Nov 2019 15:24:35 -0800 Subject: [PATCH 1/2] Added fallback to skipping comparator when offline. --- .../flutter_goldens/lib/flutter_goldens.dart | 47 ++++++++++++++----- .../test/flutter_goldens_test.dart | 16 +++++++ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index c5bf8a09bf3b8..4fbb662036bac 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:io' as io; import 'dart:typed_data'; import 'package:file/file.dart'; @@ -30,11 +31,14 @@ Future main(FutureOr testMain()) async { goldenFileComparator = await FlutterSkiaGoldFileComparator.fromDefaultComparator(platform); } else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) { goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform); - } else if (FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(platform)){ - goldenFileComparator = FlutterSkippingGoldenFileComparator.fromDefaultComparator(); + } 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); } + await testMain(); } @@ -74,7 +78,8 @@ Future main(FutureOr testMain()) async { /// 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. +/// 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 /// URIs relative to the specified [basedir], and retrieve golden baselines @@ -349,7 +354,8 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { /// 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. +/// 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: /// @@ -370,25 +376,32 @@ class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator { FlutterSkippingGoldenFileComparator( final Uri basedir, final SkiaGoldClient skiaClient, - ) : super(basedir, skiaClient); + this.reason, + ) : assert(reason != null), + super(basedir, skiaClient); + + /// Describes the reason for using the [FlutterSkippingGoldenFileComparator]. + /// + /// Cannot be null. + final String reason; /// Creates a new [FlutterSkippingGoldenFileComparator] that mirrors the /// relative path resolution of the default [goldenFileComparator]. - static FlutterSkippingGoldenFileComparator fromDefaultComparator({ + static FlutterSkippingGoldenFileComparator fromDefaultComparator( + String reason, { LocalFileComparator defaultComparator, }) { defaultComparator ??= goldenFileComparator; const FileSystem fs = LocalFileSystem(); final Uri basedir = defaultComparator.basedir; final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir)); - return FlutterSkippingGoldenFileComparator(basedir, skiaClient); + return FlutterSkippingGoldenFileComparator(basedir, skiaClient, reason); } @override Future compare(Uint8List imageBytes, Uri golden) async { print( - 'Skipping "$golden" test : Golden file testing is unavailable on LUCI and ' - 'some Cirrus shards.' + 'Skipping "$golden" test : $reason' ); return true; } @@ -457,10 +470,11 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC final Platform platform, { SkiaGoldClient goldens, LocalFileComparator defaultComparator, + @visibleForTesting + Directory baseDirectory, }) async { - defaultComparator ??= goldenFileComparator; - final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory( + baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory( defaultComparator, platform, ); @@ -470,7 +484,16 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC } goldens ??= SkiaGoldClient(baseDirectory); - await goldens.getExpectations(); + + try { + await goldens.getExpectations(); + } on io.OSError catch (_) { + return FlutterSkippingGoldenFileComparator( + baseDirectory.uri, + goldens, + 'No network connection available for contacting Gold.', + ); + } return FlutterLocalFileComparator(baseDirectory.uri, goldens); } diff --git a/packages/flutter_goldens/test/flutter_goldens_test.dart b/packages/flutter_goldens/test/flutter_goldens_test.dart index 6269d94aeb2de..e453f78a9cef6 100644 --- a/packages/flutter_goldens/test/flutter_goldens_test.dart +++ b/packages/flutter_goldens/test/flutter_goldens_test.dart @@ -762,6 +762,20 @@ void main() { shouldThrow = false; completer.complete(Future.value(false)); }); + + test('returns FlutterSkippingGoldenFileComparator when network connection is unavailable', () async { + final MockDirectory mockDirectory = MockDirectory(); + when(mockDirectory.existsSync()).thenReturn(true); + when(mockDirectory.uri).thenReturn(Uri.parse('/flutter')); + when(mockSkiaClient.getExpectations()) + .thenAnswer((_) => throw const OSError()); + final FlutterGoldenFileComparator comparator = await FlutterLocalFileComparator.fromDefaultComparator( + platform, + goldens: mockSkiaClient, + baseDirectory: mockDirectory, + ); + expect(comparator.runtimeType, FlutterSkippingGoldenFileComparator); + }); }); }); } @@ -772,6 +786,8 @@ class MockSkiaGoldClient extends Mock implements SkiaGoldClient {} class MockLocalFileComparator extends Mock implements LocalFileComparator {} +class MockDirectory extends Mock implements Directory {} + class MockHttpClient extends Mock implements HttpClient {} class MockHttpClientRequest extends Mock implements HttpClientRequest {} From 431f6035981123d69df0132d4c648a2f194d40a4 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Fri, 15 Nov 2019 11:35:43 -0800 Subject: [PATCH 2/2] Review --- packages/flutter_goldens/lib/flutter_goldens.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index 4fbb662036bac..b83a3d6a2d7f9 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -464,13 +464,12 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC /// Creates a new [FlutterLocalFileComparator] that mirrors the /// relative path resolution of the default [goldenFileComparator]. /// - /// The [goldens] and [defaultComparator] parameters are visible for testing - /// purposes only. + /// The [goldens], [defaultComparator], and [baseDirectory] parameters are + /// visible for testing purposes only. static Future fromDefaultComparator( final Platform platform, { SkiaGoldClient goldens, LocalFileComparator defaultComparator, - @visibleForTesting Directory baseDirectory, }) async { defaultComparator ??= goldenFileComparator;