charles/ws-rpc-test

Fork 0

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

Merged

charles merged 1 commit from feat/14-reporters-json-junit into main

2026-04-11 18:44:03 +00:00

charles commented

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

Owner

Closes #14. Stacked on #32.

Summary

Extracts the Reporter trait and adds JSON and JUnit XML implementations. TestRunner::run() now routes through Box<dyn Reporter> so callers pick the format via RunConfig.format.

New types

OutputFormat { Console, Json, Junit } — with a parse(&str) helper used by the CLI in #15.
Reporter trait with default no-op impls for start_run / harness_started / connected; abstract test_result and end_run.
build_reporter(format, no_color) -> Box<dyn Reporter> factory.
JsonReporter — emits a single pretty-printed JSON document at end of run.
JunitReporter — emits a <testsuites> XML document at end of run.
ConsoleReporter now impl Reporter (its existing methods are preserved for direct use).

JSON shape

{
  "tests": [
    {"name": "...", "tags": [...], "status": "passed|failed|skipped",
     "duration_ms": 123, "skip_reason": null | "...",
     "error": null | {"type": "Assertion", "expected": "...", ...}}
  ],
  "passed": 8, "failed": 1, "skipped": 1, "filtered": 0,
  "duration_ms": 21200,
  "started_at_ms": 1712825400000,
  "finished_at_ms": 1712825421200,
  "after_all_failed": false
}

Epoch-ms timestamps to avoid pulling in chrono.

JUnit shape

<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="3" failures="1" skipped="1" time="21.200">
  <testsuite name="ws-rpc-test" tests="3" failures="1" skipped="1" time="21.200">
    <testcase name="alpha" time="0.042"/>
    <testcase name="beta&lt;x&gt;" time="0.012">
      <failure message="boom">boom</failure>
    </testcase>
    <testcase name="gamma" time="0.000">
      <skipped message="no network"/>
    </testcase>
  </testsuite>
</testsuites>

xml_escape covers < > & " ' and control chars.

Runner wire-up

RunConfig.format: OutputFormat (default Console).
TestRunner::format(OutputFormat) setter.
run() builds Box<dyn Reporter> via build_reporter and dispatches start_run / harness_started / connected / test_result / end_run through the trait. Both sequential and parallel phases go through the same reporter.

