a73x

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"
    );
}