Skip to content

feat(cli_tools): Added Logger.progressStream method #108

Merged
christerswahn merged 5 commits into
mainfrom
fix-progress
May 29, 2026
Merged

feat(cli_tools): Added Logger.progressStream method #108
christerswahn merged 5 commits into
mainfrom
fix-progress

Conversation

@christerswahn
Copy link
Copy Markdown
Collaborator

@christerswahn christerswahn commented May 29, 2026

Supports displaying multiple updated status strings during the run of a progress spinner.

Also makes progress and progressStream to handle exceptions by ending the animation with a failure message and rethrowing.

Also adds cursor rule file for writing tests according to our given - when - then convention.

Summary by CodeRabbit

  • New Features

    • Streamed progress display with per-event message formatting and returns the last event value.
    • Added Dart test naming/convention rule for Given–when–then style.
  • Bug Fixes

    • More reliable progress handling: proper cleanup, failure signaling, and error propagation for streamed progress.
  • Documentation

    • Clarified progress spinner behavior and options.
  • Tests

    • New tests covering progress and streamed-progress behaviors, success/failure cases, paragraph option, and output suppression.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e4eb467c-5525-4a45-9dac-30fba3330e5c

📥 Commits

Reviewing files that changed from the base of the PR and between c8709d4 and bd6bea0.

📒 Files selected for processing (2)
  • .cursor/rules/dart-test-conventions.mdc
  • packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart
✅ Files skipped from review due to trivial changes (1)
  • .cursor/rules/dart-test-conventions.mdc
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart

📝 Walkthrough

Walkthrough

Adds Logger.progressStream and implements it in StdOutLogger and VoidLogger; StdOutLogger manages an animated progress spinner across stream events. Tests verify success, failure, empty-stream behavior, message mapping, leading-newline option, and log-level suppression. A Cursor rule for Dart test conventions is also added.

Changes

Progress Stream Feature

Layer / File(s) Summary
Logger contract and documentation
packages/cli_tools/lib/src/logger/logger.dart
Logger declares new abstract progressStream<T> for stream-driven progress display and updates progress docs to describe spinner behavior on LogLevel.info.
StdOutLogger implementation
packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart
StdOutLogger.progressStream iterates streamed events, updates the progress UI per event using an optional toMessage mapper, handles empty streams by throwing StateError, manages animation lifecycle (stop/start/cleanup), supports newParagraph, and short-circuits when verbosity is below info; progress now delegates to progressStream.
VoidLogger implementation
packages/cli_tools/lib/src/logger/loggers/void_logger.dart
VoidLogger.progressStream is a no-op that returns the stream's last event without emitting output.
Test coverage for progress and progressStream
packages/cli_tools/test/logger/progress_test.dart
New tests cover progress success/exception handling, progressStream empty-stream error, last-value return, per-event updates and mapping, fail-on-error behavior, newParagraph newline emission, and suppression of progress output at higher log levels.
Dart test naming and structure conventions
.cursor/rules/dart-test-conventions.mdc
Cursor rule defining Given–when–then style for packages/**/test/**/*_test.dart, including group and naming guidelines, assertion placement, and output collection via collectOutput.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(cli_tools): Added Logger.progressStream method' clearly identifies the main change: adding a new progressStream method to the Logger class. It is specific, concise, and directly summarizes the primary addition in this changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-progress

Comment @coderabbitai help to get the list of available commands and usage tips.

@christerswahn christerswahn requested a review from nielsenko May 29, 2026 10:16
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.cursor/rules/dart-test-conventions.mdc:
- Around line 53-56: The test assertion contains a typo: update the string
literal in the test named "then error message is logged to errors." (the expect
call checking errors[0]) to use "erroneous usage" instead of "erronous usage" so
the example and any copied tests use the correct spelling.

In `@packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart`:
- Around line 147-158: The empty-stream check using `finalEvent is! T` fails for
nullable T; replace it with an explicit boolean flag (e.g., `bool seen = false`)
declared before the loop, set `seen = true` inside the `await for (final event
in stream)` when you assign `finalEvent`, and after the loop throw
`StateError('No events in stream')` if `!seen`; then call `progress.complete()`
and return `finalEvent` cast to `T` (e.g., `return finalEvent as T`) so the
function reliably detects an empty stream regardless of nullability. Use the
existing symbols `finalEvent`, `stream`, `toMessage`, `progress.update`, and
`progress.complete` to locate and update the code.

In `@packages/cli_tools/test/logger/progress_test.dart`:
- Line 17: Several test cases destructure all three fields from collectOutput
but don't use stderr/stdin, which triggers analyzer warnings; update each
destructuring of collectOutput (e.g., the line using final (:stdout, :stderr,
:stdin) = await collectOutput(...)) to ignore unused fields by replacing the
unused names with underscores (for example use :_stderr and/or :_stdin or just
:_ for the unused positions) so only the used symbol (stdout) remains named;
apply the same change to the other occurrences mentioned (lines around where
stdout/stderr/stdin are destructured) to silence the analyzer warnings.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e5ae7154-8e4f-45ad-8c53-753661d55d28

📥 Commits

Reviewing files that changed from the base of the PR and between 2be6d0e and 23c3a23.

📒 Files selected for processing (5)
  • .cursor/rules/dart-test-conventions.mdc
  • packages/cli_tools/lib/src/logger/logger.dart
  • packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart
  • packages/cli_tools/lib/src/logger/loggers/void_logger.dart
  • packages/cli_tools/test/logger/progress_test.dart

Comment thread .cursor/rules/dart-test-conventions.mdc
Comment thread packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart
Comment thread packages/cli_tools/test/logger/progress_test.dart
Copy link
Copy Markdown
Contributor

@nielsenko nielsenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

One unrelated suggestion now I was popping by 😊

Comment thread packages/cli_tools/lib/src/logger/loggers/std_out_logger.dart
@christerswahn christerswahn merged commit fac111a into main May 29, 2026
13 checks passed
@christerswahn christerswahn deleted the fix-progress branch May 29, 2026 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants