diff --git a/packages/flutter_tools/lib/src/base/logger.dart b/packages/flutter_tools/lib/src/base/logger.dart index f0c56f422cc..d27048bb3c6 100644 --- a/packages/flutter_tools/lib/src/base/logger.dart +++ b/packages/flutter_tools/lib/src/base/logger.dart @@ -173,6 +173,107 @@ abstract class Logger { void clear(); } +/// A [Logger] that forwards all methods to another one. +/// +/// Classes can derive from this to add functionality to an existing [Logger]. +class DelegatingLogger implements Logger { + @visibleForTesting + @protected + DelegatingLogger(this._delegate); + + final Logger _delegate; + + @override + bool get quiet => _delegate.quiet; + + @override + set quiet(bool value) => _delegate.quiet = value; + + @override + bool get hasTerminal => _delegate.hasTerminal; + + @override + Terminal get _terminal => _delegate._terminal; + + @override + OutputPreferences get _outputPreferences => _delegate._outputPreferences; + + @override + TimeoutConfiguration get _timeoutConfiguration => _delegate._timeoutConfiguration; + + @override + bool get isVerbose => _delegate.isVerbose; + + @override + void printError(String message, {StackTrace stackTrace, bool emphasis, TerminalColor color, int indent, int hangingIndent, bool wrap}) { + _delegate.printError( + message, + stackTrace: stackTrace, + emphasis: emphasis, + color: color, + indent: indent, + hangingIndent: hangingIndent, + wrap: wrap, + ); + } + + @override + void printStatus(String message, {bool emphasis, TerminalColor color, bool newline, int indent, int hangingIndent, bool wrap}) { + _delegate.printStatus(message, + emphasis: emphasis, + color: color, + newline: newline, + indent: indent, + hangingIndent: hangingIndent, + wrap: wrap, + ); + } + + @override + void printTrace(String message) { + _delegate.printTrace(message); + } + + @override + void sendEvent(String name, [Map args]) { + _delegate.sendEvent(name, args); + } + + @override + Status startProgress(String message, {Duration timeout, String progressId, bool multilineOutput = false, int progressIndicatorPadding = kDefaultStatusPadding}) { + return _delegate.startProgress(message, + timeout: timeout, + progressId: progressId, + multilineOutput: multilineOutput, + progressIndicatorPadding: progressIndicatorPadding, + ); + } + + @override + bool get supportsColor => _delegate.supportsColor; + + @override + void clear() => _delegate.clear(); +} + +/// If [logger] is a [DelegatingLogger], walks the delegate chain and returns +/// the first delegate with the matching type. +/// +/// Throws a [StateError] if no matching delegate is found. +@override +T asLogger(Logger logger) { + final Logger original = logger; + while (true) { + if (logger is T) { + return logger; + } else if (logger is DelegatingLogger) { + logger = (logger as DelegatingLogger)._delegate; + } else { + throw StateError('$original has no ancestor delegate of type $T'); + } + } +} + class StdoutLogger extends Logger { StdoutLogger({ @required Terminal terminal, @@ -508,27 +609,17 @@ class BufferLogger extends Logger { } } -class VerboseLogger extends Logger { - VerboseLogger(this.parent, { +class VerboseLogger extends DelegatingLogger { + VerboseLogger(Logger parent, { StopwatchFactory stopwatchFactory = const StopwatchFactory() }) : _stopwatch = stopwatchFactory.createStopwatch(), - _stopwatchFactory = stopwatchFactory { + _stopwatchFactory = stopwatchFactory, + super(parent) { _stopwatch.start(); } - final Logger parent; - final Stopwatch _stopwatch; - @override - Terminal get _terminal => parent._terminal; - - @override - OutputPreferences get _outputPreferences => parent._outputPreferences; - - @override - TimeoutConfiguration get _timeoutConfiguration => parent._timeoutConfiguration; - final StopwatchFactory _stopwatchFactory; @override @@ -635,28 +726,19 @@ class VerboseLogger extends Logger { final String indentMessage = message.replaceAll('\n', '\n$indent'); if (type == _LogType.error) { - parent.printError(prefix + _terminal.bolden(indentMessage)); + super.printError(prefix + _terminal.bolden(indentMessage)); if (stackTrace != null) { - parent.printError(indent + stackTrace.toString().replaceAll('\n', '\n$indent')); + super.printError(indent + stackTrace.toString().replaceAll('\n', '\n$indent')); } } else if (type == _LogType.status) { - parent.printStatus(prefix + _terminal.bolden(indentMessage)); + super.printStatus(prefix + _terminal.bolden(indentMessage)); } else { - parent.printStatus(prefix + indentMessage); + super.printStatus(prefix + indentMessage); } } @override void sendEvent(String name, [Map args]) { } - - @override - bool get supportsColor => parent.supportsColor; - - @override - bool get hasTerminal => parent.hasTerminal; - - @override - void clear() => parent.clear(); } enum _LogType { error, status, trace } diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 6d45a0a2076..dbfbcb6ab02 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -54,7 +54,7 @@ class DaemonCommand extends FlutterCommand { final Daemon daemon = Daemon( stdinCommandStream, stdoutCommandResponse, - notifyingLogger: globals.logger as NotifyingLogger, + notifyingLogger: asLogger(globals.logger), ); final int code = await daemon.onExit; if (code != 0) { @@ -525,7 +525,7 @@ class AppDomain extends Domain { enableHotReload, cwd, LaunchMode.run, - globals.logger as AppRunLogger, + asLogger(globals.logger), ); } @@ -960,15 +960,14 @@ dynamic _toJsonable(dynamic obj) { return '$obj'; } -class NotifyingLogger extends Logger { - NotifyingLogger({ @required this.verbose, this.parent }) { +class NotifyingLogger extends DelegatingLogger { + NotifyingLogger({ @required this.verbose, @required Logger parent }) : super(parent) { _messageController = StreamController.broadcast( onListen: _onListen, ); } final bool verbose; - final Logger parent; final List messageBuffer = []; StreamController _messageController; @@ -1012,7 +1011,7 @@ class NotifyingLogger extends Logger { if (!verbose) { return; } - parent?.printError(message); + super.printError(message); } @override @@ -1136,83 +1135,13 @@ class EmulatorDomain extends Domain { // // TODO(devoncarew): To simplify this code a bit, we could choose to specialize // this class into two, one for each of the above use cases. -class AppRunLogger extends Logger { - AppRunLogger({ this.parent }); +class AppRunLogger extends DelegatingLogger { + AppRunLogger({ @required Logger parent }) : super(parent); AppDomain domain; AppInstance app; - final Logger parent; int _nextProgressId = 0; - @override - void printError( - String message, { - StackTrace stackTrace, - bool emphasis, - TerminalColor color, - int indent, - int hangingIndent, - bool wrap, - }) { - if (parent != null) { - parent.printError( - message, - stackTrace: stackTrace, - emphasis: emphasis, - indent: indent, - hangingIndent: hangingIndent, - wrap: wrap, - ); - } else { - if (stackTrace != null) { - _sendLogEvent({ - 'log': message, - 'stackTrace': stackTrace.toString(), - 'error': true, - }); - } else { - _sendLogEvent({ - 'log': message, - 'error': true, - }); - } - } - } - - @override - void printStatus( - String message, { - bool emphasis = false, - TerminalColor color, - bool newline = true, - int indent, - int hangingIndent, - bool wrap, - }) { - if (parent != null) { - parent.printStatus( - message, - emphasis: emphasis, - color: color, - newline: newline, - indent: indent, - hangingIndent: hangingIndent, - wrap: wrap, - ); - } else { - _sendLogEvent({'log': message}); - } - } - - @override - void printTrace(String message) { - if (parent != null) { - parent.printTrace(message); - } else { - _sendLogEvent({'log': message, 'trace': true}); - } - } - Status _status; @override @@ -1249,14 +1178,6 @@ class AppRunLogger extends Logger { domain = null; } - void _sendLogEvent(Map event) { - if (domain == null) { - printStatus('event sent after app closed: $event'); - } else { - domain._sendAppEvent(app, 'log', event); - } - } - void _sendProgressEvent({ @required String eventId, @required String eventType, diff --git a/packages/flutter_tools/test/general.shard/base/logger_test.dart b/packages/flutter_tools/test/general.shard/base/logger_test.dart index 0237ff098ee..776f6fc0df8 100644 --- a/packages/flutter_tools/test/general.shard/base/logger_test.dart +++ b/packages/flutter_tools/test/general.shard/base/logger_test.dart @@ -11,6 +11,7 @@ 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/commands/daemon.dart'; +import 'package:matcher/matcher.dart'; import 'package:mockito/mockito.dart'; import 'package:fake_async/fake_async.dart'; @@ -73,6 +74,133 @@ void main() { ), isA()); }); + testWithoutContext('DelegatingLogger delegates', () { + final FakeLogger fakeLogger = FakeLogger(); + final DelegatingLogger delegatingLogger = DelegatingLogger(fakeLogger); + + expect( + () => delegatingLogger.quiet, + _throwsInvocationFor(() => fakeLogger.quiet), + ); + + expect( + () => delegatingLogger.quiet = true, + _throwsInvocationFor(() => fakeLogger.quiet = true), + ); + + expect( + () => delegatingLogger.hasTerminal, + _throwsInvocationFor(() => fakeLogger.hasTerminal), + ); + + expect( + () => delegatingLogger.isVerbose, + _throwsInvocationFor(() => fakeLogger.isVerbose), + ); + + const String message = 'message'; + final StackTrace stackTrace = StackTrace.current; + const bool emphasis = true; + const TerminalColor color = TerminalColor.cyan; + const int indent = 88; + const int hangingIndent = 52; + const bool wrap = true; + const bool newline = true; + expect( + () => delegatingLogger.printError(message, + stackTrace: stackTrace, + emphasis: emphasis, + color: color, + indent: indent, + hangingIndent: hangingIndent, + wrap: wrap, + ), + _throwsInvocationFor(() => fakeLogger.printError(message, + stackTrace: stackTrace, + emphasis: emphasis, + color: color, + indent: indent, + hangingIndent: hangingIndent, + wrap: wrap, + )), + ); + + expect( + () => delegatingLogger.printStatus(message, + emphasis: emphasis, + color: color, + newline: newline, + indent: indent, + hangingIndent: hangingIndent, + wrap: wrap, + ), + _throwsInvocationFor(() => fakeLogger.printStatus(message, + emphasis: emphasis, + color: color, + newline: newline, + indent: indent, + hangingIndent: hangingIndent, + wrap: wrap, + )), + ); + + expect( + () => delegatingLogger.printTrace(message), + _throwsInvocationFor(() => fakeLogger.printTrace(message)), + ); + + final Map eventArgs = {}; + expect( + () => delegatingLogger.sendEvent(message, eventArgs), + _throwsInvocationFor(() => fakeLogger.sendEvent(message, eventArgs)), + ); + + const Duration timeout = Duration(seconds: 12); + const String progressId = 'progressId'; + const bool multilineOutput = true; + const int progressIndicatorPadding = kDefaultStatusPadding * 2; + expect( + () => delegatingLogger.startProgress(message, + timeout: timeout, + progressId: progressId, + multilineOutput: multilineOutput, + progressIndicatorPadding: progressIndicatorPadding, + ), + _throwsInvocationFor(() => fakeLogger.startProgress(message, + timeout: timeout, + progressId: progressId, + multilineOutput: multilineOutput, + progressIndicatorPadding: progressIndicatorPadding, + )), + ); + + expect( + () => delegatingLogger.supportsColor, + _throwsInvocationFor(() => fakeLogger.supportsColor), + ); + + expect( + () => delegatingLogger.clear(), + _throwsInvocationFor(() => fakeLogger.clear()), + ); + }); + + testWithoutContext('asLogger finds the correct delegate', () async { + final FakeLogger fakeLogger = FakeLogger(); + final VerboseLogger verboseLogger = VerboseLogger(fakeLogger); + final NotifyingLogger notifyingLogger = + NotifyingLogger(verbose: true, parent: verboseLogger); + expect(asLogger(notifyingLogger), notifyingLogger); + expect(asLogger(notifyingLogger), notifyingLogger); + expect(asLogger(notifyingLogger), verboseLogger); + expect(asLogger(notifyingLogger), fakeLogger); + + expect( + () => asLogger(notifyingLogger), + throwsA(isA()), + ); + }); + group('AppContext', () { FakeStopwatch fakeStopWatch; @@ -1079,3 +1207,38 @@ class FakeStopwatchFactory implements StopwatchFactory { return stopwatch ?? FakeStopwatch(); } } + +/// A fake [Logger] that throws the [Invocation] for any method call. +class FakeLogger implements Logger { + @override + dynamic noSuchMethod(Invocation invocation) => throw invocation; +} + +/// Returns the [Invocation] thrown from a call to [FakeLogger]. +Invocation _invocationFor(dynamic Function() fakeCall) { + try { + fakeCall(); + } on Invocation catch (invocation) { + return invocation; + } + throw UnsupportedError('_invocationFor can be used only with Fake objects ' + 'that throw Invocations'); +} + +/// Returns a [Matcher] that matches against an expected [Invocation]. +Matcher _matchesInvocation(Invocation expected) { + return const TypeMatcher() + // Compare Symbol strings instead of comparing Symbols directly for a nicer failure message. + .having((Invocation actual) => actual.memberName.toString(), 'memberName', expected.memberName.toString()) + .having((Invocation actual) => actual.isGetter, 'isGetter', expected.isGetter) + .having((Invocation actual) => actual.isSetter, 'isSetter', expected.isSetter) + .having((Invocation actual) => actual.isMethod, 'isMethod', expected.isMethod) + .having((Invocation actual) => actual.typeArguments, 'typeArguments', expected.typeArguments) + .having((Invocation actual) => actual.positionalArguments, 'positionalArguments', expected.positionalArguments) + .having((Invocation actual) => actual.namedArguments, 'namedArguments', expected.namedArguments); +} + +/// Returns a [Matcher] that matches against an [Invocation] thrown from a call +/// to [FakeLogger]. +Matcher _throwsInvocationFor(dynamic Function() fakeCall) => + throwsA(_matchesInvocation(_invocationFor(fakeCall))); diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart index cd7dd0dbdd6..0cd96a641d0 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart @@ -70,9 +70,9 @@ void main() { test('Can successfully run and connect without vmservice', () => testbed.run(() async { _setupMocks(); - final DelegateLogger delegateLogger = globals.logger as DelegateLogger; + final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger; final MockStatus mockStatus = MockStatus(); - delegateLogger.status = mockStatus; + fakeStatusLogger.status = mockStatus; final Completer connectionInfoCompleter = Completer(); unawaited(residentWebRunner.run( connectionInfoCompleter: connectionInfoCompleter, @@ -83,7 +83,7 @@ void main() { verify(mockStatus.stop()).called(1); }, overrides: { BuildSystem: () => mockBuildSystem, - Logger: () => DelegateLogger(BufferLogger( + Logger: () => FakeStatusLogger(BufferLogger( terminal: AnsiTerminal( stdio: null, platform: const LocalPlatform(), diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart index 17873075c10..15c371f16ed 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart @@ -282,10 +282,10 @@ void main() { final ResidentRunner residentWebRunner = setUpResidentRunner(mockFlutterDevice); fakeVmServiceHost = FakeVmServiceHost(requests: kAttachExpectations.toList()); _setupMocks(); - final DelegateLogger delegateLogger = globals.logger as DelegateLogger; - final BufferLogger bufferLogger = delegateLogger.delegate as BufferLogger; + final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger; + final BufferLogger bufferLogger = asLogger(fakeStatusLogger); final MockStatus status = MockStatus(); - delegateLogger.status = status; + fakeStatusLogger.status = status; final Completer connectionInfoCompleter = Completer(); unawaited(residentWebRunner.run( connectionInfoCompleter: connectionInfoCompleter, @@ -297,7 +297,7 @@ void main() { expect(bufferLogger.statusText, contains('Debug service listening on ws://127.0.0.1/abcd/')); expect(debugConnectionInfo.wsUri.toString(), 'ws://127.0.0.1/abcd/'); }, overrides: { - Logger: () => DelegateLogger(BufferLogger.test()), + Logger: () => FakeStatusLogger(BufferLogger.test()), FileSystem: () => fileSystem, ProcessManager: () => processManager, Pub: () => MockPub(), @@ -1500,9 +1500,9 @@ void main() { when(chrome.chromeConnection).thenReturn(mockChromeConnection); chromiumLauncher.testLaunchChromium(chrome); - final DelegateLogger delegateLogger = globals.logger as DelegateLogger; + final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger; final MockStatus mockStatus = MockStatus(); - delegateLogger.status = mockStatus; + fakeStatusLogger.status = mockStatus; final ResidentWebRunner runner = DwdsWebRunnerFactory().createWebRunner( mockFlutterDevice, flutterProject: FlutterProject.current(), @@ -1519,7 +1519,7 @@ void main() { await connectionInfoCompleter.future; // Ensure we got the URL and that it was already launched. - expect((delegateLogger.delegate as BufferLogger).eventText, + expect(asLogger(fakeStatusLogger).eventText, contains(json.encode({ 'name': 'app.webLaunchUrl', 'args': { @@ -1530,7 +1530,7 @@ void main() { ))); expect(fakeVmServiceHost.hasRemainingExpectations, false); }, overrides: { - Logger: () => DelegateLogger(BufferLogger.test()), + Logger: () => FakeStatusLogger(BufferLogger.test()), FileSystem: () => fileSystem, ProcessManager: () => processManager, Pub: () => MockPub(), @@ -1547,9 +1547,9 @@ void main() { return Uri.parse('http://localhost:8765/app/'); }); - final DelegateLogger delegateLogger = globals.logger as DelegateLogger; + final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger; final MockStatus mockStatus = MockStatus(); - delegateLogger.status = mockStatus; + fakeStatusLogger.status = mockStatus; final ResidentWebRunner runner = DwdsWebRunnerFactory().createWebRunner( mockFlutterDevice, flutterProject: FlutterProject.current(), @@ -1566,7 +1566,7 @@ void main() { await connectionInfoCompleter.future; // Ensure we got the URL and that it was not already launched. - expect((delegateLogger.delegate as BufferLogger).eventText, + expect(asLogger(fakeStatusLogger).eventText, contains(json.encode({ 'name': 'app.webLaunchUrl', 'args': { @@ -1577,7 +1577,7 @@ void main() { ))); expect(fakeVmServiceHost.hasRemainingExpectations, false); }, overrides: { - Logger: () => DelegateLogger(BufferLogger.test()), + Logger: () => FakeStatusLogger(BufferLogger.test()), FileSystem: () => fileSystem, ProcessManager: () => processManager, Pub: () => MockPub(), @@ -1654,9 +1654,9 @@ void main() { final ResidentRunner residentWebRunner = setUpResidentRunner(mockFlutterDevice); fakeVmServiceHost = FakeVmServiceHost(requests: []); _setupMocks(); - final DelegateLogger delegateLogger = globals.logger as DelegateLogger; + final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger; final MockStatus mockStatus = MockStatus(); - delegateLogger.status = mockStatus; + fakeStatusLogger.status = mockStatus; when(mockWebDevFS.connect(any)).thenThrow(StateError('')); @@ -1664,7 +1664,7 @@ void main() { verify(mockStatus.stop()).called(1); expect(fakeVmServiceHost.hasRemainingExpectations, false); }, overrides: { - Logger: () => DelegateLogger(BufferLogger( + Logger: () => FakeStatusLogger(BufferLogger( terminal: AnsiTerminal( stdio: null, platform: const LocalPlatform(), diff --git a/packages/flutter_tools/test/src/testbed.dart b/packages/flutter_tools/test/src/testbed.dart index 4a4e2a9bfe8..4f3361bac05 100644 --- a/packages/flutter_tools/test/src/testbed.dart +++ b/packages/flutter_tools/test/src/testbed.dart @@ -783,68 +783,15 @@ class TestFeatureFlags implements FeatureFlags { } } -class DelegateLogger implements Logger { - DelegateLogger(this.delegate); +class FakeStatusLogger extends DelegatingLogger { + FakeStatusLogger(Logger delegate) : super(delegate); - final Logger delegate; Status status; - @override - bool get quiet => delegate.quiet; - - @override - set quiet(bool value) => delegate.quiet; - - @override - bool get hasTerminal => delegate.hasTerminal; - - @override - bool get isVerbose => delegate.isVerbose; - - @override - void printError(String message, {StackTrace stackTrace, bool emphasis, TerminalColor color, int indent, int hangingIndent, bool wrap}) { - delegate.printError( - message, - stackTrace: stackTrace, - emphasis: emphasis, - color: color, - indent: indent, - hangingIndent: hangingIndent, - wrap: wrap, - ); - } - - @override - void printStatus(String message, {bool emphasis, TerminalColor color, bool newline, int indent, int hangingIndent, bool wrap}) { - delegate.printStatus(message, - emphasis: emphasis, - color: color, - indent: indent, - hangingIndent: hangingIndent, - wrap: wrap, - ); - } - - @override - void printTrace(String message) { - delegate.printTrace(message); - } - - @override - void sendEvent(String name, [Map args]) { - delegate.sendEvent(name, args); - } - @override Status startProgress(String message, {Duration timeout, String progressId, bool multilineOutput = false, int progressIndicatorPadding = kDefaultStatusPadding}) { return status; } - - @override - bool get supportsColor => delegate.supportsColor; - - @override - void clear() => delegate.clear(); } /// An implementation of the Cache which does not download or require locking.