Skip to content

Commit

Permalink
[flutter_tools] migrate flutter_goldens, flutter_goldens client to nu…
Browse files Browse the repository at this point in the history 
…ll-safety (flutter#64201)
  • Loading branch information
@jonahwilliams @smadey
jonahwilliams authored and smadey committed Aug 27, 2020
1 parent 4ba62c2 commit a56a5e0
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 92 deletions.
40 changes: 18 additions & 22 deletions packages/flutter_goldens/lib/flutter_goldens.dart
View file
Expand Up @@ -103,10 +103,7 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
this.skiaClient, {
this.fs = const LocalFileSystem(),
this.platform = const LocalPlatform(),
}) : assert(basedir != null),
assert(skiaClient != null),
assert(fs != null),
assert(platform != null);
});

/// The directory to which golden file URIs will be resolved in [compare] and
/// [update], cannot be null.
Expand All @@ -132,7 +129,7 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
}

@override
Uri getTestUri(Uri key, int version) => key;
Uri getTestUri(Uri key, int? version) => key;

/// Calculate the appropriate basedir for the current test context.
///
Expand Down Expand Up @@ -231,8 +228,8 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
/// purposes only.
static Future<FlutterPostSubmitFileComparator> fromDefaultComparator(
final Platform platform, {
SkiaGoldClient goldens,
LocalFileComparator defaultComparator,
SkiaGoldClient? goldens,
LocalFileComparator? defaultComparator,
}) async {

defaultComparator ??= goldenFileComparator as LocalFileComparator;
Expand Down Expand Up @@ -326,9 +323,9 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
/// purposes only.
static Future<FlutterGoldenFileComparator> fromDefaultComparator(
final Platform platform, {
SkiaGoldClient goldens,
LocalFileComparator defaultComparator,
final Directory testBasedir,
SkiaGoldClient? goldens,
LocalFileComparator? defaultComparator,
Directory? testBasedir,
}) async {

defaultComparator ??= goldenFileComparator as LocalFileComparator;
Expand All @@ -353,7 +350,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
// Some contributors may not have permission on Cirrus to decrypt the
// service account.
onCirrusWithPermission =
!platform.environment['GOLD_SERVICE_ACCOUNT'].startsWith('ENCRYPTED');
!platform.environment['GOLD_SERVICE_ACCOUNT']!.startsWith('ENCRYPTED');
}
final bool onLuci = platform.environment.containsKey('SWARMING_TASK_ID');
if (onCirrusWithPermission || onLuci) {
Expand Down Expand Up @@ -457,7 +454,7 @@ class _UnauthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileCompar
// low.
skiaClient.getExpectations();
final String testName = skiaClient.cleanTestName(golden.path);
final List<String> testExpectations = skiaClient.expectations[testName];
final List<String>? testExpectations = skiaClient.expectations[testName];
if (testExpectations == null) {
// This is a new test.
print('No expectations provided by Skia Gold for test: $golden. '
Expand Down Expand Up @@ -502,8 +499,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
final Uri basedir,
final SkiaGoldClient skiaClient,
this.reason,
) : assert(reason != null),
super(basedir, skiaClient);
) : super(basedir, skiaClient);

/// Describes the reason for using the [FlutterSkippingFileComparator].
///
Expand All @@ -514,12 +510,12 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
/// relative path resolution of the default [goldenFileComparator].
static FlutterSkippingFileComparator fromDefaultComparator(
String reason, {
LocalFileComparator defaultComparator,
LocalFileComparator? defaultComparator,
}) {
defaultComparator ??= goldenFileComparator as LocalFileComparator;
const FileSystem fs = LocalFileSystem();
final Uri basedir = defaultComparator.basedir;
final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir));
final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir), ci: ContinuousIntegrationEnvironment.none);
return FlutterSkippingFileComparator(basedir, skiaClient, reason);
}

Expand All @@ -532,7 +528,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
}

@override
Future<void> update(Uri golden, Uint8List imageBytes) => null;
Future<void> update(Uri golden, Uint8List imageBytes) async {}

/// Decides, based on the current environment, if this comparator should be
/// used.
Expand Down Expand Up @@ -596,9 +592,9 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
/// visible for testing purposes only.
static Future<FlutterGoldenFileComparator> fromDefaultComparator(
final Platform platform, {
SkiaGoldClient goldens,
LocalFileComparator defaultComparator,
Directory baseDirectory,
SkiaGoldClient? goldens,
LocalFileComparator? defaultComparator,
Directory? baseDirectory,
}) async {
defaultComparator ??= goldenFileComparator as LocalFileComparator;
baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
Expand All @@ -611,7 +607,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
baseDirectory.createSync(recursive: true);
}

goldens ??= SkiaGoldClient(baseDirectory);
goldens ??= SkiaGoldClient(baseDirectory, ci: ContinuousIntegrationEnvironment.none);

