Make FileSystem dependency explicit througout. (#146008)

This is part 8 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.

This particular change attempts to be minimal. A future change will apply some formatting cleanup (newlines, reordering parameters and arguments) for clarity.
This commit is contained in:
Ian Hickson 2024-04-05 16:57:21 -07:00 committed by GitHub
parent 4d1161b591
commit 3748b59e44
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 38 additions and 16 deletions

View File

@ -22,6 +22,8 @@ export 'skia_client.dart';
// titled "Inconsequential golden test" in this file: // titled "Inconsequential golden test" in this file:
// /packages/flutter/test/widgets/basic_test.dart // /packages/flutter/test/widgets/basic_test.dart
// TODO(ianh): sort the parameters and arguments in this file so they use a consistent order througout.
const String _kFlutterRootKey = 'FLUTTER_ROOT'; const String _kFlutterRootKey = 'FLUTTER_ROOT';
bool _isMainBranch(String? branch) { bool _isMainBranch(String? branch) {
@ -38,20 +40,22 @@ bool _isMainBranch(String? branch) {
/// When set, the `namePrefix` is prepended to the names of all gold images. /// When set, the `namePrefix` is prepended to the names of all gold images.
Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePrefix}) async { Future<void> testExecutable(FutureOr<void> Function() testMain, {String? namePrefix}) async {
const Platform platform = LocalPlatform(); const Platform platform = LocalPlatform();
const FileSystem fs = LocalFileSystem();
if (FlutterPostSubmitFileComparator.isForEnvironment(platform)) { if (FlutterPostSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix, log: print); goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix, log: print, fs: fs);
} else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) { } else if (FlutterPreSubmitFileComparator.isForEnvironment(platform)) {
goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix, log: print); goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix, log: print, fs: fs);
} else if (FlutterSkippingFileComparator.isForEnvironment(platform)) { } else if (FlutterSkippingFileComparator.isForEnvironment(platform)) {
goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator( goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
'Golden file testing is not executed on Cirrus, or LUCI environments ' 'Golden file testing is not executed on Cirrus, or LUCI environments '
'outside of flutter/flutter, or in test shards that are not configured ' 'outside of flutter/flutter, or in test shards that are not configured '
'for using goldctl.', 'for using goldctl.',
namePrefix: namePrefix, namePrefix: namePrefix,
log: print log: print,
fs: fs,
); );
} else { } else {
goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform, log: print); goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform, log: print, fs: fs);
} }
await testMain(); await testMain();
} }
@ -97,13 +101,13 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// locally, the [basedir] will also contain any diffs from failed tests, or /// locally, the [basedir] will also contain any diffs from failed tests, or
/// goldens generated from newly introduced tests. /// goldens generated from newly introduced tests.
/// ///
/// The [fs] and [platform] parameters are useful in tests, where the default /// The [platform] parameter is useful in tests, where the default
/// file system and platform can be replaced by mock instances. /// platform can be replaced by a mock instance.
@visibleForTesting @visibleForTesting
FlutterGoldenFileComparator( FlutterGoldenFileComparator(
this.basedir, this.basedir,
this.skiaClient, { this.skiaClient, {
this.fs = const LocalFileSystem(), required this.fs,
this.platform = const LocalPlatform(), this.platform = const LocalPlatform(),
this.namePrefix, this.namePrefix,
required this.log, required this.log,
@ -118,7 +122,6 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
final SkiaGoldClient skiaClient; final SkiaGoldClient skiaClient;
/// The file system used to perform file access. /// The file system used to perform file access.
@visibleForTesting
final FileSystem fs; final FileSystem fs;
/// A wrapper for the [dart:io.Platform] API. /// A wrapper for the [dart:io.Platform] API.
@ -153,8 +156,8 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
LocalFileComparator defaultComparator, LocalFileComparator defaultComparator,
Platform platform, { Platform platform, {
String? suffix, String? suffix,
required FileSystem fs,
}) { }) {
const FileSystem fs = LocalFileSystem();
final Directory flutterRoot = fs.directory(platform.environment[_kFlutterRootKey]); final Directory flutterRoot = fs.directory(platform.environment[_kFlutterRootKey]);
Directory comparisonRoot; Directory comparisonRoot;
@ -229,7 +232,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
FlutterPostSubmitFileComparator( FlutterPostSubmitFileComparator(
super.basedir, super.basedir,
super.skiaClient, { super.skiaClient, {
super.fs, required super.fs,
super.platform, super.platform,
super.namePrefix, super.namePrefix,
required super.log, required super.log,
@ -246,6 +249,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
LocalFileComparator? defaultComparator, LocalFileComparator? defaultComparator,
String? namePrefix, String? namePrefix,
required LogCallback log, required LogCallback log,
required FileSystem fs,
}) async { }) async {
defaultComparator ??= goldenFileComparator as LocalFileComparator; defaultComparator ??= goldenFileComparator as LocalFileComparator;
@ -253,12 +257,13 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
defaultComparator, defaultComparator,
platform, platform,
suffix: 'flutter_goldens_postsubmit.', suffix: 'flutter_goldens_postsubmit.',
fs: fs,
); );
baseDirectory.createSync(recursive: true); baseDirectory.createSync(recursive: true);
goldens ??= SkiaGoldClient(baseDirectory, log: log); goldens ??= SkiaGoldClient(baseDirectory, log: log);
await goldens.auth(); await goldens.auth();
return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens, namePrefix: namePrefix, log: log); return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens, namePrefix: namePrefix, log: log, fs: fs);
} }
@override @override
@ -308,7 +313,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
FlutterPreSubmitFileComparator( FlutterPreSubmitFileComparator(
super.basedir, super.basedir,
super.skiaClient, { super.skiaClient, {
super.fs, required super.fs,
super.platform, super.platform,
super.namePrefix, super.namePrefix,
required super.log, required super.log,
@ -326,6 +331,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
Directory? testBasedir, Directory? testBasedir,
String? namePrefix, String? namePrefix,
required LogCallback log, required LogCallback log,
required FileSystem fs,
}) async { }) async {
defaultComparator ??= goldenFileComparator as LocalFileComparator; defaultComparator ??= goldenFileComparator as LocalFileComparator;
@ -333,6 +339,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
defaultComparator, defaultComparator,
platform, platform,
suffix: 'flutter_goldens_presubmit.', suffix: 'flutter_goldens_presubmit.',
fs: fs,
); );
if (!baseDirectory.existsSync()) { if (!baseDirectory.existsSync()) {
@ -347,6 +354,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
goldens, platform: platform, goldens, platform: platform,
namePrefix: namePrefix, namePrefix: namePrefix,
log: log, log: log,
fs: fs,
); );
} }
@ -401,6 +409,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
this.reason, { this.reason, {
super.namePrefix, super.namePrefix,
required super.log, required super.log,
required super.fs,
}); });
/// Describes the reason for using the [FlutterSkippingFileComparator]. /// Describes the reason for using the [FlutterSkippingFileComparator].
@ -413,12 +422,12 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
LocalFileComparator? defaultComparator, LocalFileComparator? defaultComparator,
String? namePrefix, String? namePrefix,
required LogCallback log, required LogCallback log,
required FileSystem fs,
}) { }) {
defaultComparator ??= goldenFileComparator as LocalFileComparator; defaultComparator ??= goldenFileComparator as LocalFileComparator;
const FileSystem fs = LocalFileSystem();
final Uri basedir = defaultComparator.basedir; final Uri basedir = defaultComparator.basedir;
final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir), log: log); final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir), log: log);
return FlutterSkippingFileComparator(basedir, skiaClient, reason, namePrefix: namePrefix, log: log); return FlutterSkippingFileComparator(basedir, skiaClient, reason, namePrefix: namePrefix, log: log, fs: fs);
} }
@override @override
@ -478,7 +487,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
FlutterLocalFileComparator( FlutterLocalFileComparator(
super.basedir, super.basedir,
super.skiaClient, { super.skiaClient, {
super.fs, required super.fs,
super.platform, super.platform,
required super.log, required super.log,
}); });
@ -494,11 +503,13 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
LocalFileComparator? defaultComparator, LocalFileComparator? defaultComparator,
Directory? baseDirectory, Directory? baseDirectory,
required LogCallback log, required LogCallback log,
required FileSystem fs,
}) async { }) async {
defaultComparator ??= goldenFileComparator as LocalFileComparator; defaultComparator ??= goldenFileComparator as LocalFileComparator;
baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory( baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
defaultComparator, defaultComparator,
platform, platform,
fs: fs,
); );
if (!baseDirectory.existsSync()) { if (!baseDirectory.existsSync()) {
@ -516,6 +527,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
'OSError occurred, could not reach Gold. ' 'OSError occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.', 'Switching to FlutterSkippingGoldenFileComparator.',
log: log, log: log,
fs: fs,
); );
} on io.SocketException catch (_) { } on io.SocketException catch (_) {
return FlutterSkippingFileComparator( return FlutterSkippingFileComparator(
@ -524,6 +536,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
'SocketException occurred, could not reach Gold. ' 'SocketException occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.', 'Switching to FlutterSkippingGoldenFileComparator.',
log: log, log: log,
fs: fs,
); );
} on FormatException catch (_) { } on FormatException catch (_) {
return FlutterSkippingFileComparator( return FlutterSkippingFileComparator(
@ -532,10 +545,11 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
'FormatException occurred, could not reach Gold. ' 'FormatException occurred, could not reach Gold. '
'Switching to FlutterSkippingGoldenFileComparator.', 'Switching to FlutterSkippingGoldenFileComparator.',
log: log, log: log,
fs: fs,
); );
} }
return FlutterLocalFileComparator(baseDirectory.uri, goldens, log: log); return FlutterLocalFileComparator(baseDirectory.uri, goldens, log: log, fs: fs);
} }
@override @override

