[flutter_tools] split logger test and create log reader testing interface (#52934)

packages/flutter_tools/lib/src/ios/devices.dart

@@ -371,7 +371,11 @@ class IOSDevice extends Device {
   @override
   DeviceLogReader getLogReader({ IOSApp app }) {
     _logReaders ??= <IOSApp, DeviceLogReader>{};
-    return _logReaders.putIfAbsent(app, () => IOSDeviceLogReader(this, app));
+    return _logReaders.putIfAbsent(app, () => IOSDeviceLogReader.create(
+      device: this,
+      app: app,
+      iMobileDevice: globals.iMobileDevice,
+    ));
   }
 
   @visibleForTesting
@@ -481,7 +485,13 @@ String decodeSyslog(String line) {
 
 @visibleForTesting
 class IOSDeviceLogReader extends DeviceLogReader {
-  IOSDeviceLogReader(this.device, IOSApp app) {
+  IOSDeviceLogReader._(
+    this._iMobileDevice,
+    this._majorSdkVersion,
+    this._deviceId,
+    this.name,
+    String appName,
+  ) {
     _linesController = StreamController<String>.broadcast(
       onListen: _listenToSysLog,
       onCancel: dispose,
@@ -491,7 +501,6 @@ class IOSDeviceLogReader extends DeviceLogReader {
     //
     // iOS 9 format:  Runner[297] <Notice>:
     // iOS 10 format: Runner(Flutter)[297] <Notice>:
-    final String appName = app == null ? '' : app.name.replaceAll('.app', '');
     _runnerLineRegex = RegExp(appName + r'(\(Flutter\))?\[[\d]+\] <[A-Za-z]+>: ');
     // Similar to above, but allows ~arbitrary components instead of "Runner"
     // and "Flutter". The regex tries to strike a balance between not producing
@@ -500,7 +509,36 @@ class IOSDeviceLogReader extends DeviceLogReader {
     _loggingSubscriptions = <StreamSubscription<ServiceEvent>>[];
   }
 
-  final IOSDevice device;
+  /// Create a new [IOSDeviceLogReader].
+  factory IOSDeviceLogReader.create({
+    @required IOSDevice device,
+    @required IOSApp app,
+    @required IMobileDevice iMobileDevice,
+  }) {
+    final String appName = app == null ? '' : app.name.replaceAll('.app', '');
+    return IOSDeviceLogReader._(
+      iMobileDevice,
+      device.majorSdkVersion,
+      device.id,
+      device.name,
+      appName,
+    );
+  }
+
+  /// Create an [IOSDeviceLogReader] for testing.
+  factory IOSDeviceLogReader.test({
+    @required IMobileDevice iMobileDevice,
+    bool useSyslog = true,
+  }) {
+    return IOSDeviceLogReader._(
+      iMobileDevice, useSyslog ? 12 : 13, '1234', 'test', 'Runner');
+  }
+
+  @override
+  final String name;
+  final int _majorSdkVersion;
+  final String _deviceId;
+  final IMobileDevice _iMobileDevice;
 
   // Matches a syslog line from the runner.
   RegExp _runnerLineRegex;
@@ -513,9 +551,6 @@ class IOSDeviceLogReader extends DeviceLogReader {
   @override
   Stream<String> get logLines => _linesController.stream;
 
-  @override
-  String get name => device.name;
-
   @override
   VMService get connectedVMService => _connectedVMService;
   VMService _connectedVMService;
@@ -529,7 +564,7 @@ class IOSDeviceLogReader extends DeviceLogReader {
   static const int _minimumUniversalLoggingSdkVersion = 13;
 
   Future<void> _listenToUnifiedLoggingEvents(VMService connectedVmService) async {
-    if (device.majorSdkVersion < _minimumUniversalLoggingSdkVersion) {
+    if (_majorSdkVersion < _minimumUniversalLoggingSdkVersion) {
       return;
     }
     // The VM service will not publish logging events unless the debug stream is being listened to.
@@ -545,10 +580,10 @@ class IOSDeviceLogReader extends DeviceLogReader {
 
   void _listenToSysLog () {
     // syslog is not written on iOS 13+.
-    if (device.majorSdkVersion >= _minimumUniversalLoggingSdkVersion) {
+    if (_majorSdkVersion >= _minimumUniversalLoggingSdkVersion) {
       return;
     }
-    globals.iMobileDevice.startLogger(device.id).then<void>((Process process) {
+    _iMobileDevice.startLogger(_deviceId).then<void>((Process process) {
       process.stdout.transform<String>(utf8.decoder).transform<String>(const LineSplitter()).listen(_newSyslogLineHandler());
       process.stderr.transform<String>(utf8.decoder).transform<String>(const LineSplitter()).listen(_newSyslogLineHandler());
       process.exitCode.whenComplete(() {

packages/flutter_tools/test/general.shard/ios/devices_test.dart

@@ -250,7 +250,11 @@ void main() {
           IOSDevice device,
           IOSApp appPackage,
           Process process) {
-        final IOSDeviceLogReader logReader = IOSDeviceLogReader(device, appPackage);
+        final IOSDeviceLogReader logReader = IOSDeviceLogReader.create(
+          device: device,
+          app: appPackage,
+          iMobileDevice: null, // not used by this test.
+        );
         logReader.idevicesyslogProcess = process;
         return logReader;
       }
@@ -669,122 +673,6 @@ void main() {
       expect(diagnostics.first, 'Generic pairing error');
     });
   });
-
-  group('decodeSyslog', () {
-    testWithoutContext('decodes a syslog-encoded line', () {
-      final String decoded = decodeSyslog(r'I \M-b\M^]\M-$\M-o\M-8\M^O syslog \M-B\M-/\134_(\M-c\M^C\M^D)_/\M-B\M-/ \M-l\M^F\240!');
-      expect(decoded, r'I ❤️ syslog ¯\_(ツ)_/¯ 솠!');
-    });
-
-    testWithoutContext('passes through un-decodeable lines as-is', () {
-      final String decoded = decodeSyslog(r'I \M-b\M^O syslog!');
-      expect(decoded, r'I \M-b\M^O syslog!');
-    });
-  });
-
-  group('logging', () {
-    MockIMobileDevice mockIMobileDevice;
-    MockIosProject mockIosProject;
-    MockArtifacts mockArtifacts;
-    MockCache mockCache;
-    MockFileSystem mockFileSystem;
-    FakeProcessManager fakeProcessManager;
-    Logger logger;
-    IOSDeploy iosDeploy;
-
-    setUp(() {
-      mockIMobileDevice = MockIMobileDevice();
-      mockIosProject = MockIosProject();
-      mockArtifacts = MockArtifacts();
-      mockCache = MockCache();
-      logger = BufferLogger.test();
-      mockFileSystem = MockFileSystem();
-      fakeProcessManager = FakeProcessManager.any();
-      iosDeploy = IOSDeploy(
-        artifacts: mockArtifacts,
-        cache: mockCache,
-        logger: logger,
-        platform: macPlatform,
-        processManager: fakeProcessManager,
-      );
-    });
-
-    testUsingContext('suppresses non-Flutter lines from output', () async {
-      when(mockIMobileDevice.startLogger('123456')).thenAnswer((Invocation invocation) {
-        final Process mockProcess = MockProcess(
-          stdout: Stream<List<int>>.fromIterable(<List<int>>['''
-Runner(Flutter)[297] <Notice>: A is for ari
-Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt MobileGestaltSupport.m:153: pid 123 (Runner) does not have sandbox access for frZQaeyWLUvLjeuEK43hmg and IS NOT appropriately entitled
-Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt MobileGestalt.c:550: no access to InverseDeviceID (see <rdar://problem/11744455>)
-Runner(Flutter)[297] <Notice>: I is for ichigo
-Runner(UIKit)[297] <Notice>: E is for enpitsu"
-'''.codeUnits])
-        );
-        return Future<Process>.value(mockProcess);
-      });
-
-      final IOSDevice device = IOSDevice(
-        '123456',
-        name: 'iPhone 1',
-        sdkVersion: '10.3',
-        cpuArchitecture: DarwinArch.arm64,
-        artifacts: mockArtifacts,
-        iosDeploy: iosDeploy,
-        logger: logger,
-        platform: macPlatform,
-        fileSystem: mockFileSystem,
-      );
-      final DeviceLogReader logReader = device.getLogReader(
-        app: await BuildableIOSApp.fromProject(mockIosProject),
-      );
-
-      final List<String> lines = await logReader.logLines.toList();
-      expect(lines, <String>['A is for ari', 'I is for ichigo']);
-    }, overrides: <Type, Generator>{
-      IMobileDevice: () => mockIMobileDevice,
-    });
-
-    testUsingContext('includes multi-line Flutter logs in the output', () async {
-      when(mockIMobileDevice.startLogger('123456')).thenAnswer((Invocation invocation) {
-        final Process mockProcess = MockProcess(
-          stdout: Stream<List<int>>.fromIterable(<List<int>>['''
-Runner(Flutter)[297] <Notice>: This is a multi-line message,
-  with another Flutter message following it.
-Runner(Flutter)[297] <Notice>: This is a multi-line message,
-  with a non-Flutter log message following it.
-Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt
-'''.codeUnits]),
-        );
-        return Future<Process>.value(mockProcess);
-      });
-
-      final IOSDevice device = IOSDevice(
-        '123456',
-        name: 'iPhone 1',
-        sdkVersion: '10.3',
-        cpuArchitecture: DarwinArch.arm64,
-        artifacts: mockArtifacts,
-        iosDeploy: iosDeploy,
-        logger: logger,
-        platform: macPlatform,
-        fileSystem: mockFileSystem,
-      );
-      final DeviceLogReader logReader = device.getLogReader(
-        app: await BuildableIOSApp.fromProject(mockIosProject),
-      );
-
-      final List<String> lines = await logReader.logLines.toList();
-      expect(lines, <String>[
-        'This is a multi-line message,',
-        '  with another Flutter message following it.',
-        'This is a multi-line message,',
-        '  with a non-Flutter log message following it.',
-      ]);
-      expect(device.category, Category.mobile);
-    }, overrides: <Type, Generator>{
-      IMobileDevice: () => mockIMobileDevice,
-    });
-  });
 }
 
 class AbsoluteBuildableIOSApp extends BuildableIOSApp {

packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart

+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_tools/src/artifacts.dart';
+import 'package:flutter_tools/src/build_info.dart';
+import 'package:flutter_tools/src/device.dart';
+import 'package:flutter_tools/src/ios/devices.dart';
+import 'package:flutter_tools/src/ios/mac.dart';
+import 'package:mockito/mockito.dart';
+
+import '../../src/common.dart';
+import '../../src/context.dart';
+
+void main() {
+  FakeProcessManager processManager;
+  MockArtifacts artifacts;
+
+  setUp(() {
+    processManager = FakeProcessManager.list(<FakeCommand>[]);
+    artifacts = MockArtifacts();
+    when(artifacts.getArtifactPath(Artifact.idevicesyslog, platform: TargetPlatform.ios))
+      .thenReturn('idevice-syslog');
+  });
+
+  testWithoutContext('decodeSyslog decodes a syslog-encoded line', () {
+    final String decoded = decodeSyslog(
+      r'I \M-b\M^]\M-$\M-o\M-8\M^O syslog \M-B\M-/\'
+      r'134_(\M-c\M^C\M^D)_/\M-B\M-/ \M-l\M^F\240!');
+
+    expect(decoded, r'I ❤️ syslog ¯\_(ツ)_/¯ 솠!');
+  });
+
+  testWithoutContext('decodeSyslog passes through un-decodeable lines as-is', () {
+    final String decoded = decodeSyslog(r'I \M-b\M^O syslog!');
+
+    expect(decoded, r'I \M-b\M^O syslog!');
+  });
+
+  // IMobileDevice uses context.
+  testUsingContext('IOSDeviceLogReader suppresses non-Flutter lines from output with syslog', () async {
+    processManager.addCommand(
+      const FakeCommand(
+        command: <String>[
+          'idevice-syslog', '-u', '1234',
+        ],
+        stdout: '''
+Runner(Flutter)[297] <Notice>: A is for ari
+Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt MobileGestaltSupport.m:153: pid 123 (Runner) does not have sandbox access for frZQaeyWLUvLjeuEK43hmg and IS NOT appropriately entitled
+Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt MobileGestalt.c:550: no access to InverseDeviceID (see <rdar://problem/11744455>)
+Runner(Flutter)[297] <Notice>: I is for ichigo
+Runner(UIKit)[297] <Notice>: E is for enpitsu"
+'''
+      ),
+    );
+    final DeviceLogReader logReader = IOSDeviceLogReader.test(
+      iMobileDevice: IMobileDevice(),
+    );
+    final List<String> lines = await logReader.logLines.toList();
+
+    expect(lines, <String>['A is for ari', 'I is for ichigo']);
+  }, overrides: <Type, Generator>{
+    ProcessManager: () => processManager,
+    Artifacts: () => artifacts,
+  });
+
+  // IMobileDevice uses context.
+  testUsingContext('IOSDeviceLogReader includes multi-line Flutter logs in the output with syslog', () async {
+    processManager.addCommand(
+      const FakeCommand(
+        command: <String>[
+          'idevice-syslog', '-u', '1234',
+        ],
+        stdout: '''
+Runner(Flutter)[297] <Notice>: This is a multi-line message,
+  with another Flutter message following it.
+Runner(Flutter)[297] <Notice>: This is a multi-line message,
+  with a non-Flutter log message following it.
+Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt
+'''
+      ),
+    );
+    final DeviceLogReader logReader = IOSDeviceLogReader.test(
+      iMobileDevice: IMobileDevice()
+    );
+    final List<String> lines = await logReader.logLines.toList();
+
+    expect(lines, <String>[
+      'This is a multi-line message,',
+      '  with another Flutter message following it.',
+      'This is a multi-line message,',
+      '  with a non-Flutter log message following it.',
+    ]);
+  }, overrides: <Type, Generator>{
+    ProcessManager: () => processManager,
+    Artifacts: () => artifacts,
+  });
+
+  // IMobileDevice uses context.
+  testUsingContext('includes multi-line Flutter logs in the output', () async {
+    processManager.addCommand(
+      const FakeCommand(
+        command: <String>[
+          'idevice-syslog', '-u', '1234',
+        ],
+        stdout: '''
+Runner(Flutter)[297] <Notice>: This is a multi-line message,
+  with another Flutter message following it.
+Runner(Flutter)[297] <Notice>: This is a multi-line message,
+  with a non-Flutter log message following it.
+Runner(libsystem_asl.dylib)[297] <Notice>: libMobileGestalt
+''',
+      ),
+    );
+
+    final DeviceLogReader logReader = IOSDeviceLogReader.test(
+      iMobileDevice: IMobileDevice()
+    );
+    final List<String> lines = await logReader.logLines.toList();
+
+    expect(lines, <String>[
+      'This is a multi-line message,',
+      '  with another Flutter message following it.',
+      'This is a multi-line message,',
+      '  with a non-Flutter log message following it.',
+    ]);
+  }, overrides: <Type, Generator>{
+    ProcessManager: () => processManager,
+    Artifacts: () => artifacts,
+  });
+}
+
+class MockArtifacts extends Mock implements Artifacts {}