[flutter_tools] abstract logger construction (why can't I hold all these loggers) (#61201)
We have too many loggers, and the logger construction rules are too complicated to be untested. Capture these in a LoggerFactory and test that construction is correct.
diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart
index b0eb4ad..e69ba78 100644
--- a/packages/flutter_tools/lib/executable.dart
+++ b/packages/flutter_tools/lib/executable.dart
@@ -4,13 +4,17 @@
import 'dart:async';
+import 'package:meta/meta.dart';
+
import 'runner.dart' as runner;
import 'src/base/context.dart';
+import 'src/base/io.dart';
import 'src/base/logger.dart';
import 'src/base/template.dart';
// The build_runner code generation is provided here to make it easier to
// avoid introducing the dependency into google3. Not all build* packages
// are synced internally.
+import 'src/base/terminal.dart';
import 'src/build_runner/build_runner.dart';
import 'src/build_runner/mustache_template.dart';
import 'src/build_runner/resident_web_runner.dart';
@@ -135,36 +139,80 @@
WebRunnerFactory: () => DwdsWebRunnerFactory(),
// The mustache dependency is different in google3
TemplateRenderer: () => const MustacheTemplateRenderer(),
- if (daemon)
- Logger: () => NotifyingLogger(
- verbose: verbose,
- parent: VerboseLogger(StdoutLogger(
- timeoutConfiguration: timeoutConfiguration,
- stdio: globals.stdio,
- terminal: globals.terminal,
- outputPreferences: globals.outputPreferences,
- ),
- ))
- else if (runMachine && !verbose)
- Logger: () => AppRunLogger(parent: StdoutLogger(
- timeoutConfiguration: timeoutConfiguration,
- stdio: globals.stdio,
- terminal: globals.terminal,
+ Logger: () {
+ final LoggerFactory loggerFactory = LoggerFactory(
outputPreferences: globals.outputPreferences,
- ))
- else if (runMachine && verbose)
- Logger: () => AppRunLogger(parent: VerboseLogger(StdoutLogger(
- timeoutConfiguration: timeoutConfiguration,
- stdio: globals.stdio,
terminal: globals.terminal,
- outputPreferences: globals.outputPreferences,
- )))
- else if (verbose && !muteCommandLogging)
- Logger: () => VerboseLogger(StdoutLogger(
- timeoutConfiguration: timeoutConfiguration,
stdio: globals.stdio,
- terminal: globals.terminal,
- outputPreferences: globals.outputPreferences,
- ))
+ timeoutConfiguration: timeoutConfiguration,
+ );
+ return loggerFactory.createLogger(
+ daemon: daemon,
+ machine: runMachine,
+ verbose: verbose && !muteCommandLogging,
+ windows: globals.platform.isWindows,
+ );
+ }
});
}
+
+
+/// An abstraction for instantiation of the correct logger type.
+///
+/// Our logger class hierarchy and runtime requirements are overly complicated.
+class LoggerFactory {
+ LoggerFactory({
+ @required Terminal terminal,
+ @required Stdio stdio,
+ @required OutputPreferences outputPreferences,
+ @required TimeoutConfiguration timeoutConfiguration,
+ StopwatchFactory stopwatchFactory = const StopwatchFactory(),
+ }) : _terminal = terminal,
+ _stdio = stdio,
+ _timeoutConfiguration = timeoutConfiguration,
+ _stopwatchFactory = stopwatchFactory,
+ _outputPreferences = outputPreferences;
+
+ final Terminal _terminal;
+ final Stdio _stdio;
+ final TimeoutConfiguration _timeoutConfiguration;
+ final StopwatchFactory _stopwatchFactory;
+ final OutputPreferences _outputPreferences;
+
+ /// Create the appropriate logger for the current platform and configuration.
+ Logger createLogger({
+ @required bool verbose,
+ @required bool machine,
+ @required bool daemon,
+ @required bool windows,
+ }) {
+ Logger logger;
+ if (windows) {
+ logger = WindowsStdoutLogger(
+ terminal: _terminal,
+ stdio: _stdio,
+ outputPreferences: _outputPreferences,
+ timeoutConfiguration: _timeoutConfiguration,
+ stopwatchFactory: _stopwatchFactory,
+ );
+ } else {
+ logger = StdoutLogger(
+ terminal: _terminal,
+ stdio: _stdio,
+ outputPreferences: _outputPreferences,
+ timeoutConfiguration: _timeoutConfiguration,
+ stopwatchFactory: _stopwatchFactory
+ );
+ }
+ if (verbose) {
+ logger = VerboseLogger(logger, stopwatchFactory: _stopwatchFactory);
+ }
+ if (daemon) {
+ return NotifyingLogger(verbose: verbose, parent: logger);
+ }
+ if (machine) {
+ return AppRunLogger(parent: logger);
+ }
+ return logger;
+ }
+}
diff --git a/packages/flutter_tools/lib/src/base/logger.dart b/packages/flutter_tools/lib/src/base/logger.dart
index 075c234..20ab0e1 100644
--- a/packages/flutter_tools/lib/src/base/logger.dart
+++ b/packages/flutter_tools/lib/src/base/logger.dart
@@ -175,7 +175,7 @@
class StdoutLogger extends Logger {
StdoutLogger({
- @required AnsiTerminal terminal,
+ @required Terminal terminal,
@required Stdio stdio,
@required OutputPreferences outputPreferences,
@required TimeoutConfiguration timeoutConfiguration,
@@ -188,7 +188,7 @@
_stopwatchFactory = stopwatchFactory;
@override
- final AnsiTerminal _terminal;
+ final Terminal _terminal;
@override
final OutputPreferences _outputPreferences;
@override
@@ -345,7 +345,7 @@
/// they will show up as the unrepresentable character symbol '�'.
class WindowsStdoutLogger extends StdoutLogger {
WindowsStdoutLogger({
- @required AnsiTerminal terminal,
+ @required Terminal terminal,
@required Stdio stdio,
@required OutputPreferences outputPreferences,
@required TimeoutConfiguration timeoutConfiguration,
@@ -1006,7 +1006,7 @@
this.padding = kDefaultStatusPadding,
@required Duration timeout,
@required Stopwatch stopwatch,
- @required AnsiTerminal terminal,
+ @required Terminal terminal,
VoidCallback onFinish,
Stdio stdio,
TimeoutConfiguration timeoutConfiguration,
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 6ca3251..2130457 100644
--- a/packages/flutter_tools/test/general.shard/base/logger_test.dart
+++ b/packages/flutter_tools/test/general.shard/base/logger_test.dart
@@ -5,10 +5,12 @@
import 'dart:async';
import 'dart:convert' show jsonEncode;
+import 'package:flutter_tools/executable.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/commands/daemon.dart';
import 'package:mockito/mockito.dart';
import 'package:quiver/testing/async.dart';
@@ -24,6 +26,52 @@
class MockStdout extends Mock implements Stdout {}
void main() {
+ testWithoutContext('correct logger instance is created', () {
+ final LoggerFactory loggerFactory = LoggerFactory(
+ terminal: Terminal.test(),
+ stdio: mocks.MockStdio(),
+ outputPreferences: OutputPreferences.test(),
+ timeoutConfiguration: const TimeoutConfiguration(),
+ );
+
+ expect(loggerFactory.createLogger(
+ verbose: false,
+ machine: false,
+ daemon: false,
+ windows: false,
+ ), isA<StdoutLogger>());
+ expect(loggerFactory.createLogger(
+ verbose: false,
+ machine: false,
+ daemon: false,
+ windows: true,
+ ), isA<WindowsStdoutLogger>());
+ expect(loggerFactory.createLogger(
+ verbose: true,
+ machine: false,
+ daemon: false,
+ windows: true,
+ ), isA<VerboseLogger>());
+ expect(loggerFactory.createLogger(
+ verbose: true,
+ machine: false,
+ daemon: false,
+ windows: false,
+ ), isA<VerboseLogger>());
+ expect(loggerFactory.createLogger(
+ verbose: false,
+ machine: false,
+ daemon: true,
+ windows: false,
+ ), isA<NotifyingLogger>());
+ expect(loggerFactory.createLogger(
+ verbose: false,
+ machine: true,
+ daemon: false,
+ windows: false,
+ ), isA<AppRunLogger>());
+ });
+
group('AppContext', () {
FakeStopwatch fakeStopWatch;
