tests/interdiff_test.rs

Ref: main   Size: 16.6 KiB   History

//! Interdiff and per-revision diff behaviour.
//!
//! The defect these cover is a family: `interdiff` used to be a flat
//! tree-to-tree diff with no idea what base either revision stood on, so a
//! rebase between two revisions dragged every upstream commit rebased over
//! into the output and buried the author's actual response to review. The
//! related defects are the same mistake made at display time — recomputing a
//! merge-base when the answer was recorded at the time it was still true.

mod common;

use common::{write_raw_event, TestRepo};
use serde_json::json;

/// Every path a unified diff touches, sorted and deduplicated.
///
/// Asserted as an exact set rather than "contains the file I care about":
/// the regression being prevented is extra files, and a `contains` assertion
/// is blind to exactly that.
fn changed_files(diff: &str) -> Vec<String> {
    let mut paths: Vec<String> = diff
        .lines()
        .filter_map(|l| l.strip_prefix("diff --git a/"))
        .map(|rest| rest.split(" b/").next().unwrap_or(rest).to_string())
        .collect();
    paths.sort();
    paths.dedup();
    paths
}

fn create_patch(repo: &TestRepo, branch: &str, title: &str) -> String {
    let out = repo.run_ok(&["patch", "create", "-t", title, "-B", branch]);
    out.trim()
        .strip_prefix("Created patch ")
        .unwrap_or_else(|| panic!("unexpected patch create output: {}", out))
        .to_string()
}

/// A patch with one real edit, an upstream that advanced three commits while
/// it sat in review, and an author who rebased before answering the review.
///
/// This is the measured 75%-noise case: the flat tree diff emitted four files,
/// one real and three pure upstream churn.
fn rebase_over_busy_upstream() -> (TestRepo, String) {
    let repo = TestRepo::new("Alice", "alice@example.com");

    repo.git(&["checkout", "-b", "feat-rebase"]);
    repo.commit_file("feature.txt", "v1\n", "feature v1");
    let id = create_patch(&repo, "feat-rebase", "Rebase noise");

    // Upstream advances, touching only files the patch does not.
    repo.git(&["checkout", "main"]);
    repo.commit_file("up1.txt", "one\n", "upstream 1");
    repo.commit_file("up2.txt", "two\n", "upstream 2");
    repo.commit_file("up3.txt", "three\n", "upstream 3");

    // The author rebases, then answers the review.
    repo.git(&["checkout", "feat-rebase"]);
    repo.git(&["rebase", "main"]);
    repo.commit_file("feature.txt", "v2\n", "address review");
    repo.run_ok(&["patch", "revise", &id]);
    repo.git(&["checkout", "main"]);

    (repo, id)
}

// ===========================================================================
// 8db8d346 — interdiff must be rebase-aware
// ===========================================================================

#[test]
fn interdiff_across_a_rebase_shows_only_the_authors_changes() {
    let (repo, id) = rebase_over_busy_upstream();

    let out = repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);

    assert_eq!(
        changed_files(&out),
        vec!["feature.txt".to_string()],
        "interdiff across a rebase must exclude every upstream commit rebased \
         over; got:\n{}",
        out
    );
    assert!(
        out.contains("+v2"),
        "the author's actual change must survive: {}",
        out
    );
    assert!(
        !out.contains("up1.txt") && !out.contains("up2.txt") && !out.contains("up3.txt"),
        "upstream churn leaked into the interdiff: {}",
        out
    );
}

#[test]
fn interdiff_across_a_rebase_is_reversible() {
    let (repo, id) = rebase_over_busy_upstream();

    // Asking for 2..1 is the reverse diff, not a different comparison. The
    // newer revision is still the one shown as recorded.
    let out = repo.run_ok(&["patch", "diff", &id, "--between", "2", "1"]);

    assert_eq!(changed_files(&out), vec!["feature.txt".to_string()]);
    assert!(
        out.contains("+v1"),
        "reversed interdiff should restore v1: {}",
        out
    );
}

