Unverified Commit 4c978efe authored by Christopher Fujino's avatar Christopher Fujino Committed by GitHub

[flutter_tools] Fix bad state future already completed in flutter logs (#138517)

Fixes https://github.com/flutter/flutter/issues/138436
parent d92d2c12
...@@ -221,7 +221,10 @@ List<FlutterCommand> generateCommands({ ...@@ -221,7 +221,10 @@ List<FlutterCommand> generateCommands({
InstallCommand( InstallCommand(
verboseHelp: verboseHelp, verboseHelp: verboseHelp,
), ),
LogsCommand(), LogsCommand(
sigint: ProcessSignal.sigint,
sigterm: ProcessSignal.sigterm,
),
MakeHostAppEditableCommand(), MakeHostAppEditableCommand(),
PackagesCommand(), PackagesCommand(),
PrecacheCommand( PrecacheCommand(
......
...@@ -12,7 +12,10 @@ import '../globals.dart' as globals; ...@@ -12,7 +12,10 @@ import '../globals.dart' as globals;
import '../runner/flutter_command.dart'; import '../runner/flutter_command.dart';
class LogsCommand extends FlutterCommand { class LogsCommand extends FlutterCommand {
LogsCommand() { LogsCommand({
required this.sigint,
required this.sigterm,
}) {
argParser.addFlag('clear', argParser.addFlag('clear',
negatable: false, negatable: false,
abbr: 'c', abbr: 'c',
...@@ -38,6 +41,8 @@ class LogsCommand extends FlutterCommand { ...@@ -38,6 +41,8 @@ class LogsCommand extends FlutterCommand {
Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{}; Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{};
Device? device; Device? device;
final ProcessSignal sigint;
final ProcessSignal sigterm;
@override @override
Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) async { Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) async {
...@@ -65,26 +70,31 @@ class LogsCommand extends FlutterCommand { ...@@ -65,26 +70,31 @@ class LogsCommand extends FlutterCommand {
final Completer<int> exitCompleter = Completer<int>(); final Completer<int> exitCompleter = Completer<int>();
// First check if we already completed by another branch before completing
// with [exitCode].
void maybeComplete([int exitCode = 0]) {
if (exitCompleter.isCompleted) {
return;
}
exitCompleter.complete(exitCode);
}
// Start reading. // Start reading.
final StreamSubscription<String> subscription = logReader.logLines.listen( final StreamSubscription<String> subscription = logReader.logLines.listen(
(String message) => globals.printStatus(message, wrap: false), (String message) => globals.printStatus(message, wrap: false),
onDone: () { onDone: () => maybeComplete(),
exitCompleter.complete(0); onError: (dynamic error) => maybeComplete(error is int ? error : 1),
},
onError: (dynamic error) {
exitCompleter.complete(error is int ? error : 1);
},
); );
// When terminating, close down the log reader. // When terminating, close down the log reader.
ProcessSignal.sigint.watch().listen((ProcessSignal signal) { sigint.watch().listen((ProcessSignal signal) {
subscription.cancel(); subscription.cancel();
maybeComplete();
globals.printStatus(''); globals.printStatus('');
exitCompleter.complete(0);
}); });
ProcessSignal.sigterm.watch().listen((ProcessSignal signal) { sigterm.watch().listen((ProcessSignal signal) {
subscription.cancel(); subscription.cancel();
exitCompleter.complete(0); maybeComplete();
}); });
// Wait for the log reader to be finished. // Wait for the log reader to be finished.
......
...@@ -2,17 +2,30 @@ ...@@ -2,17 +2,30 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async';
import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/commands/logs.dart'; import 'package:flutter_tools/src/commands/logs.dart';
import 'package:flutter_tools/src/device.dart';
import 'package:test/fake.dart';
import '../../src/context.dart'; import '../../src/context.dart';
import '../../src/fake_devices.dart';
import '../../src/test_flutter_command_runner.dart'; import '../../src/test_flutter_command_runner.dart';
void main() { void main() {
group('logs', () { group('logs', () {
late Platform platform;
late FakeDeviceManager deviceManager;
const String deviceId = 'abc123';
setUp(() { setUp(() {
Cache.disableLocking(); Cache.disableLocking();
deviceManager = FakeDeviceManager();
platform = FakePlatform();
}); });
tearDown(() { tearDown(() {
...@@ -20,11 +33,46 @@ void main() { ...@@ -20,11 +33,46 @@ void main() {
}); });
testUsingContext('fail with a bad device id', () async { testUsingContext('fail with a bad device id', () async {
final LogsCommand command = LogsCommand(); final LogsCommand command = LogsCommand(
sigterm: FakeProcessSignal(),
sigint: FakeProcessSignal(),
);
await expectLater( await expectLater(
() => createTestCommandRunner(command).run(<String>['-d', 'abc123', 'logs']), () => createTestCommandRunner(command).run(<String>['-d', 'abc123', 'logs']),
throwsA(isA<ToolExit>().having((ToolExit error) => error.exitCode, 'exitCode', anyOf(isNull, 1))), throwsA(isA<ToolExit>().having((ToolExit error) => error.exitCode, 'exitCode', anyOf(isNull, 1))),
); );
}); });
testUsingContext('does not try to complete exitCompleter multiple times', () async {
final FakeDevice fakeDevice = FakeDevice('phone', deviceId);
deviceManager.attachedDevices.add(fakeDevice);
final FakeProcessSignal termSignal = FakeProcessSignal();
final FakeProcessSignal intSignal = FakeProcessSignal();
final LogsCommand command = LogsCommand(
sigterm: termSignal,
sigint: intSignal,
);
final Future<void> commandFuture = createTestCommandRunner(command).run(<String>['-d', deviceId, 'logs']);
intSignal.send(1);
termSignal.send(1);
await pumpEventQueue(times: 5);
await commandFuture;
}, overrides: <Type, Generator>{
Platform: () => platform,
DeviceManager: () => deviceManager,
});
}); });
} }
class FakeProcessSignal extends Fake implements ProcessSignal {
late final StreamController<ProcessSignal> _controller = StreamController<ProcessSignal>();
@override
Stream<ProcessSignal> watch() => _controller.stream;
@override
bool send(int pid) {
_controller.add(this);
return true;
}
}
...@@ -118,6 +118,7 @@ class FakeDevice extends Device { ...@@ -118,6 +118,7 @@ class FakeDevice extends Device {
this.connectionInterface = DeviceConnectionInterface.attached, this.connectionInterface = DeviceConnectionInterface.attached,
PlatformType type = PlatformType.web, PlatformType type = PlatformType.web,
LaunchResult? launchResult, LaunchResult? launchResult,
this.deviceLogReader,
}) : _isSupported = isSupported, }) : _isSupported = isSupported,
_isSupportedForProject = isSupportedForProject, _isSupportedForProject = isSupportedForProject,
_launchResult = launchResult ?? LaunchResult.succeeded(), _launchResult = launchResult ?? LaunchResult.succeeded(),
...@@ -131,6 +132,7 @@ class FakeDevice extends Device { ...@@ -131,6 +132,7 @@ class FakeDevice extends Device {
final bool _isSupported; final bool _isSupported;
final bool _isSupportedForProject; final bool _isSupportedForProject;
final LaunchResult _launchResult; final LaunchResult _launchResult;
DeviceLogReader? deviceLogReader;
@override @override
final String name; final String name;
...@@ -183,6 +185,12 @@ class FakeDevice extends Device { ...@@ -183,6 +185,12 @@ class FakeDevice extends Device {
@override @override
Future<String> sdkNameAndVersion = Future<String>.value('Test SDK (1.2.3)'); Future<String> sdkNameAndVersion = Future<String>.value('Test SDK (1.2.3)');
@override
FutureOr<DeviceLogReader> getLogReader({
ApplicationPackage? app,
bool includePastLogs = false,
}) => deviceLogReader ?? FakeDeviceLogReader();
} }
/// Combines fake device with its canonical JSON representation. /// Combines fake device with its canonical JSON representation.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment