Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

Commit

Permalink
Move setting the logging level into the Logger constructor, refacto…
Browse files Browse the repository at this point in the history
…r. (#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.
  • Loading branch information
matanlurey authored May 7, 2024
1 parent 1fe835b commit 880280a
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 22 deletions.
8 changes: 7 additions & 1 deletion tools/engine_tool/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);

Expand Down
4 changes: 0 additions & 4 deletions tools/engine_tool/lib/src/commands/command_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 8 additions & 9 deletions tools/engine_tool/lib/src/logger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down Expand Up @@ -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.
Expand Down
12 changes: 4 additions & 8 deletions tools/engine_tool/test/logger_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});

Expand Down Expand Up @@ -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']));
});
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 880280a

Please sign in to comment.