tests/review_test.rs
Ref: Size: 9.7 KiB History
mod common;
use common::TestRepo;
use common::{alice, bob, init_repo, test_signing_key};
use git_collab::dag;
use git_collab::event::{Action, Event, ReviewVerdict};
use git_collab::state::PatchState;
use tempfile::TempDir;
// ===========================================================================
// Review supersession: one current vote per (author, revision)
// ===========================================================================
fn setup_patch_dag(repo: &git2::Repository) -> &'static str {
let sk = test_signing_key();
let main_oid = repo.refname_to_id("refs/heads/main").unwrap();
let initial_commit = repo.find_commit(main_oid).unwrap();
repo.branch("test-branch", &initial_commit, false).unwrap();
let create = Event {
timestamp: "2026-01-01T00:00:00Z".to_string(),
author: alice(),
action: Action::PatchCreate {
title: "test patch".to_string(),
body: "body".to_string(),
base_ref: "main".to_string(),
branch: "test-branch".to_string(),
fixes: None,
commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
base_commit: None,
},
clock: 0,
};
let root_oid = dag::create_root_event(repo, &create, &sk).unwrap();
let local_ref = "refs/collab/patches/test-patch";
repo.reference(local_ref, root_oid, false, "test").unwrap();
local_ref
}
fn review_event(
author: git_collab::event::Author,
verdict: ReviewVerdict,
body: &str,
revision: u32,
ts: &str,
) -> Event {
Event {
timestamp: ts.to_string(),
author,
action: Action::PatchReview {
verdict,
body: body.to_string(),
revision: Some(revision),
},
clock: 0,
}
}
#[test]
fn duplicate_vote_same_author_same_revision_collapses_to_latest() {
let dir = TempDir::new().unwrap();
let repo = init_repo(dir.path(), &alice());
let sk = test_signing_key();
let ref_name = setup_patch_dag(&repo);
let e1 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm",
1,
"2026-01-02T00:00:00Z",
);
let e2 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm again",
1,
"2026-01-03T00:00:00Z",
);
dag::append_event(&repo, ref_name, &e1, &sk).unwrap();
dag::append_event(&repo, ref_name, &e2, &sk).unwrap();
let state = PatchState::from_ref(&repo, ref_name, "test-patch").unwrap();
assert_eq!(
state.reviews.len(),
1,
"duplicate approvals should collapse"
);
assert_eq!(state.reviews[0].body, "lgtm again", "latest review wins");
assert_eq!(state.reviews[0].verdict, ReviewVerdict::Approve);
}
#[test]
fn changed_vote_same_author_same_revision_supersedes() {
let dir = TempDir::new().unwrap();
let repo = init_repo(dir.path(), &alice());
let sk = test_signing_key();
let ref_name = setup_patch_dag(&repo);
let e1 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm",
1,
"2026-01-02T00:00:00Z",
);
let e2 = review_event(
bob(),
ReviewVerdict::RequestChanges,
"wait, found a bug",
1,
"2026-01-03T00:00:00Z",
);
dag::append_event(&repo, ref_name, &e1, &sk).unwrap();
dag::append_event(&repo, ref_name, &e2, &sk).unwrap();
let state = PatchState::from_ref(&repo, ref_name, "test-patch").unwrap();
assert_eq!(state.reviews.len(), 1, "new vote should supersede old one");
assert_eq!(state.reviews[0].verdict, ReviewVerdict::RequestChanges);
assert_eq!(state.reviews[0].body, "wait, found a bug");
}
#[test]
fn votes_on_different_revisions_are_kept() {
let dir = TempDir::new().unwrap();
let repo = init_repo(dir.path(), &alice());
let sk = test_signing_key();
let ref_name = setup_patch_dag(&repo);
let e1 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm rev 1",
1,
"2026-01-02T00:00:00Z",
);
let e2 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm rev 2",
2,
"2026-01-03T00:00:00Z",
);
dag::append_event(&repo, ref_name, &e1, &sk).unwrap();
dag::append_event(&repo, ref_name, &e2, &sk).unwrap();
let state = PatchState::from_ref(&repo, ref_name, "test-patch").unwrap();
assert_eq!(
state.reviews.len(),
2,
"votes on different revisions both count"
);
}
#[test]
fn votes_from_different_authors_are_kept() {
let dir = TempDir::new().unwrap();
let repo = init_repo(dir.path(), &alice());
let sk = test_signing_key();
let ref_name = setup_patch_dag(&repo);
let e1 = review_event(
alice(),
ReviewVerdict::Approve,
"lgtm",
1,
"2026-01-02T00:00:00Z",
);
let e2 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm too",
1,
"2026-01-03T00:00:00Z",
);
dag::append_event(&repo, ref_name, &e1, &sk).unwrap();
dag::append_event(&repo, ref_name, &e2, &sk).unwrap();
let state = PatchState::from_ref(&repo, ref_name, "test-patch").unwrap();
assert_eq!(
state.reviews.len(),
2,
"different authors' votes both count"
);
}
#[test]
fn comment_verdict_reviews_always_append() {
let dir = TempDir::new().unwrap();
let repo = init_repo(dir.path(), &alice());
let sk = test_signing_key();
let ref_name = setup_patch_dag(&repo);
let e1 = review_event(
bob(),
ReviewVerdict::Comment,
"first thought",
1,
"2026-01-02T00:00:00Z",
);
let e2 = review_event(
bob(),
ReviewVerdict::Comment,
"second thought",
1,
"2026-01-03T00:00:00Z",
);
dag::append_event(&repo, ref_name, &e1, &sk).unwrap();
dag::append_event(&repo, ref_name, &e2, &sk).unwrap();
let state = PatchState::from_ref(&repo, ref_name, "test-patch").unwrap();
assert_eq!(
state.reviews.len(),
2,
"comment-verdict reviews are not votes; keep all"
);
}
#[test]
fn vote_does_not_supersede_comment_verdict_review() {
let dir = TempDir::new().unwrap();
let repo = init_repo(dir.path(), &alice());
let sk = test_signing_key();
let ref_name = setup_patch_dag(&repo);
let e1 = review_event(
bob(),
ReviewVerdict::Comment,
"just a note",
1,
"2026-01-02T00:00:00Z",
);
let e2 = review_event(
bob(),
ReviewVerdict::Approve,
"lgtm",
1,
"2026-01-03T00:00:00Z",
);
dag::append_event(&repo, ref_name, &e1, &sk).unwrap();
dag::append_event(&repo, ref_name, &e2, &sk).unwrap();
let state = PatchState::from_ref(&repo, ref_name, "test-patch").unwrap();
assert_eq!(
state.reviews.len(),
2,
"vote should not replace a comment-verdict review"
);
}
// ===========================================================================
// CLI guard: re-submitting the same verdict for the same revision errors
// ===========================================================================
#[test]
fn cli_duplicate_approve_same_revision_errors() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = repo.patch_create("Dup approve");
repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM"]);
let err = repo.run_err(&["patch", "review", &id, "-v", "approve", "-b", "LGTM again"]);
assert!(
err.contains("already"),
"expected duplicate-approve error, got: {}",
err
);
let out = repo.run_ok(&["patch", "show", &id, "--json"]);
let json: serde_json::Value = serde_json::from_str(&out).unwrap();
assert_eq!(json["reviews"].as_array().unwrap().len(), 1);
}
#[test]
fn cli_changing_verdict_same_revision_is_allowed() {
let repo = TestRepo::new("Alice", "alice@example.com");
let id = repo.patch_create("Change verdict");
repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM"]);
repo.run_ok(&[
"patch",
"review",
&id,
"-v",
"request-changes",
"-b",
"actually, please fix X",
]);
let out = repo.run_ok(&["patch", "show", &id, "--json"]);
let json: serde_json::Value = serde_json::from_str(&out).unwrap();
let reviews = json["reviews"].as_array().unwrap();
assert_eq!(reviews.len(), 1, "changed vote supersedes the old one");
assert_eq!(reviews[0]["verdict"], "request-changes");
}
#[test]
fn cli_re_approving_new_revision_is_allowed() {
let repo = TestRepo::new("Alice", "alice@example.com");
repo.git(&["checkout", "-b", "feat-reapprove"]);
repo.commit_file("v1.txt", "v1", "initial commit");
let out = repo.run_ok(&["patch", "create", "-t", "Reapprove", "-B", "feat-reapprove"]);
let id = out
.trim()
.strip_prefix("Created patch ")
.unwrap()
.to_string();
repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM rev 1"]);
// New commit, recorded as a revision by the author — nothing else records
// one on their behalf.
repo.git(&["checkout", "feat-reapprove"]);
repo.commit_file("v2.txt", "v2", "second commit");
repo.run_ok(&["patch", "revise", &id]);
repo.git(&["checkout", "main"]);
repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM rev 2"]);
let out = repo.run_ok(&["patch", "show", &id, "--json"]);
let json: serde_json::Value = serde_json::from_str(&out).unwrap();
let reviews = json["reviews"].as_array().unwrap();
assert_eq!(
reviews.len(),
2,
"approvals on different revisions both count"
);
}