try {
await goldens.getExpectations();
Expand All @@ -638,7 +634,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
Future<bool> compare(Uint8List imageBytes, Uri golden) async {
golden = _addPrefix(golden);
final String testName = skiaClient.cleanTestName(golden.path);
final List<String> testExpectations = skiaClient.expectations[testName];
final List<String>? testExpectations = skiaClient.expectations[testName];
if (testExpectations == null) {
// There is no baseline for this test
print('No expectations provided by Skia Gold for test: $golden. '
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_goldens/pubspec.yaml
View file
Expand Up @@ -2,7 +2,7 @@ name: flutter_goldens

environment:
# The pub client defaults to an <2.0.0 sdk constraint which we need to explicitly overwrite.
sdk: ">=2.0.0-dev.68.0 <3.0.0"
sdk: ">=2.10.0-4.0.dev <2.10.0"

dependencies:
# To update these, use "flutter update-packages --force-upgrade".
Expand Down
3 changes: 3 additions & 0 deletions packages/flutter_goldens/test/flutter_goldens_test.dart
View file
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.8
import 'dart:async';
import 'dart:convert';
import 'dart:core';
Expand Down Expand Up @@ -75,6 +76,7 @@ void main() {
process: process,
platform: platform,
httpClient: mockHttpClient,
ci: ContinuousIntegrationEnvironment.luci,
);
});

Expand Down Expand Up @@ -148,6 +150,7 @@ void main() {
process: process,
platform: platform,
httpClient: mockHttpClient,
ci: ContinuousIntegrationEnvironment.luci
);

when(process.run(
Expand Down
76 changes: 34 additions & 42 deletions packages/flutter_goldens_client/lib/skia_client.dart
View file
Expand Up @@ -25,6 +25,7 @@ const String _kTestBrowserKey = 'FLUTTER_TEST_BROWSER';
enum ContinuousIntegrationEnvironment {
luci,
cirrus,
none,
}

/// A client for uploading image tests and making baseline requests to the
Expand All @@ -35,13 +36,9 @@ 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),
assert(process != null),
assert(platform != null),
httpClient = httpClient ?? io.HttpClient();
required this.ci,
io.HttpClient? httpClient,
}) : httpClient = httpClient ?? io.HttpClient();

/// The file system to use for storing the local clone of the repository.
///
Expand Down Expand Up @@ -83,7 +80,7 @@ class SkiaGoldClient {
/// [_UnauthorizedFlutterPreSubmitComparator] to test against golden masters
/// maintained in the Flutter Gold dashboard.
Map<String, List<String>> get expectations => _expectations;
Map<String, List<String>> _expectations;
late Map<String, List<String>> _expectations;

/// The local [Directory] where the Flutter repository is hosted.
///
Expand All @@ -93,13 +90,13 @@ class SkiaGoldClient {
/// The path to the local [Directory] where the goldctl tool is hosted.
///
/// Uses the [platform] environment in this implementation.
String/*!*/ get _goldctl => platform.environment[_kGoldctlKey];
String get _goldctl => platform.environment[_kGoldctlKey]!;

/// The path to the local [Directory] where the service account key is
/// hosted.
///
/// Uses the [platform] environment in this implementation.
String/*!*/ get _serviceAccount => platform.environment[_kServiceAccountKey];
String get _serviceAccount => platform.environment[_kServiceAccountKey]!;

/// Prepares the local work space for golden file testing and calls the
/// goldctl `auth` command.
Expand All @@ -116,9 +113,9 @@ class SkiaGoldClient {
return;

List<String> authArguments;
/*late*/ String failureContext;
String failureContext;

switch (ci/*!*/) {
switch (ci) {
case ContinuousIntegrationEnvironment.luci:
authArguments = <String>[
'auth',
Expand Down Expand Up @@ -154,6 +151,8 @@ class SkiaGoldClient {
'Cirrus, if the debug information below contains ENCRYPTED, the wrong '
'comparator was chosen for the test case.';
break;
case ContinuousIntegrationEnvironment.none:
return;
}

final io.ProcessResult result = await io.Process.run(
Expand Down Expand Up @@ -267,9 +266,6 @@ class SkiaGoldClient {
/// The [testName] and [goldenFile] parameters reference the current
/// comparison being evaluated by the [FlutterPostSubmitFileComparator].
Future<bool> imgtestAdd(String testName, File goldenFile) async {
assert(testName != null);
assert(goldenFile != null);

final List<String> imgtestArguments = <String>[
'imgtest', 'add',
'--work-dir', workDirectory
Expand Down Expand Up @@ -361,9 +357,6 @@ class SkiaGoldClient {
/// The [testName] and [goldenFile] parameters reference the current
/// comparison being evaluated by the [_AuthorizedFlutterPreSubmitComparator].
Future<void> tryjobAdd(String testName, File goldenFile) async {
assert(testName != null);
assert(goldenFile != null);

final List<String> imgtestArguments = <String>[
'imgtest', 'add',
'--work-dir', workDirectory
Expand Down Expand Up @@ -410,9 +403,6 @@ class SkiaGoldClient {
/// comparison being evaluated by the
/// [_UnauthorizedFlutterPreSubmitComparator].
Future<bool> imgtestCheck(String testName, File goldenFile) async {
assert(testName != null);
assert(goldenFile != null);

final List<String> imgtestArguments = <String>[
'imgtest', 'check',
'--work-dir', workDirectory
Expand Down Expand Up @@ -440,15 +430,15 @@ class SkiaGoldClient {
);
const String mainKey = 'master';
const String temporaryKey = 'master_str';
String rawResponse;
late String rawResponse;
try {
final io.HttpClientRequest request = await httpClient.getUrl(requestForExpectations);
final io.HttpClientResponse response = await request.close();
rawResponse = await utf8.decodeStream(response);
final dynamic jsonResponse = json.decode(rawResponse);
if (jsonResponse is! Map<String, dynamic>)
throw const FormatException('Skia gold expectations do not match expected format.');
final Map<String, dynamic> skiaJson = (jsonResponse[mainKey] ?? jsonResponse[temporaryKey]) as Map<String, dynamic>;
final Map<String, dynamic>? skiaJson = (jsonResponse[mainKey] ?? jsonResponse[temporaryKey]) as Map<String, dynamic>?;
if (skiaJson == null)
throw FormatException('Skia gold expectations are missing the "$mainKey" key (and also doesn\'t have "$temporaryKey")! Available keys: ${jsonResponse.keys.join(", ")}');
skiaJson.forEach((String key, dynamic value) {
Expand Down Expand Up @@ -506,7 +496,7 @@ class SkiaGoldClient {
Future<bool> testIsIgnoredForPullRequest(String pullRequest, String testName) async {
bool ignoreIsActive = false;
testName = cleanTestName(testName);
String rawResponse;
late String rawResponse;
await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
final Uri requestForIgnores = Uri.parse(
'https://flutter-gold.skia.org/json/ignores'
Expand Down Expand Up @@ -565,7 +555,7 @@ class SkiaGoldClient {
Future<bool> isValidDigestForExpectation(String expectation, String testName) async {
bool isValid = false;
testName = cleanTestName(testName);
String rawResponse;
late String rawResponse;
await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
final Uri requestForDigest = Uri.parse(
'https://flutter-gold.skia.org/json/details?test=$testName&digest=$expectation'
Expand Down Expand Up @@ -653,22 +643,24 @@ class SkiaGoldClient {
/// Returns a list of arguments for initializing a tryjob based on the testing
/// environment.
List<String> getCIArguments() {
/*late*/ String/*!*/ pullRequest;
/*late*/ String/*!*/ jobId;
/*late*/ String cis;
String pullRequest;
String jobId;
String cis;

switch (ci/*!*/) {
switch (ci) {
case ContinuousIntegrationEnvironment.luci:
jobId = platform.environment['LOGDOG_STREAM_PREFIX'].split('/').last;
final List<String> refs = platform.environment['GOLD_TRYJOB'].split('/');
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'];
pullRequest = platform.environment['CIRRUS_PR']!;
jobId = platform.environment['CIRRUS_TASK_ID']!;
cis = 'cirrus';
break;
case ContinuousIntegrationEnvironment.none:
return <String>[];
}

return <String>[
Expand All @@ -685,17 +677,17 @@ class SkiaGoldHttpOverrides extends io.HttpOverrides {}
/// A digest returned from a request to the Flutter Gold dashboard.
class SkiaGoldDigest {
const SkiaGoldDigest({
this.imageHash,
this.paramSet,
this.testName,
this.status,
required this.imageHash,
required this.paramSet,
required this.testName,
required this.status,
});

/// Create a digest from requested JSON.
factory SkiaGoldDigest.fromJson(Map<String, dynamic> json) {
return SkiaGoldDigest(
imageHash: json['digest'] as String,
paramSet: Map<String, dynamic>.from(json['paramset'] as Map<String, dynamic> ??
paramSet: Map<String, dynamic>.from(json['paramset'] as Map<String, dynamic>? ??
<String, List<String>>{
'Platform': <String>[],
'Browser' : <String>[],
Expand All @@ -706,16 +698,16 @@ class SkiaGoldDigest {
}

/// Unique identifier for the image associated with the digest.
final String/*!*/ imageHash;
final String imageHash;

/// Parameter set for the given test, e.g. Platform : Windows.
final Map<String, dynamic>/*!*/ paramSet;
final Map<String, dynamic> paramSet;

/// Test name associated with the digest, e.g. positive or un-triaged.
final String/*!*/ testName;
final String testName;

/// Status of the given digest, e.g. positive or un-triaged.
final String/*!*/ status;
final String status;

/// Validates a given digest against the current testing conditions.
bool isValid(Platform platform, String name, String expectation) {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_goldens_client/pubspec.yaml
View file
Expand Up @@ -2,7 +2,7 @@ name: flutter_goldens_client

environment:
# The pub client defaults to an <2.0.0 sdk constraint which we need to explicitly overwrite.
sdk: ">=2.0.0-dev.68.0 <3.0.0"
sdk: ">=2.10.0-4.0.dev <2.10.0 <3.0.0"

dependencies:
# To update these, use "flutter update-packages --force-upgrade".
Expand Down

0 comments on commit a56a5e0

Please sign in to comment.