tests/timeline_test.rs
Ref: Size: 16.0 KiB History
//! `patch log --timeline`: the unified event timeline for a patch.
//!
//! `patch log` lists revisions; `patch show` lists comments and reviews. Neither
//! answers the question a reviewer actually asks — *what answered what*. A
//! comment on revision 1 followed by revision 2 is the causal chain this
//! project's revision anchoring exists to record, and reconstructing it by
//! interleaving two commands' output by eye is exactly the work the tool should
//! be doing.
//!
//! These tests pin three things: that the sequence is rendered in order with
//! every event kind in it, that each entry names the revision it is anchored to,
//! and that rendering it writes nothing.
mod common;
use common::TestRepo;
/// Build a patch with a full review round-trip on it:
///
/// r1 -> comment -> request-changes -> r2 -> approve -> merged
///
/// Returns the patch's short ID.
fn patch_with_a_review_round(repo: &TestRepo) -> String {
repo.git(&["checkout", "-b", "feat-timeline"]);
repo.commit_file("a.txt", "first", "v1");
let out = repo.run_ok(&[
"patch",
"create",
"-t",
"Timeline patch",
"-B",
"feat-timeline",
]);
let id = out
.trim()
.strip_prefix("Created patch ")
.unwrap()
.to_string();
repo.run_ok(&["patch", "comment", &id, "-b", "needs a test"]);
repo.run_ok(&[
"patch",
"review",
&id,
"-v",
"request-changes",
"-b",
"not yet",
]);
repo.git(&["checkout", "feat-timeline"]);
repo.commit_file("b.txt", "second", "v2");
repo.run_ok(&["patch", "revise", &id, "-b", "added the test"]);
repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "looks good"]);
repo.git(&["checkout", "main"]);
id
}
/// The whole point: one command, every event kind, in the order they happened.
#[test]
fn timeline_interleaves_revisions_comments_and_reviews_in_order() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
// Every kind of event is present. `patch log` alone shows only the
// revisions; `patch show` alone shows only the comment and the reviews.
assert!(out.contains("r1"), "timeline must show revision 1: {}", out);
assert!(
out.contains("needs a test"),
"timeline must show the comment: {}",
out
);
assert!(
out.contains("request-changes"),
"timeline must show the review verdict: {}",
out
);
assert!(out.contains("r2"), "timeline must show revision 2: {}", out);
assert!(
out.contains("approve"),
"timeline must show the approval: {}",
out
);
// ...and in the order they happened. This is the assertion that makes it a
// timeline rather than two lists printed one after the other.
let at = |needle: &str| {
out.find(needle)
.unwrap_or_else(|| panic!("{:?} missing from timeline:\n{}", needle, out))
};
assert!(
at("needs a test") < at("request-changes"),
"the comment came before the review:\n{}",
out
);
assert!(
at("request-changes") < at("added the test"),
"revision 2 answered the request for changes and must follow it:\n{}",
out
);
assert!(
at("added the test") < at("looks good"),
"the approval came after revision 2:\n{}",
out
);
}
/// A timeline that does not say which revision a comment was made against
/// cannot answer "what answered what" — it is just a sorted list.
#[test]
fn timeline_anchors_each_event_to_its_revision() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
let line = |needle: &str| {
out.lines()
.find(|l| l.contains(needle))
.unwrap_or_else(|| panic!("{:?} missing from timeline:\n{}", needle, out))
};
assert!(
line("not yet").contains("@r1"),
"the request-changes review was made against r1: {:?}",
line("not yet")
);
assert!(
line("looks good").contains("@r2"),
"the approval was made against r2: {:?}",
line("looks good")
);
}
/// A thread comment carries no revision of its own, so it is anchored to the
/// revision that was current where it sits in the sequence. Without that, the
/// view answers "what answered what" for reviews only, and a thread comment —
/// the most common kind — floats free of the round it belongs to.
#[test]
fn timeline_anchors_a_thread_comment_to_the_revision_it_was_written_on() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
// Written after r2 exists, so it belongs to r2, not to r1.
repo.run_ok(&["patch", "comment", &id, "-b", "one more thought"]);
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
let line = out
.lines()
.find(|l| l.contains("one more thought"))
.unwrap_or_else(|| panic!("comment missing:\n{}", out));
assert!(
line.contains("@r2"),
"a comment written after r2 is anchored to r2: {:?}",
line
);
// And the earlier one is still anchored to r1 — the anchor tracks position
// in the sequence, not simply the latest revision.
let earlier = out
.lines()
.find(|l| l.contains("needs a test"))
.unwrap_or_else(|| panic!("earlier comment missing:\n{}", out));
assert!(
earlier.contains("@r1"),
"a comment written before r2 stays anchored to r1: {:?}",
earlier
);
}
/// The merge is the last beat of the story, and for a squash or a rebase-merge
/// the recorded commit is the only route from the patch back to the code.
#[test]
fn timeline_ends_with_the_recorded_merge() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
repo.git(&["merge", "--no-ff", "-m", "land it", "feat-timeline"]);
let landed = repo.git(&["rev-parse", "HEAD"]).trim().to_string();
repo.run_ok(&["patch", "merge", &id]);
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
let last = out.lines().rfind(|l| !l.trim().is_empty()).unwrap();
assert!(
last.contains("merged"),
"the timeline ends with the merge: {:?}",
last
);
assert!(
last.contains(&landed[..8]),
"the merge entry names the commit that landed the patch: {:?}",
last
);
}
/// Scripted callers need the same sequence without parsing columns.
#[test]
fn timeline_json_carries_the_same_sequence() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
let out = repo.run_ok(&["patch", "log", &id, "--timeline", "--json"]);
let entries: Vec<serde_json::Value> = serde_json::from_str(&out).unwrap();
let kinds: Vec<&str> = entries
.iter()
.map(|e| e["type"].as_str().unwrap())
.collect();
assert_eq!(
kinds,
vec!["revision", "comment", "review", "revision", "review"],
"timeline JSON: {}",
out
);
assert_eq!(entries[0]["revision"], 1);
assert_eq!(entries[1]["revision"], 1, "the comment was made against r1");
assert_eq!(entries[2]["revision"], 1, "the review was made against r1");
assert_eq!(entries[3]["revision"], 2);
assert_eq!(
entries[4]["revision"], 2,
"the approval was made against r2"
);
assert_eq!(entries[2]["verdict"], "request-changes");
assert_eq!(entries[4]["verdict"], "approve");
assert_eq!(entries[1]["author"]["email"], "alice@example.com");
}
/// The governing constraint of this project: surfacing recorded facts is a pure
/// read. A view that appends an event or moves a ref corrupts the record it
/// claims to be showing.
///
/// Note `patch show` deliberately moves `refs/collab/local/seen/patches/<id>`,
/// a purely local read-marker that is not a collab event. The timeline does
/// *not* extend that pattern: it moves nothing at all, which is what the
/// `refs/collab/` sweep below pins.
#[test]
fn rendering_a_timeline_writes_nothing() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
repo.git(&["merge", "--no-ff", "-m", "land it", "feat-timeline"]);
repo.run_ok(&["patch", "merge", &id]);
let refs_before = repo.git(&["for-each-ref", "--format=%(refname) %(objectname)"]);
repo.run_ok(&["patch", "log", &id, "--timeline"]);
repo.run_ok(&["patch", "log", &id, "--timeline", "--json"]);
let refs_after = repo.git(&["for-each-ref", "--format=%(refname) %(objectname)"]);
assert_eq!(
refs_before, refs_after,
"rendering a timeline must not create, move or delete any ref"
);
}
// ---------------------------------------------------------------------------
// Corrections
// ---------------------------------------------------------------------------
//
// A `BodyEdit` supersedes an earlier body and a `CommentDelete` tombstones one.
// Both are events in the DAG, appended rather than applied in place, precisely
// so the record stays an audit trail rather than a summary. A timeline that
// quietly showed the corrected text at the original position and nothing else
// would turn it back into a summary — so a correction gets its own slot, at the
// point in the sequence where it actually happened.
/// The 8-char event ID `patch show` prints for the first thread comment.
fn first_comment_id(repo: &TestRepo, patch: &str) -> String {
let show = repo.run_ok(&["patch", "show", patch]);
let comments = show
.split_once("--- Comments ---")
.unwrap_or_else(|| panic!("no comments section in:\n{}", show))
.1;
let start = comments.find('[').unwrap() + 1;
let end = comments[start..].find(']').unwrap() + start;
comments[start..end].to_string()
}
#[test]
fn timeline_records_an_edit_as_its_own_event() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
let comment_id = first_comment_id(&repo, &id);
repo.run_ok(&[
"patch",
"edit-comment",
&id,
&comment_id,
"-b",
"needs a test, and a changelog entry",
]);
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
// The comment keeps its original slot, carrying the corrected text and
// saying that it was corrected.
let comment_line = out
.lines()
.find(|l| l.contains("needs a test, and a changelog entry"))
.unwrap_or_else(|| panic!("corrected comment missing:\n{}", out));
assert!(
comment_line.contains("(edited)"),
"a corrected comment must say so: {:?}",
comment_line
);
// ...and the correction is its own entry, later in the sequence, naming
// the comment it corrected.
let edit_line = out
.lines()
.find(|l| l.contains("edited") && l.contains(&comment_id))
.unwrap_or_else(|| panic!("the edit itself is missing from the timeline:\n{}", out));
assert!(
out.find(comment_line).unwrap() < out.find(edit_line).unwrap(),
"the edit happened after the comment it corrected:\n{}",
out
);
}
#[test]
fn timeline_keeps_a_deleted_comments_slot_and_records_the_deletion() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
let comment_id = first_comment_id(&repo, &id);
repo.run_ok(&["patch", "delete-comment", &id, &comment_id]);
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
assert!(
!out.contains("needs a test"),
"a tombstone drops the text, it does not merely hide it:\n{}",
out
);
assert!(
out.contains("[deleted]"),
"the deleted comment keeps its slot, marked as a tombstone:\n{}",
out
);
let delete_line = out
.lines()
.find(|l| l.contains("deleted") && l.contains(&comment_id))
.unwrap_or_else(|| panic!("the deletion itself is missing:\n{}", out));
assert!(
out.find("[deleted]").unwrap() < out.find(delete_line).unwrap(),
"the deletion happened after the comment it removed:\n{}",
out
);
}
/// The fold ignores a correction whose author is not the author of the event it
/// names — anyone holding the DAG can append anything to it, so the rule that
/// nobody rewrites somebody else's words holds where state is derived.
///
/// The timeline has to honour the same rule. Listing every `BodyEdit` event it
/// finds would show a forged edit to someone else's words as though it had
/// happened, which is worse than not having a timeline at all.
#[test]
fn timeline_ignores_a_correction_the_fold_refused() {
use git_collab::dag;
use git_collab::event::Action;
// The patch's comments are Bob's, so an edit authored by Alice — which is
// what `write_raw_event` signs — is one author correcting another.
let repo = TestRepo::new("Bob", "bob@example.com");
let id = patch_with_a_review_round(&repo);
let git_repo = git2::Repository::open(repo.dir.path()).unwrap();
let events_ref = format!(
"refs/collab/patches/{}/events",
git_repo
.references_glob("refs/collab/patches/*/events")
.unwrap()
.filter_map(|r| r.ok())
.find_map(|r| {
let name = r.name()?.to_string();
let full = name
.strip_prefix("refs/collab/patches/")?
.strip_suffix("/events")?;
full.starts_with(&id).then(|| full.to_string())
})
.expect("patch events ref")
);
// The OID of Bob's thread comment: what a forged edit would name.
let target = dag::walk_events(&git_repo, &events_ref)
.unwrap()
.into_iter()
.find(|(_, e)| matches!(e.action, Action::PatchComment { .. }))
.map(|(oid, _)| oid.to_string())
.expect("thread comment event");
let tip = git_repo.refname_to_id(&events_ref).unwrap();
let forged = common::write_raw_event(
&git_repo,
Some(tip),
serde_json::json!({
"type": "body.edit",
"target": target,
"body": "words Bob never wrote",
}),
99,
);
git_repo
.reference(&events_ref, forged, true, "forged edit")
.unwrap();
let out = repo.run_ok(&["patch", "log", &id, "--timeline"]);
assert!(
!out.contains("words Bob never wrote"),
"a refused correction must not appear in the timeline:\n{}",
out
);
assert!(
out.contains("needs a test"),
"the original comment stands unchanged:\n{}",
out
);
assert!(
!out.contains("(edited)"),
"nothing was in fact edited:\n{}",
out
);
}
/// `patch log` and `patch log --json` are consumed by scripts and by the
/// existing suite. The timeline is strictly additive: without the flag, nothing
/// about either changes.
#[test]
fn timeline_flag_does_not_change_default_patch_log_output() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = patch_with_a_review_round(&repo);
let plain = repo.run_ok(&["patch", "log", &id]);
assert!(plain.contains("r1"), "{}", plain);
assert!(plain.contains("r2"), "{}", plain);
assert!(
!plain.contains("needs a test"),
"default patch log stays a revision log, with no comments in it: {}",
plain
);
assert!(
!plain.contains("request-changes"),
"default patch log stays a revision log, with no reviews in it: {}",
plain
);
// Default --json is still the bare revision array.
let json = repo.run_ok(&["patch", "log", &id, "--json"]);
let revisions: Vec<serde_json::Value> = serde_json::from_str(&json).unwrap();
assert_eq!(revisions.len(), 2);
assert_eq!(revisions[0]["number"], 1);
assert!(
revisions[0].get("type").is_none(),
"default --json keeps the revision shape, not the timeline shape: {}",
json
);
}