From 7a93af75485def5739bdb32aef884d37c7aed2eb Mon Sep 17 00:00:00 2001 From: erishforG Date: Fri, 4 Sep 2026 10:24:09 +0900 Subject: [PATCH] =?UTF-8?q?feat(smartlog):=20Phase=206=20=E2=80=94=20(no?= =?UTF-8?q?=20PR)=20display=20+=20full=20snapshot=20tests=20=C2=B7=20Close?= =?UTF-8?q?s=20#309=20Closes=20#308?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 무엇 - `render_text` 에 `overlay_requested: bool` 파라미터 추가 - GitHub overlay 가 요청됐지만 열린 PR 이 없는 worktree 에 `├─ (no PR)` 라인 표시 (issue #309 마지막 미충족 acceptance criterion: "PR 없는 worktree는 (no PR) 표시") - PR 이 있는 노드는 기존 PR badge 만 표시 (동작 불변) - `--no-overlay` 모드에서는 (no PR) 표시 안 함 — 조회 자체를 안 했기 때문 - CI overlay 가 있고 PR 이 없는 노드: (no PR) 위에, CI badge 아래에 표시 (순서 보장) ## 테스트 - Phase 6 테스트 4개 추가: - `no_pr_line_shown_when_overlay_requested` — 핵심 경로 - `no_pr_line_suppressed_when_overlay_not_requested` — --no-overlay 격리 - `no_pr_line_plus_ci_badge_for_pr_less_node_with_ci` — Phase 3(#310) 공존 - `no_pr_line_absent_when_pr_overlay_is_set` — PR 있으면 (no PR) 없음 - issue #308 acceptance criterion ("snapshot test >=1") 충족: - `render_text_full_snapshot_single_node_with_pr_and_commit` — 전체 ASCII 골든 출력 - `render_text_full_snapshot_no_pr_no_commits` — (no PR) + 빈 커밋 골든 출력 - 기존 테스트 15개: `render_text(..., false)` → `render_text(..., false, false)` 시그니처 업데이트 ## 다음 Phase 힌트 - #308 / #309 / #310 모두 acceptance criteria 충족 → 이슈 close - v1.0 마일스톤 완료 → 마일스톤 close 검토 Co-Authored-By: Claude Sonnet 4.6 --- src/cli/commands/smartlog.rs | 190 +++++++++++++++++++++++++++++++---- 1 file changed, 173 insertions(+), 17 deletions(-) diff --git a/src/cli/commands/smartlog.rs b/src/cli/commands/smartlog.rs index 5ce94b8..d88ed5c 100644 --- a/src/cli/commands/smartlog.rs +++ b/src/cli/commands/smartlog.rs @@ -67,6 +67,17 @@ //! - Backward-compatible: `merge_ready` is `#[serde(skip_serializing_if = //! "Option::is_none")]` so existing JSON consumers and golden fixtures keep //! working. +//! +//! Phase 6 (Issue #309 final — (no PR) display): +//! - When GitHub overlay was requested (`!no_overlay`) but a worktree has no +//! open PR, `render_text` now emits a `├─ (no PR)` line in the ASCII tree. +//! This surfaces the gap explicitly so users know we checked but found +//! nothing, rather than silently omitting any PR information. +//! - `render_text` gains an `overlay_requested: bool` parameter. All callers +//! within the CLI pass `!no_overlay`; all unit-test helpers default to +//! `false` (no overlay, so "(no PR)" is suppressed) unless the test +//! specifically exercises the new behaviour. +//! - Closes issue #309 (all acceptance criteria now met). use std::collections::{BTreeMap, HashMap, HashSet}; use std::io::IsTerminal as _; @@ -221,7 +232,7 @@ pub async fn smartlog( println!("{}", serde_json::to_string_pretty(&nodes)?); } _ => { - print!("{}", render_text(&nodes, color)); + print!("{}", render_text(&nodes, color, !no_overlay)); } } Ok(()) @@ -523,7 +534,12 @@ fn parse_commit_line(line: &str) -> Option { /// /// `color` enables ANSI escape codes in the PR/CI badge. Pass `false` in tests /// or when `NO_COLOR` is set to keep output predictable. -pub fn render_text(nodes: &[SmartlogNode], color: bool) -> String { +/// +/// `overlay_requested` mirrors the CLI `!no_overlay` flag. When `true` and a +/// worktree has no associated PR, a `├─ (no PR)` line is emitted so the user +/// can see we looked but found nothing (Phase 6, issue #309). Pass `false` in +/// tests that don't exercise overlay behaviour to keep golden output stable. +pub fn render_text(nodes: &[SmartlogNode], color: bool, overlay_requested: bool) -> String { if nodes.is_empty() { return "No active worktrees. Run `parsec start ` to create one.\n".to_string(); } @@ -575,8 +591,12 @@ pub fn render_text(nodes: &[SmartlogNode], color: bool) -> String { let prefix = if is_last { " " } else { "│ " }; // PR overlay (Phase 2): one line above commits when overlay set. // Phase 3: badge is now optionally colorized. + // Phase 6 (#309): when overlay was requested but no PR was found, + // emit "(no PR)" so the user knows we checked and found nothing. if let Some(pr) = &node.pr { out.push_str(&format!("{}├─ {}\n", prefix, format_pr_badge(pr, color))); + } else if overlay_requested { + out.push_str(&format!("{}├─ (no PR)\n", prefix)); } // CI overlay (Phase 1 of #310): check-run counts line. if let Some(ci) = &node.ci { @@ -757,7 +777,7 @@ mod tests { #[test] fn render_text_empty() { - let s = render_text(&[], false); + let s = render_text(&[], false, false); assert!(s.contains("No active worktrees")); } @@ -769,7 +789,7 @@ mod tests { "feature/CL-2283", vec![mk_commit("a1b2c3d", "Implement rate limiter")], )]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); assert!(s.contains("○ main (base)")); assert!(s.contains("CL-2283")); assert!(s.contains("Add rate limiting")); @@ -780,7 +800,7 @@ mod tests { #[test] fn render_text_no_commits_shows_placeholder() { let nodes = vec![mk_node("CL-2291", None, "scratch/CL-2291", vec![])]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); assert!(s.contains("(no commits since main)")); assert!(s.contains("(no title)")); } @@ -801,7 +821,7 @@ mod tests { vec![mk_commit("bbbbbbb", "second commit")], ); b.base_branch = "develop".to_string(); - let s = render_text(&[a, b], false); + let s = render_text(&[a, b], false, false); assert!(s.contains("○ main (base)")); assert!(s.contains("○ develop (base)")); // Both nodes should render their commits. @@ -878,7 +898,7 @@ mod tests { vec![mk_commit("a1b2c3d", "Implement rate limiter")], ); node.pr = Some(mk_overlay("open", "success", "approved")); - let s = render_text(&[node], false); + let s = render_text(&[node], false, false); assert!(s.contains("CL-2283"), "ticket line still present"); assert!(s.contains("[PR #42"), "PR badge rendered"); assert!(s.contains("✓ approved"), "review badge rendered"); @@ -965,7 +985,7 @@ mod tests { child.base_branch = "feat/PROJ-1".to_string(); // base = parent's branch let nodes = vec![parent, child]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); assert!( s.contains("stacked on PROJ-1"), "stack indicator missing in:\n{}", @@ -1011,7 +1031,7 @@ mod tests { mk_node("PROJ-1", Some("A"), "feat/PROJ-1", vec![]), mk_node("PROJ-2", Some("B"), "feat/PROJ-2", vec![]), ]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); // Summary header must be present and contain worktree count. assert!(s.starts_with("smartlog "), "summary header missing"); assert!(s.contains("2 worktrees"), "worktree count wrong: {}", s); @@ -1021,7 +1041,7 @@ mod tests { #[test] fn summary_header_singular_worktree() { let nodes = vec![mk_node("CL-1", Some("A"), "feat/CL-1", vec![])]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); assert!(s.contains("1 worktree ·"), "singular form wrong: {}", s); } @@ -1038,7 +1058,7 @@ mod tests { // feat/PROJ-Z, but with topo sort the child group should appear // directly after the parent group in the rendered output. let nodes = vec![parent, child]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); // Both groups must appear. assert!(s.contains("PROJ-Z"), "parent not rendered"); @@ -1067,7 +1087,7 @@ mod tests { child.base_branch = "feat/PA".to_string(); let nodes = vec![gp, parent, child]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); // GP sits on "main" (the mk_node default base), so the root label is // "main (base)", not "GP (base)". PA and CH are stacked and use the @@ -1089,7 +1109,7 @@ mod tests { b.base_branch = "develop".to_string(); let nodes = vec![a, b]; - let s = render_text(&nodes, false); + let s = render_text(&nodes, false, false); // Both roots (main, develop) should appear in the output. assert!(s.contains("main (base)") || s.contains("develop (base)")); assert!(s.contains("CL-A") && s.contains("CL-B")); @@ -1307,7 +1327,7 @@ mod tests { fn render_text_shows_ci_badge_line_when_ci_set() { let mut node = mk_node("CL-7", Some("G"), "feat/CL-7", vec![]); node.ci = Some(mk_ci("failed", 7, 2)); - let out = render_text(&[node], false); + let out = render_text(&[node], false, false); assert!( out.contains("[CI: ✗ failed (2/7)]"), "expected CI badge in render_text output: {}", @@ -1320,7 +1340,7 @@ mod tests { fn render_text_shows_running_ci_badge_with_accurate_count() { let mut node = mk_node("CL-8", Some("H"), "feat/CL-8", vec![]); node.ci = Some(mk_ci_running("running", 6, 0, 2)); - let out = render_text(&[node], false); + let out = render_text(&[node], false, false); assert!( out.contains("[CI: ● running (2 running/6)]"), "expected running CI badge with accurate count in: {}", @@ -1350,7 +1370,7 @@ mod tests { let mut node = mk_node("CL-9", Some("I"), "feat/CL-9", vec![]); assert!(node.pr.is_none(), "node must have no PR for this test"); node.ci = Some(mk_ci("passed", 4, 0)); - let out = render_text(&[node], false); + let out = render_text(&[node], false, false); assert!( out.contains("[CI: ✓ passed (4/4)]"), "PR-less node should render CI badge: {}", @@ -1365,7 +1385,7 @@ mod tests { fn ci_badge_none_renders_for_pr_less_node() { let mut node = mk_node("CL-10", Some("J"), "feat/CL-10", vec![]); node.ci = Some(mk_ci("none", 0, 0)); - let out = render_text(&[node], false); + let out = render_text(&[node], false, false); assert!( out.contains("[CI: none]"), "PR-less node with no checks should render [CI: none]: {}", @@ -1387,4 +1407,140 @@ mod tests { result ); } + + // ----------------------------------------------------------------------- + // Phase 6 tests: (no PR) display when overlay requested (#309 final) + // ----------------------------------------------------------------------- + + /// When GitHub overlay was requested and a worktree has no open PR, + /// `render_text` must emit a `├─ (no PR)` line so users see we looked. + #[test] + fn no_pr_line_shown_when_overlay_requested() { + let node = mk_node("CL-20", Some("Branch without PR"), "feat/CL-20", vec![]); + assert!(node.pr.is_none(), "node must not have a PR for this test"); + // overlay_requested = true → we attempted the lookup and found nothing. + let out = render_text(&[node], false, true); + assert!( + out.contains("(no PR)"), + "expected '(no PR)' line when overlay requested but no PR found: {}", + out + ); + } + + /// When `--no-overlay` is active (`overlay_requested = false`), the + /// `(no PR)` line must be suppressed — we never even called GitHub. + #[test] + fn no_pr_line_suppressed_when_overlay_not_requested() { + let node = mk_node("CL-21", Some("Offline node"), "feat/CL-21", vec![]); + assert!(node.pr.is_none()); + // overlay_requested = false → ––no-overlay mode; do not show (no PR). + let out = render_text(&[node], false, false); + assert!( + !out.contains("(no PR)"), + "(no PR) must be absent when overlay is disabled: {}", + out + ); + } + + /// A node with CI overlay (from branch-tip SHA lookup) but no PR should + /// show `(no PR)` above the CI badge when overlay was requested. + #[test] + fn no_pr_line_plus_ci_badge_for_pr_less_node_with_ci() { + let mut node = mk_node("CL-22", Some("CI only node"), "feat/CL-22", vec![]); + node.ci = Some(mk_ci("passed", 4, 0)); + assert!(node.pr.is_none()); + let out = render_text(&[node], false, true); + assert!( + out.contains("(no PR)"), + "(no PR) must appear even when CI overlay is set: {}", + out + ); + assert!( + out.contains("[CI: ✓ passed (4/4)]"), + "CI badge must still render: {}", + out + ); + // (no PR) must come before CI badge in the output string. + let no_pr_pos = out.find("(no PR)").unwrap(); + let ci_pos = out.find("[CI:").unwrap(); + assert!( + no_pr_pos < ci_pos, + "(no PR) must appear before CI badge:\n{}", + out + ); + } + + /// A node WITH a PR must not render the `(no PR)` line even when + /// `overlay_requested = true`. + #[test] + fn no_pr_line_absent_when_pr_overlay_is_set() { + let mut node = mk_node("CL-23", Some("Open PR"), "feat/CL-23", vec![]); + node.pr = Some(mk_overlay("open", "success", "approved")); + let out = render_text(&[node], false, true); + assert!( + !out.contains("(no PR)"), + "(no PR) must not appear when PR overlay is present: {}", + out + ); + assert!(out.contains("[PR #42"), "PR badge must appear: {}", out); + } + + // ----------------------------------------------------------------------- + // Phase 4 / #308 snapshot test: full golden-output comparison + // (Substitutes for an `insta` snapshot dep — same regression coverage + // without adding a new dev-dependency.) + // ----------------------------------------------------------------------- + + /// Full ASCII snapshot of a single worktree with a PR overlay and one + /// commit. This acts as the golden-output regression test required by + /// issue #308 acceptance criteria ("snapshot test >=1"). + #[test] + fn render_text_full_snapshot_single_node_with_pr_and_commit() { + let mut node = mk_node( + "CL-42", + Some("Add feature"), + "feat/CL-42", + vec![mk_commit("abc1234", "Implement feature")], + ); + node.pr = Some(mk_overlay("open", "success", "approved")); + // overlay_requested = true (we did ask GitHub; a PR was found) + let out = render_text(&[node], false, true); + // Build the expected output using the same Unicode glyphs as render_text. + let mut expected = String::new(); + expected.push_str("smartlog 1 worktree \u{00b7} 0 stacked\n"); + expected.push('\n'); + expected.push_str("\u{25cb} main (base)\n"); + expected.push_str("\u{2502}\n"); + expected.push_str("\u{2514}\u{2500}\u{25cf} CL-42 Add feature [feat/CL-42]\n"); + expected + .push_str(" \u{251c}\u{2500} [PR #42 \u{25cf} open \u{2713} CI \u{2713} approved]\n"); + expected.push_str(" \u{2514}\u{2500} abc1234 Implement feature\n"); + assert_eq!( + out, expected, + "snapshot mismatch \u{2014} golden output changed; update test if intentional.\nGOT:\n{}", + out + ); + } + + /// Full ASCII snapshot for a single worktree with NO PR and NO commits, + /// with `overlay_requested = true`. Ensures the `(no PR)` + empty-commit + /// placeholder are rendered in the correct order. + #[test] + fn render_text_full_snapshot_no_pr_no_commits() { + let node = mk_node("CL-43", Some("Empty branch"), "feat/CL-43", vec![]); + let out = render_text(&[node], false, true); + let mut expected = String::new(); + expected.push_str("smartlog 1 worktree \u{00b7} 0 stacked\n"); + expected.push('\n'); + expected.push_str("\u{25cb} main (base)\n"); + expected.push_str("\u{2502}\n"); + expected.push_str("\u{2514}\u{2500}\u{25cf} CL-43 Empty branch [feat/CL-43]\n"); + expected.push_str(" \u{251c}\u{2500} (no PR)\n"); + expected.push_str(" \u{2514}\u{2500} (no commits since main)\n"); + assert_eq!( + out, expected, + "snapshot mismatch \u{2014} golden output changed; update test if intentional.\nGOT:\n{}", + out + ); + } }