From 861fe0a276aa39eb497112016d5a8058e500ad27 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 9 Oct 2019 16:27:39 -0700 Subject: [PATCH] Ensure precache --web works on dev branch (#42289) --- packages/flutter_tools/lib/src/cache.dart | 13 +- .../lib/src/commands/precache.dart | 4 + packages/flutter_tools/lib/src/features.dart | 18 +- .../hermetic/precache_test.dart | 303 ++++++++++-------- .../test/general.shard/cache_test.dart | 2 +- packages/flutter_tools/test/src/testbed.dart | 17 + 6 files changed, 210 insertions(+), 147 deletions(-) diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index 889ff74129..07c1b73977 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -16,21 +16,25 @@ import 'base/net.dart'; import 'base/os.dart'; import 'base/platform.dart'; import 'base/process.dart'; +import 'features.dart'; import 'globals.dart'; /// A tag for a set of development artifacts that need to be cached. class DevelopmentArtifact { - const DevelopmentArtifact._(this.name, {this.unstable = false}); + const DevelopmentArtifact._(this.name, {this.unstable = false, this.feature}); /// The name of the artifact. /// /// This should match the flag name in precache.dart final String name; - /// Whether this artifact should be unavailable on stable branches. + /// Whether this artifact should be unavailable on master branch only. final bool unstable; + /// A feature to control the visibility of this artifact. + final Feature feature; + /// Artifacts required for Android development. static const DevelopmentArtifact androidGenSnapshot = DevelopmentArtifact._('android_gen_snapshot'); static const DevelopmentArtifact androidMaven = DevelopmentArtifact._('android_maven'); @@ -41,7 +45,7 @@ class DevelopmentArtifact { static const DevelopmentArtifact iOS = DevelopmentArtifact._('ios'); /// Artifacts required for web development. - static const DevelopmentArtifact web = DevelopmentArtifact._('web', unstable: true); + static const DevelopmentArtifact web = DevelopmentArtifact._('web', feature: flutterWebFeature); /// Artifacts required for desktop macOS. static const DevelopmentArtifact macOS = DevelopmentArtifact._('macos', unstable: true); @@ -75,6 +79,9 @@ class DevelopmentArtifact { universal, flutterRunner, ]; + + @override + String toString() => 'Artifact($name, $unstable)'; } /// A wrapper around the `bin/cache/` directory. diff --git a/packages/flutter_tools/lib/src/commands/precache.dart b/packages/flutter_tools/lib/src/commands/precache.dart index e7f7103923..d8ccd3a107 100644 --- a/packages/flutter_tools/lib/src/commands/precache.dart +++ b/packages/flutter_tools/lib/src/commands/precache.dart @@ -5,6 +5,7 @@ import 'dart:async'; import '../cache.dart'; +import '../features.dart'; import '../globals.dart'; import '../runner/flutter_command.dart'; import '../version.dart'; @@ -65,6 +66,9 @@ class PrecacheCommand extends FlutterCommand { if (!FlutterVersion.instance.isMaster && artifact.unstable) { continue; } + if (artifact.feature != null && !featureFlags.isEnabled(artifact.feature)) { + continue; + } if (argResults[artifact.name]) { requiredArtifacts.add(artifact); } diff --git a/packages/flutter_tools/lib/src/features.dart b/packages/flutter_tools/lib/src/features.dart index b0d979c26a..332506d3f7 100644 --- a/packages/flutter_tools/lib/src/features.dart +++ b/packages/flutter_tools/lib/src/features.dart @@ -17,7 +17,7 @@ FeatureFlags get featureFlags => context.get(); /// The interface used to determine if a particular [Feature] is enabled. /// /// The rest of the tools code should use this class instead of looking up -/// features directly. To faciliate rolls to google3 and other clients, all +/// features directly. To facilitate rolls to google3 and other clients, all /// flags should be provided with a default implementation here. Clients that /// use this class should extent instead of implement, so that new flags are /// picked up automatically. @@ -25,22 +25,24 @@ class FeatureFlags { const FeatureFlags(); /// Whether flutter desktop for linux is enabled. - bool get isLinuxEnabled => _isEnabled(flutterLinuxDesktopFeature); + bool get isLinuxEnabled => isEnabled(flutterLinuxDesktopFeature); /// Whether flutter desktop for macOS is enabled. - bool get isMacOSEnabled => _isEnabled(flutterMacOSDesktopFeature); + bool get isMacOSEnabled => isEnabled(flutterMacOSDesktopFeature); /// Whether flutter web is enabled. - bool get isWebEnabled => _isEnabled(flutterWebFeature); + bool get isWebEnabled => isEnabled(flutterWebFeature); /// Whether flutter desktop for Windows is enabled. - bool get isWindowsEnabled => _isEnabled(flutterWindowsDesktopFeature); + bool get isWindowsEnabled => isEnabled(flutterWindowsDesktopFeature); /// Whether the new Android embedding is enabled. - bool get isNewAndroidEmbeddingEnabled => _isEnabled(flutterNewAndroidEmbeddingFeature); + bool get isNewAndroidEmbeddingEnabled => isEnabled(flutterNewAndroidEmbeddingFeature); - // Calculate whether a particular feature is enabled for the current channel. - static bool _isEnabled(Feature feature) { + /// Whether a particular feature is enabled for the current channel. + /// + /// Prefer using one of the specific getters above instead of this API. + bool isEnabled(Feature feature) { final String currentChannel = FlutterVersion.instance.channel; final FeatureChannelSetting featureSetting = feature.getSettingForChannel(currentChannel); if (!featureSetting.available) { diff --git a/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart index 128662eb3f..3794ff2bd4 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/precache.dart'; +import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/version.dart'; import 'package:mockito/mockito.dart'; @@ -11,151 +12,183 @@ import 'package:mockito/mockito.dart'; import '../../src/common.dart'; import '../../src/context.dart'; import '../../src/mocks.dart'; +import '../../src/testbed.dart'; void main() { - group('precache', () { - final MockCache cache = MockCache(); - Set artifacts; + MockCache cache; + Set artifacts; + MockFlutterVersion flutterVersion; + MockFlutterVersion masterFlutterVersion; + + setUp(() { + cache = MockCache(); + // Release lock between test cases. + Cache.releaseLockEarly(); when(cache.isUpToDate()).thenReturn(false); when(cache.updateAll(any)).thenAnswer((Invocation invocation) { artifacts = invocation.positionalArguments.first; return Future.value(null); }); - - testUsingContext('Adds artifact flags to requested artifacts', () async { - final PrecacheCommand command = PrecacheCommand(); - applyMocksToCommand(command); - await createTestCommandRunner(command).run( - const [ - 'precache', - '--ios', - '--android', - '--web', - '--macos', - '--linux', - '--windows', - '--fuchsia', - '--flutter_runner', - ], - ); - expect(artifacts, unorderedEquals({ - DevelopmentArtifact.universal, - DevelopmentArtifact.iOS, - DevelopmentArtifact.androidGenSnapshot, - DevelopmentArtifact.androidMaven, - DevelopmentArtifact.androidInternalBuild, - DevelopmentArtifact.web, - DevelopmentArtifact.macOS, - DevelopmentArtifact.linux, - DevelopmentArtifact.windows, - DevelopmentArtifact.fuchsia, - DevelopmentArtifact.flutterRunner, - })); - }, overrides: { - Cache: () => cache, - }); - - testUsingContext('Expands android artifacts when the android flag is used', () async { - // Release lock between test cases. - Cache.releaseLockEarly(); - - final PrecacheCommand command = PrecacheCommand(); - applyMocksToCommand(command); - await createTestCommandRunner(command).run( - const [ - 'precache', - '--no-ios', - '--android', - ], - ); - expect(artifacts, unorderedEquals({ - DevelopmentArtifact.universal, - DevelopmentArtifact.androidGenSnapshot, - DevelopmentArtifact.androidMaven, - DevelopmentArtifact.androidInternalBuild, - })); - }, overrides: { - Cache: () => cache, - }); - - testUsingContext('Adds artifact flags to requested android artifacts', () async { - // Release lock between test cases. - Cache.releaseLockEarly(); - - final PrecacheCommand command = PrecacheCommand(); - applyMocksToCommand(command); - await createTestCommandRunner(command).run( - const [ - 'precache', - '--no-ios', - '--android_gen_snapshot', - '--android_maven', - '--android_internal_build', - ], - ); - expect(artifacts, unorderedEquals({ - DevelopmentArtifact.universal, - DevelopmentArtifact.androidGenSnapshot, - DevelopmentArtifact.androidMaven, - DevelopmentArtifact.androidInternalBuild, - })); - }, overrides: { - Cache: () => cache, - }); - - final MockFlutterVersion flutterVersion = MockFlutterVersion(); + flutterVersion = MockFlutterVersion(); when(flutterVersion.isMaster).thenReturn(false); + masterFlutterVersion = MockFlutterVersion(); + when(masterFlutterVersion.isMaster).thenReturn(true); + }); - testUsingContext('Adds artifact flags to requested artifacts on stable', () async { - // Release lock between test cases. - Cache.releaseLockEarly(); - final PrecacheCommand command = PrecacheCommand(); - applyMocksToCommand(command); - await createTestCommandRunner(command).run( - const [ - 'precache', - '--ios', - '--android_gen_snapshot', - '--android_maven', - '--android_internal_build', - '--web', - '--macos', - '--linux', - '--windows', - '--fuchsia', - '--flutter_runner', - ], - ); - expect(artifacts, unorderedEquals({ - DevelopmentArtifact.universal, - DevelopmentArtifact.iOS, - DevelopmentArtifact.androidGenSnapshot, - DevelopmentArtifact.androidMaven, - DevelopmentArtifact.androidInternalBuild, - })); - }, overrides: { - Cache: () => cache, - FlutterVersion: () => flutterVersion, - }); - testUsingContext('Downloads artifacts when --force is provided', () async { - when(cache.isUpToDate()).thenReturn(true); - // Release lock between test cases. - Cache.releaseLockEarly(); - final PrecacheCommand command = PrecacheCommand(); - applyMocksToCommand(command); - await createTestCommandRunner(command).run(const ['precache', '--force']); - expect(artifacts, unorderedEquals({ - DevelopmentArtifact.universal, - DevelopmentArtifact.iOS, - DevelopmentArtifact.androidGenSnapshot, - DevelopmentArtifact.androidMaven, - DevelopmentArtifact.androidInternalBuild, - })); - }, overrides: { - Cache: () => cache, - FlutterVersion: () => flutterVersion, - }); + testUsingContext('precache downloads web artifacts on dev branch when feature is enabled.', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run(const ['precache', '--web', '--no-android', '--no-ios']); + + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.web, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + })); + }, overrides: { + Cache: () => cache, + FeatureFlags: () => TestFeatureFlags(isWebEnabled: true), + }); + + testUsingContext('precache does not download web artifacts on dev branch when feature is enabled.', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run(const ['precache', '--web', '--no-android', '--no-ios']); + + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + })); + }, overrides: { + Cache: () => cache, + FeatureFlags: () => TestFeatureFlags(isWebEnabled: false), + }); + + testUsingContext('precache adds artifact flags to requested artifacts', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run( + const [ + 'precache', + '--ios', + '--android', + '--web', + '--macos', + '--linux', + '--windows', + '--fuchsia', + '--flutter_runner', + ], + ); + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.iOS, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + DevelopmentArtifact.web, + DevelopmentArtifact.macOS, + DevelopmentArtifact.linux, + DevelopmentArtifact.windows, + DevelopmentArtifact.fuchsia, + DevelopmentArtifact.flutterRunner, + })); + }, overrides: { + Cache: () => cache, + FeatureFlags: () => TestFeatureFlags(isWebEnabled: true), + FlutterVersion: () => masterFlutterVersion, + }); + + testUsingContext('precache expands android artifacts when the android flag is used', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run( + const [ + 'precache', + '--no-ios', + '--android', + ], + ); + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + })); + }, overrides: { + Cache: () => cache, + }); + + testUsingContext('precache adds artifact flags to requested android artifacts', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run( + const [ + 'precache', + '--no-ios', + '--android_gen_snapshot', + '--android_maven', + '--android_internal_build', + ], + ); + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + })); + }, overrides: { + Cache: () => cache, + }); + + testUsingContext('precache adds artifact flags to requested artifacts on stable', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run( + const [ + 'precache', + '--ios', + '--android_gen_snapshot', + '--android_maven', + '--android_internal_build', + '--web', + '--macos', + '--linux', + '--windows', + '--fuchsia', + '--flutter_runner', + ], + ); + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.iOS, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + })); + }, overrides: { + Cache: () => cache, + FlutterVersion: () => flutterVersion, + FeatureFlags: () => TestFeatureFlags(isWebEnabled: false), + }); + testUsingContext('precache downloads artifacts when --force is provided', () async { + when(cache.isUpToDate()).thenReturn(true); + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + await createTestCommandRunner(command).run(const ['precache', '--force']); + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.iOS, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + })); + }, overrides: { + Cache: () => cache, + FlutterVersion: () => flutterVersion, }); } diff --git a/packages/flutter_tools/test/general.shard/cache_test.dart b/packages/flutter_tools/test/general.shard/cache_test.dart index cc6bb6203c..a6a20a2576 100644 --- a/packages/flutter_tools/test/general.shard/cache_test.dart +++ b/packages/flutter_tools/test/general.shard/cache_test.dart @@ -209,7 +209,7 @@ void main() { }); test('Unstable artifacts', () { - expect(DevelopmentArtifact.web.unstable, true); + expect(DevelopmentArtifact.web.unstable, false); expect(DevelopmentArtifact.linux.unstable, true); expect(DevelopmentArtifact.macOS.unstable, true); expect(DevelopmentArtifact.windows.unstable, true); diff --git a/packages/flutter_tools/test/src/testbed.dart b/packages/flutter_tools/test/src/testbed.dart index d8db9fa905..37acf093a6 100644 --- a/packages/flutter_tools/test/src/testbed.dart +++ b/packages/flutter_tools/test/src/testbed.dart @@ -712,6 +712,23 @@ class TestFeatureFlags implements FeatureFlags { @override final bool isNewAndroidEmbeddingEnabled; + + @override + bool isEnabled(Feature feature) { + switch (feature) { + case flutterWebFeature: + return isWebEnabled; + case flutterLinuxDesktopFeature: + return isLinuxEnabled; + case flutterMacOSDesktopFeature: + return isMacOSEnabled; + case flutterWindowsDesktopFeature: + return isWindowsEnabled; + case flutterNewAndroidEmbeddingFeature: + return isNewAndroidEmbeddingEnabled; + } + return false; + } } class ThrowingPub implements Pub {