From 8d3cca43b224ec28935973d8159166bd9cc02499 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 5 Nov 2024 15:00:05 -0800 Subject: [PATCH] use root directory as the default for rootOverride in Cache.test constructor (#158201) While doing some hacking on `Cache` in https://github.com/flutter/flutter/pull/158081, I noticed that [`Cache.test`](https://github.com/flutter/flutter/blob/de9318275391a6d2f10ae33c576f4113b25fd156/packages/flutter_tools/lib/src/cache.dart#L139) allows the caller to tell Cache to use some given directory as the flutter root (instead of depending on the static global [`Cache.flutterRoot`](https://github.com/flutter/flutter/blob/4f3976a4f2b722d26c3353158dcd26590859dde0/packages/flutter_tools/lib/src/cache.dart#L206)). This has a default value, `/cache`. However, `/cache` is an unintuitive name for the root directory of a Flutter installation. This led to confusion when updating some tests. I wanted to create `/bin/cache/engine-dart-sdk.stamp` for tests, but in reality I needed to create `/cache/bin/cache/engine-dart-sdk.stamp`. This PR changes this default to the current directory of the file system (which I'm guessing is `/` for all intents and purposes).
Pre-launch checklist
--- packages/flutter_tools/lib/src/cache.dart | 11 ++++++++++- .../test/general.shard/android/gradle_utils_test.dart | 2 +- .../flutter_tools/test/general.shard/cache_test.dart | 11 ++++------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index ef2e24fc3a..95c71bbb22 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -144,11 +144,20 @@ class Cache { Platform? platform, required ProcessManager processManager, }) { + if (rootOverride?.fileSystem != null && + fileSystem != null && + rootOverride!.fileSystem != fileSystem) { + throw ArgumentError( + 'If rootOverride and fileSystem are both non-null, ' + 'rootOverride.fileSystem must be the same as fileSystem.', + 'fileSystem' + ); + } fileSystem ??= rootOverride?.fileSystem ?? MemoryFileSystem.test(); platform ??= FakePlatform(environment: {}); logger ??= BufferLogger.test(); return Cache( - rootOverride: rootOverride ?? fileSystem.directory('cache'), + rootOverride: rootOverride ?? fileSystem.currentDirectory, artifacts: artifacts ?? [], logger: logger, fileSystem: fileSystem, diff --git a/packages/flutter_tools/test/general.shard/android/gradle_utils_test.dart b/packages/flutter_tools/test/general.shard/android/gradle_utils_test.dart index 61c242f777..86221f722f 100644 --- a/packages/flutter_tools/test/general.shard/android/gradle_utils_test.dart +++ b/packages/flutter_tools/test/general.shard/android/gradle_utils_test.dart @@ -24,7 +24,7 @@ void main() { setUp(() { fileSystem = MemoryFileSystem.test(); gradleWrapperDirectory = - fileSystem.directory('cache/bin/cache/artifacts/gradle_wrapper'); + fileSystem.directory('bin/cache/artifacts/gradle_wrapper'); gradleWrapperDirectory.createSync(recursive: true); gradleWrapperDirectory .childFile('gradlew') diff --git a/packages/flutter_tools/test/general.shard/cache_test.dart b/packages/flutter_tools/test/general.shard/cache_test.dart index 1522f63d91..e79aa6e85c 100644 --- a/packages/flutter_tools/test/general.shard/cache_test.dart +++ b/packages/flutter_tools/test/general.shard/cache_test.dart @@ -338,7 +338,6 @@ void main() { testWithoutContext('a non-empty realm is included in the storage url', () async { final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final Directory internalDir = fileSystem.currentDirectory - .childDirectory('cache') .childDirectory('bin') .childDirectory('internal'); final File engineVersionFile = internalDir.childFile('engine.version'); @@ -803,7 +802,6 @@ void main() { testWithoutContext('FlutterWebSdk fetches web artifacts and deletes previous directory contents', () async { final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final Directory internalDir = fileSystem.currentDirectory - .childDirectory('cache') .childDirectory('bin') .childDirectory('internal'); final File canvasKitVersionFile = internalDir.childFile('canvaskit.version'); @@ -842,7 +840,7 @@ void main() { ]); expect(locations, [ - 'cache/bin/cache/flutter_web_sdk', + '/bin/cache/flutter_web_sdk', ]); expect(webCacheDirectory.childFile('foo'), exists); @@ -852,7 +850,6 @@ void main() { testWithoutContext('FlutterWebSdk CanvasKit URL can be overridden via FLUTTER_STORAGE_BASE_URL', () async { final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final Directory internalDir = fileSystem.currentDirectory - .childDirectory('cache') .childDirectory('bin') .childDirectory('internal'); final File canvasKitVersionFile = internalDir.childFile('canvaskit.version'); @@ -909,7 +906,7 @@ void main() { handler.addError(webCacheDirectory, FileSystemOp.delete, const FileSystemException('', '', OSError('', 2))); await expectLater(() => webSdk.updateInner(artifactUpdater, fileSystem, FakeOperatingSystemUtils()), throwsToolExit( - message: RegExp('Unable to delete file or directory at "cache/bin/cache/flutter_web_sdk"'), + message: RegExp('Unable to delete file or directory at "/bin/cache/flutter_web_sdk"'), )); }); @@ -1108,11 +1105,11 @@ void main() { Platform: () => FakePlatform(), ProcessManager: () => FakeProcessManager.list([ const FakeCommand(command: [ - '/cache/bin/cache/flutter_gradle_wrapper.rand0/gradlew', + '/bin/cache/flutter_gradle_wrapper.rand0/gradlew', '-b', 'packages/flutter_tools/gradle/resolve_dependencies.gradle', '--project-cache-dir', - 'cache/bin/cache/flutter_gradle_wrapper.rand0', + '/bin/cache/flutter_gradle_wrapper.rand0', 'resolveDependencies', ]), ]),