Skip to content

feat(smartlog): Phase 6 — (no PR) display + full snapshot tests · Closes #309 Closes #308 #422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erishforG merged 1 commit into from
Sep 4, 2026
Merged

feat(smartlog): Phase 6 — (no PR) display + full snapshot tests · Closes #309 Closes #308 #422

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 173 additions & 17 deletions src/cli/commands/smartlog.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 _;
Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -523,7 +534,12 @@ fn parse_commit_line(line: &str) -> Option<CommitSummary> {
///
/// `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 <ticket>` to create one.\n".to_string();
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"));
}

Expand All @@ -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"));
Expand All @@ -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)"));
}
Expand All @@ -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.
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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{}",
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand All @@ -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");
Expand Down Expand Up @@ -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
Expand All @@ -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"));
Expand Down Expand Up @@ -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: {}",
Expand All @@ -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: {}",
Expand Down Expand Up @@ -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: {}",
Expand All @@ -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]: {}",
Expand All @@ -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
);
}
}
Loading