From d65c98b4a2ec30c515eaf4d0b65a9853f9466c04 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 17 Feb 2021 21:26:03 -0800 Subject: [PATCH] [flutter_tools] replace some mock file/directories with new op handle (#76268) --- .../test/general.shard/asset_bundle_test.dart | 19 ++-- .../test/general.shard/base/os_test.dart | 24 +++-- .../build_system/file_store_test.dart | 26 +++--- .../test/general.shard/dart/pub_get_test.dart | 78 +++------------- .../test/general.shard/web/chrome_test.dart | 92 +++++-------------- packages/flutter_tools/test/src/common.dart | 63 ++++++++----- .../flutter_tools/test/template_test.dart | 14 ++- 7 files changed, 121 insertions(+), 195 deletions(-) diff --git a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart index 2d4546b55a8..299dcdce56e 100644 --- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart +++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart @@ -14,7 +14,6 @@ import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/bundle.dart'; import 'package:flutter_tools/src/devfs.dart'; import 'package:flutter_tools/src/globals.dart' as globals; -import 'package:mockito/mockito.dart'; import '../src/common.dart'; import '../src/context.dart'; @@ -180,14 +179,16 @@ flutter: }); testUsingContext('Failed directory delete shows message', () async { - final MockDirectory mockDirectory = MockDirectory(); - when(mockDirectory.existsSync()).thenReturn(true); - when(mockDirectory.deleteSync(recursive: true)).thenThrow(const FileSystemException('ABCD')); + final FileExceptionHandler handler = FileExceptionHandler(); + final FileSystem fileSystem = MemoryFileSystem.test(opHandle: handler.opHandle); - await writeBundle(mockDirectory, {}, loggerOverride: testLogger); + final Directory directory = fileSystem.directory('foo') + ..createSync(); + handler.addError(directory, FileSystemOp.delete, const FileSystemException('Expected Error Text')); - verify(mockDirectory.createSync(recursive: true)).called(1); - expect(testLogger.errorText, contains('ABCD')); + await writeBundle(directory, {}, loggerOverride: testLogger); + + expect(testLogger.errorText, contains('Expected Error Text')); }); testUsingContext('does not unnecessarily recreate asset manifest, font manifest, license', () async { @@ -450,7 +451,7 @@ flutter: '''); globals.fs.file('assets/foo.txt').createSync(recursive: true); - // Potential build artifacts outisde of build directory. + // Potential build artifacts outside of build directory. globals.fs.file('linux/flutter/foo.txt').createSync(recursive: true); globals.fs.file('windows/flutter/foo.txt').createSync(recursive: true); globals.fs.file('windows/CMakeLists.txt').createSync(); @@ -468,5 +469,3 @@ flutter: Platform: () => FakePlatform(operatingSystem: 'linux'), }); } - -class MockDirectory extends Mock implements Directory {} diff --git a/packages/flutter_tools/test/general.shard/base/os_test.dart b/packages/flutter_tools/test/general.shard/base/os_test.dart index 8ef2372fc39..d77a77292d3 100644 --- a/packages/flutter_tools/test/general.shard/base/os_test.dart +++ b/packages/flutter_tools/test/general.shard/base/os_test.dart @@ -383,25 +383,26 @@ void main() { testWithoutContext('If unzip fails, include stderr in exception text', () { const String exceptionMessage = 'Something really bad happened.'; + final FileExceptionHandler handler = FileExceptionHandler(); + final FileSystem fileSystem = MemoryFileSystem.test(opHandle: handler.opHandle); fakeProcessManager.addCommand( const FakeCommand(command: [ 'unzip', '-o', '-q', - null, + 'bar.zip', '-d', - null, + 'foo', ], exitCode: 1, stderr: exceptionMessage), ); - final MockFileSystem fileSystem = MockFileSystem(); - final MockFile mockFile = MockFile(); - final MockDirectory mockDirectory = MockDirectory(); - when(fileSystem.file(any)).thenReturn(mockFile); - when(mockFile.readAsBytesSync()).thenThrow( - const FileSystemException(exceptionMessage), - ); + final Directory foo = fileSystem.directory('foo') + ..createSync(); + final File bar = fileSystem.file('bar.zip') + ..createSync(); + handler.addError(bar, FileSystemOp.read, const FileSystemException(exceptionMessage)); + final OperatingSystemUtils osUtils = OperatingSystemUtils( fileSystem: fileSystem, logger: BufferLogger.test(), @@ -410,7 +411,7 @@ void main() { ); expect( - () => osUtils.unzip(mockFile, mockDirectory), + () => osUtils.unzip(bar, foo), throwsProcessException(message: exceptionMessage), ); }); @@ -470,6 +471,3 @@ void main() { } class MockProcessManager extends Mock implements ProcessManager {} -class MockDirectory extends Mock implements Directory {} -class MockFileSystem extends Mock implements FileSystem {} -class MockFile extends Mock implements File {} diff --git a/packages/flutter_tools/test/general.shard/build_system/file_store_test.dart b/packages/flutter_tools/test/general.shard/build_system/file_store_test.dart index 7a7d4dd8662..dbfdab84e2e 100644 --- a/packages/flutter_tools/test/general.shard/build_system/file_store_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/file_store_test.dart @@ -11,7 +11,6 @@ import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/build_system/file_store.dart'; -import 'package:mockito/mockito.dart'; import '../../src/common.dart'; @@ -143,14 +142,16 @@ void main() { }); testWithoutContext('FileStore handles failure to persist file cache', () async { - final MockFile mockFile = MockFile(); + final FileExceptionHandler handler = FileExceptionHandler(); + final FileSystem fileSystem = MemoryFileSystem.test(opHandle: handler.opHandle); final BufferLogger logger = BufferLogger.test(); - when(mockFile.writeAsBytesSync(any)).thenThrow(const FileSystemException('Out of space!')); - when(mockFile.readAsBytesSync()).thenReturn(Uint8List(0)); - when(mockFile.existsSync()).thenReturn(true); + + final File cacheFile = fileSystem.file('foo') + ..createSync(); + handler.addError(cacheFile, FileSystemOp.write, const FileSystemException('Out of space!')); final FileStore fileCache = FileStore( - cacheFile: mockFile, + cacheFile: cacheFile, logger: logger, ); @@ -161,13 +162,16 @@ void main() { }); testWithoutContext('FileStore handles failure to restore file cache', () async { - final MockFile mockFile = MockFile(); + final FileExceptionHandler handler = FileExceptionHandler(); + final FileSystem fileSystem = MemoryFileSystem.test(opHandle: handler.opHandle); final BufferLogger logger = BufferLogger.test(); - when(mockFile.readAsBytesSync()).thenThrow(const FileSystemException('Out of space!')); - when(mockFile.existsSync()).thenReturn(true); + + final File cacheFile = fileSystem.file('foo') + ..createSync(); + handler.addError(cacheFile, FileSystemOp.read, const FileSystemException('Out of space!')); final FileStore fileCache = FileStore( - cacheFile: mockFile, + cacheFile: cacheFile, logger: logger, ); @@ -198,5 +202,3 @@ void main() { expect(fileCache.currentAssetKeys['foo.dart'], '5d41402abc4b2a76b9719d911017c592'); }); } - -class MockFile extends Mock implements File {} diff --git a/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart b/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart index d254c0f06f2..6b783cfa240 100644 --- a/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart +++ b/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart @@ -15,7 +15,6 @@ import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/reporting/reporting.dart'; -import 'package:mockito/mockito.dart'; import 'package:process/process.dart'; import 'package:fake_async/fake_async.dart'; @@ -28,10 +27,6 @@ void main() { Cache.flutterRoot = ''; }); - tearDown(() { - MockDirectory.findCache = false; - }); - testWithoutContext('checkUpToDate skips pub get if the package config is newer than the pubspec ' 'and the current framework version is the same as the last version', () async { final FakeProcessManager processManager = FakeProcessManager.list([]); @@ -300,8 +295,9 @@ void main() { final MockProcessManager processMock = MockProcessManager(69); final BufferLogger logger = BufferLogger.test(); + final FileSystem fileSystem = MemoryFileSystem.test(); final Pub pub = Pub( - fileSystem: MockFileSystem(), + fileSystem: fileSystem, logger: logger, processManager: processMock, usage: TestUsage(), @@ -370,9 +366,10 @@ void main() { testWithoutContext('pub get 66 shows message from pub', () async { final BufferLogger logger = BufferLogger.test(); + final FileSystem fileSystem = MemoryFileSystem.test(); final Pub pub = Pub( platform: FakePlatform(environment: const {}), - fileSystem: MockFileSystem(), + fileSystem: fileSystem, logger: logger, usage: TestUsage(), botDetector: const BotDetectorAlwaysNo(), @@ -400,18 +397,19 @@ void main() { testWithoutContext('pub cache in root is used', () async { String error; final MockProcessManager processMock = MockProcessManager(69); - final MockFileSystem fsMock = MockFileSystem(); + final FileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.directory(Cache.flutterRoot).childDirectory('.pub-cache').createSync(); + final Pub pub = Pub( platform: FakePlatform(environment: const {}), usage: TestUsage(), - fileSystem: fsMock, + fileSystem: fileSystem, logger: BufferLogger.test(), processManager: processMock, botDetector: const BotDetectorAlwaysNo(), ); FakeAsync().run((FakeAsync time) { - MockDirectory.findCache = true; expect(processMock.lastPubEnvironment, isNull); expect(processMock.lastPubCache, isNull); pub.get(context: PubContext.flutterTests).then((void value) { @@ -421,15 +419,17 @@ void main() { }); time.elapse(const Duration(milliseconds: 500)); - expect(processMock.lastPubCache, equals(fsMock.path.join(Cache.flutterRoot, '.pub-cache'))); + expect(processMock.lastPubCache, equals(fileSystem.path.join(Cache.flutterRoot, '.pub-cache'))); expect(error, isNull); }); }); testWithoutContext('pub cache in environment is used', () async { + final FileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.directory('custom/pub-cache/path').createSync(recursive: true); final MockProcessManager processMock = MockProcessManager(69); final Pub pub = Pub( - fileSystem: MockFileSystem(), + fileSystem: fileSystem, logger: BufferLogger.test(), processManager: processMock, usage: TestUsage(), @@ -442,7 +442,6 @@ void main() { ); FakeAsync().run((FakeAsync time) { - MockDirectory.findCache = true; expect(processMock.lastPubEnvironment, isNull); expect(processMock.lastPubCache, isNull); @@ -534,13 +533,13 @@ void main() { ); }); - testWithoutContext('analytics sent on failure', () async { - MockDirectory.findCache = true; + final FileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.directory('custom/pub-cache/path').createSync(recursive: true); final TestUsage usage = TestUsage(); final Pub pub = Pub( usage: usage, - fileSystem: MockFileSystem(), + fileSystem: fileSystem, logger: BufferLogger.test(), processManager: MockProcessManager(1), botDetector: const BotDetectorAlwaysNo(), @@ -721,50 +720,3 @@ class MockProcessManager implements ProcessManager { @override dynamic noSuchMethod(Invocation invocation) => null; } - -class MockFileSystem extends ForwardingFileSystem { - MockFileSystem() : super(MemoryFileSystem.test()); - - @override - File file(dynamic path) { - return MockFile(); - } - - @override - Directory directory(dynamic path) { - return MockDirectory(path as String); - } -} - -class MockFile implements File { - @override - Future open({ FileMode mode = FileMode.read }) async { - return MockRandomAccessFile(); - } - - @override - bool existsSync() => true; - - @override - DateTime lastModifiedSync() => DateTime(0); - - @override - dynamic noSuchMethod(Invocation invocation) => null; -} - -class MockDirectory implements Directory { - MockDirectory(this.path); - - @override - final String path; - - static bool findCache = false; - - @override - bool existsSync() => findCache && path.endsWith('.pub-cache'); - - @override - dynamic noSuchMethod(Invocation invocation) => null; -} - -class MockRandomAccessFile extends Mock implements RandomAccessFile {} diff --git a/packages/flutter_tools/test/general.shard/web/chrome_test.dart b/packages/flutter_tools/test/general.shard/web/chrome_test.dart index 22b23ba6552..94c7f38407d 100644 --- a/packages/flutter_tools/test/general.shard/web/chrome_test.dart +++ b/packages/flutter_tools/test/general.shard/web/chrome_test.dart @@ -30,6 +30,7 @@ const List kChromeArgs = [ const String kDevtoolsStderr = '\n\nDevTools listening\n\n'; void main() { + FileExceptionHandler exceptionHandler; ChromiumLauncher chromeLauncher; FileSystem fileSystem; Platform platform; @@ -37,6 +38,7 @@ void main() { OperatingSystemUtils operatingSystemUtils; setUp(() { + exceptionHandler = FileExceptionHandler(); operatingSystemUtils = MockOperatingSystemUtils(); when(operatingSystemUtils.findFreePort()) .thenAnswer((Invocation invocation) async { @@ -45,7 +47,7 @@ void main() { platform = FakePlatform(operatingSystem: 'macos', environment: { kChromeEnvironment: 'example_chrome', }); - fileSystem = MemoryFileSystem.test(); + fileSystem = MemoryFileSystem.test(opHandle: exceptionHandler.opHandle); processManager = FakeProcessManager.list([]); chromeLauncher = ChromiumLauncher( fileSystem: fileSystem, @@ -105,10 +107,8 @@ void main() { testWithoutContext('does not crash if saving profile information fails due to a file system exception.', () async { final BufferLogger logger = BufferLogger.test(); - final ErrorThrowingFileSystem errorFileSystem = ErrorThrowingFileSystem(fileSystem); - errorFileSystem.addErrorEntity(FakeDirectory('/.tmp_rand0/flutter_tools_chrome_device.rand0/Default', errorFileSystem)); chromeLauncher = ChromiumLauncher( - fileSystem: errorFileSystem, + fileSystem: fileSystem, platform: platform, processManager: processManager, operatingSystemUtils: operatingSystemUtils, @@ -132,9 +132,18 @@ void main() { cacheDir: fileSystem.currentDirectory, ); - // Create cache dir that the Chrome launcher will atttempt to persist. - fileSystem.directory('/.tmp_rand0/flutter_tools_chrome_device.rand0/Default/Local Storage') + // Create cache dir that the Chrome launcher will atttempt to persist, and a file + // that will thrown an exception when it is read. + const String directoryPrefix = '/.tmp_rand0/flutter_tools_chrome_device.rand0/Default'; + fileSystem.directory('$directoryPrefix/Local Storage') .createSync(recursive: true); + final File file = fileSystem.file('$directoryPrefix/Local Storage/foo') + ..createSync(recursive: true); + exceptionHandler.addError( + file, + FileSystemOp.read, + const FileSystemException(), + ); await chrome.close(); // does not exit with error. expect(logger.errorText, contains('Failed to save Chrome preferences')); @@ -142,11 +151,15 @@ void main() { testWithoutContext('does not crash if restoring profile information fails due to a file system exception.', () async { final BufferLogger logger = BufferLogger.test(); - final ErrorThrowingFileSystem errorFileSystem = ErrorThrowingFileSystem(fileSystem); - errorFileSystem.addErrorEntity(FakeDirectory('/Default', errorFileSystem)); - + final File file = fileSystem.file('/Default/foo') + ..createSync(recursive: true); + exceptionHandler.addError( + file, + FileSystemOp.read, + const FileSystemException(), + ); chromeLauncher = ChromiumLauncher( - fileSystem: errorFileSystem, + fileSystem: fileSystem, platform: platform, processManager: processManager, operatingSystemUtils: operatingSystemUtils, @@ -169,7 +182,7 @@ void main() { final Chromium chrome = await chromeLauncher.launch( 'example_url', skipCheck: true, - cacheDir: errorFileSystem.currentDirectory, + cacheDir: fileSystem.currentDirectory, ); // Create cache dir that the Chrome launcher will atttempt to persist. @@ -363,60 +376,3 @@ Future _testLaunchChrome(String userDataDir, FakeProcessManager proces skipCheck: true, ); } - -class FakeDirectory extends Fake implements Directory { - FakeDirectory(this.path, this.fileSystem); - - @override - final FileSystem fileSystem; - - @override - final String path; - - @override - void createSync({bool recursive = false}) {} - - @override - bool existsSync() { - return true; - } - - @override - List listSync({bool recursive = false, bool followLinks = true}) { - throw FileSystemException(path, ''); - } -} - -class ErrorThrowingFileSystem extends ForwardingFileSystem { - ErrorThrowingFileSystem(FileSystem delegate) : super(delegate); - - final Map errorEntities = {}; - - void addErrorEntity(FileSystemEntity entity) { - errorEntities[entity.path] = entity; - } - - @override - Directory directory(dynamic path) { - if (errorEntities.containsKey(path)) { - return errorEntities[path] as Directory; - } - return delegate.directory(path); - } - - @override - File file(dynamic path) { - if (errorEntities.containsKey(path)) { - return errorEntities[path] as File; - } - return delegate.file(path); - } - - @override - Link link(dynamic path) { - if (errorEntities.containsKey(path)) { - return errorEntities[path] as Link; - } - return delegate.link(path); - } -} diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index bc13fce2480..5058a5d22bf 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart @@ -8,6 +8,7 @@ import 'dart:async'; import 'package:args/args.dart'; import 'package:args/command_runner.dart'; +import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/user_messages.dart'; @@ -419,27 +420,6 @@ class TestFlutterCommandRunner extends FlutterCommandRunner { } } -/// A file system that allows preconfiguring certain entities. -/// -/// This is useful for inserting mocks/entities which throw errors or -/// have other behavior that is not easily configured through the -/// filesystem interface. -class ConfiguredFileSystem extends ForwardingFileSystem { - ConfiguredFileSystem(FileSystem delegate, {@required this.entities}) : super(delegate); - - final Map entities; - - @override - File file(dynamic path) { - return (entities[path] as File) ?? super.file(path); - } - - @override - Directory directory(dynamic path) { - return (entities[path] as Directory) ?? super.directory(path); - } -} - /// Matches a doctor validation result. Matcher matchDoctorValidation({ ValidationType validationType, @@ -451,3 +431,44 @@ Matcher matchDoctorValidation({ .having((ValidationResult result) => result.statusInfo, 'statusInfo', statusInfo) .having((ValidationResult result) => result.messages, 'messages', messages); } + +/// Allows inserting file system exceptions into certain +/// [MemoryFileSystem] operations by tagging path/op combinations. +/// +/// Example use: +/// +/// ``` +/// void main() { +/// var handler = FileExceptionHandler(); +/// var fs = MemoryFileSystem(opHandle: handler.opHandle); +/// +/// var file = fs.file('foo')..createSync(); +/// handler.addError(file, FileSystemOp.read, FileSystemException('Error Reading foo')); +/// +/// expect(() => file.writeAsStringSync('A'), throwsA(isA())); +/// } +/// ``` +class FileExceptionHandler { + final Map> _contextErrors = >{}; + + /// Add an exception that will be thrown whenever the file system attached to this + /// handler performs the [operation] on the [entity]. + void addError(FileSystemEntity entity, FileSystemOp operation, FileSystemException exception) { + final String path = entity.path; + _contextErrors[path] ??= {}; + _contextErrors[path][operation] = exception; + } + + // Tear-off this method and pass it to the memory filesystem `opHandle` parameter. + void opHandle(String path, FileSystemOp operation) { + final Map exceptions = _contextErrors[path]; + if (exceptions == null) { + return; + } + final FileSystemException exception = exceptions[operation]; + if (exception == null) { + return; + } + throw exception; + } +} diff --git a/packages/flutter_tools/test/template_test.dart b/packages/flutter_tools/test/template_test.dart index b704e85c5a9..71b4edcd983 100644 --- a/packages/flutter_tools/test/template_test.dart +++ b/packages/flutter_tools/test/template_test.dart @@ -10,12 +10,12 @@ import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/template.dart'; import 'package:flutter_tools/src/template.dart'; -import 'package:mockito/mockito.dart'; import 'src/common.dart'; void main() { testWithoutContext('Template.render throws ToolExit when FileSystem exception is raised', () { - final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + final FileExceptionHandler handler = FileExceptionHandler(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(opHandle: handler.opHandle); final Template template = Template( fileSystem.directory('examples'), fileSystem.currentDirectory, @@ -25,11 +25,11 @@ void main() { templateRenderer: FakeTemplateRenderer(), templateManifest: null, ); - final MockDirectory mockDirectory = MockDirectory(); - when(mockDirectory.createSync(recursive: true)).thenThrow(const FileSystemException()); + final Directory directory = fileSystem.directory('foo'); + handler.addError(directory, FileSystemOp.create, const FileSystemException()); - expect(() => template.render(mockDirectory, {}), - throwsToolExit()); + expect(() => template.render(directory, {}), + throwsToolExit()); }); testWithoutContext('Template.render replaces .img.tmpl files with files from the image source', () { @@ -60,8 +60,6 @@ void main() { }); } -class MockDirectory extends Mock implements Directory {} - class FakeTemplateRenderer extends TemplateRenderer { @override String renderString(String template, dynamic context, {bool htmlEscapeValues = false}) {