From 224385af374b97a6999132068f1b6acf77bc4ae3 Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 31 May 2026 15:13:16 +0200 Subject: [PATCH] =?UTF-8?q?hive-forge:=20extract=20format=5Fevent=20helper?= =?UTF-8?q?;=20tests=20call=20it=20directly=20(argus=20=F0=9F=9F=A1=20on?= =?UTF-8?q?=20#798)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit was: `print_event` did the format + println inline; tests had a parallel `captured` helper that re-implemented the dispatch and only covered 4 of 14 event types. brittle — a new arm in `print_event` silently went uncovered. now: pure `format_event(ev: &Value) -> String` builds the line; `print_event` is a thin wrapper that adds the trailing newline. tests assert on `format_event` output directly so every new arm gets test coverage by construction. bonus: added 3 more test cases (assignees add/remove, pull_push commit count + force-push, commit_ref sha truncation) since the helper extraction made them cheap. 9 tests total, all green. --- hive-forge/src/verbs/timeline.rs | 162 ++++++++++++++++++++----------- 1 file changed, 104 insertions(+), 58 deletions(-) diff --git a/hive-forge/src/verbs/timeline.rs b/hive-forge/src/verbs/timeline.rs index 056aff98..52c35f1e 100644 --- a/hive-forge/src/verbs/timeline.rs +++ b/hive-forge/src/verbs/timeline.rs @@ -50,14 +50,19 @@ pub fn run(client: &Client, args: Args) -> Result<()> { Ok(()) } -/// Render one timeline event as `**actor @ ts**: summary`. Comment -/// rows print their full body; structured event types (label, -/// assignees, close, etc.) get a one-line human summary derived from -/// the per-type fields the API populates. Unknown / future types -/// fall through to a `[]` placeholder so a forge schema bump -/// doesn't panic the verb — operator still sees that the event -/// existed, with timestamp + actor. -fn print_event(ev: &Value) { +/// Render one timeline event as a single `**actor @ ts**: summary` +/// line. Comment rows inline their full body; structured event types +/// (label, assignees, close, etc.) get a one-line human summary +/// derived from the per-type fields the API populates. Unknown / +/// future types fall through to a `[]` placeholder so a forge +/// schema bump doesn't panic the verb — operator still sees that the +/// event existed, with timestamp + actor. +/// +/// Pure function (no I/O) so the tests below can pin the formatted +/// output for every supported event type without re-implementing the +/// per-arm dispatch. `print_event` is the only caller that adds the +/// terminating newline. +fn format_event(ev: &Value) -> String { let event_type = ev.get("type").and_then(Value::as_str).unwrap_or("?"); let user = ev .get("user") @@ -67,11 +72,13 @@ fn print_event(ev: &Value) { let ts = ev.get("created_at").and_then(Value::as_str).unwrap_or("?"); let summary = match event_type { "comment" => { - // Comments get the full body — matches `comments` verb shape. - let body = ev.get("body").and_then(Value::as_str).unwrap_or(""); - println!("**{user} @ {ts}**: {body}"); - println!(); - return; + // Comments get the full body inlined — matches `comments` + // verb shape so the operator sees the same line they'd + // get from the head-of-thread listing. + ev.get("body") + .and_then(Value::as_str) + .unwrap_or("") + .to_owned() } "label" => { // Forgejo encodes label add/remove via `body = "1"` (added) @@ -180,52 +187,27 @@ fn print_event(ev: &Value) { // whatever new event a forge bump invents. other => format!("[{other}]"), }; - println!("**{user} @ {ts}**: {summary}"); + format!("**{user} @ {ts}**: {summary}") +} + +/// Print one event followed by a blank line spacer. Thin wrapper +/// around `format_event` so the tests can pin per-arm output without +/// duplicating the dispatch. +fn print_event(ev: &Value) { + println!("{}", format_event(ev)); println!(); } #[cfg(test)] mod tests { + //! Tests call `format_event` directly so any new event-type arm + //! added in `print_event`'s dispatch is automatically covered by + //! the rendering path (no parallel test-side dispatch to keep in + //! sync). Argus on PR #798 🟡: "extract a `format_event(ev) -> + //! String` helper and test that function directly instead of + //! duplicating the logic" — addressed. use super::*; - /// Render one mock event and assert the formatted line matches. - /// Tests don't hit the network — we synthesise `Value` shapes - /// matching the Forgejo schema directly. - fn captured(ev: &Value) -> String { - // Tests run in-process; `println!` would interleave with the - // test harness. We rebuild the line via the same format - // expression. Keep this helper in lockstep with `print_event`. - let event_type = ev.get("type").and_then(Value::as_str).unwrap_or("?"); - let user = ev - .get("user") - .and_then(|u| u.get("login")) - .and_then(Value::as_str) - .unwrap_or("?"); - let ts = ev.get("created_at").and_then(Value::as_str).unwrap_or("?"); - let summary = match event_type { - "comment" => { - let body = ev.get("body").and_then(Value::as_str).unwrap_or(""); - return format!("**{user} @ {ts}**: {body}"); - } - "label" => { - let action = match ev.get("body").and_then(Value::as_str).unwrap_or("") { - "1" => "added", - "0" => "removed", - _ => "changed", - }; - let label = ev - .get("label") - .and_then(|l| l.get("name")) - .and_then(Value::as_str) - .unwrap_or("?"); - format!("{action} label `{label}`") - } - "close" => "closed".to_owned(), - other => format!("[{other}]"), - }; - format!("**{user} @ {ts}**: {summary}") - } - #[test] fn comment_renders_body_inline() { let ev = serde_json::json!({ @@ -234,7 +216,7 @@ mod tests { "created_at": "2026-05-31T12:00:00Z", "body": "looks good to me", }); - assert_eq!(captured(&ev), "**iris @ 2026-05-31T12:00:00Z**: looks good to me"); + assert_eq!(format_event(&ev), "**iris @ 2026-05-31T12:00:00Z**: looks good to me"); } #[test] @@ -247,7 +229,7 @@ mod tests { "label": { "name": "area:harness" }, }); assert_eq!( - captured(&ev), + format_event(&ev), "**triage @ 2026-05-31T12:00:00Z**: added label `area:harness`" ); } @@ -262,7 +244,7 @@ mod tests { "label": { "name": "needs-review" }, }); assert_eq!( - captured(&ev), + format_event(&ev), "**mara @ 2026-05-31T12:00:00Z**: removed label `needs-review`" ); } @@ -274,7 +256,71 @@ mod tests { "user": { "login": "mara" }, "created_at": "2026-05-31T12:00:00Z", }); - assert_eq!(captured(&ev), "**mara @ 2026-05-31T12:00:00Z**: closed"); + assert_eq!(format_event(&ev), "**mara @ 2026-05-31T12:00:00Z**: closed"); + } + + #[test] + fn assignees_added_and_removed() { + let added = serde_json::json!({ + "type": "assignees", + "user": { "login": "triage" }, + "created_at": "2026-05-31T12:00:00Z", + "assignee": { "login": "damocles" }, + "removed_assignee": false, + }); + assert_eq!( + format_event(&added), + "**triage @ 2026-05-31T12:00:00Z**: assigned @damocles" + ); + let removed = serde_json::json!({ + "type": "assignees", + "user": { "login": "triage" }, + "created_at": "2026-05-31T12:00:00Z", + "assignee": { "login": "damocles" }, + "removed_assignee": true, + }); + assert_eq!( + format_event(&removed), + "**triage @ 2026-05-31T12:00:00Z**: unassigned @damocles" + ); + } + + #[test] + fn pull_push_counts_commits_and_marks_force() { + let normal = serde_json::json!({ + "type": "pull_push", + "user": { "login": "damocles" }, + "created_at": "2026-05-31T12:00:00Z", + "body": r#"{"is_force_push":false,"commit_ids":["a","b","c"]}"#, + }); + assert_eq!( + format_event(&normal), + "**damocles @ 2026-05-31T12:00:00Z**: pushed 3 commit(s)" + ); + let forced = serde_json::json!({ + "type": "pull_push", + "user": { "login": "damocles" }, + "created_at": "2026-05-31T12:00:00Z", + "body": r#"{"is_force_push":true,"commit_ids":["a"]}"#, + }); + assert_eq!( + format_event(&forced), + "**damocles @ 2026-05-31T12:00:00Z**: force-pushed 1 commit(s)" + ); + } + + #[test] + fn commit_ref_truncates_sha_to_seven() { + let ev = serde_json::json!({ + "type": "commit_ref", + "user": { "login": "damocles" }, + "created_at": "2026-05-31T12:00:00Z", + "ref_commit_sha": "abcdef0123456789abcdef0123456789abcdef01", + }); + assert_eq!( + format_event(&ev), + "**damocles @ 2026-05-31T12:00:00Z**: referenced from commit abcdef0" + ); } #[test] @@ -287,7 +333,7 @@ mod tests { "created_at": "2026-05-31T12:00:00Z", }); assert_eq!( - captured(&ev), + format_event(&ev), "**ci-bot @ 2026-05-31T12:00:00Z**: [deploy_status]" ); } @@ -299,6 +345,6 @@ mod tests { "type": "close", "created_at": "2026-05-31T12:00:00Z", }); - assert_eq!(captured(&ev), "**? @ 2026-05-31T12:00:00Z**: closed"); + assert_eq!(format_event(&ev), "**? @ 2026-05-31T12:00:00Z**: closed"); } }