From 8e4f704abc4576206c3d56e1a430f66457b706f3 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Thu, 20 Jun 2024 10:45:09 -0700 Subject: [PATCH] [CLI tool] in `flutter test`, consider `--flavor` when validating the cached asset bundle (#150461) Fixes https://github.com/flutter/flutter/issues/150296 **Context.** `flutter test` has its own code path for writing flutter app [assets](https://docs.flutter.dev/ui/assets/assets-and-images). https://github.com/flutter/flutter/pull/132985 introduced [flavor-conditional asset bundling ](https://docs.flutter.dev/deployment/flavors#conditionally-bundling-assets-based-on-flavor), which lets users control which assets get bundled based on `--flavor`. `--flavor` is supported in `flutter test`. **Bug and fix.** `--flavor` isn't considered when deciding whether we need to rebuild this asset bundle: https://github.com/flutter/flutter/blob/5e448f4ce57723ac0792ae822ebac69df3188ba1/packages/flutter_tools/lib/src/commands/test.dart#L709 This PR address this by writing the value of `--flavor` to a file in the build directory and checking that when validating the cached asset bundle. --- .../flutter_tools/lib/src/commands/test.dart | 32 +++++++++-- .../commands.shard/hermetic/test_test.dart | 54 +++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart index 6b9a12722b..490130e6af 100644 --- a/packages/flutter_tools/lib/src/commands/test.dart +++ b/packages/flutter_tools/lib/src/commands/test.dart @@ -695,7 +695,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { if (build != 0) { throwToolExit('Error: Failed to build asset bundle'); } - if (_needRebuild(assetBundle.entries)) { + if (_needsRebuild(assetBundle.entries, flavor)) { await writeBundle( globals.fs.directory(globals.fs.path.join('build', 'unit_test_assets')), assetBundle.entries, @@ -708,14 +708,25 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { projectDir: globals.fs.currentDirectory, buildMode: buildMode, ); + + final File cachedFlavorFile = globals.fs.file( + globals.fs.path.join('build', 'test_cache', 'flavor.txt'), + ); + if (cachedFlavorFile.existsSync()) { + await cachedFlavorFile.delete(); + } + if (flavor != null) { + cachedFlavorFile.createSync(recursive: true); + cachedFlavorFile.writeAsStringSync(flavor); + } } } - bool _needRebuild(Map entries) { + bool _needsRebuild(Map entries, String? flavor) { // TODO(andrewkolos): This logic might fail in the future if we change the - // schema of the contents of the asset manifest file and the user does not - // perform a `flutter clean` after upgrading. - // See https://github.com/flutter/flutter/issues/128563. + // schema of the contents of the asset manifest file and the user does not + // perform a `flutter clean` after upgrading. + // See https://github.com/flutter/flutter/issues/128563. final File manifest = globals.fs.file(globals.fs.path.join('build', 'unit_test_assets', 'AssetManifest.bin')); if (!manifest.existsSync()) { return true; @@ -736,6 +747,17 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { return true; } } + + final File cachedFlavorFile = globals.fs.file( + globals.fs.path.join('build', 'test_cache', 'flavor.txt'), + ); + final String? cachedFlavor = cachedFlavorFile.existsSync() + ? cachedFlavorFile.readAsStringSync() + : null; + if (cachedFlavor != flavor) { + return true; + } + return false; } } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart index a925dae8e9..7e2f271869 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart @@ -7,6 +7,7 @@ import 'dart:convert'; import 'package:args/command_runner.dart'; import 'package:file/memory.dart'; +import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/async_guard.dart'; import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/file_system.dart'; @@ -1178,6 +1179,59 @@ dev_dependencies: DeviceManager: () => _FakeDeviceManager([]), }); + testUsingContext('correctly considers --flavor when validating the cached asset bundle', () async { + final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0); + fs.file('vanilla.txt').writeAsStringSync('vanilla'); + fs.file('flavorless.txt').writeAsStringSync('flavorless'); + fs.file('pubspec.yaml').writeAsStringSync(''' +flutter: + assets: + - path: vanilla.txt + flavors: + - vanilla + - flavorless.txt +dev_dependencies: + flutter_test: + sdk: flutter + integration_test: + sdk: flutter'''); + final TestCommand testCommand = TestCommand(testRunner: testRunner); + final CommandRunner commandRunner = createTestCommandRunner(testCommand); + + const List buildArgsFlavorless = [ + 'test', + '--no-pub', + ]; + + const List buildArgsVanilla = [ + 'test', + '--no-pub', + '--flavor', + 'vanilla', + ]; + + final File builtVanillaAssetFile = fs.file( + fs.path.join('build', 'unit_test_assets', 'vanilla.txt'), + ); + final File builtFlavorlessAssetFile = fs.file( + fs.path.join('build', 'unit_test_assets', 'flavorless.txt'), + ); + + await commandRunner.run(buildArgsVanilla); + await commandRunner.run(buildArgsFlavorless); + + expect(builtVanillaAssetFile, isNot(exists)); + expect(builtFlavorlessAssetFile, exists); + + await commandRunner.run(buildArgsVanilla); + + expect(builtVanillaAssetFile, exists); + }, overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.empty(), + DeviceManager: () => _FakeDeviceManager([]), + }); + testUsingContext("Don't build the asset manifest if --no-test-assets if informed", () async { final FakeFlutterTestRunner testRunner = FakeFlutterTestRunner(0);