#[test]
fn squashing_without_changing_the_tree_gives_an_empty_interdiff() {
    let repo = TestRepo::new("Alice", "alice@example.com");

    repo.git(&["checkout", "-b", "feat-squash"]);
    repo.commit_file("a.txt", "a\n", "first");
    repo.commit_file("b.txt", "b\n", "second");
    let id = create_patch(&repo, "feat-squash", "Squash");

    // Squash both commits into one. Same tree, different commit.
    repo.git(&["reset", "--soft", "main"]);
    repo.git(&["commit", "-m", "squashed"]);
    repo.run_ok(&["patch", "revise", &id]);
    repo.git(&["checkout", "main"]);

    let out = repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);

    assert!(
        changed_files(&out).is_empty(),
        "a pure squash changes no code, so the interdiff must be empty: {}",
        out
    );
    assert!(
        out.contains("No diff available"),
        "an empty interdiff should say so rather than print nothing: {}",
        out
    );
}

#[test]
fn interdiff_reports_a_conflict_rather_than_a_misleading_diff() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    repo.commit_file("shared.txt", "original\n", "add shared");

    repo.git(&["checkout", "-b", "feat-conflict"]);
    repo.commit_file("shared.txt", "feature version\n", "feature edits shared");
    let id = create_patch(&repo, "feat-conflict", "Conflicting rebase");

    // Upstream rewrites the same line.
    repo.git(&["checkout", "main"]);
    repo.commit_file("shared.txt", "upstream version\n", "upstream edits shared");

    // The author rebases onto that and resolves in favour of the feature.
    repo.git(&["checkout", "feat-conflict"]);
    repo.git(&["reset", "--hard", "main"]);
    repo.commit_file(
        "shared.txt",
        "feature version, revised\n",
        "rebased and revised",
    );
    repo.run_ok(&["patch", "revise", &id]);
    repo.git(&["checkout", "main"]);

    let err = repo.run_err(&["patch", "diff", &id, "--between", "1", "2"]);

    assert!(
        err.contains("conflict"),
        "a conflicting replay must be reported as a conflict: {}",
        err
    );
    assert!(
        err.contains("shared.txt"),
        "the conflict should name the file: {}",
        err
    );
}

// ===========================================================================
// 243b7bc0 — differing bases must be disclosed
// ===========================================================================

#[test]
fn interdiff_discloses_that_the_bases_differ() {
    let (repo, id) = rebase_over_busy_upstream();

    let shown = repo.run_ok(&["patch", "show", &id, "--json"]);
    let json: serde_json::Value = serde_json::from_str(&shown).unwrap();
    let revisions = json["revisions"].as_array().unwrap();
    let base1 = revisions[0]["base"].as_str().unwrap().to_string();
    let base2 = revisions[1]["base"].as_str().unwrap().to_string();
    assert_ne!(base1, base2, "the fixture must actually rebase");

    let out = repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);

    assert!(
        out.contains("bases differ"),
        "a reviewer cannot tell noise from signal unless the rebase is \
         disclosed: {}",
        out
    );
    assert!(
        out.contains(&base1[..8]) && out.contains(&base2[..8]),
        "the disclosure should name both bases: {}",
        out
    );
}

#[test]
fn interdiff_on_a_shared_base_says_nothing_about_bases() {
    let repo = TestRepo::new("Alice", "alice@example.com");

    repo.git(&["checkout", "-b", "feat-same-base"]);
    repo.commit_file("a.txt", "a\n", "v1");
    let id = create_patch(&repo, "feat-same-base", "Same base");
    repo.commit_file("a.txt", "a2\n", "v2");
    repo.run_ok(&["patch", "revise", &id]);
    repo.git(&["checkout", "main"]);

    let out = repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);

    assert_eq!(changed_files(&out), vec!["a.txt".to_string()]);
    assert!(
        !out.contains("bases differ"),
        "an unrebased interdiff is already exactly the author's changes and \
         needs no caveat: {}",
        out
    );
}

#[test]
fn patch_show_displays_each_revisions_base() {
    let (repo, id) = rebase_over_busy_upstream();

    let shown = repo.run_ok(&["patch", "show", &id, "--json"]);
    let json: serde_json::Value = serde_json::from_str(&shown).unwrap();
    let base2 = json["revisions"][1]["base"].as_str().unwrap().to_string();

    let out = repo.run_ok(&["patch", "show", &id]);
    assert!(
        out.contains(&base2[..8]),
        "`patch show` must display the base each revision stands on: {}",
        out
    );
}

