diff --git a/dev/devicelab/bin/tasks/flutter_attach_test_android.dart b/dev/devicelab/bin/tasks/flutter_attach_test_android.dart index 95bf4220b2..f44c6f121a 100644 --- a/dev/devicelab/bin/tasks/flutter_attach_test_android.dart +++ b/dev/devicelab/bin/tasks/flutter_attach_test_android.dart @@ -57,14 +57,10 @@ Future testReload(Process process, { Future Function() onListening } } await eventOrExit(listening.future); - await eventOrExit(ready.future).timeout(const Duration(seconds: 5), onTimeout: () { - // If it can't attach in 5 seconds, it's not capable of finding the - // observatory URL in the logs. - throw TaskResult.failure('Failed to attach to running Flutter process'); - }); + await eventOrExit(ready.future); if (exitCode != null) - throw TaskResult.failure('Failed to attach to test app; command unexpected exited, with exit code $exitCode.'); + throw 'Failed to attach to test app; command unexpected exited, with exit code $exitCode.'; process.stdin.write('r'); process.stdin.flush(); @@ -79,10 +75,10 @@ Future testReload(Process process, { Future Function() onListening } await process.exitCode; if (stderr.isNotEmpty) - throw TaskResult.failure('flutter attach had output on standard error.'); + throw 'flutter attach had output on standard error.'; if (exitCode != 0) - throw TaskResult.failure('exit code was not 0'); + throw 'exit code was not 0'; } void main() { @@ -124,7 +120,7 @@ void main() { // After the delay, force-stopping it shouldn't do anything, but doesn't hurt. await device.shellExec('am', ['force-stop', kAppId]); - String currentTime = (await device.shellEval('date', ['"+%F %R:%S.000"'])).trim(); + final String currentTime = (await device.shellEval('date', ['"+%F %R:%S.000"'])).trim(); print('Start time on device: $currentTime'); section('Relaunching application'); await device.shellExec('am', ['start', '-n', kActivityId]); @@ -144,26 +140,6 @@ void main() { ); await testReload(attachProcess); - // Give the device the time to really shut down the app. - await Future.delayed(const Duration(milliseconds: 200)); - // After the delay, force-stopping it shouldn't do anything, but doesn't hurt. - await device.shellExec('am', ['force-stop', kAppId]); - - section('Attaching after relaunching application'); - await device.shellExec('am', ['start', '-n', kActivityId]); - - // Let the application launch. Sync to the next time an observatory is ready. - currentTime = (await device.shellEval('date', ['"+%F %R:%S.000"'])).trim(); - await device.adb(['logcat', '-e', 'Observatory listening on http:', '-m', '1', '-T', currentTime]); - - // Attach again now that the VM is already running. - attachProcess = await startProcess( - path.join(flutterDirectory.path, 'bin', 'flutter'), - ['--suppress-analytics', 'attach', '-d', device.deviceId], - isBot: false, // we just want to test the output, not have any debugging info - ); - // Verify that it can discover the observatory port from past logs. - await testReload(attachProcess); } finally { section('Uninstalling'); await device.adb(['uninstall', kAppId]); diff --git a/packages/flutter_tools/doc/attach.md b/packages/flutter_tools/doc/attach.md index 0683d68555..fd7dcca1c8 100644 --- a/packages/flutter_tools/doc/attach.md +++ b/packages/flutter_tools/doc/attach.md @@ -9,17 +9,19 @@ without `flutter run` and provides a HotRunner (enabling hot reload/restart). There are four ways for the attach command to discover a running app: +1. If the app is already running and the observatory port is known, it can be +explicitly provided to attach via the command-line, e.g. `$ flutter attach +--debug-port 12345` +1. If the app is already running and the platform is iOS, attach can use mDNS +to lookup the observatory port via the application ID, with just `$ flutter +attach` 1. If the platform is Fuchsia the module name must be provided, e.g. `$ flutter attach --module=mod_name`. This can be called either before or after the application is started, attach will poll the device if it cannot immediately discover the port -1. On Android and iOS, just running `flutter attach` suffices. Flutter tools -will search for an already running Flutter app or module if available. -Otherwise, the tool will wait for the next Flutter app or module to launch -before attaching. -1. If the app or module is already running and the specific observatory port is -known, it can be explicitly provided to attach via the command-line, e.g. -`$ flutter attach --debug-port 12345` +1. On other platforms (i.e. Android), if the app is not yet running attach +will listen and wait for the app to be (manually) started with the default +command: `$ flutter attach` ## Source diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 0a3b9e680a..d1bd4e6138 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -213,7 +213,6 @@ class AndroidDevice extends Device { Future get apiVersion => _getProperty('ro.build.version.sdk'); AdbLogReader _logReader; - AdbLogReader _pastLogReader; _AndroidDevicePortForwarder _portForwarder; List adbCommandForDevice(List args) { @@ -674,23 +673,9 @@ class AndroidDevice extends Device { } @override - FutureOr getLogReader({ - AndroidApk app, - bool includePastLogs = false, - }) async { - // The Android log reader isn't app-specific. The `app` parameter isn't used. - if (includePastLogs) { - return _pastLogReader ??= await AdbLogReader.createLogReader( - this, - globals.processManager, - includePastLogs: true, - ); - } else { - return _logReader ??= await AdbLogReader.createLogReader( - this, - globals.processManager, - ); - } + FutureOr getLogReader({ AndroidApk app }) async { + // The Android log reader isn't app-specific. + return _logReader ??= await AdbLogReader.createLogReader(this, globals.processManager); } @override @@ -739,7 +724,6 @@ class AndroidDevice extends Device { @override Future dispose() async { _logReader?._stop(); - _pastLogReader?._stop(); await _portForwarder?.dispose(); } } @@ -917,9 +901,6 @@ class AdbLogReader extends DeviceLogReader { static Future createLogReader( AndroidDevice device, ProcessManager processManager, - { - bool includePastLogs = false, - } ) async { // logcat -T is not supported on Android releases before Lollipop. const int kLollipopVersionCode = 21; @@ -934,12 +915,7 @@ class AdbLogReader extends DeviceLogReader { 'logcat', '-v', 'time', - // If we include logs from the past, filter for 'flutter' logs only. - if (includePastLogs) ...[ - '-s', - 'flutter', - ] else if (apiVersion != null && apiVersion >= kLollipopVersionCode) ...[ - // Otherwise, filter for logs appearing past the present. + if (apiVersion != null && apiVersion >= kLollipopVersionCode) ...[ // Empty `-T` means the timestamp of the logcat command invocation. '-T', device.lastLogcatTimestamp ?? '', diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index e6d201ef63..ef68aa2048 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -6,7 +6,6 @@ import 'dart:async'; import 'package:meta/meta.dart'; -import '../android/android_device.dart'; import '../artifacts.dart'; import '../base/common.dart'; import '../base/context.dart'; @@ -246,9 +245,7 @@ class AttachCommand extends FlutterCommand { if (observatoryUri == null) { final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory( - // If it's an Android device, attaching relies on past log searching - // to find the service protocol. - await device.getLogReader(includePastLogs: device is AndroidDevice), + await device.getLogReader(), portForwarder: device.portForwarder, ipv6: ipv6, devicePort: deviceVmservicePort, diff --git a/packages/flutter_tools/lib/src/desktop_device.dart b/packages/flutter_tools/lib/src/desktop_device.dart index f5d4041ecf..39c54ed367 100644 --- a/packages/flutter_tools/lib/src/desktop_device.dart +++ b/packages/flutter_tools/lib/src/desktop_device.dart @@ -63,11 +63,7 @@ abstract class DesktopDevice extends Device { Future get sdkNameAndVersion async => globals.os.name; @override - DeviceLogReader getLogReader({ - ApplicationPackage app, - bool includePastLogs = false, - }) { - assert(!includePastLogs, 'Past log reading not supported on desktop.'); + DeviceLogReader getLogReader({ ApplicationPackage app }) { return _deviceLogReader; } diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 9ccb5a2868..df03261207 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -415,17 +415,9 @@ abstract class Device { Future get sdkNameAndVersion; /// Get a log reader for this device. - /// - /// If `app` is specified, this will return a log reader specific to that + /// If [app] is specified, this will return a log reader specific to that /// application. Otherwise, a global log reader will be returned. - /// - /// If `includePastLogs` is true and the device type supports it, the log - /// reader will also include log messages from before the invocation time. - /// Defaults to false. - FutureOr getLogReader({ - covariant ApplicationPackage app, - bool includePastLogs = false, - }); + FutureOr getLogReader({ covariant ApplicationPackage app }); /// Get the port forwarder for this device. DevicePortForwarder get portForwarder; diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart index 19551462a0..4c3d6ff906 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart @@ -508,13 +508,8 @@ class FuchsiaDevice extends Device { } @override - DeviceLogReader getLogReader({ - ApplicationPackage app, - bool includePastLogs = false, - }) { - assert(!includePastLogs, 'Past log reading not supported on Fuchsia.'); - return _logReader ??= _FuchsiaLogReader(this, app); - } + DeviceLogReader getLogReader({ApplicationPackage app}) => + _logReader ??= _FuchsiaLogReader(this, app); _FuchsiaLogReader _logReader; @override diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index faf121875f..20e8a319c2 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -372,11 +372,7 @@ class IOSDevice extends Device { Future get sdkNameAndVersion async => 'iOS $_sdkVersion'; @override - DeviceLogReader getLogReader({ - IOSApp app, - bool includePastLogs = false, - }) { - assert(!includePastLogs, 'Past log reading not supported on iOS devices.'); + DeviceLogReader getLogReader({ IOSApp app }) { _logReaders ??= {}; return _logReaders.putIfAbsent(app, () => IOSDeviceLogReader.create( device: this, diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 617e500d16..f6943660e3 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -519,12 +519,8 @@ class IOSSimulator extends Device { } @override - DeviceLogReader getLogReader({ - covariant IOSApp app, - bool includePastLogs = false, - }) { + DeviceLogReader getLogReader({ covariant IOSApp app }) { assert(app is IOSApp); - assert(!includePastLogs, 'Past log reading not supported on iOS simulators.'); _logReaders ??= {}; return _logReaders.putIfAbsent(app, () => _IOSSimulatorLogReader(this, app)); } diff --git a/packages/flutter_tools/lib/src/tester/flutter_tester.dart b/packages/flutter_tools/lib/src/tester/flutter_tester.dart index 1158886447..4cd7c9d7a0 100644 --- a/packages/flutter_tools/lib/src/tester/flutter_tester.dart +++ b/packages/flutter_tools/lib/src/tester/flutter_tester.dart @@ -79,12 +79,7 @@ class FlutterTesterDevice extends Device { _FlutterTesterDeviceLogReader(); @override - DeviceLogReader getLogReader({ - ApplicationPackage app, - bool includePastLogs = false, - }) { - return _logReader; - } + DeviceLogReader getLogReader({ ApplicationPackage app }) => _logReader; @override Future installApp(ApplicationPackage app) async => true; diff --git a/packages/flutter_tools/lib/src/web/web_device.dart b/packages/flutter_tools/lib/src/web/web_device.dart index 649afa70b8..a47cc157cc 100644 --- a/packages/flutter_tools/lib/src/web/web_device.dart +++ b/packages/flutter_tools/lib/src/web/web_device.dart @@ -60,10 +60,7 @@ class ChromeDevice extends Device { DeviceLogReader _logReader; @override - DeviceLogReader getLogReader({ - ApplicationPackage app, - bool includePastLogs = false, - }) { + DeviceLogReader getLogReader({ApplicationPackage app}) { return _logReader ??= NoOpDeviceLogReader(app?.name); } @@ -224,10 +221,7 @@ class WebServerDevice extends Device { DeviceLogReader _logReader; @override - DeviceLogReader getLogReader({ - ApplicationPackage app, - bool includePastLogs = false, - }) { + DeviceLogReader getLogReader({ApplicationPackage app}) { return _logReader ??= NoOpDeviceLogReader(app?.name); } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart index edef46efcc..9d271893a5 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart @@ -97,13 +97,12 @@ void main() { }); testUsingContext('finds observatory port and forwards', () async { - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) - .thenAnswer((_) { - // Now that the reader is used, start writing messages to it. - mockLogReader.addLine('Foo'); - mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); - return mockLogReader; - }); + when(device.getLogReader()).thenAnswer((_) { + // Now that the reader is used, start writing messages to it. + mockLogReader.addLine('Foo'); + mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); + return mockLogReader; + }); testDeviceManager.addDevice(device); final Completer completer = Completer(); final StreamSubscription loggerSubscription = logger.stream.listen((String message) { @@ -135,14 +134,12 @@ void main() { ..writeAsStringSync('{}'); when(device.name).thenReturn('MockAndroidDevice'); - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) + when(device.getLogReader()).thenReturn(mockLogReader); - .thenReturn(mockLogReader); + final Process dartProcess = MockProcess(); + final StreamController> compilerStdoutController = StreamController>(); - final Process dartProcess = MockProcess(); - final StreamController> compilerStdoutController = StreamController>(); - - when(dartProcess.stdout).thenAnswer((_) => compilerStdoutController.stream); + when(dartProcess.stdout).thenAnswer((_) => compilerStdoutController.stream); when(dartProcess.stderr) .thenAnswer((_) => Stream>.fromFuture(Future>.value(const []))); @@ -235,14 +232,13 @@ void main() { }); testUsingContext('Fails with tool exit on bad Observatory uri', () async { - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) - .thenAnswer((_) { - // Now that the reader is used, start writing messages to it. - mockLogReader.addLine('Foo'); - mockLogReader.addLine('Observatory listening on http:/:/127.0.0.1:$devicePort'); - mockLogReader.dispose(); - return mockLogReader; - }); + when(device.getLogReader()).thenAnswer((_) { + // Now that the reader is used, start writing messages to it. + mockLogReader.addLine('Foo'); + mockLogReader.addLine('Observatory listening on http:/:/127.0.0.1:$devicePort'); + mockLogReader.dispose(); + return mockLogReader; + }); testDeviceManager.addDevice(device); expect(createTestCommandRunner(AttachCommand()).run(['attach']), throwsToolExit()); @@ -253,13 +249,12 @@ void main() { }); testUsingContext('accepts filesystem parameters', () async { - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) - .thenAnswer((_) { - // Now that the reader is used, start writing messages to it. - mockLogReader.addLine('Foo'); - mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); - return mockLogReader; - }); + when(device.getLogReader()).thenAnswer((_) { + // Now that the reader is used, start writing messages to it. + mockLogReader.addLine('Foo'); + mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); + return mockLogReader; + }); testDeviceManager.addDevice(device); const String filesystemScheme = 'foo'; @@ -333,13 +328,12 @@ void main() { }); testUsingContext('exits when ipv6 is specified and debug-port is not', () async { - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) - .thenAnswer((_) { - // Now that the reader is used, start writing messages to it. - mockLogReader.addLine('Foo'); - mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); - return mockLogReader; - }); + when(device.getLogReader()).thenAnswer((_) { + // Now that the reader is used, start writing messages to it. + mockLogReader.addLine('Foo'); + mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); + return mockLogReader; + }); testDeviceManager.addDevice(device); final AttachCommand command = AttachCommand(); @@ -356,13 +350,12 @@ void main() { },); testUsingContext('exits when observatory-port is specified and debug-port is not', () async { - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) - .thenAnswer((_) { - // Now that the reader is used, start writing messages to it. - mockLogReader.addLine('Foo'); - mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); - return mockLogReader; - }); + when(device.getLogReader()).thenAnswer((_) { + // Now that the reader is used, start writing messages to it. + mockLogReader.addLine('Foo'); + mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); + return mockLogReader; + }); testDeviceManager.addDevice(device); final AttachCommand command = AttachCommand(); @@ -409,7 +402,7 @@ void main() { when(mockHotRunner.isWaitingForObservatory).thenReturn(false); testDeviceManager.addDevice(device); - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) + when(device.getLogReader()) .thenAnswer((_) { // Now that the reader is used, start writing messages to it. mockLogReader.addLine('Foo'); @@ -448,8 +441,7 @@ void main() { final MockHotRunner mockHotRunner = MockHotRunner(); final MockHotRunnerFactory mockHotRunnerFactory = MockHotRunnerFactory(); when(device.portForwarder).thenReturn(portForwarder); - when(device.getLogReader(includePastLogs: anyNamed('includePastLogs'))) - .thenAnswer((_) => mockLogReader); + when(device.getLogReader()).thenAnswer((_) => mockLogReader); when(portForwarder.forward(devicePort, hostPort: anyNamed('hostPort'))) .thenAnswer((_) async => hostPort); when(portForwarder.forwardedPorts) diff --git a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart index af6aaf2a2f..ded8172591 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart @@ -633,10 +633,7 @@ class FakeDevice extends Fake implements Device { Future get sdkNameAndVersion => Future.value(''); @override - DeviceLogReader getLogReader({ - ApplicationPackage app, - bool includePastLogs = false, - }) { + DeviceLogReader getLogReader({ ApplicationPackage app }) { return FakeDeviceLogReader(); } diff --git a/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart b/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart index 9bee6206c3..40bb838e10 100644 --- a/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart +++ b/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart @@ -79,30 +79,6 @@ void main() { expect(processManager.hasRemainingExpectations, false); }); - testWithoutContext('AdbLogReader calls adb logcat with expected flags when requesting past logs', () async { - final FakeProcessManager processManager = FakeProcessManager.list([ - const FakeCommand( - command: [ - 'adb', - '-s', - '1234', - 'logcat', - '-v', - 'time', - '-s', - 'flutter', - ], - ) - ]); - await AdbLogReader.createLogReader( - createMockDevice(null), - processManager, - includePastLogs: true, - ); - - expect(processManager.hasRemainingExpectations, false); - }); - testWithoutContext('AdbLogReader handles process early exit', () async { final FakeProcessManager processManager = FakeProcessManager.list([ FakeCommand( diff --git a/packages/flutter_tools/test/general.shard/android/android_device_test.dart b/packages/flutter_tools/test/general.shard/android/android_device_test.dart index 952eb502f6..b02630055e 100644 --- a/packages/flutter_tools/test/general.shard/android/android_device_test.dart +++ b/packages/flutter_tools/test/general.shard/android/android_device_test.dart @@ -468,48 +468,17 @@ flutter: }); }); - group('logcat', () { + group('lastLogcatTimestamp', () { final ProcessManager mockProcessManager = MockProcessManager(); final AndroidDevice device = AndroidDevice('1234'); - testUsingContext('lastLogcatTimestamp returns null if shell command failed', () async { + testUsingContext('returns null if shell command failed', () async { when(mockProcessManager.runSync(argThat(contains('logcat')))) .thenReturn(ProcessResult(0, 1, '', '')); expect(device.lastLogcatTimestamp, isNull); }, overrides: { ProcessManager: () => mockProcessManager, }); - - testUsingContext('AdbLogReaders for past+future and future logs are not the same', () async { - when(mockProcessManager.run( - argThat(contains('getprop')), - stderrEncoding: anyNamed('stderrEncoding'), - stdoutEncoding: anyNamed('stdoutEncoding'), - )).thenAnswer((_) { - final StringBuffer buf = StringBuffer() - ..writeln('[ro.build.version.sdk]: [23]'); - final ProcessResult result = ProcessResult(1, exitCode, buf.toString(), ''); - return Future.value(result); - }); - when(mockProcessManager.run( - argThat(contains('shell')), - stderrEncoding: anyNamed('stderrEncoding'), - stdoutEncoding: anyNamed('stdoutEncoding'), - )).thenAnswer((_) { - final StringBuffer buf = StringBuffer() - ..writeln('11-27 15:39:04.506'); - final ProcessResult result = ProcessResult(1, exitCode, buf.toString(), ''); - return Future.value(result); - }); - final DeviceLogReader pastLogReader = await device.getLogReader(includePastLogs: true); - final DeviceLogReader defaultLogReader = await device.getLogReader(); - expect(pastLogReader, isNot(equals(defaultLogReader))); - // Getting again is cached. - expect(pastLogReader, equals(await device.getLogReader(includePastLogs: true))); - expect(defaultLogReader, equals(await device.getLogReader())); - }, overrides: { - ProcessManager: () => mockProcessManager, - }); }); test('Can parse adb shell dumpsys info', () {