[flutter_tools] Isolate userHomePath, other cleanups (#50125)

packages/flutter_tools/lib/src/base/config.dart

@@ -3,17 +3,40 @@
 // found in the LICENSE file.
 
 import 'package:meta/meta.dart';
+import 'package:platform/platform.dart';
 
 import '../convert.dart';
 import 'file_system.dart';
 import 'logger.dart';
 import 'utils.dart';
 
+/// A class to abstract configuration files.
 class Config {
-  Config({
-    @required File file,
+  /// Constructs a new [Config] object from a file called [name] in the
+  /// current user's home directory as determined by the [Platform] and
+  /// [FileSystem].
+  factory Config(
+    String name, {
+    @required FileSystem fileSystem,
     @required Logger logger,
-  }) : _file = file, _logger = logger {
+    @required Platform platform,
+  }) {
+    final File file = fileSystem.file(fileSystem.path.join(
+      _userHomePath(platform),
+      name,
+    ));
+    return Config._(file, logger);
+  }
+
+  /// Constructs a new [Config] object from a file called [name] in
+  /// the given [Directory].
+  factory Config.test(
+    String name, {
+    @required Directory directory,
+    @required Logger logger,
+  }) => Config._(directory.childFile(name), logger);
+
+  Config._(File file, Logger logger) : _file = file, _logger = logger {
     if (!_file.existsSync()) {
       return;
     }
@@ -30,11 +53,13 @@ class Config {
     }
   }
 
+  /// The default name for the Flutter config file.
   static const String kFlutterSettings = '.flutter_settings';
 
-  final File _file;
   final Logger _logger;
 
+  File _file;
+
   String get configPath => _file.path;
 
   Map<String, dynamic> _values = <String, dynamic>{};
@@ -60,4 +85,16 @@ class Config {
     json = '$json\n';
     _file.writeAsStringSync(json);
   }
+
+  // Reads the process environment to find the current user's home directory.
+  //
+  // If the searched environment variables are not set, '.' is returned instead.
+  //
+  // Note that this is different from FileSystemUtils.homeDirPath.
+  static String _userHomePath(Platform platform) {
+    final String envKey = platform.operatingSystem == 'windows'
+      ? 'APPDATA'
+      : 'HOME';
+    return platform.environment[envKey] ?? '.';
+  }
 }

packages/flutter_tools/lib/src/base/file_system.dart

@@ -133,14 +133,6 @@ class FileSystemUtils {
         && referenceFile.statSync().modified.isAfter(entity.statSync().modified);
   }
 
-  /// Reads the process environment to find the current user's home directory.
-  ///
-  /// If the searched environment variables are not set, '.' is returned instead.
-  String get userHomePath {
-    final String envKey = _platform.operatingSystem == 'windows' ? 'APPDATA' : 'HOME';
-    return _platform.environment[envKey] ?? '.';
-  }
-
   /// Return the absolute path of the user's home directory
   String get homeDirPath {
     String path = _platform.isWindows

packages/flutter_tools/lib/src/commands/upgrade.dart

@@ -13,7 +13,6 @@ import '../base/process.dart';
 import '../cache.dart';
 import '../dart/pub.dart';
 import '../globals.dart' as globals;
-import '../persistent_tool_state.dart';
 import '../runner/flutter_command.dart';
 import '../version.dart';
 import 'channel.dart';
@@ -144,12 +143,12 @@ class UpgradeCommandRunner {
   // re-entrantly with the `--continue` flag
   Future<void> runCommandSecondHalf(FlutterVersion flutterVersion) async {
     // Make sure the welcome message re-display is delayed until the end.
-    persistentToolState.redisplayWelcomeMessage = false;
+    globals.persistentToolState.redisplayWelcomeMessage = false;
     await precacheArtifacts();
     await updatePackages(flutterVersion);
     await runDoctor();
     // Force the welcome message to re-display following the upgrade.
-    persistentToolState.redisplayWelcomeMessage = true;
+    globals.persistentToolState.redisplayWelcomeMessage = true;
   }
 
   Future<bool> hasUncomittedChanges() async {

packages/flutter_tools/lib/src/context_runner.dart

@@ -88,11 +88,10 @@ Future<T> runInContext<T>(
       CocoaPods: () => CocoaPods(),
       CocoaPodsValidator: () => const CocoaPodsValidator(),
       Config: () => Config(
-        file: globals.fs.file(globals.fs.path.join(
-          globals.fsUtils.userHomePath,
         Config.kFlutterSettings,
-        )),
+        fileSystem: globals.fs,
         logger: globals.logger,
+        platform: globals.platform,
       ),
       DevFSConfig: () => DevFSConfig(),
       DeviceManager: () => DeviceManager(),
@@ -137,7 +136,11 @@ Future<T> runInContext<T>(
         platform: globals.platform,
         processManager: globals.processManager,
       ),
-      PersistentToolState: () => PersistentToolState(),
+      PersistentToolState: () => PersistentToolState(
+        fileSystem: globals.fs,
+        logger: globals.logger,
+        platform: globals.platform,
+      ),
       ProcessInfo: () => ProcessInfo(),
       ProcessUtils: () => ProcessUtils(
         processManager: globals.processManager,

packages/flutter_tools/lib/src/globals.dart

@@ -17,6 +17,7 @@ import 'base/terminal.dart';
 import 'cache.dart';
 import 'ios/mac.dart';
 import 'macos/xcode.dart';
+import 'persistent_tool_state.dart';
 import 'version.dart';
 
 Artifacts get artifacts => context.get<Artifacts>();
@@ -24,6 +25,7 @@ Cache get cache => context.get<Cache>();
 Config get config => context.get<Config>();
 Logger get logger => context.get<Logger>();
 OperatingSystemUtils get os => context.get<OperatingSystemUtils>();
+PersistentToolState get persistentToolState => PersistentToolState.instance;
 
 const FileSystem _kLocalFs = LocalFileSystem();
 

packages/flutter_tools/lib/src/persistent_tool_state.dart

@@ -2,18 +2,34 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:meta/meta.dart';
+import 'package:platform/platform.dart';
+
 import 'base/config.dart';
 import 'base/context.dart';
 import 'base/file_system.dart';
-import 'globals.dart' as globals;
-
-PersistentToolState get persistentToolState => PersistentToolState.instance;
+import 'base/logger.dart';
 
 /// A class that represents global (non-project-specific) internal state that
 /// must persist across tool invocations.
 abstract class PersistentToolState {
-  factory PersistentToolState([File configFile]) =>
-    _DefaultPersistentToolState(configFile);
+  factory PersistentToolState({
+    @required FileSystem fileSystem,
+    @required Logger logger,
+    @required Platform platform,
+  }) => _DefaultPersistentToolState(
+    fileSystem: fileSystem,
+    logger: logger,
+    platform: platform,
+  );
+
+  factory PersistentToolState.test({
+    @required Directory directory,
+    @required Logger logger,
+  }) => _DefaultPersistentToolState.test(
+    directory: directory,
+    logger: logger,
+  );
 
   static PersistentToolState get instance => context.get<PersistentToolState>();
 
@@ -24,13 +40,25 @@ abstract class PersistentToolState {
 }
 
 class _DefaultPersistentToolState implements PersistentToolState {
-  _DefaultPersistentToolState([File configFile]) :
-    _config = Config(
-      file: configFile ?? globals.fs.file(globals.fs.path.join(
-        globals.fsUtils.userHomePath,
+  _DefaultPersistentToolState({
+    @required FileSystem fileSystem,
+    @required Logger logger,
+    @required Platform platform,
+  }) : _config = Config(
       _kFileName,
-      )),
-      logger: globals.logger,
+      fileSystem: fileSystem,
+      logger: logger,
+      platform: platform,
+    );
+
+  @visibleForTesting
+  _DefaultPersistentToolState.test({
+    @required Directory directory,
+    @required Logger logger,
+  }) : _config = Config.test(
+      _kFileName,
+      directory: directory,
+      logger: logger,
     );
 
   static const String _kFileName = '.flutter_tool_state';
@@ -39,7 +67,9 @@ class _DefaultPersistentToolState implements PersistentToolState {
   final Config _config;
 
   @override
-  bool get redisplayWelcomeMessage => _config.getValue(_kRedisplayWelcomeMessage) as bool;
+  bool get redisplayWelcomeMessage {
+    return _config.getValue(_kRedisplayWelcomeMessage) as bool;
+  }
 
   @override
   set redisplayWelcomeMessage(bool value) {

packages/flutter_tools/lib/src/reporting/reporting.dart

@@ -18,7 +18,6 @@ import '../base/time.dart';
 import '../doctor.dart';
 import '../features.dart';
 import '../globals.dart' as globals;
-import '../persistent_tool_state.dart';
 import '../runner/flutter_command.dart';
 import '../version.dart';
 

packages/flutter_tools/lib/src/reporting/usage.dart

@@ -376,10 +376,10 @@ class _DefaultUsage implements Usage {
         // Display the welcome message if we are not on master, and if the
         // persistent tool state instructs that we should.
         (!globals.flutterVersion.isMaster &&
-        (persistentToolState.redisplayWelcomeMessage ?? true))) {
+        (globals.persistentToolState.redisplayWelcomeMessage ?? true))) {
       _printWelcome();
       _printedWelcome = true;
-      persistentToolState.redisplayWelcomeMessage = false;
+      globals.persistentToolState.redisplayWelcomeMessage = false;
     }
   }
 }

packages/flutter_tools/test/commands.shard/permeable/upgrade_test.dart

@@ -251,12 +251,17 @@ void main() {
           ],
         );
 
-        expect(json.decode(flutterToolState.readAsStringSync()),
-          containsPair('redisplay-welcome-message', true));
+        expect(
+          json.decode(flutterToolState.readAsStringSync()),
+          containsPair('redisplay-welcome-message', true),
+        );
       }, overrides: <Type, Generator>{
         FlutterVersion: () => mockFlutterVersion,
         ProcessManager: () => fakeProcessManager,
-        PersistentToolState: () => PersistentToolState(flutterToolState),
+        PersistentToolState: () => PersistentToolState.test(
+          directory: tempDir,
+          logger: testLogger,
+        ),
       });
     });
   });

packages/flutter_tools/test/general.shard/config_test.dart

@@ -17,11 +17,22 @@ class MockLogger extends Mock implements Logger {}
 void main() {
   Config config;
   MemoryFileSystem memoryFileSystem;
+  FakePlatform fakePlatform;
 
   setUp(() {
     memoryFileSystem = MemoryFileSystem();
-    final File file = memoryFileSystem.file('example');
-    config = Config(file: file, logger: MockLogger());
+    fakePlatform = FakePlatform(
+      operatingSystem: 'linux',
+      environment: <String, String>{
+        'HOME': '/',
+      },
+    );
+    config = Config(
+      'example',
+      fileSystem: memoryFileSystem,
+      logger: MockLogger(),
+      platform: fakePlatform,
+    );
   });
   testWithoutContext('Config get set value', () async {
     expect(config.getValue('foo'), null);
@@ -63,7 +74,12 @@ void main() {
     );
     final File file = memoryFileSystem.file('example')
       ..writeAsStringSync('{"hello":"bar');
-    config = Config(file: file, logger: bufferLogger);
+    config = Config(
+      'example',
+      fileSystem: memoryFileSystem,
+      logger: bufferLogger,
+      platform: fakePlatform,
+    );
 
     expect(file.existsSync(), false);
     expect(bufferLogger.errorText, contains('Failed to decode preferences'));

packages/flutter_tools/test/general.shard/persistent_tool_state_test.dart

@@ -2,23 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/persistent_tool_state.dart';
-import 'package:flutter_tools/src/globals.dart' as globals;
+import 'package:mockito/mockito.dart';
 
 import '../src/common.dart';
-import '../src/testbed.dart';
 
-void main() {
-  Testbed testbed;
-
-  setUp(() {
-    testbed = Testbed();
-  });
+class MockLogger extends Mock implements Logger {}
 
-  test('state can be set and persists', () => testbed.run(() {
-    final File stateFile = globals.fs.file('.flutter_tool_state');
-    final PersistentToolState state1 = PersistentToolState(stateFile);
+void main() {
+  testWithoutContext('state can be set and persists', () {
+    final MemoryFileSystem fs = MemoryFileSystem();
+    final Directory directory = fs.directory('state_dir');
+    directory.createSync();
+    final File stateFile = directory.childFile('.flutter_tool_state');
+    final PersistentToolState state1 = PersistentToolState.test(
+      directory: directory,
+      logger: MockLogger(),
+    );
     expect(state1.redisplayWelcomeMessage, null);
     state1.redisplayWelcomeMessage = true;
     expect(stateFile.existsSync(), true);
@@ -26,7 +29,10 @@ void main() {
     state1.redisplayWelcomeMessage = false;
     expect(state1.redisplayWelcomeMessage, false);
 
-    final PersistentToolState state2 = PersistentToolState(stateFile);
+    final PersistentToolState state2 = PersistentToolState.test(
+      directory: directory,
+      logger: MockLogger(),
+    );
     expect(state2.redisplayWelcomeMessage, false);
-  }));
+  });
 }

packages/flutter_tools/test/src/context.dart

@@ -78,17 +78,23 @@ void testUsingContext(
     }
   });
   Config buildConfig(FileSystem fs) {
-    configDir ??= globals.fs.systemTempDirectory.createTempSync('flutter_config_dir_test.');
-    final File settingsFile = globals.fs.file(
-      globals.fs.path.join(configDir.path, '.flutter_settings')
+    configDir ??= globals.fs.systemTempDirectory.createTempSync(
+      'flutter_config_dir_test.',
+    );
+    return Config.test(
+      Config.kFlutterSettings,
+      directory: configDir,
+      logger: globals.logger,
     );
-    return Config(file: settingsFile, logger: globals.logger);
   }
   PersistentToolState buildPersistentToolState(FileSystem fs) {
-    configDir ??= globals.fs.systemTempDirectory.createTempSync('flutter_config_dir_test.');
-    final File toolStateFile = globals.fs.file(
-      globals.fs.path.join(configDir.path, '.flutter_tool_state'));
-    return PersistentToolState(toolStateFile);
+    configDir ??= globals.fs.systemTempDirectory.createTempSync(
+      'flutter_config_dir_test.',
+    );
+    return PersistentToolState.test(
+      directory: configDir,
+      logger: globals.logger,
+    );
   }
 
   test(description, () async {