Checklist (from issue #14)

Reporter trait

Abstracts Console / JSON / JUnit
start_run, harness_started, connected, test_result, end_run

JSON

Single JSON object at end of run
tests array with {name, tags, status, duration_ms, error?}
Aggregate passed/failed/skipped/filtered/duration_ms
started_at_ms / finished_at_ms for timing
Stable key order (via serde_json::to_string_pretty)

JUnit

<testsuites> with one <testsuite> per run
<testcase> with <failure> / <skipped> children as appropriate
Escaped strings throughout
time attribute per testcase and suite

General

Non-console formats suppress per-test progress lines (the Reporter trait's default impls for harness_started / connected are no-ops for JSON and JUnit)
No ANSI colour in JSON or XML even without --no-color

Test plan

4 new unit tests in src/report.rs (4 total for JSON/JUnit + OutputFormat::parse)
just qa green — 98 unit + 5 integration = 103/103
CI green on Forgejo

Notes for the reviewer

Using epoch milliseconds instead of RFC 3339 avoids adding chrono. A future PR can swap to ISO 8601 without changing the JSON structure (just add the extra string field).
JunitReporter stores TestCaseResult clones with the error wrapped as TestError::Connection(formatted_message) because TestError isn't Clone. The wrapping is invisible to users — we only ever read the error back as a formatted string at end_run time.
The Reporter::start_run method has a default no-op so reporters that don't care (Console, JUnit) don't have to override it. Only JsonReporter needs it (to capture started_at_ms).
Round-trip testing for the JSON output lives in the unit tests by operating on the tests accumulator directly. A true end-to-end round trip (parse the printed stdout) would need stdout capture, which is brittle and deferred.
The JUnit output is well-formed but not validated against the strict XSD. Real CI systems (Jenkins, GitLab) are lenient about JUnit XML and will accept this shape; if strict XSD compliance becomes a requirement we can tighten it in a follow-up.

Closes #14. **Stacked on #32.** ## Summary Extracts the `Reporter` trait and adds JSON and JUnit XML implementations. `TestRunner::run()` now routes through `Box<dyn Reporter>` so callers pick the format via `RunConfig.format`. ### New types - `OutputFormat { Console, Json, Junit }` — with a `parse(&str)` helper used by the CLI in #15. - `Reporter` trait with default no-op impls for `start_run` / `harness_started` / `connected`; abstract `test_result` and `end_run`. - `build_reporter(format, no_color) -> Box<dyn Reporter>` factory. - `JsonReporter` — emits a single pretty-printed JSON document at end of run. - `JunitReporter` — emits a `<testsuites>` XML document at end of run. - `ConsoleReporter` now `impl Reporter` (its existing methods are preserved for direct use). ### JSON shape ```json { "tests": [ {"name": "...", "tags": [...], "status": "passed|failed|skipped", "duration_ms": 123, "skip_reason": null | "...", "error": null | {"type": "Assertion", "expected": "...", ...}} ], "passed": 8, "failed": 1, "skipped": 1, "filtered": 0, "duration_ms": 21200, "started_at_ms": 1712825400000, "finished_at_ms": 1712825421200, "after_all_failed": false } ``` Epoch-ms timestamps to avoid pulling in `chrono`. ### JUnit shape ```xml <?xml version="1.0" encoding="UTF-8"?> <testsuites tests="3" failures="1" skipped="1" time="21.200"> <testsuite name="ws-rpc-test" tests="3" failures="1" skipped="1" time="21.200"> <testcase name="alpha" time="0.042"/> <testcase name="beta<x>" time="0.012"> <failure message="boom">boom</failure> </testcase> <testcase name="gamma" time="0.000"> <skipped message="no network"/> </testcase> </testsuite> </testsuites> ``` `xml_escape` covers `< > & " '` and control chars. ### Runner wire-up - `RunConfig.format: OutputFormat` (default `Console`). - `TestRunner::format(OutputFormat)` setter. - `run()` builds `Box<dyn Reporter>` via `build_reporter` and dispatches `start_run / harness_started / connected / test_result / end_run` through the trait. Both sequential and parallel phases go through the same reporter. ## Checklist (from issue #14) ### Reporter trait - [x] Abstracts Console / JSON / JUnit - [x] `start_run`, `harness_started`, `connected`, `test_result`, `end_run` ### JSON - [x] Single JSON object at end of run - [x] `tests` array with `{name, tags, status, duration_ms, error?}` - [x] Aggregate `passed/failed/skipped/filtered/duration_ms` - [x] `started_at_ms` / `finished_at_ms` for timing - [x] Stable key order (via `serde_json::to_string_pretty`) ### JUnit - [x] `<testsuites>` with one `<testsuite>` per run - [x] `<testcase>` with `<failure>` / `<skipped>` children as appropriate - [x] Escaped strings throughout - [x] `time` attribute per testcase and suite ### General - [x] Non-console formats suppress per-test progress lines (the `Reporter` trait's default impls for `harness_started` / `connected` are no-ops for JSON and JUnit) - [x] No ANSI colour in JSON or XML even without `--no-color` ## Test plan - [x] 4 new unit tests in `src/report.rs` (4 total for JSON/JUnit + `OutputFormat::parse`) - [x] `just qa` green — 98 unit + 5 integration = 103/103 - [ ] CI green on Forgejo ## Notes for the reviewer - Using epoch milliseconds instead of RFC 3339 avoids adding `chrono`. A future PR can swap to ISO 8601 without changing the JSON structure (just add the extra string field). - `JunitReporter` stores `TestCaseResult` clones with the error wrapped as `TestError::Connection(formatted_message)` because `TestError` isn't `Clone`. The wrapping is invisible to users — we only ever read the error back as a formatted string at `end_run` time. - The `Reporter::start_run` method has a default no-op so reporters that don't care (Console, JUnit) don't have to override it. Only `JsonReporter` needs it (to capture `started_at_ms`). - Round-trip testing for the JSON output lives in the unit tests by operating on the `tests` accumulator directly. A true end-to-end round trip (parse the printed stdout) would need stdout capture, which is brittle and deferred. - The JUnit output is well-formed but not validated against the strict XSD. Real CI systems (Jenkins, GitLab) are lenient about JUnit XML and will accept this shape; if strict XSD compliance becomes a requirement we can tighten it in a follow-up.

charles added the

type:user-story

area:reporting

labels

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

charles added 1 commit

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

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

Implements ticket #14.

Reporter trait
- pub trait Reporter: Send with default no-op impls for start_run,
  harness_started, connected; abstract test_result and end_run.
- ConsoleReporter, JsonReporter, JunitReporter all implement it.
- build_reporter(format, no_color) factory returns Box<dyn Reporter>.
- OutputFormat enum with a parse() helper for CLI input (#15 uses it).

JsonReporter
- Accumulates TestCaseResult clones as serde_json::Value.
- start_run captures started_at as epoch ms.
- end_run prints a single pretty-printed JSON document to stdout:
    {
      tests: [{name, tags, status, duration_ms, skip_reason, error}],
      passed/failed/skipped/filtered counts,
      duration_ms, started_at_ms, finished_at_ms,
      after_all_failed,
    }
- Error serialisation maps each TestError variant to a tagged object
  (Assertion, Timeout, Connection, RpcError, Skip, Other).
- No chrono dep: epoch ms via SystemTime, keeps the dep tree minimal.

JunitReporter
- Accumulates test results (since TestError isn't Clone, the error
  field is stored as a TestError::Connection wrapping the formatted
  message — used only for rendering).
- end_run writes a <testsuites>/<testsuite> document with one
  <testcase> per test. Failed tests get a <failure>, skipped get
  a <skipped message="..."/>.
- xml_escape helper covers <, >, &, ", ', and control chars.

runner.rs wire-up
- RunConfig.format: OutputFormat (default Console).
- runner.format(OutputFormat) setter.
- run() calls build_reporter(format, no_color) and dispatches through
  the trait instead of hard-wiring ConsoleReporter. Sequential AND
  parallel phases both route through the same Box<dyn Reporter>.

Tests (4 new in src/report.rs, additive)
- output_format_parse_is_case_insensitive
- json_reporter_emits_all_top_level_fields (builds a reporter,
  feeds two results, asserts the serde_json::Value accumulator
  has the right shape — covers status mapping and Assertion error)
- junit_xml_escapes_special_chars
- junit_reporter_builds_well_formed_element_per_test

just qa green: 98 unit + 5 integration = 103/103, fmt + clippy clean.

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

charles referenced this pull request

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

feat(cli): clap-based CLI + parse_cli_args() helper (#15) #34

charles reviewed

2026-04-11 17:58:37 +00:00

charles left a comment

Review PR #33 — `Reporter` trait + JSON / JUnit reporters

Code qualité globale : solide. Trait bien défini, factory propre, reporters complets, tests unitaires pertinents. Quelques observations à décharger.

1. Délégation redondante dans `impl Reporter for ConsoleReporter`

fn harness_started(&mut self, binary: &str, duration: Duration) {
    ConsoleReporter::harness_started(self, binary, duration);
}

Les méthodes implémentent le trait en appelant les méthodes intrinsèques du même nom avec la syntaxe chemin complet. C'est valide, mais verbose. self.harness_started(binary, duration) est équivalent ici et plus idiomatique. La seule raison d'utiliser le chemin complet serait d'éviter une récursion infinie si le nom de méthode est identique dans le trait et dans l'impl ; mais comme les méthodes intrinsèques préexistaient, Rust résoud sans ambiguïté. Mineur, pas un bug.

2. `started_at_ms = 0` si `start_run()` n'est pas appelé

JsonReporter initialise started_at_ms: 0 (via #[derive(Default)]). Si un consommateur appelle directement test_result + end_run sans start_run, la sortie JSON contiendra "started_at_ms": 0. Le runner appelle désormais start_run() en tête de run(), donc en pratique ça ne se produit pas. Mais le contrat du trait pourrait rendre start_run obligatoire en le retirant de l'impl par défaut, ou au moins documenter l'invariant.

3. Perte de type d'erreur dans `JunitReporter`

let cloned_error = result.error.as_ref().map(|e| format!("{e}"));
self.tests.push(TestCaseResult {
    error: cloned_error.map(TestError::Connection),
    ...
});

L'erreur est convertie en Display string puis re-wrappée dans TestError::Connection. Toutes les infos de type (Assertion, Timeout, RpcError, Skip, Other) sont perdues. Pour JUnit XML, seul le message apparaît dans <failure message="...">, donc en pratique ça ne pose pas de problème. C'est une décision pragmatique acceptable mais mérite un commentaire explicite (pourquoi Connection spécifiquement plutôt qu'un variant dédié comme TestError::StringOnly).

4. Nom de suite JUnit codé en dur : `"ws-rpc-test"`

let name = "ws-rpc-test";

Le nom de la <testsuite> est toujours "ws-rpc-test". Ce serait plus flexible de l'exposer dans RunConfig ou de le passer à build_reporter(). Pas bloquant pour v0.1 mais à garder en tête si le framework est utilisé pour d'autres projets.

5. `OutputFormat::parse()` vs `FromStr`

Choix de design valide — évite d'importer le trait au point d'usage. Fonctionnellement identique. Pas d'objection.

Points positifs

xml_escape couvre correctement les 5 entités XML + les caractères de contrôle interdits en XML 1.0.
error_json préserve fidèlement la structure de chaque variant de TestError.
build_reporter() factory centralisée : propre et extensible.
Tests suffisants (format parse, structure JSON, XML escape, éléments JUnit).
Le runner appelle désormais reporter.start_run() avant tout, et reporter.end_run() au lieu de reporter.summary() : bonne transition vers le trait.

✅ Aucun bug bloquant. Peut merger.

## Review PR #33 — `Reporter` trait + JSON / JUnit reporters Code qualité globale : solide. Trait bien défini, factory propre, reporters complets, tests unitaires pertinents. Quelques observations à décharger. --- ### 1. Délégation redondante dans `impl Reporter for ConsoleReporter` ```rust fn harness_started(&mut self, binary: &str, duration: Duration) { ConsoleReporter::harness_started(self, binary, duration); } ``` Les méthodes implémentent le trait en appelant les méthodes intrinsèques du même nom avec la syntaxe chemin complet. C'est valide, mais verbose. `self.harness_started(binary, duration)` est équivalent ici et plus idiomatique. La seule raison d'utiliser le chemin complet serait d'éviter une récursion infinie si le nom de méthode est identique dans le trait et dans l'impl ; mais comme les méthodes intrinsèques préexistaient, Rust résoud sans ambiguïté. Mineur, pas un bug. --- ### 2. `started_at_ms = 0` si `start_run()` n'est pas appelé `JsonReporter` initialise `started_at_ms: 0` (via `#[derive(Default)]`). Si un consommateur appelle directement `test_result` + `end_run` sans `start_run`, la sortie JSON contiendra `"started_at_ms": 0`. Le runner appelle désormais `start_run()` en tête de `run()`, donc en pratique ça ne se produit pas. Mais le contrat du trait pourrait rendre `start_run` obligatoire en le retirant de l'impl par défaut, ou au moins documenter l'invariant. --- ### 3. Perte de type d'erreur dans `JunitReporter` ```rust let cloned_error = result.error.as_ref().map(|e| format!("{e}")); self.tests.push(TestCaseResult { error: cloned_error.map(TestError::Connection), ... }); ``` L'erreur est convertie en `Display` string puis re-wrappée dans `TestError::Connection`. Toutes les infos de type (`Assertion`, `Timeout`, `RpcError`, `Skip`, `Other`) sont perdues. Pour JUnit XML, seul le message apparaît dans `<failure message="...">`, donc en pratique ça ne pose pas de problème. C'est une décision pragmatique acceptable mais mérite un commentaire explicite (pourquoi `Connection` spécifiquement plutôt qu'un variant dédié comme `TestError::StringOnly`). --- ### 4. Nom de suite JUnit codé en dur : `"ws-rpc-test"` ```rust let name = "ws-rpc-test"; ``` Le nom de la `<testsuite>` est toujours `"ws-rpc-test"`. Ce serait plus flexible de l'exposer dans `RunConfig` ou de le passer à `build_reporter()`. Pas bloquant pour v0.1 mais à garder en tête si le framework est utilisé pour d'autres projets. --- ### 5. `OutputFormat::parse()` vs `FromStr` Choix de design valide — évite d'importer le trait au point d'usage. Fonctionnellement identique. Pas d'objection. --- ### Points positifs - `xml_escape` couvre correctement les 5 entités XML + les caractères de contrôle interdits en XML 1.0. - `error_json` préserve fidèlement la structure de chaque variant de `TestError`. - `build_reporter()` factory centralisée : propre et extensible. - Tests suffisants (format parse, structure JSON, XML escape, éléments JUnit). - Le runner appelle désormais `reporter.start_run()` avant tout, et `reporter.end_run()` au lieu de `reporter.summary()` : bonne transition vers le trait. ✅ Aucun bug bloquant. Peut merger.

charles force-pushed feat/14-reporters-json-junit from 14ddd2dfda to 552ba2b9ed

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

Compare

charles changed target branch from feat/13-console-reporter to main

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

charles merged commit 519b62b5b9 into main

2026-04-11 18:44:03 +00:00

charles deleted branch feat/14-reporters-json-junit

2026-04-11 18:44:03 +00:00

charles referenced this pull request from a commit

2026-04-11 18:44:05 +00:00

Merge pull request 'feat(report): Reporter trait + JSON and JUnit reporters (#14)' (#33) from feat/14-reporters-json-junit into main