Skip to content

[flutter_tools] return terminal to echo and line mode before exiting resident_runner #105283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christopherfujino merged 1 commit into from
Jun 6, 2022
Merged

[flutter_tools] return terminal to echo and line mode before exiting resident_runner #105283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/flutter_tools/lib/src/commands/run.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,10 @@ class RunCommand extends RunCommandBase {
throwToolExit('Lost connection to device.');
}
rethrow;
} finally {
// However we exited from the runner, ensure the terminal has line mode
// and echo mode enabled before we return the user to the shell.
globals.terminal.singleCharMode = false;
Copy link
Member

@zanderso zanderso Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the resident runner modify any other modes that should be reset?

Copy link
Contributor Author

@christopherfujino christopherfujino Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is actually a convenience setter around two modes:

    if (value) {
      stdin.echoMode = false;
      stdin.lineMode = false;
    } else {
      stdin.lineMode = true;
      stdin.echoMode = true;
    }

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/base/terminal.dart#L288

I'll take one more pass at the code to check for anything else suspicious, however.

}
return FlutterCommandResult(
ExitStatus.success,
Expand Down
52 changes: 48 additions & 4 deletions packages/flutter_tools/test/commands.shard/hermetic/run_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/base/user_messages.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/cache.dart';
Expand All @@ -47,14 +48,14 @@ import '../../src/fakes.dart';
import '../../src/test_flutter_command_runner.dart';

void main() {
setUpAll(() {
Cache.disableLocking();
});

group('run', () {
FakeDeviceManager mockDeviceManager;
FileSystem fileSystem;

setUpAll(() {
Cache.disableLocking();
});

setUp(() {
mockDeviceManager = FakeDeviceManager();
fileSystem = MemoryFileSystem.test();
Expand Down Expand Up @@ -657,6 +658,35 @@ void main() {
});
});

group('terminal', () {
FakeAnsiTerminal fakeTerminal;

setUp(() {
fakeTerminal = FakeAnsiTerminal();
});

testUsingContext('Flutter run sets terminal singleCharMode to false on exit', () async {
final FakeResidentRunner residentRunner = FakeResidentRunner();
final TestRunCommandWithFakeResidentRunner command = TestRunCommandWithFakeResidentRunner();
command.fakeResidentRunner = residentRunner;

await createTestCommandRunner(command).run(<String>[
'run',
'--no-pub',
]);
// The sync completer where we initially set `terminal.singleCharMode` to
// `true` does not execute in unit tests, so explicitly check the
// `setSingleCharModeHistory` that the finally block ran, setting this
// back to `false`.
expect(fakeTerminal.setSingleCharModeHistory, contains(false));
}, overrides: <Type, Generator>{
AnsiTerminal: () => fakeTerminal,
Cache: () => Cache.test(processManager: FakeProcessManager.any()),
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
});
});

testUsingContext('Flutter run catches service has disappear errors and throws a tool exit', () async {
final FakeResidentRunner residentRunner = FakeResidentRunner();
residentRunner.rpcError = RPCError('flutter._listViews', RPCErrorCodes.kServiceDisappeared, '');
Expand Down Expand Up @@ -1019,3 +1049,17 @@ class CapturingAppDomain extends AppDomain {
throwToolExit('');
}
}

class FakeAnsiTerminal extends Fake implements AnsiTerminal {
@override
bool usesTerminalUi = false;

/// A list of all the calls to the [singleCharMode] setter.
List<bool> setSingleCharModeHistory = <bool>[];

@override
set singleCharMode(bool value) => setSingleCharModeHistory.add(value);

@override
bool get singleCharMode => setSingleCharModeHistory.last;
}