Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove hidden dependencies on the default LocalPlatform #147342

Merged
merged 1 commit into from May 9, 2024
+68 −47
Merged

Remove hidden dependencies on the default LocalPlatform #147342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 57 additions & 34 deletions packages/flutter_goldens/lib/flutter_goldens.dart
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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