[flutter_tools] remove some globals from flutter_tester device (#60787)

Remove globals from flutter_tester device and cleanup test case. Not completely gone since the Kernel Builder will still use them, but a good incremental improvement.

packages/flutter_tools/lib/src/application_package.dart

@@ -51,7 +51,7 @@ class ApplicationPackageFactory {
             ? await IOSApp.fromIosProject(FlutterProject.current().ios, buildInfo)
             : IOSApp.fromPrebuiltApp(applicationBinary);
       case TargetPlatform.tester:
-        return FlutterTesterApp.fromCurrentDirectory();
+        return FlutterTesterApp.fromCurrentDirectory(globals.fs);
       case TargetPlatform.darwin_x64:
         return applicationBinary == null
             ? MacOSApp.fromMacOSProject(FlutterProject.current().macos)

packages/flutter_tools/lib/src/tester/flutter_tester.dart

@@ -5,12 +5,13 @@
 import 'dart:async';
 
 import 'package:meta/meta.dart';
+import 'package:process/process.dart';
 
 import '../application_package.dart';
 import '../artifacts.dart';
-import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/io.dart';
+import '../base/logger.dart';
 import '../build_info.dart';
 import '../bundle.dart';
 import '../convert.dart';
@@ -21,8 +22,8 @@ import '../protocol_discovery.dart';
 import '../version.dart';
 
 class FlutterTesterApp extends ApplicationPackage {
-  factory FlutterTesterApp.fromCurrentDirectory() {
-    return FlutterTesterApp._(globals.fs.currentDirectory);
+  factory FlutterTesterApp.fromCurrentDirectory(FileSystem fileSystem) {
+    return FlutterTesterApp._(fileSystem.currentDirectory);
   }
 
   FlutterTesterApp._(Directory directory)
@@ -38,17 +39,42 @@ class FlutterTesterApp extends ApplicationPackage {
   File get packagesFile => _directory.childFile('.packages');
 }
 
+/// The device interface for running on the flutter_tester shell.
+///
+/// Normally this is only used as the runner for `flutter test`, but it can
+/// also be used as a regular device when `--show-test-device` is provided
+/// to the flutter command.
 // TODO(scheglov): This device does not currently work with full restarts.
 class FlutterTesterDevice extends Device {
-  FlutterTesterDevice(String deviceId) : super(
+  FlutterTesterDevice(String deviceId, {
+    @required ProcessManager processManager,
+    @required FlutterVersion flutterVersion,
+    @required Logger logger,
+    @required String buildDirectory,
+    @required FileSystem fileSystem,
+    @required Artifacts artifacts,
+  }) : _processManager = processManager,
+       _flutterVersion = flutterVersion,
+       _logger = logger,
+       _buildDirectory = buildDirectory,
+       _fileSystem = fileSystem,
+       _artifacts = artifacts,
+       super(
         deviceId,
         platformType: null,
         category: null,
         ephemeral: false,
       );
 
+  final ProcessManager _processManager;
+  final FlutterVersion _flutterVersion;
+  final Logger _logger;
+  final String _buildDirectory;
+  final FileSystem _fileSystem;
+  final Artifacts _artifacts;
+
   Process _process;
-  final DevicePortForwarder _portForwarder = _NoopPortForwarder();
+  final DevicePortForwarder _portForwarder = const NoOpDevicePortForwarder();
 
   @override
   Future<bool> get isLocalEmulator async => false;
@@ -64,7 +90,7 @@ class FlutterTesterDevice extends Device {
 
   @override
   Future<String> get sdkNameAndVersion async {
-    final FlutterVersion flutterVersion = globals.flutterVersion;
+    final FlutterVersion flutterVersion = _flutterVersion;
     return 'Flutter ${flutterVersion.frameworkRevisionShort}';
   }
 
@@ -106,9 +132,6 @@ class FlutterTesterDevice extends Device {
   @override
   bool isSupported() => true;
 
-  bool _isRunning = false;
-  bool get isRunning => _isRunning;
-
   @override
   Future<LaunchResult> startApp(
     ApplicationPackage package, {
@@ -121,42 +144,17 @@ class FlutterTesterDevice extends Device {
     String userIdentifier,
   }) async {
     final BuildInfo buildInfo = debuggingOptions.buildInfo;
-
     if (!buildInfo.isDebug) {
-      globals.printError('This device only supports debug mode.');
+      _logger.printError('This device only supports debug mode.');
       return LaunchResult.failed();
     }
 
-    final String shellPath = globals.artifacts.getArtifactPath(Artifact.flutterTester);
-    if (!globals.fs.isFileSync(shellPath)) {
-      throwToolExit('Cannot find Flutter shell at $shellPath');
-    }
-
-    final List<String> command = <String>[
-      shellPath,
-      '--run-forever',
-      '--non-interactive',
-      '--enable-dart-profiling',
-      '--packages=${debuggingOptions.buildInfo.packagesPath}',
-    ];
-    if (debuggingOptions.debuggingEnabled) {
-      if (debuggingOptions.startPaused) {
-        command.add('--start-paused');
-      }
-      if (debuggingOptions.disableServiceAuthCodes) {
-        command.add('--disable-service-auth-codes');
-      }
-      if (debuggingOptions.hasObservatoryPort) {
-        command.add('--observatory-port=${debuggingOptions.hostVmServicePort}');
-      }
-    }
-
-    // Build assets and perform initial compilation.
-    final String assetDirPath = getAssetBuildDirectory();
+    final String assetDirPath = _fileSystem.path.join(_buildDirectory, 'flutter_assets');
     final String applicationKernelFilePath = getKernelPathForTransformerOptions(
-      globals.fs.path.join(getBuildDirectory(), 'flutter-tester-app.dill'),
+      _fileSystem.path.join(_buildDirectory, 'flutter-tester-app.dill'),
       trackWidgetCreation: buildInfo.trackWidgetCreation,
     );
+    // Build assets and perform initial compilation.
     await BundleBuilder().build(
       buildInfo: buildInfo,
       mainPath: mainPath,
@@ -167,34 +165,39 @@ class FlutterTesterDevice extends Device {
       platform: getTargetPlatformForName(getNameForHostPlatform(getCurrentHostPlatform())),
       treeShakeIcons: buildInfo.treeShakeIcons,
     );
-    command.add('--flutter-assets-dir=$assetDirPath');
 
-    command.add(applicationKernelFilePath);
+    final List<String> command = <String>[
+      _artifacts.getArtifactPath(Artifact.flutterTester),
+      '--run-forever',
+      '--non-interactive',
+      '--enable-dart-profiling',
+      '--packages=${debuggingOptions.buildInfo.packagesPath}',
+      '--flutter-assets-dir=$assetDirPath',
+      if (debuggingOptions.startPaused)
+        '--start-paused',
+      if (debuggingOptions.disableServiceAuthCodes)
+        '--disable-service-auth-codes',
+      if (debuggingOptions.hasObservatoryPort)
+        '--observatory-port=${debuggingOptions.hostVmServicePort}',
+      applicationKernelFilePath
+    ];
 
     ProtocolDiscovery observatoryDiscovery;
     try {
-      globals.printTrace(command.join(' '));
-
-      _isRunning = true;
-      _process = await globals.processManager.start(command,
+      _logger.printTrace(command.join(' '));
+      _process = await _processManager.start(command,
         environment: <String, String>{
           'FLUTTER_TEST': 'true',
         },
       );
-      // Setting a bool can't fail in the callback.
-      unawaited(_process.exitCode.then<void>((_) => _isRunning = false));
       _process.stdout
         .transform<String>(utf8.decoder)
         .transform<String>(const LineSplitter())
-        .listen((String line) {
-          _logReader.addLine(line);
-        });
+        .listen(_logReader.addLine);
       _process.stderr
         .transform<String>(utf8.decoder)
         .transform<String>(const LineSplitter())
-        .listen((String line) {
-          _logReader.addLine(line);
-        });
+        .listen(_logReader.addLine);
 
       if (!debuggingOptions.debuggingEnabled) {
         return LaunchResult.succeeded();
@@ -211,12 +214,12 @@ class FlutterTesterDevice extends Device {
       if (observatoryUri != null) {
         return LaunchResult.succeeded(observatoryUri: observatoryUri);
       }
-      globals.printError(
+      _logger.printError(
         'Failed to launch $package: '
         'The log reader failed unexpectedly.',
       );
     } on Exception catch (error) {
-      globals.printError('Failed to launch $package: $error');
+      _logger.printError('Failed to launch $package: $error');
     } finally {
       await observatoryDiscovery?.cancel();
     }
@@ -256,8 +259,15 @@ class FlutterTesterDevices extends PollingDeviceDiscovery {
 
   static bool showFlutterTesterDevice = false;
 
-  final FlutterTesterDevice _testerDevice =
-      FlutterTesterDevice(kTesterDeviceId);
+  final FlutterTesterDevice _testerDevice = FlutterTesterDevice(
+    kTesterDeviceId,
+    fileSystem: globals.fs,
+    artifacts: globals.artifacts,
+    processManager: globals.processManager,
+    buildDirectory: getBuildDirectory(),
+    logger: globals.logger,
+    flutterVersion: globals.flutterVersion,
+  );
 
   @override
   bool get canListAnything => true;
@@ -289,24 +299,3 @@ class _FlutterTesterDeviceLogReader extends DeviceLogReader {
   @override
   void dispose() {}
 }
-
-/// A fake port forwarder that doesn't do anything. Used by flutter tester
-/// where the VM is running on the same machine and does not need ports forwarding.
-class _NoopPortForwarder extends DevicePortForwarder {
-  @override
-  Future<int> forward(int devicePort, { int hostPort }) {
-    if (hostPort != null && hostPort != devicePort) {
-      throw 'Forwarding to a different port is not supported by flutter tester';
-    }
-    return Future<int>.value(devicePort);
-  }
-
-  @override
-  List<ForwardedPort> get forwardedPorts => <ForwardedPort>[];
-
-  @override
-  Future<void> unforward(ForwardedPort forwardedPort) async { }
-
-  @override
-  Future<void> dispose() async { }
-}

packages/flutter_tools/test/general.shard/tester/flutter_tester_test.dart

@@ -7,6 +7,7 @@ import 'dart:async';
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/artifacts.dart';
+import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/build_system/build_system.dart';
@@ -23,22 +24,18 @@ void main() {
   MemoryFileSystem fileSystem;
 
   setUp(() {
-    fileSystem = MemoryFileSystem();
+    fileSystem = MemoryFileSystem.test();
   });
 
-  group('FlutterTesterApp', () {
-    testUsingContext('fromCurrentDirectory', () async {
+  testWithoutContext('FlutterTesterApp can be created from the current directory', () async {
     const String projectPath = '/home/my/projects/my_project';
     await fileSystem.directory(projectPath).create(recursive: true);
     fileSystem.currentDirectory = projectPath;
 
-      final FlutterTesterApp app = FlutterTesterApp.fromCurrentDirectory();
+    final FlutterTesterApp app = FlutterTesterApp.fromCurrentDirectory(fileSystem);
+
     expect(app.name, 'my_project');
     expect(app.packagesFile.path, fileSystem.path.join(projectPath, '.packages'));
-    }, overrides: <Type, Generator>{
-      FileSystem: () => fileSystem,
-      ProcessManager: () => FakeProcessManager.any(),
-    });
   });
 
   group('FlutterTesterDevices', () {
@@ -74,41 +71,11 @@ void main() {
       expect(devices, hasLength(1));
     });
   });
-
-  group('FlutterTesterDevice', () {
+  group('startApp', () {
     FlutterTesterDevice device;
     List<String> logLines;
-
-    setUp(() {
-      device = FlutterTesterDevice('flutter-tester');
-
-      logLines = <String>[];
-      device.getLogReader().logLines.listen(logLines.add);
-    });
-
-    testUsingContext('getters', () async {
-      expect(device.id, 'flutter-tester');
-      expect(await device.isLocalEmulator, isFalse);
-      expect(device.name, 'Flutter test device');
-      expect(device.portForwarder, isNot(isNull));
-      expect(await device.targetPlatform, TargetPlatform.tester);
-
-      expect(await device.installApp(null), isTrue);
-      expect(await device.isAppInstalled(null), isFalse);
-      expect(await device.isLatestBuildInstalled(null), isFalse);
-      expect(await device.uninstallApp(null), isTrue);
-
-      expect(device.isSupported(), isTrue);
-    });
-
-    group('startApp', () {
-      String flutterRoot;
-      String flutterTesterPath;
-
-      String projectPath;
     String mainPath;
 
-      MockArtifacts mockArtifacts;
     MockProcessManager mockProcessManager;
     MockProcess mockProcess;
     MockBuildSystem mockBuildSystem;
@@ -117,55 +84,71 @@ void main() {
       Platform: () => FakePlatform(operatingSystem: 'linux'),
       FileSystem: () => fileSystem,
       ProcessManager: () => mockProcessManager,
-        Artifacts: () => mockArtifacts,
+      Artifacts: () => Artifacts.test(),
       BuildSystem: () => mockBuildSystem,
     };
 
     setUp(() {
       mockBuildSystem = MockBuildSystem();
-        flutterRoot = fileSystem.path.join('home', 'me', 'flutter');
-        flutterTesterPath = fileSystem.path.join(flutterRoot, 'bin', 'cache',
-            'artifacts', 'engine', 'linux-x64', 'flutter_tester');
-        final File flutterTesterFile = fileSystem.file(flutterTesterPath);
-        flutterTesterFile.parent.createSync(recursive: true);
-        flutterTesterFile.writeAsBytesSync(const <int>[]);
-
-        projectPath = fileSystem.path.join('home', 'me', 'hello');
-        mainPath = fileSystem.path.join(projectPath, 'lin', 'main.dart');
-
       mockProcessManager = MockProcessManager();
       mockProcessManager.processFactory =
           (List<String> commands) => mockProcess;
 
-        mockArtifacts = MockArtifacts();
-        final String artifactPath = fileSystem.path.join(flutterRoot, 'artifact');
-        fileSystem.file(artifactPath).createSync(recursive: true);
-        when(mockArtifacts.getArtifactPath(
-          any,
-          mode: anyNamed('mode')
-        )).thenReturn(artifactPath);
-        when(mockArtifacts.isLocalEngine)
-          .thenReturn(false);
-
       when(mockBuildSystem.build(
         any,
         any,
       )).thenAnswer((_) async {
-          fileSystem.file('$mainPath.dill').createSync(recursive: true);
         return BuildResult(success: true);
       });
+      device = FlutterTesterDevice('flutter-tester',
+        fileSystem: fileSystem,
+        processManager: mockProcessManager,
+        artifacts: Artifacts.test(),
+        buildDirectory: 'build',
+        logger: BufferLogger.test(),
+        flutterVersion: MockFlutterVersion(),
+      );
+      logLines = <String>[];
+      device.getLogReader().logLines.listen(logLines.add);
     });
 
-      testUsingContext('not debug', () async {
-        final LaunchResult result = await device.startApp(null,
+    testWithoutContext('default settings', () async {
+      expect(device.id, 'flutter-tester');
+      expect(await device.isLocalEmulator, isFalse);
+      expect(device.name, 'Flutter test device');
+      expect(device.portForwarder, isNot(isNull));
+      expect(await device.targetPlatform, TargetPlatform.tester);
+
+      expect(await device.installApp(null), isTrue);
+      expect(await device.isAppInstalled(null), isFalse);
+      expect(await device.isLatestBuildInstalled(null), isFalse);
+      expect(await device.uninstallApp(null), isTrue);
+
+      expect(device.isSupported(), isTrue);
+    });
+
+    testWithoutContext('does not accept profile, release, or jit-release builds', () async {
+      final LaunchResult releaseResult = await device.startApp(null,
         mainPath: mainPath,
-            debuggingOptions: DebuggingOptions.disabled(const BuildInfo(BuildMode.release, null, treeShakeIcons: false)));
+        debuggingOptions: DebuggingOptions.disabled(BuildInfo.release),
+      );
+      final LaunchResult profileResult = await device.startApp(null,
+        mainPath: mainPath,
+        debuggingOptions: DebuggingOptions.disabled(BuildInfo.profile),
+      );
+      final LaunchResult jitReleaseResult = await device.startApp(null,
+        mainPath: mainPath,
+        debuggingOptions: DebuggingOptions.disabled(BuildInfo.jitRelease),
+      );
 
-        expect(result.started, isFalse);
-      }, overrides: startOverrides);
+      expect(releaseResult.started, isFalse);
+      expect(profileResult.started, isFalse);
+      expect(jitReleaseResult.started, isFalse);
+    });
 
 
-      testUsingContext('start', () async {
+    testUsingContext('performs a build and starts in debug mode', () async {
+      final FlutterTesterApp app = FlutterTesterApp.fromCurrentDirectory(fileSystem);
       final Uri observatoryUri = Uri.parse('http://127.0.0.1:6666/');
       mockProcess = MockProcess(stdout: Stream<List<int>>.fromIterable(<List<int>>[
         '''
@@ -175,16 +158,16 @@ Hello!
             .codeUnits,
       ]));
 
-        final LaunchResult result = await device.startApp(null,
+      final LaunchResult result = await device.startApp(app,
         mainPath: mainPath,
-            debuggingOptions: DebuggingOptions.enabled(const BuildInfo(BuildMode.debug, null, treeShakeIcons: false)));
+        debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug)
+      );
+
       expect(result.started, isTrue);
       expect(result.observatoryUri, observatoryUri);
       expect(logLines.last, 'Hello!');
     }, overrides: startOverrides);
   });
-  });
 }
 
 class MockBuildSystem extends Mock implements BuildSystem {}
-class MockArtifacts extends Mock implements Artifacts {}