Move setting the logging level into the Logger constructor, refactor. (#52624)

Based on @zanderso's feedback here: #52619 (comment).

I think this is the most succinct way to setup logging, it also doesn't seem to make sense to allow the level to be configured at runtime, so boom.

Author: matanlurey

tools/engine_tool/lib/main.dart

@@ -60,12 +60,18 @@ void main(List<String> args) async {
     io.exitCode = 1;
   }
 
+  final Logger logger;
+  if (verbose) {
+    logger = Logger(level: Logger.infoLevel);
+  } else {
+    logger = Logger();
+  }
   final Environment environment = Environment(
     abi: ffi.Abi.current(),
     engine: engine,
     platform: const LocalPlatform(),
     processRunner: ProcessRunner(),
-    logger: Logger(),
+    logger: logger,
     verbose: verbose,
   );
 

tools/engine_tool/lib/src/commands/command_runner.dart

@@ -6,7 +6,6 @@ import 'package:args/command_runner.dart';
 import 'package:engine_build_configs/engine_build_configs.dart';
 
 import '../environment.dart';
-import '../logger.dart';
 import 'build_command.dart';
 import 'fetch_command.dart';
 import 'flags.dart';
@@ -94,9 +93,6 @@ final class ToolCommandRunner extends CommandRunner<int> {
 
   @override
   Future<int> run(Iterable<String> args) async {
-    if (environment.verbose) {
-      environment.logger.level = Logger.infoLevel;
-    }
     try {
       return await runCommand(parse(args)) ?? 0;
     } on FormatException catch (e) {

tools/engine_tool/lib/src/logger.dart

@@ -26,21 +26,25 @@ import 'package:meta/meta.dart';
 /// which can be inspected by unit tetss.
 class Logger {
   /// Constructs a logger for use in the tool.
-  Logger()
+  Logger({
+    log.Level level = statusLevel,
+  })
       : _logger = log.Logger.detached('et'),
         _test = false {
-    _logger.level = statusLevel;
+    _logger.level = level;
     _logger.onRecord.listen(_handler);
     _setupIoSink(io.stderr);
     _setupIoSink(io.stdout);
   }
 
   /// A logger for tests.
   @visibleForTesting
-  Logger.test()
+  Logger.test({
+    log.Level level = statusLevel,
+  })
       : _logger = log.Logger.detached('et'),
         _test = true {
-    _logger.level = statusLevel;
+    _logger.level = level;
     _logger.onRecord.listen((log.LogRecord r) => _testLogs.add(r));
   }
 
@@ -105,11 +109,6 @@ class Logger {
   /// Get the current logging level.
   log.Level get level => _logger.level;
 
-  /// Set the current logging level.
-  set level(log.Level l) {
-    _logger.level = l;
-  }
-
   /// Record a log message level [Logger.error] and throw a FatalError.
   /// This should only be called when the program has entered an impossible
   /// to recover from state or when something isn't implemented yet.

tools/engine_tool/test/logger_test.dart

@@ -12,8 +12,7 @@ void main() {
   }
 
   test('Setting the level works', () {
-    final Logger logger = Logger.test();
-    logger.level = Logger.infoLevel;
+    final Logger logger = Logger.test(level: Logger.infoLevel);
     expect(logger.level, equals(Logger.infoLevel));
   });
 
@@ -42,8 +41,7 @@ void main() {
   });
 
   test('info messages are recorded at the infoLevel log level', () {
-    final Logger logger = Logger.test();
-    logger.level = Logger.infoLevel;
+    final Logger logger = Logger.test(level: Logger.infoLevel);
     logger.info('info');
     expect(stringsFromLogs(logger.testLogs), equals(<String>['info\n']));
   });
@@ -73,15 +71,13 @@ void main() {
   });
 
   test('newlines in info() can be disabled', () {
-    final Logger logger = Logger.test();
-    logger.level = Logger.infoLevel;
+    final Logger logger = Logger.test(level: Logger.infoLevel);
     logger.info('info', newline: false);
     expect(stringsFromLogs(logger.testLogs), equals(<String>['info']));
   });
 
   test('fatal throws exception', () {
-    final Logger logger = Logger.test();
-    logger.level = Logger.infoLevel;
+    final Logger logger = Logger.test(level: Logger.infoLevel);
     bool caught = false;
     try {
       logger.fatal('test', newline: false);