charles/ws-rpc-test

Fork 0

feat(report): console reporter with colours + TTY detection (#13) #32

Merged

charles merged 1 commit from feat/13-console-reporter into main

2026-04-11 18:43:54 +00:00

charles commented

2026-04-11 13:01:45 +00:00

Owner

Closes #13. Stacked on #31.

Summary

Introduces ConsoleReporter (in src/report.rs) and wires it into TestRunner::run(), replacing the inline println! scaffolding from earlier tickets.

Output (matches spec §5.1)

Starting <binary>...  OK (1.8s)
Connecting to WebSocket...  OK

 PASS  health_check (0.1s)
 FAIL  cancel_running (0.5s)

       ASSERTION FAILED

       Expected: true
       Got:      false
       Context:  after cancel

       at src/foo.rs:42

 SKIP  cloud_sync (—) (requires network)

8 passed, 1 failed, 1 skipped (21.2s)

Colours

Green PASS, red FAIL, yellow SKIP via the colored crate.
Coloured output is disabled whenever any of:
- no_color: true is passed to ConsoleReporter::new
- NO_COLOR env var is set (per https://no-color.org)
- stdout is not a TTY (std::io::IsTerminal)
Disabling flips the whole crate globally via colored::control::set_override(false).

Failure rendering

TestError::Assertion → full block with expected / got / context / location
TestError::Timeout → TIMEOUT waiting for <event> after <duration>
TestError::RpcError → RPC ERROR <code>: <message>
TestError::Connection → CONNECTION ERROR: <msg>

Runner wire-up

RunConfig.no_color: bool + TestRunner::no_color(bool) setter
run() creates one ConsoleReporter and uses it for harness start, WebSocket connect, per-test lines, and the final summary. Both the sequential and parallel phases route through the same reporter.
Helpers print_result / status_label from #12 are deleted — the reporter owns that formatting now.

Checklist (from issue #13)

Format

Header (Starting... OK (X.Xs), Connecting to WebSocket... OK)
Per-test PASS / FAIL / SKIP lines with timing
Indented failure block
Summary totals

Colours

Green PASS, red FAIL, yellow SKIP
Bold totals in summary
--no-color and NO_COLOR support
TTY auto-detection

Failure detail

Full TestError::Assertion fields + location
Timeout / RpcError / Connection get appropriate blocks

Header

Includes harness startup duration

Test plan

5 new unit tests in src/report.rs (pass / fail / skip / summary / header)
just qa green locally — 94 unit + 5 integration = 99/99
CI green on Forgejo

Notes for the reviewer

The reporter is currently a plain struct, not a trait. The Reporter trait abstraction lands in #14 alongside the JSON and JUnit implementations. Extracting it now would mean one level of indirection for zero current benefit.
harness_started captures the startup duration via Instant::now() in run() just before harness.start().await, then calls the reporter after the harness is up. This matches the spec's "header includes harness startup time" requirement without needing to thread the duration through the harness API.
verbose field is present on ConsoleReporter with #[allow(dead_code)]; it's reserved for #15's -v flag (which will append stdout/stderr tail to failure blocks). Added now to avoid thrashing the struct in #15.
The failure block for non-Assertion errors is less rich than the Assertion one — matches the spec §5.1 example, which only shows the Assertion format in detail.

Closes #13. **Stacked on #31.** ## Summary Introduces `ConsoleReporter` (in `src/report.rs`) and wires it into `TestRunner::run()`, replacing the inline `println!` scaffolding from earlier tickets. ### Output (matches spec §5.1) ``` Starting <binary>... OK (1.8s) Connecting to WebSocket... OK PASS health_check (0.1s) FAIL cancel_running (0.5s) ASSERTION FAILED Expected: true Got: false Context: after cancel at src/foo.rs:42 SKIP cloud_sync (—) (requires network) 8 passed, 1 failed, 1 skipped (21.2s) ``` ### Colours - Green `PASS`, red `FAIL`, yellow `SKIP` via the `colored` crate. - Coloured output is disabled whenever **any** of: - `no_color: true` is passed to `ConsoleReporter::new` - `NO_COLOR` env var is set (per https://no-color.org) - stdout is not a TTY (`std::io::IsTerminal`) - Disabling flips the whole crate globally via `colored::control::set_override(false)`. ### Failure rendering - `TestError::Assertion` → full block with expected / got / context / location - `TestError::Timeout` → `TIMEOUT waiting for <event> after <duration>` - `TestError::RpcError` → `RPC ERROR <code>: <message>` - `TestError::Connection` → `CONNECTION ERROR: <msg>` ### Runner wire-up - `RunConfig.no_color: bool` + `TestRunner::no_color(bool)` setter - `run()` creates one `ConsoleReporter` and uses it for harness start, WebSocket connect, per-test lines, and the final summary. Both the sequential and parallel phases route through the same reporter. - Helpers `print_result` / `status_label` from #12 are deleted — the reporter owns that formatting now. ## Checklist (from issue #13) ### Format - [x] Header (`Starting... OK (X.Xs)`, `Connecting to WebSocket... OK`) - [x] Per-test `PASS` / `FAIL` / `SKIP` lines with timing - [x] Indented failure block - [x] Summary totals ### Colours - [x] Green PASS, red FAIL, yellow SKIP - [x] Bold totals in summary - [x] `--no-color` and `NO_COLOR` support - [x] TTY auto-detection ### Failure detail - [x] Full `TestError::Assertion` fields + location - [x] `Timeout` / `RpcError` / `Connection` get appropriate blocks ### Header - [x] Includes harness startup duration ## Test plan - [x] 5 new unit tests in `src/report.rs` (pass / fail / skip / summary / header) - [x] `just qa` green locally — 94 unit + 5 integration = 99/99 - [ ] CI green on Forgejo ## Notes for the reviewer - The reporter is currently a plain struct, not a trait. The `Reporter` trait abstraction lands in #14 alongside the JSON and JUnit implementations. Extracting it now would mean one level of indirection for zero current benefit. - `harness_started` captures the startup duration via `Instant::now()` in `run()` just before `harness.start().await`, then calls the reporter after the harness is up. This matches the spec's "header includes harness startup time" requirement without needing to thread the duration through the harness API. - `verbose` field is present on `ConsoleReporter` with `#[allow(dead_code)]`; it's reserved for #15's `-v` flag (which will append stdout/stderr tail to failure blocks). Added now to avoid thrashing the struct in #15. - The failure block for non-Assertion errors is less rich than the Assertion one — matches the spec §5.1 example, which only shows the Assertion format in detail.

charles added the

type:user-story

area:reporting

labels

2026-04-11 13:01:45 +00:00

charles added 1 commit

2026-04-11 13:01:46 +00:00

feat(report): console reporter with colours + TTY detection (closes #13 ) 98b12110c2

Implements ticket #13.

src/report.rs: new ConsoleReporter struct
- Constructor disables coloured output when any of:
  - `no_color: true` is passed by the caller
  - the `NO_COLOR` env var is set (https://no-color.org)
  - stdout is not a TTY (std::io::IsTerminal)
- Disabled via `colored::control::set_override(false)` so the whole
  crate's coloured output falls back to plain text.

Output (matches spec §5.1)
- harness_started(binary, duration) prints the
  `Starting <binary>...  OK (X.Xs)` header with the startup wall clock.
- connected() prints `Connecting to WebSocket...  OK`.
- test_result(&TestCaseResult) prints one line with a coloured
  `PASS` (green) / `FAIL` (red) / `SKIP` (yellow) label, the test
  name, the duration, and the optional skip reason in parens.
- On failure, an indented block renders the error variant:
    - Assertion → `ASSERTION FAILED\n  Expected: ...\n  Got: ...\n
       Context: ...\n  at file:line`
    - Timeout / RpcError / Connection get their own sections
- summary(&TestReport) prints
  `N passed, N failed, N skipped (XX.Xs)` with coloured totals;
  also prints `after_all hook failed` when `report.after_all_failed`.

runner.rs wire-up
- `RunConfig` gains `no_color: bool`.
- `TestRunner::no_color(bool)` setter.
- `run()` creates a `ConsoleReporter::new(config.no_color)` at the top,
  captures the harness startup time via Instant::now() before
  harness.start(), then calls the reporter for header / test_result /
  summary in both sequential and parallel phases. The inline println!
  calls from earlier tickets are gone.
- Helper `print_result` and `status_label` from #12 are deleted — the
  reporter owns that formatting now.

Tests (5 new in src/report.rs)
- reporter_formats_pass_label
- reporter_formats_fail_with_assertion_block
- reporter_formats_skip_with_reason
- reporter_summary_no_panic_on_all_zero
- harness_started_captures_startup_time

just qa green: 94 unit + 5 integration = 99/99, fmt + clippy clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

charles referenced this pull request

2026-04-11 13:05:02 +00:00

feat(report): Reporter trait + JSON and JUnit reporters (#14) #33

charles reviewed

2026-04-11 17:55:00 +00:00

charles left a comment

Review — console reporter avec couleurs + TTY detection (#13)

TTY detection + `NO_COLOR`

let disable = no_color
    || std::env::var_os("NO_COLOR").is_some()
    || !std::io::IsTerminal::is_terminal(&std::io::stdout());

Correct et conforme à la spec. colored::control::set_override(false) est une mutation globale — ça fonctionne mais dans un processus multi-thread qui lance des tests en parallèle, deux reporters instanciés avec des settings différents se marcheraient dessus. Non-problème pour l'usage courant (un seul reporter par run).

Bloc d'erreur structuré

Un match par variant de TestError avec des formats différents — correct. Chaque type d'erreur a son propre formatage (assertion block avec Expected/Got/Context, timeout, RPC error, connection error).

Champ `verbose: bool` mort

#[allow(dead_code)]
verbose: bool,

Réservé pour usage futur, mais aucune logique n'utilise encore ce flag. À implémenter ou à supprimer — ne pas laisser en allow(dead_code) dans le code final.

Tests : pas de capture stdout

Les tests vérifient uniquement que les méthodes ne paniquent pas, pas le contenu du rendu. C'est acceptable pour une première implémentation, mais du snapshot testing (ou au moins une assertion sur une string capturée) renforcerait la confiance dans les formats de sortie.

Aucun bloquant.

## Review — console reporter avec couleurs + TTY detection (#13) ### TTY detection + `NO_COLOR` ```rust let disable = no_color || std::env::var_os("NO_COLOR").is_some() || !std::io::IsTerminal::is_terminal(&std::io::stdout()); ``` Correct et conforme à la spec. `colored::control::set_override(false)` est une mutation globale — ça fonctionne mais dans un processus multi-thread qui lance des tests en parallèle, deux reporters instanciés avec des settings différents se marcheraient dessus. Non-problème pour l'usage courant (un seul reporter par run). ### Bloc d'erreur structuré Un `match` par variant de `TestError` avec des formats différents — correct. Chaque type d'erreur a son propre formatage (assertion block avec Expected/Got/Context, timeout, RPC error, connection error). ### Champ `verbose: bool` mort ```rust #[allow(dead_code)] verbose: bool, ``` Réservé pour usage futur, mais aucune logique n'utilise encore ce flag. À implémenter ou à supprimer — ne pas laisser en `allow(dead_code)` dans le code final. ### Tests : pas de capture stdout Les tests vérifient uniquement que les méthodes ne paniquent pas, pas le contenu du rendu. C'est acceptable pour une première implémentation, mais du snapshot testing (ou au moins une assertion sur une string capturée) renforcerait la confiance dans les formats de sortie. Aucun bloquant.

charles reviewed

2026-04-11 17:55:17 +00:00

charles left a comment

✅ Pas de bloquant. Points à adresser avant release : champ verbose mort (#[allow(dead_code)]), et tests sans assertion sur le contenu de la sortie.

✅ Pas de bloquant. Points à adresser avant release : champ `verbose` mort (`#[allow(dead_code)]`), et tests sans assertion sur le contenu de la sortie.

charles force-pushed feat/13-console-reporter from 98b12110c2 to 81953fee00

2026-04-11 18:41:55 +00:00

Compare

charles changed target branch from feat/16-mock-server-tests to main

2026-04-11 18:43:46 +00:00

charles merged commit ffc8d12bb6 into main

2026-04-11 18:43:54 +00:00

charles deleted branch feat/13-console-reporter

2026-04-11 18:43:55 +00:00

charles referenced this pull request from a commit

2026-04-11 18:43:56 +00:00

Merge pull request 'feat(report): console reporter with colours + TTY detection (#13)' (#32) from feat/13-console-reporter into main