From 5757500931421e53efe99cb359cd827e815957c4 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Wed, 17 Jan 2024 13:23:01 -0800 Subject: [PATCH] Make test file systems/platforms used in asset_bundle_test.dart less dependent on the host platform (#141657) Part of work on https://github.com/flutter/flutter/pull/141214. See [this discussion](https://github.com/flutter/flutter/pull/141214#discussion_r1446727495) for the inspiration for this PR. ## Issue Many tests in [packages/flutter_tools/test/general.shard/asset_bundle_test.dart](https://github.com/flutter/flutter/blob/4cd0a3252d2b45e243714b3ce93c5b2313bce671/packages/flutter_tools/test/general.shard/asset_bundle_test.dart) aren't hermetic. When setting up fake `FileSystem` and `Platform` objects, the host OS is referenced: https://github.com/flutter/flutter/blob/f2745e97d53c6a29c7d40003dfaa4fc4c688cdd2/packages/flutter_tools/test/general.shard/asset_bundle_test.dart#L35-L40 https://github.com/flutter/flutter/blob/f2745e97d53c6a29c7d40003dfaa4fc4c688cdd2/packages/flutter_tools/test/general.shard/asset_bundle_test.dart#L43 To improve hermeticity here, we could instead run each once _per_ valid combination of file system style and platform. However, it is unclear if these tests even depend on the file system style (integration tests should catch most cases where this might matter). As a result, I think it's sufficient to improve hermeticity by always assuming a Linux environment, which is generally our default (as `MemoryFileSystem` does, and most of our fakes of `Platform` do by default). In general, if a test needs to run other kinds of environments, it should make this clear, ideally through the test name. --- .../test/general.shard/asset_bundle_test.dart | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 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 e838560b19..8864e871fc 100644 --- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart +++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart @@ -33,14 +33,10 @@ void main() { late Platform platform; setUp(() async { - testFileSystem = MemoryFileSystem( - style: globals.platform.isWindows - ? FileSystemStyle.windows - : FileSystemStyle.posix, - ); + testFileSystem = MemoryFileSystem(); testFileSystem.currentDirectory = testFileSystem.systemTempDirectory.createTempSync('flutter_asset_bundle_test.'); logger = BufferLogger.test(); - platform = FakePlatform(operatingSystem: globals.platform.operatingSystem); + platform = FakePlatform(); }); testUsingContext('nonempty', () async { @@ -49,6 +45,7 @@ void main() { expect(ab.entries.length, greaterThan(0)); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -75,6 +72,7 @@ void main() { }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -120,6 +118,7 @@ flutter: ])); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -150,6 +149,7 @@ flutter: 'assets/foo/fizz.txt'])); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -192,6 +192,7 @@ name: example''') 'AssetManifest.bin', 'FontManifest.json', 'NOTICES.Z', 'assets/foo/bar.txt'])); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -217,6 +218,7 @@ flutter: expect(bundle.needsBuild(), false); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -252,6 +254,7 @@ flutter: expect(bundle.needsBuild(), false); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -282,6 +285,7 @@ flutter: expect(bundle.needsBuild(), false); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -330,6 +334,7 @@ flutter: expect(bundle.deferredComponentsEntries['component1']!.length, 3); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => platform, ProcessManager: () => FakeProcessManager.any(), }); @@ -515,14 +520,12 @@ flutter: group('AssetBundle.build (web builds)', () { late FileSystem testFileSystem; + late Platform testPlatform; setUp(() async { - testFileSystem = MemoryFileSystem( - style: globals.platform.isWindows - ? FileSystemStyle.windows - : FileSystemStyle.posix, - ); + testFileSystem = MemoryFileSystem(); testFileSystem.currentDirectory = testFileSystem.systemTempDirectory.createTempSync('flutter_asset_bundle_test.'); + testPlatform = FakePlatform(); }); testUsingContext('empty pubspec', () async { @@ -550,6 +553,7 @@ flutter: ); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => testPlatform, ProcessManager: () => FakeProcessManager.any(), }); @@ -605,6 +609,7 @@ flutter: reason: 'JSON-encoded binary content should be identical to BIN file.'); }, overrides: { FileSystem: () => testFileSystem, + Platform: () => testPlatform, ProcessManager: () => FakeProcessManager.any(), }); }); @@ -657,6 +662,7 @@ assets: expect(license, bundle.entries['NOTICES']); }, overrides: { FileSystem: () => MemoryFileSystem.test(), + Platform: () => FakePlatform(), ProcessManager: () => FakeProcessManager.any(), }); @@ -678,6 +684,7 @@ flutter: expect(bundle.additionalDependencies.single.path, contains('DOES_NOT_EXIST_RERUN_FOR_WILDCARD')); }, overrides: { FileSystem: () => MemoryFileSystem.test(), + Platform: () => FakePlatform(), ProcessManager: () => FakeProcessManager.any(), }); @@ -699,6 +706,7 @@ flutter: expect(bundle.additionalDependencies, isEmpty); }, overrides: { FileSystem: () => MemoryFileSystem.test(), + Platform: () => FakePlatform(), ProcessManager: () => FakeProcessManager.any(), });