// ===========================================================================
// 40001f77 — --between on a single-revision patch
// ===========================================================================

#[test]
fn between_on_a_single_revision_patch_says_so() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    let id = repo.patch_create("Solo");

    let err = repo.run_err(&["patch", "diff", &id, "--between", "1"]);

    assert!(
        err.contains("1 revision"),
        "an empty diff reads as 'these revisions are identical'; the truth is \
         'there is no second revision': {}",
        err
    );
}

#[test]
fn between_a_revision_and_itself_says_so() {
    let repo = TestRepo::new("Alice", "alice@example.com");

    repo.git(&["checkout", "-b", "feat-self"]);
    repo.commit_file("a.txt", "a\n", "v1");
    let id = create_patch(&repo, "feat-self", "Self compare");
    repo.commit_file("a.txt", "a2\n", "v2");
    repo.run_ok(&["patch", "revise", &id]);
    repo.git(&["checkout", "main"]);

    let err = repo.run_err(&["patch", "diff", &id, "--between", "2", "2"]);
    assert!(
        err.contains("2 revisions"),
        "comparing a revision with itself should name how many exist: {}",
        err
    );
}

// ===========================================================================
// 57575b50 — a merged patch's historical diff stays readable
// ===========================================================================

#[test]
fn revision_diff_survives_the_patch_being_merged() {
    let repo = TestRepo::new("Alice", "alice@example.com");

    repo.git(&["checkout", "-b", "feat-merged"]);
    repo.commit_file("a.txt", "hello\n", "add a");
    let id = create_patch(&repo, "feat-merged", "Merged patch");

    let before = repo.run_ok(&["patch", "diff", &id, "--revision", "1"]);
    assert_eq!(changed_files(&before), vec!["a.txt".to_string()]);

    repo.git(&["checkout", "main"]);
    repo.git(&["merge", "--ff-only", "feat-merged"]);

    let after = repo.run_ok(&["patch", "diff", &id, "--revision", "1"]);
    assert_eq!(
        changed_files(&after),
        vec!["a.txt".to_string()],
        "a fast-forward merge collapses the recomputed merge-base onto the \
         revision itself; the recorded base must be used instead: {}",
        after
    );
    assert!(after.contains("+hello"), "{}", after);
}

#[test]
fn patch_diff_survives_the_patch_being_merged() {
    let repo = TestRepo::new("Alice", "alice@example.com");

    repo.git(&["checkout", "-b", "feat-merged-head"]);
    repo.commit_file("a.txt", "hello\n", "add a");
    let id = create_patch(&repo, "feat-merged-head", "Merged patch head");

    repo.git(&["checkout", "main"]);
    repo.git(&["merge", "--ff-only", "feat-merged-head"]);

    let out = repo.run_ok(&["patch", "diff", &id]);
    assert_eq!(
        changed_files(&out),
        vec!["a.txt".to_string()],
        "the head diff is lost to the same recompute: {}",
        out
    );
}

// ===========================================================================
// Revisions with no recorded base degrade, they do not fail
//
// Not a migration case and never was: a revision written before `base` was
// stored genuinely has none, nothing can recover it, and "unknown" is a state
// this tool has to be able to express and say out loud. Issue `e5096ffc`
// stripped the pre-release compatibility around it and deliberately kept this.
// ===========================================================================

