From f177d8aab483c64c411971fde2a5c842ed4819fa Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 16 Dec 2024 14:59:16 -0800 Subject: [PATCH] Migrate `test/commands.shard` (mostly) to `explicit-package-dependencies`. (#160288) Towards https://github.com/flutter/flutter/issues/160257. I intentionally skipped `create_test.dart`, as that requires updating the generated template which uses l10n, a feature changing. I'll do that in the "big bang" PR that finally enables the feature to avoid getting us into a bad state. --- .../hermetic/build_web_test.dart | 14 +++++++++++ .../hermetic/generate_localizations_test.dart | 18 +++++++++++--- .../commands.shard/hermetic/pub_test.dart | 18 ++++++++++++++ .../permeable/packages_test.dart | 24 +++++++++++++++++-- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart index 84661ace67..d1bfaa1afd 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart @@ -16,12 +16,14 @@ import 'package:flutter_tools/src/build_system/targets/web.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/build.dart'; import 'package:flutter_tools/src/commands/build_web.dart'; +import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/web/compile.dart'; import '../../src/common.dart'; import '../../src/context.dart'; +import '../../src/fake_pub_deps.dart'; import '../../src/fakes.dart'; import '../../src/test_build_system.dart'; import '../../src/test_flutter_command_runner.dart'; @@ -38,6 +40,16 @@ void main() { late ProcessManager processManager; late Artifacts artifacts; + // TODO(matanlurey): Remove after `explicit-package-dependencies` is enabled by default. + // See https://github.com/flutter/flutter/issues/160257 for details. + FeatureFlags enableExplicitPackageDependencies() { + return TestFeatureFlags( + isExplicitPackageDependenciesEnabled: true, + // Assumed to be true below. + isWebEnabled: true, + ); + } + setUpAll(() { Cache.flutterRoot = ''; Cache.disableLocking(); @@ -384,6 +396,8 @@ void main() { FileSystem: () => fileSystem, ProcessManager: () => processManager, Logger: () => logger, + FeatureFlags: enableExplicitPackageDependencies, + Pub: FakePubWithPrimedDeps.new, }); } /// Do test all the deprecated WebRendererModes diff --git a/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart index b911f8a159..d1ec39f7d2 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart @@ -10,12 +10,14 @@ import 'package:flutter_tools/src/build_system/build_system.dart'; import 'package:flutter_tools/src/build_system/targets/localizations.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/generate_localizations.dart'; +import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/localizations/gen_l10n_types.dart'; import '../../integration.shard/test_data/basic_project.dart'; import '../../src/common.dart'; import '../../src/context.dart'; import '../../src/fake_process_manager.dart'; +import '../../src/fakes.dart'; import '../../src/test_flutter_command_runner.dart'; void main() { @@ -24,6 +26,14 @@ void main() { late Artifacts artifacts; late FakeProcessManager processManager; + // TODO(matanlurey): Remove after `explicit-package-dependencies` is enabled by default. + // See https://github.com/flutter/flutter/issues/160257 for details. + FeatureFlags enableExplicitPackageDependencies() { + return TestFeatureFlags( + isExplicitPackageDependenciesEnabled: true, + ); + } + setUpAll(() { Cache.disableLocking(); }); @@ -399,8 +409,8 @@ format: true command: [ 'Artifact.engineDartBinary', 'format', - '/.dart_tool/flutter_gen/gen_l10n/app_localizations_en.dart', - '/.dart_tool/flutter_gen/gen_l10n/app_localizations.dart', + '/lib/l10n/app_localizations_en.dart', + '/lib/l10n/app_localizations.dart', ] ) ); @@ -413,11 +423,13 @@ format: true ); await buildTarget.build(environment); - final Directory outputDirectory = fileSystem.directory(fileSystem.path.join('.dart_tool', 'flutter_gen', 'gen_l10n')); + final Directory outputDirectory = fileSystem.directory(fileSystem.path.join('lib', 'l10n')); expect(outputDirectory.existsSync(), true); expect(outputDirectory.childFile('app_localizations_en.dart').existsSync(), true); expect(outputDirectory.childFile('app_localizations.dart').existsSync(), true); expect(processManager, hasNoRemainingExpectations); + }, overrides: { + FeatureFlags: enableExplicitPackageDependencies, }); testUsingContext('nullable-getter defaults to true', () async { diff --git a/packages/flutter_tools/test/commands.shard/hermetic/pub_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/pub_test.dart index d5199b94db..9ed95d2c35 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/pub_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/pub_test.dart @@ -8,12 +8,15 @@ import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/packages.dart'; import 'package:flutter_tools/src/dart/pub.dart'; +import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/project.dart'; import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:test/fake.dart'; import '../../src/context.dart'; +import '../../src/fake_pub_deps.dart'; +import '../../src/fakes.dart'; import '../../src/test_flutter_command_runner.dart'; const String minimalV2EmbeddingManifest = r''' @@ -31,6 +34,15 @@ void main() { late FileSystem fileSystem; late FakePub pub; + // TODO(matanlurey): Remove after `flutter_gen` is removed. + // See https://github.com/flutter/flutter/issues/102983 for details. + FeatureFlags disableExplicitPackageDependencies() { + return TestFeatureFlags( + // ignore: avoid_redundant_argument_values + isExplicitPackageDependenciesEnabled: false, + ); + } + setUp(() { Cache.disableLocking(); fileSystem = MemoryFileSystem.test(); @@ -262,6 +274,7 @@ void main() { Pub: () => pub, ProcessManager: () => FakeProcessManager.any(), FileSystem: () => fileSystem, + FeatureFlags: disableExplicitPackageDependencies, }); } @@ -292,4 +305,9 @@ class FakePub extends Fake implements Pub { ..writeAsStringSync('{"configVersion":2,"packages":[]}'); } } + + @override + Future> deps(FlutterProject project) { + return FakePubWithPrimedDeps().deps(project); + } } diff --git a/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart b/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart index bbba64cf0a..a3479be903 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart @@ -30,6 +30,23 @@ import '../../src/test_flutter_command_runner.dart'; void main() { late FakeStdio mockStdio; + // TODO(matanlurey): Remove after `explicit-package-dependencies` is enabled by default. + // See https://github.com/flutter/flutter/issues/160257 for details. + FeatureFlags enableExplicitPackageDependencies() { + return TestFeatureFlags( + isExplicitPackageDependenciesEnabled: true, + ); + } + + // TODO(matanlurey): Remove after `flutter_gen` is removed. + // See https://github.com/flutter/flutter/issues/102983 for details. + FeatureFlags disableExplicitPackageDependencies() { + return TestFeatureFlags( + // ignore: avoid_redundant_argument_values + isExplicitPackageDependenciesEnabled: false, + ); + } + setUp(() { mockStdio = FakeStdio()..stdout.terminalColumns = 80; @@ -328,6 +345,7 @@ flutter: botDetector: globals.botDetector, platform: globals.platform, ), + FeatureFlags: disableExplicitPackageDependencies, }); testUsingContext('get fetches packages for a workspace', () async { @@ -593,7 +611,7 @@ workspace: await runCommandIn(projectPath, 'get'); expectDependenciesResolved(projectPath); - expectModulePluginInjected(projectPath, includeLegacyPluginsList: true); + expectModulePluginInjected(projectPath, includeLegacyPluginsList: false); }, overrides: { Stdio: () => mockStdio, Pub: () => Pub.test( @@ -605,6 +623,7 @@ workspace: platform: globals.platform, stdio: mockStdio, ), + FeatureFlags: enableExplicitPackageDependencies, }); testUsingContext('get fetches packages and injects plugin in plugin project', () async { @@ -623,7 +642,7 @@ workspace: await runCommandIn(exampleProjectPath, 'get'); expectDependenciesResolved(exampleProjectPath); - expectPluginInjected(exampleProjectPath, includeLegacyPluginsList: true); + expectPluginInjected(exampleProjectPath, includeLegacyPluginsList: false); }, overrides: { Stdio: () => mockStdio, Pub: () => Pub.test( @@ -635,6 +654,7 @@ workspace: platform: globals.platform, stdio: mockStdio, ), + FeatureFlags: enableExplicitPackageDependencies, }); testUsingContext('get explicit-packages-resolution omits ".flutter-plugins"', () async {