From 3748b59e44c908b60bc0878b10ae3288a38b5c5c Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Fri, 5 Apr 2024 16:57:21 -0700 Subject: [PATCH] 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. --- .../flutter_goldens/lib/flutter_goldens.dart | 46 ++++++++++++------- .../test/flutter_goldens_test.dart | 8 ++++ 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index bec4d8a7c3..63abf6ba18 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -22,6 +22,8 @@ export 'skia_client.dart'; // titled "Inconsequential golden test" in this file: // /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'; 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. Future testExecutable(FutureOr Function() testMain, {String? namePrefix}) async { const Platform platform = LocalPlatform(); + const FileSystem fs = LocalFileSystem(); 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)) { - 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)) { goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator( '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.', namePrefix: namePrefix, - log: print + log: print, + fs: fs, ); } else { - goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform, log: print); + goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform, log: print, fs: fs); } await testMain(); } @@ -97,13 +101,13 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { /// locally, the [basedir] will also contain any diffs from failed tests, or /// goldens generated from newly introduced tests. /// - /// The [fs] and [platform] parameters are useful in tests, where the default - /// file system and platform can be replaced by mock instances. + /// The [platform] parameter is useful in tests, where the default + /// platform can be replaced by a mock instance. @visibleForTesting FlutterGoldenFileComparator( this.basedir, this.skiaClient, { - this.fs = const LocalFileSystem(), + required this.fs, this.platform = const LocalPlatform(), this.namePrefix, required this.log, @@ -118,7 +122,6 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { final SkiaGoldClient skiaClient; /// The file system used to perform file access. - @visibleForTesting final FileSystem fs; /// A wrapper for the [dart:io.Platform] API. @@ -153,8 +156,8 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { LocalFileComparator defaultComparator, Platform platform, { String? suffix, + required FileSystem fs, }) { - const FileSystem fs = LocalFileSystem(); final Directory flutterRoot = fs.directory(platform.environment[_kFlutterRootKey]); Directory comparisonRoot; @@ -229,7 +232,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator { FlutterPostSubmitFileComparator( super.basedir, super.skiaClient, { - super.fs, + required super.fs, super.platform, super.namePrefix, required super.log, @@ -246,6 +249,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator { LocalFileComparator? defaultComparator, String? namePrefix, required LogCallback log, + required FileSystem fs, }) async { defaultComparator ??= goldenFileComparator as LocalFileComparator; @@ -253,12 +257,13 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator { defaultComparator, platform, suffix: 'flutter_goldens_postsubmit.', + fs: fs, ); baseDirectory.createSync(recursive: true); goldens ??= SkiaGoldClient(baseDirectory, log: log); await goldens.auth(); - return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens, namePrefix: namePrefix, log: log); + return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens, namePrefix: namePrefix, log: log, fs: fs); } @override @@ -308,7 +313,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { FlutterPreSubmitFileComparator( super.basedir, super.skiaClient, { - super.fs, + required super.fs, super.platform, super.namePrefix, required super.log, @@ -326,6 +331,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { Directory? testBasedir, String? namePrefix, required LogCallback log, + required FileSystem fs, }) async { defaultComparator ??= goldenFileComparator as LocalFileComparator; @@ -333,6 +339,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { defaultComparator, platform, suffix: 'flutter_goldens_presubmit.', + fs: fs, ); if (!baseDirectory.existsSync()) { @@ -347,6 +354,7 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { goldens, platform: platform, namePrefix: namePrefix, log: log, + fs: fs, ); } @@ -401,6 +409,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { this.reason, { super.namePrefix, required super.log, + required super.fs, }); /// Describes the reason for using the [FlutterSkippingFileComparator]. @@ -413,12 +422,12 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { LocalFileComparator? defaultComparator, String? namePrefix, required LogCallback log, + required FileSystem fs, }) { defaultComparator ??= goldenFileComparator as LocalFileComparator; - const FileSystem fs = LocalFileSystem(); final Uri basedir = defaultComparator.basedir; 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 @@ -478,7 +487,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC FlutterLocalFileComparator( super.basedir, super.skiaClient, { - super.fs, + required super.fs, super.platform, required super.log, }); @@ -494,11 +503,13 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC LocalFileComparator? defaultComparator, Directory? baseDirectory, required LogCallback log, + required FileSystem fs, }) async { defaultComparator ??= goldenFileComparator as LocalFileComparator; baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory( defaultComparator, platform, + fs: fs, ); if (!baseDirectory.existsSync()) { @@ -516,6 +527,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC 'OSError occurred, could not reach Gold. ' 'Switching to FlutterSkippingGoldenFileComparator.', log: log, + fs: fs, ); } on io.SocketException catch (_) { return FlutterSkippingFileComparator( @@ -524,6 +536,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC 'SocketException occurred, could not reach Gold. ' 'Switching to FlutterSkippingGoldenFileComparator.', log: log, + fs: fs, ); } on FormatException catch (_) { return FlutterSkippingFileComparator( @@ -532,10 +545,11 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC 'FormatException occurred, could not reach Gold. ' 'Switching to FlutterSkippingGoldenFileComparator.', log: log, + fs: fs, ); } - return FlutterLocalFileComparator(baseDirectory.uri, goldens, log: log); + return FlutterLocalFileComparator(baseDirectory.uri, goldens, log: log, fs: fs); } @override diff --git a/packages/flutter_goldens/test/flutter_goldens_test.dart b/packages/flutter_goldens/test/flutter_goldens_test.dart index 38c0b10c98..3ffcf703c2 100644 --- a/packages/flutter_goldens/test/flutter_goldens_test.dart +++ b/packages/flutter_goldens/test/flutter_goldens_test.dart @@ -18,6 +18,8 @@ import 'package:process/process.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'; // 1x1 transparent pixel @@ -714,6 +716,7 @@ void main() { final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory( defaultComparator, platform, + fs: fs, ); expect( basedir.uri, @@ -852,6 +855,7 @@ void main() { platform, goldens: fakeSkiaClient, log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + fs: fs, ); expect(fakeSkiaClient.initCalls, 0); }); @@ -1049,6 +1053,7 @@ void main() { platform, goldens: fakeSkiaClient, log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + fs: fs, ); expect(fakeSkiaClient.tryInitCalls, 0); }); @@ -1358,6 +1363,7 @@ void main() { goldens: fakeSkiaClient, baseDirectory: fakeDirectory, log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + fs: fs, ); expect(comparator1.runtimeType, FlutterSkippingFileComparator); @@ -1367,6 +1373,7 @@ void main() { goldens: fakeSkiaClient, baseDirectory: fakeDirectory, log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + fs: fs, ); expect(comparator2.runtimeType, FlutterSkippingFileComparator); @@ -1376,6 +1383,7 @@ void main() { goldens: fakeSkiaClient, baseDirectory: fakeDirectory, log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + fs: fs, ); expect(comparator3.runtimeType, FlutterSkippingFileComparator);