#[test]
fn revisions_without_a_recorded_base_still_diff_and_say_the_base_is_unknown() {
    let work = TestRepo::new("Alice", "alice@example.com");

    work.git(&["checkout", "-b", "legacy"]);
    work.commit_file("x.txt", "one\n", "c1");
    let c1 = work.git(&["rev-parse", "HEAD"]).trim().to_string();
    let t1 = work.git(&["rev-parse", "HEAD^{tree}"]).trim().to_string();
    work.commit_file("y.txt", "two\n", "c2");
    let c2 = work.git(&["rev-parse", "HEAD"]).trim().to_string();
    let t2 = work.git(&["rev-parse", "HEAD^{tree}"]).trim().to_string();
    work.git(&["checkout", "main"]);

    let repo = git2::Repository::open(work.dir.path()).unwrap();
    let root = write_raw_event(
        &repo,
        None,
        json!({
            "type": "patch.create",
            "title": "Legacy without a base",
            "body": "",
            "base_ref": "main",
            "branch": "legacy",
            "commit": c1,
            "tree": t1,
        }),
        1,
    );
    let tip = write_raw_event(
        &repo,
        Some(root),
        json!({
            "type": "patch.revision",
            "commit": c2,
            "tree": t2,
        }),
        2,
    );
    let id = root.to_string();
    // The current ref layout. What makes this fixture the case under test is
    // that neither event carries a `base` key — not the shape of its refs.
    repo.reference(
        &format!("refs/collab/patches/{}/events", id),
        tip,
        false,
        "events",
    )
    .unwrap();
    drop(repo);

    let out = work.run_ok(&["patch", "diff", &id[..8], "--between", "1", "2"]);

    assert_eq!(
        changed_files(&out),
        vec!["y.txt".to_string()],
        "old patches must degrade to the flat tree diff, not fail: {}",
        out
    );
    assert!(
        out.contains("no base recorded"),
        "an undisclosed unknown base is the misleading case this family of \
         defects is about: {}",
        out
    );
}

// ===========================================================================
// Rendering a diff is a read
// ===========================================================================

fn count_objects(dir: &std::path::Path) -> usize {
    let mut n = 0;
    if let Ok(entries) = std::fs::read_dir(dir) {
        for e in entries.flatten() {
            let p = e.path();
            if p.is_dir() {
                n += count_objects(&p);
            } else {
                n += 1;
            }
        }
    }
    n
}

/// A rebase where upstream and the patch edit the same file in different
/// regions. The replay has to auto-merge rather than reuse a blob wholesale,
/// which is the case that would otherwise tempt an implementation into writing
/// a merged tree to the object database in the middle of a read.
#[test]
fn an_auto_merging_replay_shows_only_the_authors_hunk_and_writes_nothing() {
    let repo = TestRepo::new("Alice", "alice@example.com");
    repo.commit_file("both.txt", "a\nb\nc\nd\ne\nf\ng\n", "seed");
    repo.git(&["checkout", "-b", "feat-automerge"]);
    repo.commit_file(
        "both.txt",
        "a\nb\nc\nd\ne\nf\nGGG\n",
        "patch edits the tail",
    );
    let id = create_patch(&repo, "feat-automerge", "Auto-merging replay");
    repo.git(&["checkout", "main"]);
    repo.commit_file(
        "both.txt",
        "AAA\nb\nc\nd\ne\nf\ng\n",
        "upstream edits the head",
    );
    repo.git(&["checkout", "feat-automerge"]);
    repo.git(&["rebase", "main"]);
    repo.commit_file("both.txt", "AAA\nb\nc\nd\ne\nf\nHHH\n", "address review");
    repo.run_ok(&["patch", "revise", &id]);
    repo.git(&["checkout", "main"]);

    let objects = repo.dir.path().join(".git/objects");
    let before = count_objects(&objects);
    let out = repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);
    let after = count_objects(&objects);

    assert_eq!(changed_files(&out), vec!["both.txt".to_string()], "{}", out);
    assert!(
        out.contains("-GGG") && out.contains("+HHH"),
        "the author's hunk must be the one shown: {}",
        out
    );
    assert!(
        !out.contains("AAA"),
        "upstream's hunk in the same file must cancel: {}",
        out
    );
    assert_eq!(
        before, after,
        "replaying a revision is done in memory; rendering a diff must not add \
         objects to the object database"
    );
}

#[test]
fn rendering_a_diff_moves_no_refs() {
    let (repo, id) = rebase_over_busy_upstream();

    let before = repo.git(&["for-each-ref", "refs/collab"]);
    repo.run_ok(&["patch", "diff", &id]);
    repo.run_ok(&["patch", "diff", &id, "--revision", "1"]);
    repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);
    let after = repo.git(&["for-each-ref", "refs/collab"]);

    assert_eq!(
        before, after,
        "rendering a diff must not append events or move refs"
    );
}