Skip to content

Commit

Permalink
Remove hidden dependencies on the default LocalPlatform (#147342)
Browse files Browse the repository at this point in the history 
This is part 13 of a broken down version of the #140101 refactor.

This only makes one dependency explicit. Further PRs will do the same
for other dependencies, until these APIs have no hidden dependencies.
  • Loading branch information
@Hixie
Hixie committed May 9, 2024
1 parent 94a7151 commit e1e742a
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 47 deletions.
91 changes: 57 additions & 34 deletions packages/flutter_goldens/lib/flutter_goldens.dart
View file
Expand Up @@ -59,15 +59,15 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
if (FlutterPostSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPostSubmitFileComparator.fromLocalFileComparator(
localFileComparator: goldenFileComparator as LocalFileComparator,
platform,
platform: platform,
namePrefix: namePrefix,
log: print,
fs: fs,
);
} else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromLocalFileComparator(
localFileComparator: goldenFileComparator as LocalFileComparator,
platform,
platform: platform,
namePrefix: namePrefix,
log: print,
fs: fs,
Expand All @@ -78,14 +78,15 @@ Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePre
'Golden file testing is not executed on Cirrus, or LUCI environments '
'outside of flutter/flutter, or in test shards that are not configured '
'for using goldctl.',
platform: platform,
namePrefix: namePrefix,
log: print,
fs: fs,
);
} else {
goldenFileComparator = await FlutterLocalFileComparator.fromLocalFileComparator(
localFileComparator: goldenFileComparator as LocalFileComparator,
platform,
platform: platform,
log: print,
fs: fs,
);
Expand Down Expand Up @@ -133,15 +134,12 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// information and files for interacting with the [skiaClient]. When testing
/// locally, the [basedir] will also contain any diffs from failed tests, or
/// goldens generated from newly introduced tests.
///
/// The [platform] parameter is useful in tests, where the default
/// platform can be replaced by a mock instance.
@visibleForTesting
FlutterGoldenFileComparator(
this.basedir,
this.skiaClient, {
required this.fs,
this.platform = const LocalPlatform(),
required this.platform,
this.namePrefix,
required this.log,
});
Expand All @@ -157,8 +155,8 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// The file system used to perform file access.
final FileSystem fs;

/// A wrapper for the [dart:io.Platform] API.
@visibleForTesting
/// The environment (current working directory, identity of the OS,
/// environment variables, etc).
final Platform platform;

/// The prefix that is added to all golden names.
Expand Down Expand Up @@ -186,8 +184,8 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
@protected
@visibleForTesting
static Directory getBaseDirectory(
LocalFileComparator defaultComparator,
Platform platform, {
LocalFileComparator defaultComparator, {
required Platform platform,
String? suffix,
required FileSystem fs,
}) {
Expand Down Expand Up @@ -260,13 +258,13 @@ 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.
/// The [fs] parameter is useful in tests, where the default
/// file system can be replaced by mock instances.
FlutterPostSubmitFileComparator(
super.basedir,
super.skiaClient, {
required super.fs,
super.platform,
required super.platform,
super.namePrefix,
required super.log,
});
Expand All @@ -275,27 +273,32 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
/// path resolution of the provided `localFileComparator`.
///
/// The [goldens] parameter is visible for testing purposes only.
static Future<FlutterPostSubmitFileComparator> fromLocalFileComparator(
final Platform platform, {
static Future<FlutterPostSubmitFileComparator> fromLocalFileComparator({
SkiaGoldClient? goldens,
required LocalFileComparator localFileComparator,
required Platform platform,
String? namePrefix,
required LogCallback log,
required FileSystem fs,
}) async {
final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
localFileComparator,
platform,
platform: platform,
suffix: 'flutter_goldens_postsubmit.',
fs: fs,
);
baseDirectory.createSync(recursive: true);

goldens ??= SkiaGoldClient(baseDirectory, log: log);
goldens ??= SkiaGoldClient(
baseDirectory,
log: log,
platform: platform,
);
await goldens.auth();
return FlutterPostSubmitFileComparator(
baseDirectory.uri,
goldens,
platform: platform,
namePrefix: namePrefix,
log: log,
fs: fs,
Expand Down Expand Up @@ -342,13 +345,13 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
/// Creates a [FlutterPreSubmitFileComparator] that will test golden file
/// images against baselines requested from Flutter Gold.
///
/// The [fs] and [platform] parameters are useful in tests, where the default
/// file system and platform can be replaced by mock instances.
/// The [fs] parameter is useful in tests, where the default
/// file system can be replaced by mock instances.
FlutterPreSubmitFileComparator(
super.basedir,
super.skiaClient, {
required super.fs,
super.platform,
required super.platform,
super.namePrefix,
required super.log,
});
Expand All @@ -357,18 +360,18 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
/// relative path resolution of the default [goldenFileComparator].
///
/// The [goldens] parameter is visible for testing purposes only.
static Future<FlutterGoldenFileComparator> fromLocalFileComparator(
final Platform platform, {
static Future<FlutterGoldenFileComparator> fromLocalFileComparator({
SkiaGoldClient? goldens,
required LocalFileComparator localFileComparator,
required Platform platform,
Directory? testBasedir,
String? namePrefix,
required LogCallback log,
required FileSystem fs,
}) async {
final Directory baseDirectory = testBasedir ?? FlutterGoldenFileComparator.getBaseDirectory(
localFileComparator,
platform,
platform: platform,
suffix: 'flutter_goldens_presubmit.',
fs: fs,
);
Expand All @@ -377,12 +380,17 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
baseDirectory.createSync(recursive: true);
}

goldens ??= SkiaGoldClient(baseDirectory, log: log);
goldens ??= SkiaGoldClient(
baseDirectory,
platform: platform,
log: log,
);

await goldens.auth();
return FlutterPreSubmitFileComparator(
baseDirectory.uri,
goldens, platform: platform,
goldens,
platform: platform,
namePrefix: namePrefix,
log: log,
fs: fs,
Expand Down Expand Up @@ -439,6 +447,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
super.skiaClient,
this.reason, {
super.namePrefix,
required super.platform,
required super.log,
required super.fs,
});
Expand All @@ -452,16 +461,22 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
String reason, {
required LocalFileComparator localFileComparator,
String? namePrefix,
required Platform platform,
required LogCallback log,
required FileSystem fs,
}) {
final Uri basedir = localFileComparator.basedir;
final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir), log: log);
final SkiaGoldClient skiaClient = SkiaGoldClient(
fs.directory(basedir),
platform: platform,
log: log,
);
return FlutterSkippingFileComparator(
basedir,
skiaClient,
reason,
namePrefix: namePrefix,
platform: platform,
log: log,
fs: fs,
);
Expand Down Expand Up @@ -519,13 +534,13 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
/// Creates a [FlutterLocalFileComparator] that will test golden file
/// images against baselines requested from Flutter Gold.
///
/// The [fs] and [platform] parameters are useful in tests, where the default
/// file system and platform can be replaced by mock instances.
/// The [fs] parameter is useful in tests, where the default
/// file system can be replaced by mock instances.
FlutterLocalFileComparator(
super.basedir,
super.skiaClient, {
required super.fs,
super.platform,
required super.platform,
required super.log,
});

Expand All @@ -534,25 +549,29 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
///
/// The [goldens] and [baseDirectory] parameters are
/// visible for testing purposes only.
static Future<FlutterGoldenFileComparator> fromLocalFileComparator(
final Platform platform, {
static Future<FlutterGoldenFileComparator> fromLocalFileComparator({
SkiaGoldClient? goldens,
required LocalFileComparator localFileComparator,
required Platform platform,
Directory? baseDirectory,
required LogCallback log,
required FileSystem fs,
}) async {
baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
localFileComparator,
platform,
platform: platform,
fs: fs,
);

if (!baseDirectory.existsSync()) {
baseDirectory.createSync(recursive: true);
}

goldens ??= SkiaGoldClient(baseDirectory, log: log);
goldens ??= SkiaGoldClient(
baseDirectory,
platform: platform,
log: log,
);
try {
// Check if we can reach Gold.
await goldens.getExpectationForTest('');
Expand All @@ -562,6 +581,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
goldens,
'OSError occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.',
platform: platform,
log: log,
fs: fs,
);
Expand All @@ -571,6 +591,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
goldens,
'SocketException occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.',
platform: platform,
log: log,
fs: fs,
);
Expand All @@ -580,6 +601,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
goldens,
'FormatException occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.',
platform: platform,
log: log,
fs: fs,
);
Expand All @@ -588,6 +610,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
return FlutterLocalFileComparator(
baseDirectory.uri,
goldens,
platform: platform,
log: log,
fs: fs,
);
Expand Down
12 changes: 5 additions & 7 deletions packages/flutter_goldens/lib/skia_client.dart
View file
Expand Up @@ -44,15 +44,15 @@ class SkiaException implements Exception {
/// A client for uploading image tests and making baseline requests to the
/// Flutter Gold Dashboard.
class SkiaGoldClient {
/// Creates a [SkiaGoldClient] with the given [workDirectory].
/// Creates a [SkiaGoldClient] with the given [workDirectory] and [Platform].
///
/// All other parameters are optional. They may be provided in tests to
/// override the defaults for [fs], [process], [platform], and [httpClient].
/// override the defaults for [fs], [process], and [httpClient].
SkiaGoldClient(
this.workDirectory, {
this.fs = const LocalFileSystem(),
this.process = const LocalProcessManager(),
this.platform = const LocalPlatform(),
required this.platform,
Abi? abi,
io.HttpClient? httpClient,
required this.log,
Expand All @@ -65,10 +65,8 @@ class SkiaGoldClient {
/// 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.
/// The environment (current working directory, identity of the OS,
/// environment variables, etc).
final Platform platform;

/// A controller for launching sub-processes.
Expand Down
12 changes: 6 additions & 6 deletions packages/flutter_goldens/test/flutter_goldens_test.dart
View file
Expand Up @@ -716,7 +716,7 @@ void main() {
defaultComparator.basedir = flutterRoot.childDirectory('baz').uri;
final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory(
defaultComparator,
platform,
platform: platform,
fs: fs,
);
expect(
Expand Down Expand Up @@ -854,7 +854,7 @@ void main() {
expect(fakeSkiaClient.initCalls, 0);
FlutterPostSubmitFileComparator.fromLocalFileComparator(
localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix),
platform,
platform: platform,
goldens: fakeSkiaClient,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
Expand Down Expand Up @@ -940,7 +940,7 @@ void main() {
expect(fakeSkiaClient.tryInitCalls, 0);
FlutterPostSubmitFileComparator.fromLocalFileComparator(
localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix),
platform,
platform: platform,
goldens: fakeSkiaClient,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
Expand Down Expand Up @@ -1042,7 +1042,7 @@ void main() {
fakeSkiaClient.getExpectationForTestThrowable = const OSError("Can't reach Gold");
final FlutterGoldenFileComparator comparator1 = await FlutterLocalFileComparator.fromLocalFileComparator(
localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix),
platform,
platform: platform,
goldens: fakeSkiaClient,
baseDirectory: fakeDirectory,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
Expand All @@ -1053,7 +1053,7 @@ void main() {
fakeSkiaClient.getExpectationForTestThrowable = const SocketException("Can't reach Gold");
final FlutterGoldenFileComparator comparator2 = await FlutterLocalFileComparator.fromLocalFileComparator(
localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix),
platform,
platform: platform,
goldens: fakeSkiaClient,
baseDirectory: fakeDirectory,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
Expand All @@ -1064,7 +1064,7 @@ void main() {
fakeSkiaClient.getExpectationForTestThrowable = const FormatException("Can't reach Gold");
final FlutterGoldenFileComparator comparator3 = await FlutterLocalFileComparator.fromLocalFileComparator(
localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix),
platform,
platform: platform,
goldens: fakeSkiaClient,
baseDirectory: fakeDirectory,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
Expand Down

0 comments on commit e1e742a

Please sign in to comment.