[flutter_tools] macOS cleanups, attach to log reader in release mode (#61913)

packages/flutter_tools/lib/src/context_runner.dart

@@ -142,6 +142,10 @@ Future<T> runInContext<T>(
         config: globals.config,
         fuchsiaWorkflow: fuchsiaWorkflow,
         xcDevice: globals.xcdevice,
+        macOSWorkflow: MacOSWorkflow(
+          platform: globals.platform,
+          featureFlags: featureFlags,
+        ),
       ),
       Doctor: () => Doctor(logger: globals.logger),
       DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,
@@ -193,7 +197,10 @@ Future<T> runInContext<T>(
             outputPreferences: globals.outputPreferences,
             timeoutConfiguration: timeoutConfiguration,
           ),
-      MacOSWorkflow: () => const MacOSWorkflow(),
+      MacOSWorkflow: () => MacOSWorkflow(
+        featureFlags: featureFlags,
+        platform: globals.platform,
+      ),
       MDnsObservatoryDiscovery: () => MDnsObservatoryDiscovery(),
       OperatingSystemUtils: () => OperatingSystemUtils(
         fileSystem: globals.fs,

packages/flutter_tools/lib/src/desktop_device.dart

@@ -5,10 +5,12 @@
 import 'dart:async';
 
 import 'package:meta/meta.dart';
+import 'package:process/process.dart';
 
 import 'application_package.dart';
 import 'base/common.dart';
 import 'base/io.dart';
+import 'base/logger.dart';
 import 'build_info.dart';
 import 'convert.dart';
 import 'device.dart';
@@ -18,15 +20,23 @@ import 'protocol_discovery.dart';
 /// A partial implementation of Device for desktop-class devices to inherit
 /// from, containing implementations that are common to all desktop devices.
 abstract class DesktopDevice extends Device {
-  DesktopDevice(String identifier, {@required PlatformType platformType, @required bool ephemeral}) : super(
-      identifier,
-      category: Category.desktop,
-      platformType: platformType,
-      ephemeral: ephemeral,
-  );
-
+  DesktopDevice(String identifier, {
+      @required PlatformType platformType,
+      @required bool ephemeral,
+      Logger logger,
+      ProcessManager processManager,
+    }) : _logger = logger ?? globals.logger, // TODO(jonahwilliams): remove after updating google3
+         _processManager = processManager ?? globals.processManager,
+         super(
+          identifier,
+          category: Category.desktop,
+          platformType: platformType,
+          ephemeral: ephemeral,
+        );
+
+  final Logger _logger;
+  final ProcessManager _processManager;
   final Set<Process> _runningProcesses = <Process>{};
-
   final DesktopLogReader _deviceLogReader = DesktopLogReader();
 
   // Since the host and target devices are the same, no work needs to be done
@@ -108,20 +118,20 @@ abstract class DesktopDevice extends Device {
     final BuildMode buildMode = debuggingOptions?.buildInfo?.mode;
     final String executable = executablePathForDevice(package, buildMode);
     if (executable == null) {
-      globals.printError('Unable to find executable to run');
+      _logger.printError('Unable to find executable to run');
       return LaunchResult.failed();
     }
 
-    final Process process = await globals.processManager.start(<String>[
+    final Process process = await _processManager.start(<String>[
       executable,
     ]);
     _runningProcesses.add(process);
     unawaited(process.exitCode.then((_) => _runningProcesses.remove(process)));
 
+    _deviceLogReader.initializeProcess(process);
     if (debuggingOptions?.buildInfo?.isRelease == true) {
       return LaunchResult.succeeded();
     }
-    _deviceLogReader.initializeProcess(process);
     final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader,
       devicePort: debuggingOptions?.deviceVmServicePort,
       hostPort: debuggingOptions?.hostVmServicePort,
@@ -133,12 +143,12 @@ abstract class DesktopDevice extends Device {
         onAttached(package, buildMode, process);
         return LaunchResult.succeeded(observatoryUri: observatoryUri);
       }
-      globals.printError(
+      _logger.printError(
         'Error waiting for a debug connection: '
         'The log reader stopped unexpectedly.',
       );
     } on Exception catch (error) {
-      globals.printError('Error waiting for a debug connection: $error');
+      _logger.printError('Error waiting for a debug connection: $error');
     } finally {
       await observatoryDiscovery.cancel();
     }
@@ -186,9 +196,7 @@ class DesktopLogReader extends DeviceLogReader {
   void initializeProcess(Process process) {
     process.stdout.listen(_inputController.add);
     process.stderr.listen(_inputController.add);
-    process.exitCode.then((int result) {
-      _inputController.close();
-    });
+    process.exitCode.whenComplete(_inputController.close);
   }
 
   @override

packages/flutter_tools/lib/src/device.dart

@@ -33,6 +33,7 @@ import 'ios/ios_workflow.dart';
 import 'ios/simulators.dart';
 import 'linux/linux_device.dart';
 import 'macos/macos_device.dart';
+import 'macos/macos_workflow.dart';
 import 'macos/xcode.dart';
 import 'project.dart';
 import 'tester/flutter_tester.dart';
@@ -292,6 +293,7 @@ class FlutterDeviceManager extends DeviceManager {
     @required FlutterVersion flutterVersion,
     @required Config config,
     @required Artifacts artifacts,
+    @required MacOSWorkflow macOSWorkflow,
   }) : deviceDiscoverers =  <DeviceDiscovery>[
     AndroidDevices(
       logger: logger,
@@ -322,10 +324,17 @@ class FlutterDeviceManager extends DeviceManager {
       logger: logger,
       artifacts: artifacts,
     ),
-    MacOSDevices(),
+    MacOSDevices(
+      processManager: processManager,
+      macOSWorkflow: macOSWorkflow,
+      logger: logger,
+      platform: platform,
+    ),
     LinuxDevices(
       platform: platform,
       featureFlags: featureFlags,
+      processManager: processManager,
+      logger: logger,
     ),
     WindowsDevices(),
     WebDevices(

packages/flutter_tools/lib/src/doctor.dart

@@ -65,6 +65,11 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
     featureFlags: featureFlags,
   );
 
+  final MacOSWorkflow macOSWorkflow = MacOSWorkflow(
+    platform: globals.platform,
+    featureFlags: featureFlags,
+  );
+
   @override
   List<DoctorValidator> get validators {
     if (_validators != null) {

packages/flutter_tools/lib/src/linux/linux_device.dart

@@ -3,12 +3,15 @@
 // found in the LICENSE file.
 
 import 'package:meta/meta.dart';
+import 'package:process/process.dart';
 
+import '../base/logger.dart';
 import '../base/platform.dart';
 import '../build_info.dart';
 import '../desktop_device.dart';
 import '../device.dart';
 import '../features.dart';
+import '../globals.dart' as globals;
 import '../project.dart';
 import 'application_package.dart';
 import 'build_linux.dart';
@@ -16,10 +19,15 @@ import 'linux_workflow.dart';
 
 /// A device that represents a desktop Linux target.
 class LinuxDevice extends DesktopDevice {
-  LinuxDevice() : super(
+  LinuxDevice({
+    @required ProcessManager processManager,
+    @required Logger logger,
+  }) : super(
       'linux',
       platformType: PlatformType.linux,
       ephemeral: false,
+      logger: logger,
+      processManager: processManager,
   );
 
   @override
@@ -59,15 +67,21 @@ class LinuxDevices extends PollingDeviceDiscovery {
   LinuxDevices({
     @required Platform platform,
     @required FeatureFlags featureFlags,
-  }) : _platform = platform,
+    ProcessManager processManager,
+    Logger logger,
+  }) : _platform = platform ?? globals.platform, // TODO(jonahwilliams): remove after google3 roll
        _linuxWorkflow = LinuxWorkflow(
           platform: platform,
           featureFlags: featureFlags,
        ),
+       _logger = logger,
+       _processManager = processManager ?? globals.processManager,
        super('linux devices');
 
   final Platform _platform;
   final LinuxWorkflow _linuxWorkflow;
+  final ProcessManager _processManager;
+  final Logger _logger;
 
   @override
   bool get supportsPlatform => _platform.isLinux;
@@ -81,7 +95,10 @@ class LinuxDevices extends PollingDeviceDiscovery {
       return const <Device>[];
     }
     return <Device>[
-      LinuxDevice(),
+      LinuxDevice(
+        logger: _logger,
+        processManager: _processManager,
+      ),
     ];
   }
 

packages/flutter_tools/lib/src/macos/macos_device.dart

@@ -2,11 +2,15 @@
 // 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:process/process.dart';
+
 import '../base/io.dart';
+import '../base/logger.dart';
+import '../base/platform.dart';
 import '../build_info.dart';
 import '../desktop_device.dart';
 import '../device.dart';
-import '../globals.dart' as globals;
 import '../macos/application_package.dart';
 import '../project.dart';
 import 'build_macos.dart';
@@ -14,11 +18,21 @@ import 'macos_workflow.dart';
 
 /// A device that represents a desktop MacOS target.
 class MacOSDevice extends DesktopDevice {
-  MacOSDevice() : super(
-      'macos',
-      platformType: PlatformType.macos,
-      ephemeral: false,
-  );
+  MacOSDevice({
+    @required ProcessManager processManager,
+    @required Logger logger,
+  }) : _processManager = processManager,
+       _logger = logger,
+       super(
+        'macos',
+        platformType: PlatformType.macos,
+        ephemeral: false,
+        processManager: processManager,
+        logger: logger,
+      );
+
+  final ProcessManager _processManager;
+  final Logger _logger;
 
   @override
   bool isSupported() => true;
@@ -44,7 +58,7 @@ class MacOSDevice extends DesktopDevice {
       flutterProject: FlutterProject.current(),
       buildInfo: buildInfo,
       targetOverride: mainPath,
-      verboseLogging: globals.logger.isVerbose,
+      verboseLogging: _logger.isVerbose,
     );
   }
 
@@ -59,7 +73,7 @@ class MacOSDevice extends DesktopDevice {
     // than post-attach, since this won't run for release builds, but there's
     // no general-purpose way of knowing when a process is far enoug along in
     // the launch process for 'open' to foreground it.
-    globals.processManager.run(<String>[
+    _processManager.run(<String>[
       'open', package.applicationBundle(buildMode),
     ]).then((ProcessResult result) {
       if (result.exitCode != 0) {
@@ -70,13 +84,27 @@ class MacOSDevice extends DesktopDevice {
 }
 
 class MacOSDevices extends PollingDeviceDiscovery {
-  MacOSDevices() : super('macOS devices');
+  MacOSDevices({
+    @required Platform platform,
+    @required MacOSWorkflow macOSWorkflow,
+    @required ProcessManager processManager,
+    @required Logger logger,
+  }) : _logger = logger,
+       _platform = platform,
+       _macOSWorkflow = macOSWorkflow,
+       _processManager = processManager,
+       super('macOS devices');
+
+  final MacOSWorkflow _macOSWorkflow;
+  final Platform _platform;
+  final ProcessManager _processManager;
+  final Logger _logger;
 
   @override
-  bool get supportsPlatform => globals.platform.isMacOS;
+  bool get supportsPlatform => _platform.isMacOS;
 
   @override
-  bool get canListAnything => macOSWorkflow.canListDevices;
+  bool get canListAnything => _macOSWorkflow.canListDevices;
 
   @override
   Future<List<Device>> pollingGetDevices({ Duration timeout }) async {
@@ -84,7 +112,7 @@ class MacOSDevices extends PollingDeviceDiscovery {
       return const <Device>[];
     }
     return <Device>[
-      MacOSDevice(),
+      MacOSDevice(processManager: _processManager, logger: _logger),
     ];
   }
 

packages/flutter_tools/lib/src/macos/macos_workflow.dart

@@ -2,29 +2,35 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import '../base/context.dart';
+import 'package:meta/meta.dart';
+
+import '../base/platform.dart';
 import '../doctor.dart';
 import '../features.dart';
-import '../globals.dart' as globals;
 
-/// The [MacOSWorkflow] instance.
-MacOSWorkflow get macOSWorkflow => context.get<MacOSWorkflow>();
 
 /// The macOS-specific implementation of a [Workflow].
 ///
 /// This workflow requires the flutter-desktop-embedding as a sibling
 /// repository to the flutter repo.
 class MacOSWorkflow implements Workflow {
-  const MacOSWorkflow();
+  const MacOSWorkflow({
+    @required Platform platform,
+    @required FeatureFlags featureFlags,
+  }) : _platform = platform,
+       _featureFlags = featureFlags;
+
+  final Platform _platform;
+  final FeatureFlags _featureFlags;
 
   @override
-  bool get appliesToHostPlatform => globals.platform.isMacOS && featureFlags.isMacOSEnabled;
+  bool get appliesToHostPlatform => _platform.isMacOS && _featureFlags.isMacOSEnabled;
 
   @override
-  bool get canLaunchDevices => globals.platform.isMacOS && featureFlags.isMacOSEnabled;
+  bool get canLaunchDevices => _platform.isMacOS && _featureFlags.isMacOSEnabled;
 
   @override
-  bool get canListDevices => globals.platform.isMacOS && featureFlags.isMacOSEnabled;
+  bool get canListDevices => _platform.isMacOS && _featureFlags.isMacOSEnabled;
 
   @override
   bool get canListEmulators => false;

packages/flutter_tools/test/general.shard/linux/linux_device_test.dart

@@ -4,6 +4,7 @@
 
 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/base/platform.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/device.dart';
@@ -17,15 +18,21 @@ import '../../src/common.dart';
 import '../../src/context.dart';
 import '../../src/testbed.dart';
 
-void main() {
-  final LinuxDevice device = LinuxDevice();
-  final MockPlatform notLinux = MockPlatform();
-  when(notLinux.isLinux).thenReturn(false);
+final FakePlatform linux = FakePlatform(
+  operatingSystem: 'linux',
+);
+final FakePlatform windows = FakePlatform(
+  operatingSystem: 'windows',
+);
 
-  final MockPlatform mockLinuxPlatform = MockPlatform();
-  when(mockLinuxPlatform.isLinux).thenReturn(true);
+void main() {
 
   testWithoutContext('LinuxDevice defaults', () async {
+    final LinuxDevice device = LinuxDevice(
+      processManager: FakeProcessManager.any(),
+      logger: BufferLogger.test(),
+    );
+
     final PrebuiltLinuxApp linuxApp = PrebuiltLinuxApp(executable: 'foo');
     expect(await device.targetPlatform, TargetPlatform.linux_x64);
     expect(device.name, 'Linux');
@@ -44,30 +51,38 @@ void main() {
 
   testWithoutContext('LinuxDevice: no devices listed if platform unsupported', () async {
     expect(await LinuxDevices(
-      platform: notLinux,
+      platform: windows,
       featureFlags: TestFeatureFlags(isLinuxEnabled: true),
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
     ).devices, <Device>[]);
   });
 
   testWithoutContext('LinuxDevice: no devices listed if Linux feature flag disabled', () async {
     expect(await LinuxDevices(
-      platform: mockLinuxPlatform,
+      platform: linux,
       featureFlags: TestFeatureFlags(isLinuxEnabled: false),
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
     ).devices, <Device>[]);
   });
 
   testWithoutContext('LinuxDevice: devices', () async {
     expect(await LinuxDevices(
-      platform: mockLinuxPlatform,
+      platform: linux,
       featureFlags: TestFeatureFlags(isLinuxEnabled: true),
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
     ).devices, hasLength(1));
   });
 
   testWithoutContext('LinuxDevice: discoverDevices', () async {
     // Timeout ignored.
     final List<Device> devices = await LinuxDevices(
-      platform: mockLinuxPlatform,
+      platform: linux,
       featureFlags: TestFeatureFlags(isLinuxEnabled: true),
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
     ).discoverDevices(timeout: const Duration(seconds: 10));
     expect(devices, hasLength(1));
   });
@@ -78,7 +93,10 @@ void main() {
     globals.fs.directory('linux').createSync();
     final FlutterProject flutterProject = FlutterProject.current();
 
-    expect(LinuxDevice().isSupportedForProject(flutterProject), true);
+    expect(LinuxDevice(
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
+    ).isSupportedForProject(flutterProject), true);
   }, overrides: <Type, Generator>{
     FileSystem: () => MemoryFileSystem(),
     ProcessManager: () => FakeProcessManager.any(),
@@ -89,7 +107,10 @@ void main() {
     globals.fs.file('.packages').createSync();
     final FlutterProject flutterProject = FlutterProject.current();
 
-    expect(LinuxDevice().isSupportedForProject(flutterProject), false);
+    expect(LinuxDevice(
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
+    ).isSupportedForProject(flutterProject), false);
   }, overrides: <Type, Generator>{
     FileSystem: () => MemoryFileSystem(),
     ProcessManager: () => FakeProcessManager.any(),
@@ -97,6 +118,10 @@ void main() {
 
   testUsingContext('LinuxDevice.executablePathForDevice uses the correct package executable', () async {
     final MockLinuxApp mockApp = MockLinuxApp();
+    final LinuxDevice device = LinuxDevice(
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.any(),
+    );
     const String debugPath = 'debug/executable';
     const String profilePath = 'profile/executable';
     const String releasePath = 'release/executable';
@@ -104,15 +129,13 @@ void main() {
     when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath);
     when(mockApp.executable(BuildMode.release)).thenReturn(releasePath);
 
-    expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath);
-    expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath);
-    expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath);
+    expect(device.executablePathForDevice(mockApp, BuildMode.debug), debugPath);
+    expect(device.executablePathForDevice(mockApp, BuildMode.profile), profilePath);
+    expect(device.executablePathForDevice(mockApp, BuildMode.release), releasePath);
   }, overrides: <Type, Generator>{
     FileSystem: () => MemoryFileSystem(),
     ProcessManager: () => FakeProcessManager.any(),
   });
 }
 
-class MockPlatform extends Mock implements Platform {}
-
 class MockLinuxApp extends Mock implements LinuxApp {}

packages/flutter_tools/test/general.shard/macos/macos_workflow_test.dart

@@ -3,60 +3,53 @@
 // found in the LICENSE file.
 
 import 'package:flutter_tools/src/base/platform.dart';
-import 'package:flutter_tools/src/features.dart';
 import 'package:flutter_tools/src/macos/macos_workflow.dart';
-import 'package:mockito/mockito.dart';
-import 'package:process/process.dart';
 
 import '../../src/common.dart';
-import '../../src/context.dart';
 import '../../src/testbed.dart';
 
+final FakePlatform macOS = FakePlatform(
+  operatingSystem: 'macos',
+);
+
+final FakePlatform linux = FakePlatform(
+  operatingSystem: 'linux',
+);
+
 void main() {
-  MockPlatform mac;
-  MockPlatform notMac;
-  Testbed testbed;
-
-  setUp(() {
-    mac = MockPlatform();
-    notMac = MockPlatform();
-    when(mac.isMacOS).thenReturn(true);
-    when(notMac.isMacOS).thenReturn(false);
-    testbed = Testbed(overrides: <Type, Generator>{
-      Platform: () => mac,
-      FeatureFlags: () => TestFeatureFlags(isMacOSEnabled: true),
-    });
-  });
+  testWithoutContext('Applies to macOS platform', () {
+    final MacOSWorkflow macOSWorkflow = MacOSWorkflow(
+      platform: macOS,
+      featureFlags: TestFeatureFlags(isMacOSEnabled: true),
+    );
 
-  test('Applies to macOS platform', () => testbed.run(() {
     expect(macOSWorkflow.appliesToHostPlatform, true);
     expect(macOSWorkflow.canListDevices, true);
     expect(macOSWorkflow.canLaunchDevices, true);
     expect(macOSWorkflow.canListEmulators, false);
-  }));
+  });
+
+  testWithoutContext('Does not apply to non-macOS platform', () {
+    final MacOSWorkflow macOSWorkflow = MacOSWorkflow(
+      platform: linux,
+      featureFlags: TestFeatureFlags(isMacOSEnabled: true),
+    );
 
-  test('Does not apply to non-macOS platform', () => testbed.run(() {
     expect(macOSWorkflow.appliesToHostPlatform, false);
     expect(macOSWorkflow.canListDevices, false);
     expect(macOSWorkflow.canLaunchDevices, false);
     expect(macOSWorkflow.canListEmulators, false);
-  }, overrides: <Type, Generator>{
-    Platform: () => notMac,
-  }));
+  });
+
+  testWithoutContext('Does not apply when feature is disabled', () {
+    final MacOSWorkflow macOSWorkflow = MacOSWorkflow(
+      platform: macOS,
+      featureFlags: TestFeatureFlags(isMacOSEnabled: false),
+    );
 
-  test('Does not apply when feature is disabled', () => testbed.run(() {
     expect(macOSWorkflow.appliesToHostPlatform, false);
     expect(macOSWorkflow.canListDevices, false);
     expect(macOSWorkflow.canLaunchDevices, false);
     expect(macOSWorkflow.canListEmulators, false);
-  }, overrides: <Type, Generator>{
-    FeatureFlags: () => TestFeatureFlags(isMacOSEnabled: false),
-  }));
-}
-
-class MockPlatform extends Mock implements Platform {
-  @override
-  Map<String, String> environment = <String, String>{};
+  });
 }
-
-class MockProcessManager extends Mock implements ProcessManager {}