From 37c8df1690ffb0e41877aa4e52191707aa056a5a Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Fri, 23 Feb 2024 15:39:49 -0800 Subject: [PATCH] allow optional direct injection of Config instance into DevFS (#144002) In service of https://github.com/flutter/flutter/issues/143348 This will make testing of asset transformation hot reload behavior easier (specifically in that we can avoid having to write a test that looks like this: https://github.com/flutter/flutter/blob/5d02c27248d9ebb0db79547f3ed0fce8060ff041/packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart#L167-L249 --- .../flutter_tools/lib/src/build_info.dart | 6 +++-- packages/flutter_tools/lib/src/devfs.dart | 10 +++++--- .../test/general.shard/devfs_test.dart | 23 ++++++------------- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index feca4d7931f..ba1eace61ae 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -800,6 +800,7 @@ HostPlatform getCurrentHostPlatform() { /// Returns the top-level build output directory. String getBuildDirectory([Config? config, FileSystem? fileSystem]) { + // TODO(andrewkolos): Prefer required parameters instead of falling back to globals. // TODO(johnmccutchan): Stop calling this function as part of setting // up command line argument processing. final Config localConfig = config ?? globals.config; @@ -825,8 +826,9 @@ String getAotBuildDirectory() { } /// Returns the asset build output directory. -String getAssetBuildDirectory() { - return globals.fs.path.join(getBuildDirectory(), 'flutter_assets'); +String getAssetBuildDirectory([Config? config, FileSystem? fileSystem]) { + return (fileSystem ?? globals.fs) + .path.join(getBuildDirectory(config, fileSystem), 'flutter_assets'); } /// Returns the iOS build output directory. diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index ef4ce8698af..ced41441959 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart @@ -8,6 +8,7 @@ import 'package:package_config/package_config.dart'; import 'package:vm_service/vm_service.dart' as vm_service; import 'asset.dart'; +import 'base/config.dart'; import 'base/context.dart'; import 'base/file_system.dart'; import 'base/io.dart'; @@ -456,6 +457,7 @@ class DevFS { HttpClient? httpClient, Duration? uploadRetryThrottle, StopwatchFactory stopwatchFactory = const StopwatchFactory(), + Config? config, }) : _vmService = serviceProtocol, _logger = logger, _fileSystem = fileSystem, @@ -468,13 +470,15 @@ class DevFS { httpClient: httpClient ?? ((context.get() == null) ? HttpClient() : context.get()!())), - _stopwatchFactory = stopwatchFactory; + _stopwatchFactory = stopwatchFactory, + _config = config; final FlutterVmService _vmService; final _DevFSHttpWriter _httpWriter; final Logger _logger; final FileSystem _fileSystem; final StopwatchFactory _stopwatchFactory; + final Config? _config; final String fsName; final Directory? rootDirectory; @@ -615,8 +619,8 @@ class DevFS { // The tool writes the assets into the AssetBundle working dir so that they // are in the same location in DevFS and the iOS simulator. - final String assetBuildDirPrefix = _asUriPath(getAssetBuildDirectory()); - final String assetDirectory = getAssetBuildDirectory(); + final String assetDirectory = getAssetBuildDirectory(_config, _fileSystem); + final String assetBuildDirPrefix = _asUriPath(assetDirectory); bundle.entries.forEach((String archivePath, AssetBundleEntry entry) { // If the content is backed by a real file, isModified will file stat and return true if // it was modified since the last time this was called. diff --git a/packages/flutter_tools/test/general.shard/devfs_test.dart b/packages/flutter_tools/test/general.shard/devfs_test.dart index a6a9b431cd4..117226c8186 100644 --- a/packages/flutter_tools/test/general.shard/devfs_test.dart +++ b/packages/flutter_tools/test/general.shard/devfs_test.dart @@ -11,6 +11,7 @@ import 'package:file/memory.dart'; import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/asset.dart'; +import 'package:flutter_tools/src/base/config.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; @@ -587,15 +588,8 @@ void main() { }); group('Shader compilation', () { - late FileSystem fileSystem; - late ProcessManager processManager; - - setUp(() { - fileSystem = MemoryFileSystem.test(); - processManager = FakeProcessManager.any(); - }); - - testUsingContext('DevFS recompiles shaders', () async { + testWithoutContext('DevFS recompiles shaders', () async { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost( requests: [createDevFSRequest], httpAddress: Uri.parse('http://localhost'), @@ -609,6 +603,7 @@ void main() { logger: logger, osUtils: FakeOperatingSystemUtils(), httpClient: FakeHttpClient.any(), + config: Config.test(), ); await devFS.create(); @@ -647,12 +642,10 @@ void main() { expect(report.success, true); expect(devFS.shaderPathsToEvict, {'foo.frag'}); expect(devFS.assetPathsToEvict, {'not.frag'}); - }, overrides: { - FileSystem: () => fileSystem, - ProcessManager: () => processManager, }); - testUsingContext('DevFS tracks when FontManifest is updated', () async { + testWithoutContext('DevFS tracks when FontManifest is updated', () async { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost( requests: [createDevFSRequest], httpAddress: Uri.parse('http://localhost'), @@ -666,6 +659,7 @@ void main() { logger: logger, osUtils: FakeOperatingSystemUtils(), httpClient: FakeHttpClient.any(), + config: Config.test(), ); await devFS.create(); @@ -702,9 +696,6 @@ void main() { expect(devFS.shaderPathsToEvict, {}); expect(devFS.assetPathsToEvict, {'FontManifest.json'}); expect(devFS.didUpdateFontManifest, true); - }, overrides: { - FileSystem: () => fileSystem, - ProcessManager: () => processManager, }); }); }