View File

@ -18,6 +18,8 @@ import 'package:process/process.dart';
import 'json_templates.dart'; import 'json_templates.dart';
// TODO(ianh): make sure all constructors order their arguments in a manner consistent with the defined parameter order
const String _kFlutterRoot = '/flutter'; const String _kFlutterRoot = '/flutter';
// 1x1 transparent pixel // 1x1 transparent pixel
@ -714,6 +716,7 @@ void main() {
final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory( final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory(
defaultComparator, defaultComparator,
platform, platform,
fs: fs,
); );
expect( expect(
basedir.uri, basedir.uri,
@ -852,6 +855,7 @@ void main() {
platform, platform,
goldens: fakeSkiaClient, goldens: fakeSkiaClient,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'), log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
); );
expect(fakeSkiaClient.initCalls, 0); expect(fakeSkiaClient.initCalls, 0);
}); });
@ -1049,6 +1053,7 @@ void main() {
platform, platform,
goldens: fakeSkiaClient, goldens: fakeSkiaClient,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'), log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
); );
expect(fakeSkiaClient.tryInitCalls, 0); expect(fakeSkiaClient.tryInitCalls, 0);
}); });
@ -1358,6 +1363,7 @@ void main() {
goldens: fakeSkiaClient, goldens: fakeSkiaClient,
baseDirectory: fakeDirectory, baseDirectory: fakeDirectory,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'), log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
); );
expect(comparator1.runtimeType, FlutterSkippingFileComparator); expect(comparator1.runtimeType, FlutterSkippingFileComparator);
@ -1367,6 +1373,7 @@ void main() {
goldens: fakeSkiaClient, goldens: fakeSkiaClient,
baseDirectory: fakeDirectory, baseDirectory: fakeDirectory,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'), log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
); );
expect(comparator2.runtimeType, FlutterSkippingFileComparator); expect(comparator2.runtimeType, FlutterSkippingFileComparator);
@ -1376,6 +1383,7 @@ void main() {
goldens: fakeSkiaClient, goldens: fakeSkiaClient,
baseDirectory: fakeDirectory, baseDirectory: fakeDirectory,
log: (String message) => fail('skia gold client printed unexpected output: "$message"'), log: (String message) => fail('skia gold client printed unexpected output: "$message"'),
fs: fs,
); );
expect(comparator3.runtimeType, FlutterSkippingFileComparator); expect(comparator3.runtimeType, FlutterSkippingFileComparator);