tests/sync_diagnostics_test.rs
Ref: Size: 12.0 KiB History
//! Sync diagnostics: telling the user something true about a failed push,
//! and not lying to them about `init` having done something.
//!
//! Both tests here drive the real CLI against a real local bare remote. The
//! refname-conflict case in particular is built end to end — an actual stale
//! bare ref planted on the remote, an actual `git push` rejected by an actual
//! `receive-pack` — because the whole point of the diagnostic is that it keys
//! off git's own wording. A test that fed a hand-written error string into the
//! classifier would only prove the classifier matches its own constant, and
//! would keep passing on the day git rephrases the rejection.
mod common;
use std::process::Command;
use tempfile::TempDir;
use common::TestRepo;
// ---------------------------------------------------------------------------
// Harness
// ---------------------------------------------------------------------------
/// A `TestRepo` with a local bare `origin`, with collab refspecs configured.
/// Never a real network remote. The returned `TempDir` owns the bare repo —
/// keep it alive for the duration of the test.
fn repo_with_origin() -> (TestRepo, TempDir) {
let bare = TempDir::new().unwrap();
let status = Command::new("git")
.args(["init", "--bare", "-b", "main"])
.arg(bare.path())
.status()
.unwrap();
assert!(status.success());
let repo = TestRepo::new("Alice", "alice@example.com");
repo.git(&["remote", "add", "origin", bare.path().to_str().unwrap()]);
repo.git(&["push", "-u", "origin", "main"]);
repo.run_ok(&["init"]);
(repo, bare)
}
/// The full 40-char ids of every patch in the repo, deduplicated. `patch
/// create` only prints the 8-char short form, and the ref names a conflict has
/// to talk about are full ids.
fn patch_ids(repo: &TestRepo) -> Vec<String> {
let refs = repo.git(&[
"for-each-ref",
"--format=%(refname)",
"refs/collab/patches/",
]);
let mut ids: Vec<String> = Vec::new();
for name in refs.lines() {
let Some(rest) = name.strip_prefix("refs/collab/patches/") else {
continue;
};
let Some(id) = rest.split('/').next() else {
continue;
};
if !ids.iter().any(|existing| existing == id) {
ids.push(id.to_string());
}
}
ids
}
/// The full id of the single patch in the repo, asserting there is exactly one.
fn only_patch_id(repo: &TestRepo) -> String {
let ids = patch_ids(repo);
assert_eq!(ids.len(), 1, "expected exactly one patch, found: {:?}", ids);
ids.into_iter().next().unwrap()
}
/// Plant the pre-migration layout on the remote: a bare `refs/collab/patches/<id>`
/// holding the event DAG, exactly as a remote that has never been migrated
/// still carries it. This is what makes the new `<id>/events` unpushable.
fn plant_legacy_bare_ref(repo: &TestRepo, id: &str) {
repo.git(&[
"push",
"origin",
&format!(
"refs/collab/patches/{}/events:refs/collab/patches/{}",
id, id
),
]);
}
// ---------------------------------------------------------------------------
// Fix 1 — a refname conflict is not retryable (issue 866b28c5)
// ---------------------------------------------------------------------------
#[test]
fn refname_conflict_is_diagnosed_not_reported_as_retryable() {
let (repo, _bare) = repo_with_origin();
repo.patch_create("Needs a migrated remote");
let id = only_patch_id(&repo);
plant_legacy_bare_ref(&repo, &id);
let stderr = repo.run_err(&["sync", "--remote", "origin"]);
// The retry advice must be gone: retrying can never clear this.
assert!(
!stderr.contains("again to retry"),
"refname conflict was reported as retryable:\n{}",
stderr
);
// And the old wrong binary name must not appear anywhere.
assert!(
!stderr.contains("`collab sync"),
"stderr uses the wrong binary name `collab`:\n{}",
stderr
);
// It must name the condition, the specific patch, and the exact remedy.
assert!(
stderr.contains("refname conflict"),
"diagnostic does not name the condition:\n{}",
stderr
);
assert!(
stderr.contains(&format!(
"git push origin --delete refs/collab/patches/{}",
id
)),
"diagnostic does not contain the pasteable fix for {}:\n{}",
id,
stderr
);
// The remedy must be scoped to the conflicting id, never a blanket glob
// over every patch ref on the remote.
assert!(
!stderr.contains("refs/collab/patches/*"),
"diagnostic suggests a blanket glob deletion:\n{}",
stderr
);
}
/// The remedy the diagnostic prints has to actually work. Running it verbatim
/// must unblock the very sync that produced it.
#[test]
fn the_suggested_deletion_actually_unblocks_the_sync() {
let (repo, _bare) = repo_with_origin();
repo.patch_create("Needs a migrated remote");
let id = only_patch_id(&repo);
plant_legacy_bare_ref(&repo, &id);
repo.run_err(&["sync", "--remote", "origin"]);
// Exactly the command the diagnostic tells the user to paste.
repo.git(&[
"push",
"origin",
"--delete",
&format!("refs/collab/patches/{}", id),
]);
repo.run_ok(&["sync", "--remote", "origin"]);
}
/// A push failure that is *not* a refname conflict keeps the ordinary retry
/// advice — spelled with the real binary name. A stale lock on the remote
/// produces "cannot lock ref" too, so this pins the boundary: the classifier
/// must not fire on every locking failure.
#[test]
fn ordinary_push_failure_still_gets_retry_advice() {
let (repo, bare) = repo_with_origin();
repo.patch_create("Ordinary failure");
let id = only_patch_id(&repo);
// A stale lock file on the remote: rejected, but nothing to do with the
// ref layout, and clearing it does make a retry succeed.
let lock_dir = bare.path().join("refs/collab/patches").join(&id);
std::fs::create_dir_all(&lock_dir).unwrap();
std::fs::write(
lock_dir.join("events.lock"),
"0000000000000000000000000000000000000000\n",
)
.unwrap();
let stderr = repo.run_err(&["sync", "--remote", "origin"]);
assert!(
stderr.contains("git-collab sync --remote origin"),
"ordinary failure lost its retry advice:\n{}",
stderr
);
assert!(
!stderr.contains("--delete"),
"ordinary failure was misclassified as a refname conflict:\n{}",
stderr
);
}
/// A conflict and an ordinary failure in the same sync each need their own
/// answer, and neither may suppress the other. The retry count must cover only
/// the ordinary failure — telling the user to retry the conflicted ref is
/// exactly what the conflict advice says not to do.
#[test]
fn a_conflict_and_an_ordinary_failure_are_both_reported() {
let (repo, bare) = repo_with_origin();
repo.patch_create("Conflicting");
let conflicted = only_patch_id(&repo);
plant_legacy_bare_ref(&repo, &conflicted);
repo.patch_create("Merely locked");
let locked = patch_ids(&repo)
.into_iter()
.find(|id| *id != conflicted)
.expect("second patch id");
let lock_dir = bare.path().join("refs/collab/patches").join(&locked);
std::fs::create_dir_all(&lock_dir).unwrap();
std::fs::write(
lock_dir.join("events.lock"),
"0000000000000000000000000000000000000000\n",
)
.unwrap();
let stderr = repo.run_err(&["sync", "--remote", "origin"]);
assert!(
stderr.contains(&format!(
"git push origin --delete refs/collab/patches/{}",
conflicted
)),
"conflict advice missing when an ordinary failure was also present:\n{}",
stderr
);
assert!(
stderr.contains("git-collab sync --remote origin"),
"retry advice missing when a conflict was also present:\n{}",
stderr
);
// The locked patch must not be offered up for deletion.
assert!(
!stderr.contains(&format!("--delete refs/collab/patches/{}", locked))
&& !stderr.contains(&format!(" refs/collab/patches/{} ", locked)),
"the merely-locked patch {} was offered for deletion:\n{}",
locked,
stderr
);
// Only the locked ref is retryable, not the two conflicted ones.
assert!(
stderr.contains("retry 1 failed ref(s)"),
"retry count includes conflicted refs:\n{}",
stderr
);
}
// ---------------------------------------------------------------------------
// Fix 2 — `init` is idempotent (issue 7824bd7b)
// ---------------------------------------------------------------------------
#[test]
fn init_twice_leaves_exactly_one_collab_refspec() {
let (repo, _bare) = repo_with_origin();
// repo_with_origin already ran `init` once.
repo.run_ok(&["init"]);
repo.run_ok(&["init"]);
let fetch_specs = repo.git(&["config", "--get-all", "remote.origin.fetch"]);
let collab_specs: Vec<&str> = fetch_specs
.lines()
.filter(|l| l.contains("refs/collab/*"))
.collect();
assert_eq!(
collab_specs.len(),
1,
"expected exactly one collab refspec after repeated init, got {:?}",
collab_specs
);
}
#[test]
fn init_says_already_configured_on_a_second_run() {
let (repo, _bare) = repo_with_origin();
let out = repo.run_ok(&["init"]);
assert!(
out.contains("already configured"),
"second init claimed to configure something new:\n{}",
out
);
}
// ---------------------------------------------------------------------------
// Fix 3 — the keyless-clone trust warning (issue a18e9b76)
// ---------------------------------------------------------------------------
/// A second `TestRepo` pointed at an existing bare remote, with collab
/// refspecs configured and no trusted keys — the state a fresh clone is in.
fn peer_of(bare: &TempDir, name: &str, email: &str) -> TestRepo {
let repo = TestRepo::new(name, email);
repo.git(&["remote", "add", "origin", bare.path().to_str().unwrap()]);
repo.git(&["fetch", "origin"]);
repo.run_ok(&["init"]);
repo
}
/// The warning is one statement about one local trust store, so it fires once
/// per sync however many *kinds* of collab ref that sync reconciles. The bug
/// was a per-kind flag inside `reconcile_refs`, which is called once for
/// issues and once for patches: a clone with both to fetch was told twice.
#[test]
fn trust_warning_appears_once_per_sync_and_names_a_real_command() {
let (alice, bare) = repo_with_origin();
// Both kinds, because the duplicate only appeared when there were issue
// refs *and* patch refs to reconcile.
alice.issue_open("An issue to fetch");
alice.patch_create("A patch to fetch");
alice.run_ok(&["sync", "--remote", "origin"]);
let bob = peer_of(&bare, "Bob", "bob@example.com");
let output = bob.run(&["sync", "--remote", "origin"]);
assert!(
output.status.success(),
"bob's sync failed:\nstdout: {}\nstderr: {}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let stderr = String::from_utf8(output.stderr).unwrap();
let warnings = stderr.matches("no trusted keys configured").count();
assert_eq!(
warnings, 1,
"expected exactly one trust warning for a single-remote sync, got {}:\n{}",
warnings, stderr
);
// The behaviour being described must not have changed: a keyless clone
// still accepts every valid signature, and still says so.
assert!(
!stderr.contains("Rejecting"),
"a keyless clone rejected something it should have accepted:\n{}",
stderr
);
// And the remedy it names must be a command that exists.
assert!(
stderr.contains("git-collab key add --self"),
"trust warning does not name the real command:\n{}",
stderr
);
assert!(
!stderr.contains("'collab "),
"trust warning uses the wrong binary name `collab`:\n{}",
stderr
);
}