Move Cache.flutterRoot initialization up sooner (#16057)

It's required to be set before we detect local engine.

Was broken by #15984

packages/flutter_tools/lib/src/runner/flutter_command_runner.dart

@@ -238,6 +238,11 @@ class FlutterCommandRunner extends CommandRunner<Null> {
       VMService.enableReplayConnection(replayFrom);
     }
 
+    // We must set Cache.flutterRoot early because other features use it (e.g.
+    // enginePath's initializer uses it).
+    final String flutterRoot = topLevelResults['flutter-root'] ?? _defaultFlutterRoot;
+    Cache.flutterRoot = fs.path.normalize(fs.path.absolute(flutterRoot));
+
     // Set up the tooling configuration.
     final String enginePath = _findEnginePath(topLevelResults);
     if (enginePath != null) {
@@ -256,11 +261,6 @@ class FlutterCommandRunner extends CommandRunner<Null> {
         if (topLevelResults.wasParsed('color'))
           logger.supportsColor = topLevelResults['color'];
 
-        // We must set Cache.flutterRoot early because other features use it (e.g.
-        // enginePath's initializer uses it).
-        final String flutterRoot = topLevelResults['flutter-root'] ?? _defaultFlutterRoot;
-        Cache.flutterRoot = fs.path.normalize(fs.path.absolute(flutterRoot));
-
         if (platform.environment['FLUTTER_ALREADY_LOCKED'] != 'true')
           await Cache.lock();
 

packages/flutter_tools/test/runner/flutter_command_runner_test.dart

@@ -8,52 +8,94 @@ import 'package:flutter_tools/src/runner/flutter_command_runner.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/version.dart';
 import 'package:mockito/mockito.dart';
+import 'package:platform/platform.dart';
 import 'package:test/test.dart';
 
 import '../src/common.dart';
 import '../src/context.dart';
 import 'flutter_command_test.dart';
 
+const String _kFlutterRoot = '/flutter/flutter';
+const String _kEngineRoot = '/flutter/engine';
+const String _kProjectRoot = '/project';
+const String _kDotPackages = '.packages';
+
 void main() {
   group('FlutterCommandRunner', () {
-    testUsingContext('checks that Flutter installation is up-to-date', () async {
-      final MockFlutterVersion version = FlutterVersion.instance;
-      bool versionChecked = false;
-      when(version.checkFlutterVersionFreshness()).thenAnswer((_) async {
-        versionChecked = true;
+    MemoryFileSystem fs;
+    Platform platform;
+    FlutterCommandRunner runner;
+
+    setUpAll(() {
+      Cache.disableLocking();
+    });
+
+    setUp(() {
+      fs = new MemoryFileSystem();
+      fs.directory(_kFlutterRoot).createSync(recursive: true);
+      fs.directory(_kProjectRoot).createSync(recursive: true);
+      fs.currentDirectory = _kProjectRoot;
+
+      platform = new FakePlatform(environment: <String, String>{
+        'FLUTTER_ROOT': _kFlutterRoot,
       });
 
-      await createTestCommandRunner(new DummyFlutterCommand(shouldUpdateCache: false))
-          .run(<String>['dummy']);
+      runner = createTestCommandRunner(new DummyFlutterCommand());
+    });
 
-      expect(versionChecked, isTrue);
+    group('run', () {
+      testUsingContext('checks that Flutter installation is up-to-date', () async {
+        final MockFlutterVersion version = FlutterVersion.instance;
+        bool versionChecked = false;
+        when(version.checkFlutterVersionFreshness()).thenAnswer((_) async {
+          versionChecked = true;
+        });
+
+        await runner.run(<String>['dummy']);
+
+        expect(versionChecked, isTrue);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => fs,
+        Platform: () => platform,
+      }, initializeFlutterRoot: false);
+
+      testUsingContext('works if --local-engine is specified', () async {
+        fs.file(_kDotPackages).writeAsStringSync('sky_engine:file://$_kFlutterRoot/bin/cache/pkg/sky_engine/lib/');
+        fs.directory('$_kEngineRoot/src/out/ios_debug').createSync(recursive: true);
+        fs.directory('$_kEngineRoot/src/out/host_debug').createSync(recursive: true);
+        await runner.run(<String>['dummy', '--local-engine=ios_debug']);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => fs,
+        Platform: () => platform,
+      }, initializeFlutterRoot: false);
+    });
+
+    group('getRepoPackages', () {
+      setUp(() {
+        fs.directory(fs.path.join(_kFlutterRoot, 'examples'))
+            .createSync(recursive: true);
+        fs.directory(fs.path.join(_kFlutterRoot, 'packages'))
+            .createSync(recursive: true);
+        fs.directory(fs.path.join(_kFlutterRoot, 'dev', 'tools', 'aatool'))
+            .createSync(recursive: true);
+
+        fs.file(fs.path.join(_kFlutterRoot, 'dev', 'tools', 'pubspec.yaml'))
+            .createSync();
+        fs.file(fs.path.join(_kFlutterRoot, 'dev', 'tools', 'aatool', 'pubspec.yaml'))
+            .createSync();
+      });
+
+      testUsingContext('', () {
+        final List<String> packagePaths = runner.getRepoPackages()
+            .map((Directory d) => d.path).toList();
+        expect(packagePaths, <String>[
+          fs.directory(fs.path.join(_kFlutterRoot, 'dev', 'tools', 'aatool')).path,
+          fs.directory(fs.path.join(_kFlutterRoot, 'dev', 'tools')).path,
+        ]);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => fs,
+        Platform: () => platform,
+      }, initializeFlutterRoot: false);
     });
   });
-
-  MemoryFileSystem fs;
-
-  setUp(() {
-    fs = new MemoryFileSystem();
-  });
-
-  testUsingContext('getRepoPackages', () {
-    final FlutterCommandRunner runner = new FlutterCommandRunner();
-    final String root = fs.path.absolute(Cache.flutterRoot);
-    fs.directory(fs.path.join(root, 'examples'))
-      .createSync(recursive: true);
-    fs.directory(fs.path.join(root, 'packages'))
-      .createSync(recursive: true);
-    fs.directory(fs.path.join(root, 'dev', 'tools', 'aatool'))
-      .createSync(recursive: true);
-
-    fs.file(fs.path.join(root, 'dev', 'tools', 'pubspec.yaml')).createSync();
-    fs.file(fs.path.join(root, 'dev', 'tools', 'aatool', 'pubspec.yaml')).createSync();
-
-    final List<String> packagePaths = runner.getRepoPackages()
-      .map((Directory d) => d.path).toList();
-    expect(packagePaths, <String>[
-      fs.directory(fs.path.join(root, 'dev', 'tools', 'aatool')).path,
-      fs.directory(fs.path.join(root, 'dev', 'tools')).path,
-    ]);
-  }, overrides: <Type, Generator>{ FileSystem: () => fs });
 }

packages/flutter_tools/test/src/context.dart

@@ -40,6 +40,7 @@ typedef void ContextInitializer(AppContext testContext);
 void testUsingContext(String description, dynamic testMethod(), {
   Timeout timeout,
   Map<Type, Generator> overrides: const <Type, Generator>{},
+  bool initializeFlutterRoot: true,
   String testOn,
   bool skip, // should default to `false`, but https://github.com/dart-lang/test/issues/545 doesn't allow this
 }) {
@@ -90,9 +91,11 @@ void testUsingContext(String description, dynamic testMethod(), {
                 overrides: overrides,
                 name: 'test-specific overrides',
                 body: () async {
-                  // Provide a sane default for the flutterRoot directory. Individual
-                  // tests can override this either in the test or during setup.
-                  Cache.flutterRoot ??= flutterRoot;
+                  if (initializeFlutterRoot) {
+                    // Provide a sane default for the flutterRoot directory. Individual
+                    // tests can override this either in the test or during setup.
+                    Cache.flutterRoot ??= flutterRoot;
+                  }
 
                   return await testMethod();
                 },