From 15b1518b2af185aa1c87fe60f1178844826c5091 Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Wed, 27 Mar 2024 23:31:11 +0900 Subject: [PATCH] feat: Change concurrent log to sequential log (#679) * feat: Change concurrent log to sequential log * test: Add analyze -c 2 case * fix: logger error label --------- Co-authored-by: Lukas Klingsbo --- packages/melos/lib/src/commands/analyze.dart | 32 ++-- packages/melos/lib/src/common/utils.dart | 18 ++- packages/melos/lib/src/logging.dart | 147 ++++++++++++++++-- .../melos/test/commands/analyze_test.dart | 84 ++++++++++ 4 files changed, 250 insertions(+), 31 deletions(-) diff --git a/packages/melos/lib/src/commands/analyze.dart b/packages/melos/lib/src/commands/analyze.dart index fcfb895c5..45f61b690 100644 --- a/packages/melos/lib/src/commands/analyze.dart +++ b/packages/melos/lib/src/commands/analyze.dart @@ -36,7 +36,7 @@ mixin _AnalyzeMixin on _Melos { fatalWarnings: fatalWarnings, concurrency: concurrency, ).join(' '); - final prefixLogs = concurrency != 1 && packages.length != 1; + final useGroupBuffer = concurrency != 1 && packages.length != 1; logger.command('melos analyze', withDollarSign: true); @@ -44,20 +44,12 @@ mixin _AnalyzeMixin on _Melos { .child(targetStyle(analyzeArgsString)) .child('$runningLabel (in ${packages.length} packages)') .newLine(); - if (prefixLogs) { - logger.horizontalLine(); - } - - final packageResults = Map.fromEntries( - packages.map((package) => MapEntry(package.name, Completer())), - ); await pool.forEach(packages, (package) async { - if (!prefixLogs) { - logger - ..horizontalLine() - ..log(AnsiStyles.bgBlack.bold.italic('${package.name}:')); - } + final group = useGroupBuffer ? package.name : null; + logger + ..horizontalLine(group: group) + ..log(AnsiStyles.bgBlack.bold.italic('${package.name}:'), group: group); final packageExitCode = await _analyzeForPackage( workspace, @@ -67,20 +59,22 @@ mixin _AnalyzeMixin on _Melos { fatalInfos: fatalInfos, fatalWarnings: fatalWarnings, ), + group: group, ); - packageResults[package.name]?.complete(packageExitCode); - if (packageExitCode > 0) { failures[package.name] = packageExitCode; - } else if (!prefixLogs) { + } else { logger.log( AnsiStyles.bgBlack.bold.italic('${package.name}: ') + AnsiStyles.bgBlack(successLabel), + group: group, ); } }).drain(); + logger.flushGroupBufferIfNeed(); + logger ..horizontalLine() ..newLine() @@ -149,8 +143,9 @@ mixin _AnalyzeMixin on _Melos { Future _analyzeForPackage( MelosWorkspace workspace, Package package, - List analyzeArgs, - ) async { + List analyzeArgs, { + String? group, + }) async { final environment = { EnvironmentVariableKey.melosRootPath: config.path, if (workspace.sdkPath != null) @@ -164,6 +159,7 @@ mixin _AnalyzeMixin on _Melos { logger: logger, environment: environment, workingDirectory: package.path, + group: group, ); } diff --git a/packages/melos/lib/src/common/utils.dart b/packages/melos/lib/src/common/utils.dart index 5f78bc571..b4793ce1e 100644 --- a/packages/melos/lib/src/common/utils.dart +++ b/packages/melos/lib/src/common/utils.dart @@ -469,6 +469,7 @@ Future startCommand( bool onlyOutputOnError = false, bool includeParentEnvironment = true, required MelosLogger logger, + String? group, }) async { final processedCommand = command // Remove empty arguments. @@ -521,7 +522,10 @@ Future startCommand( (event) { processStdout.addAll(event); if (!onlyOutputOnError) { - logger.write(utf8.decode(event, allowMalformed: true)); + logger.logWithoutNewLine( + utf8.decode(event, allowMalformed: true), + group: group, + ); } }, onDone: processStdoutCompleter.complete, @@ -530,7 +534,7 @@ Future startCommand( (event) { processStderr.addAll(event); if (!onlyOutputOnError) { - logger.stderr(utf8.decode(event, allowMalformed: true)); + logger.error(utf8.decode(event, allowMalformed: true), group: group); } }, onDone: processStderrCompleter.complete, @@ -543,8 +547,14 @@ Future startCommand( _runningPids.remove(process.pid); if (onlyOutputOnError && exitCode > 0) { - logger.stdout(utf8.decode(processStdout, allowMalformed: true)); - logger.stderr(utf8.decode(processStderr, allowMalformed: true)); + logger.log( + utf8.decode(processStdout, allowMalformed: true), + group: group, + ); + logger.error( + utf8.decode(processStderr, allowMalformed: true), + group: group, + ); } return exitCode; diff --git a/packages/melos/lib/src/logging.dart b/packages/melos/lib/src/logging.dart index 6a28eabc6..0f5575311 100644 --- a/packages/melos/lib/src/logging.dart +++ b/packages/melos/lib/src/logging.dart @@ -49,7 +49,23 @@ class MelosLogger with _DelegateLogger { final String _indentation; final String _childIndentation; - void log(String message) => stdout(message); + void log(String message, {String? group}) { + if (group != null) { + _stdoutGroup(message, group); + return; + } + + stdout(message); + } + + void logWithoutNewLine(String message, {String? group}) { + if (group != null) { + _writeGroup(message, group); + return; + } + + write(message); + } void command(String command, {bool withDollarSign = false}) { if (withDollarSign) { @@ -59,7 +75,17 @@ class MelosLogger with _DelegateLogger { } } - void success(String message, {bool dryRun = false}) { + void success(String message, {String? group, bool dryRun = false}) { + if (group != null) { + if (dryRun) { + _stdoutGroup(successMessageColor(message), group); + } else { + _stdoutGroup(successMessageColor(successStyle(message)), group); + } + + return; + } + if (dryRun) { stdout(successMessageColor(message)); } else { @@ -67,11 +93,27 @@ class MelosLogger with _DelegateLogger { } } - void warning(String message, {bool label = true, bool dryRun = false}) { + void warning( + String message, { + String? group, + bool label = true, + bool dryRun = false, + }) { final labelColor = dryRun ? dryRunWarningLabelColor : dryRunWarningMessageColor; final messageColor = dryRun ? dryRunWarningMessageColor : warningMessageColor; + + if (group != null) { + if (label) { + _stdoutGroup('$warningLabel${labelColor(':')} $message', group); + } else { + _stdoutGroup(messageColor(message), group); + } + + return; + } + if (label) { stdout('$warningLabel${labelColor(':')} $message'); } else { @@ -79,7 +121,17 @@ class MelosLogger with _DelegateLogger { } } - void error(String message, {bool label = true}) { + void error(String message, {String? group, bool label = true}) { + if (group != null) { + if (label) { + _stderrGroup('$errorLabel${errorLabelColor(':')} $message', group); + } else { + _stderrGroup(errorMessageColor(message), group); + } + + return; + } + if (label) { stderr('$errorLabel${errorLabelColor(':')} $message'); } else { @@ -87,7 +139,17 @@ class MelosLogger with _DelegateLogger { } } - void hint(String message, {bool label = true}) { + void hint(String message, {String? group, bool label = true}) { + if (group != null) { + if (label) { + _stdoutGroup('$hintLabel${hintLabelColor(':')} $message', group); + } else { + _stdoutGroup(hintMessageColor(message), group); + } + + return; + } + if (label) { stdout(hintMessageColor('$hintLabel: $message')); } else { @@ -95,14 +157,29 @@ class MelosLogger with _DelegateLogger { } } - void newLine() => _logger.write('\n'); + void newLine({String? group}) { + if (group != null) { + _stdoutGroup('', group); + return; + } + + _logger.stdout(''); + } - void horizontalLine() => _logger.stdout('-' * terminalWidth); + void horizontalLine({String? group}) { + if (group != null) { + _stdoutGroup('-' * terminalWidth, group); + return; + } + + _logger.stdout('-' * terminalWidth); + } MelosLogger child( String message, { String prefix = '└> ', bool stderr = false, + String? group, }) { final childIndentation = ' ' * AnsiStyles.strip(prefix).length; final logger = MelosLogger( @@ -116,9 +193,9 @@ class MelosLogger with _DelegateLogger { for (final line in lines) { final prefixedLine = '${isFirstLine ? prefix : childIndentation}$line'; if (stderr) { - logger.stderr(prefixedLine); + logger.error(prefixedLine, group: group, label: false); } else { - logger.stdout(prefixedLine); + logger.log(prefixedLine, group: group); } isFirstLine = false; } @@ -141,6 +218,58 @@ class MelosLogger with _DelegateLogger { @override void trace(String message) => _logger.trace('$_indentation$message'); + + final _groupBuffer = >{}; + + void _stdoutGroup(String message, String group) { + final previous = _groupBuffer[group] ?? const []; + _groupBuffer[group] = [...previous, _GroupBufferStdoutMessage(message)]; + } + + void _stderrGroup(String message, String group) { + final previous = _groupBuffer[group] ?? const []; + _groupBuffer[group] = [...previous, _GroupBufferStderrMessage(message)]; + } + + void _writeGroup(String message, String group) { + final previous = _groupBuffer[group] ?? const []; + _groupBuffer[group] = [...previous, _GroupBufferWriteMessage(message)]; + } + + void flushGroupBufferIfNeed() { + for (final entry in _groupBuffer.entries) { + for (final value in entry.value) { + switch (value) { + case _GroupBufferStdoutMessage(:final message): + stdout(message); + case _GroupBufferStderrMessage(:final message): + stderr(message); + case _GroupBufferWriteMessage(:final message): + write(message); + } + } + } + + _groupBuffer.clear(); + } +} + +sealed class _GroupBufferMessage { + const _GroupBufferMessage(this.message); + + final String message; +} + +class _GroupBufferStdoutMessage extends _GroupBufferMessage { + const _GroupBufferStdoutMessage(super.message); +} + +class _GroupBufferStderrMessage extends _GroupBufferMessage { + const _GroupBufferStderrMessage(super.message); +} + +class _GroupBufferWriteMessage extends _GroupBufferMessage { + const _GroupBufferWriteMessage(super.message); } mixin _DelegateLogger implements Logger { diff --git a/packages/melos/test/commands/analyze_test.dart b/packages/melos/test/commands/analyze_test.dart index 292074788..c423cd71d 100644 --- a/packages/melos/test/commands/analyze_test.dart +++ b/packages/melos/test/commands/analyze_test.dart @@ -276,5 +276,89 @@ $ melos analyze expect(regex.hasMatch(logger.output.normalizeNewLines()), isTrue); }); + + test('should run analysis with --concurrency 2 flag', () async { + await melos.analyze(concurrency: 2); + + expect( + logger.output.normalizeNewLines(), + ignoringAnsii(r''' +$ melos analyze + └> dart analyze --concurrency 2 + └> RUNNING (in 3 packages) + +-------------------------------------------------------------------------------- +a: +Analyzing a... +No issues found! +a: SUCCESS +-------------------------------------------------------------------------------- +b: +Analyzing b... +No issues found! +b: SUCCESS +-------------------------------------------------------------------------------- +c: +Analyzing c... +No issues found! +c: SUCCESS +-------------------------------------------------------------------------------- + +$ melos analyze + └> dart analyze --concurrency 2 + └> SUCCESS +'''), + ); + }); + + test('should run analysis with --concurrency 2 and --fatal-infos flag', + () async { + writeTextFile( + p.join(aDir.path, 'main.dart'), + r''' + void main() { + for (var i = 0; i < 10; i++) { + print('hello ${i + 1}'); + } + } + ''', + ); + + await melos.analyze(concurrency: 2, fatalInfos: true); + + expect( + logger.output.normalizeNewLines(), + ignoringAnsii(r''' +$ melos analyze + └> dart analyze --fatal-infos --concurrency 2 + └> RUNNING (in 3 packages) + +-------------------------------------------------------------------------------- +a: +Analyzing a... + + info - main.dart:3:13 - Don't invoke 'print' in production code. Try using a logging framework. - avoid_print + info - main.dart:5:10 - Missing a newline at the end of the file. Try adding a newline at the end of the file. - eol_at_end_of_file + +2 issues found. +-------------------------------------------------------------------------------- +b: +Analyzing b... +No issues found! +b: SUCCESS +-------------------------------------------------------------------------------- +c: +Analyzing c... +No issues found! +c: SUCCESS +-------------------------------------------------------------------------------- + +$ melos analyze + └> dart analyze --fatal-infos --concurrency 2 + └> FAILED (in 1 packages) + └> a (with exit code 1) +'''), + ); + }); }); }