tests/early_validation_test.rs
Ref: Size: 15.9 KiB History
//! Commands must reject a meaningless argument at the point it is given, with a
//! message naming the fix — not accept it and fail somewhere else later, or
//! never fail at all.
//!
//! Each test here pins the *message*, because the message is the feature. An
//! exit code alone tells the user nothing, and these bugs were all cases where
//! the user learned nothing until much later, if ever.
mod common;
use common::TestRepo;
/// A repo with `main` plus a `feat` branch carrying one commit, left checked
/// out on `main`. The common starting point: a patch that *should* be creatable.
fn repo_with_feature_branch() -> TestRepo {
let repo = TestRepo::new("Alice", "alice@example.com");
repo.git(&["checkout", "-b", "feat"]);
repo.commit_file("a.txt", "a", "do the work");
repo.git(&["checkout", "main"]);
repo
}
fn full_issue_id(repo: &TestRepo, short: &str) -> String {
repo.git(&["for-each-ref", "--format=%(refname)", "refs/collab/issues/"])
.lines()
.filter_map(|l| l.trim().strip_prefix("refs/collab/issues/"))
.find(|id| id.starts_with(short))
.unwrap_or_else(|| panic!("no issue ref matching {}", short))
.to_string()
}
/// Give an issue a second ref under a near-identical id, so its 8-character
/// short id matches two issues. Random ids practically never collide, so
/// ambiguity has to be constructed.
fn duplicate_issue_ref_with_colliding_id(repo: &TestRepo, short: &str) {
let full = full_issue_id(repo, short);
let last = full.chars().last().unwrap();
let replacement = if last == '0' { '1' } else { '0' };
let twin: String = full[..full.len() - 1]
.chars()
.chain([replacement])
.collect();
let tip = repo
.git(&["rev-parse", &format!("refs/collab/issues/{}", full)])
.trim()
.to_string();
repo.git(&["update-ref", &format!("refs/collab/issues/{}", twin), &tip]);
}
fn assert_no_patches(repo: &TestRepo) {
let out = repo.run_ok(&["patch", "list"]);
assert!(
out.contains("No patches found"),
"a rejected create must leave nothing behind, got: {}",
out
);
}
// ===========================================================================
// 6b1db172 — `patch create --fixes` accepts an unresolvable issue reference
// ===========================================================================
#[test]
fn patch_create_rejects_a_fixes_reference_that_resolves_to_nothing() {
let repo = repo_with_feature_branch();
let stderr = repo.run_err(&[
"patch", "create", "-t", "T", "-B", "feat", "--fixes", "deadbeef",
]);
assert!(
stderr.contains("no issue found matching 'deadbeef'"),
"must fail with the same message `patch merge` reports, got: {}",
stderr
);
assert!(
stderr.contains("--fixes"),
"the message must name the flag at fault, got: {}",
stderr
);
assert_no_patches(&repo);
}
/// The failure that motivated the issue: a script guessed a verb wrong, captured
/// `issue open`'s usage text into a variable, and passed the whole thing on as a
/// `--fixes` value. It was accepted without complaint.
#[test]
fn patch_create_rejects_usage_text_passed_as_a_fixes_value() {
let repo = repo_with_feature_branch();
let usage = "Usage: git-collab issue open --title <TITLE>";
let stderr = repo.run_err(&["patch", "create", "-t", "T", "-B", "feat", "--fixes", usage]);
assert!(
stderr.contains(&format!("no issue found matching '{}'", usage)),
"the message must quote the value verbatim so the caller can see what it sent, got: {}",
stderr
);
assert_no_patches(&repo);
}
#[test]
fn patch_create_rejects_an_ambiguous_fixes_prefix() {
let repo = repo_with_feature_branch();
let issue = repo.issue_open("Login bug");
duplicate_issue_ref_with_colliding_id(&repo, &issue);
let stderr = repo.run_err(&[
"patch", "create", "-t", "T", "-B", "feat", "--fixes", &issue,
]);
assert!(
stderr.contains("ambiguous issue prefix"),
"an ambiguous prefix must fail at create time too, got: {}",
stderr
);
assert!(
stderr.contains(&issue),
"the message must name the prefix given, got: {}",
stderr
);
assert_no_patches(&repo);
}
/// Resolving at create time is only durable if what gets *stored* is the
/// resolved id. A stored prefix that is unique today can go ambiguous tomorrow,
/// which is the very failure the create-time check exists to prevent.
#[test]
fn patch_create_stores_the_resolved_issue_id_not_the_prefix_given() {
let repo = repo_with_feature_branch();
let issue = repo.issue_open("Login bug");
let full = full_issue_id(&repo, &issue);
let out = repo.run_ok(&[
"patch", "create", "-t", "T", "-B", "feat", "--fixes", &issue,
]);
let patch_id = out.trim().strip_prefix("Created patch ").unwrap();
let json = repo.run_ok(&["patch", "show", patch_id, "--json"]);
let v: serde_json::Value = serde_json::from_str(&json).unwrap();
assert_eq!(
v["fixes"], full,
"stored `fixes` must be the fully resolved issue id"
);
}
#[test]
fn patch_create_still_accepts_a_fixes_reference_that_resolves() {
let repo = repo_with_feature_branch();
let issue = repo.issue_open("Login bug");
let out = repo.run_ok(&[
"patch", "create", "-t", "T", "-B", "feat", "--fixes", &issue,
]);
assert!(out.starts_with("Created patch "));
}
// ===========================================================================
// 03163871 — `patch create` succeeds with nothing in it
// ===========================================================================
#[test]
fn patch_create_rejects_a_branch_with_no_commits_ahead_of_base() {
let repo = TestRepo::new("Alice", "alice@example.com");
repo.git(&["checkout", "-b", "empty"]);
repo.git(&["checkout", "main"]);
let stderr = repo.run_err(&["patch", "create", "-t", "T", "-B", "empty"]);
assert!(
stderr.contains("branch 'empty' has no commits ahead of base 'main'"),
"the message must name both the branch and the base it compared against, got: {}",
stderr
);
assert!(
stderr.contains("commit"),
"the message must name the fix, got: {}",
stderr
);
assert_no_patches(&repo);
}
/// A branch that is merely *behind* base has no commits of its own either. It
/// must be rejected for the same reason and with the same message, not slip
/// through because it is not literally at the base tip.
#[test]
fn patch_create_rejects_a_branch_that_is_only_behind_base() {
let repo = TestRepo::new("Alice", "alice@example.com");
repo.git(&["checkout", "-b", "stale"]);
repo.git(&["checkout", "main"]);
repo.commit_file("moved.txt", "m", "main moves on");
let stderr = repo.run_err(&["patch", "create", "-t", "T", "-B", "stale"]);
assert!(
stderr.contains("branch 'stale' has no commits ahead of base 'main'"),
"got: {}",
stderr
);
assert_no_patches(&repo);
}
#[test]
fn patch_create_names_a_base_branch_that_does_not_exist() {
let repo = repo_with_feature_branch();
let stderr = repo.run_err(&["patch", "create", "-t", "T", "-B", "feat", "--base", "nope"]);
assert!(
stderr.contains("base branch 'nope' not found"),
"an unknown base must be reported as such, got: {}",
stderr
);
assert_no_patches(&repo);
}
/// From a detached HEAD, `patch create` invents a branch to hang the patch off
/// before it validates anything. A rejected create must not leave that branch
/// behind — the whole point of failing early is that nothing is left over.
#[test]
fn a_rejected_create_from_a_detached_head_leaves_no_branch_behind() {
let repo = repo_with_feature_branch();
repo.git(&["checkout", "feat"]);
repo.git(&["checkout", "--detach"]);
let before = repo.git(&["branch", "--list", "collab/*"]);
assert!(before.trim().is_empty(), "precondition");
repo.run_err(&["patch", "create", "-t", "T", "--fixes", "deadbeef"]);
let after = repo.git(&["branch", "--list", "collab/*"]);
assert!(
after.trim().is_empty(),
"a rejected create must not leave its auto-created branch behind, got: {}",
after
);
}
// ===========================================================================
// aa0d482e — `patch show --revision N` for a revision that does not exist
// ===========================================================================
fn repo_with_one_revision_patch() -> (TestRepo, String) {
let repo = repo_with_feature_branch();
let out = repo.run_ok(&["patch", "create", "-t", "T", "-B", "feat"]);
let id = out
.trim()
.strip_prefix("Created patch ")
.unwrap()
.to_string();
(repo, id)
}
#[test]
fn patch_show_rejects_a_revision_that_does_not_exist() {
let (repo, id) = repo_with_one_revision_patch();
let stderr = repo.run_err(&["patch", "show", &id, "--revision", "5"]);
assert!(
stderr.contains("revision 5 not found"),
"must use the same message interdiff already gives, got: {}",
stderr
);
}
/// A failed read must not leave a mark. `patch show` records a seen-ref to drive
/// unread counts; a rejected invocation showed the user nothing, so it must not
/// claim they have seen anything.
#[test]
fn a_rejected_patch_show_does_not_mark_the_patch_as_seen() {
let (repo, id) = repo_with_one_revision_patch();
repo.run_err(&["patch", "show", &id, "--revision", "5"]);
let seen = repo.git(&[
"for-each-ref",
"--format=%(refname)",
"refs/collab/local/seen/",
]);
assert!(
seen.trim().is_empty(),
"a failed show must write nothing, but left: {}",
seen
);
}
#[test]
fn patch_show_still_accepts_a_revision_that_exists() {
let (repo, id) = repo_with_one_revision_patch();
let out = repo.run_ok(&["patch", "show", &id, "--revision", "1"]);
assert!(out.contains("Patch "));
}
// ---------------------------------------------------------------------------
// --json callers must be able to see the failure. An error that only reaches
// stderr is invisible to a script parsing stdout.
// ---------------------------------------------------------------------------
#[test]
fn a_json_command_reports_its_error_as_json_on_stdout() {
let (repo, id) = repo_with_one_revision_patch();
let out = repo.run(&["patch", "show", &id, "--json", "--revision", "5"]);
assert!(!out.status.success(), "must still exit non-zero");
let stdout = String::from_utf8(out.stdout).unwrap();
let v: serde_json::Value = serde_json::from_str(&stdout)
.unwrap_or_else(|e| panic!("--json failure must still be JSON ({}): {}", e, stdout));
assert_eq!(v["error"], "revision 5 not found");
let stderr = String::from_utf8(out.stderr).unwrap();
assert!(
stderr.contains("revision 5 not found"),
"the human-readable error must still reach stderr, got: {}",
stderr
);
}
#[test]
fn a_non_json_command_does_not_print_json_on_failure() {
let (repo, id) = repo_with_one_revision_patch();
let out = repo.run(&["patch", "show", &id, "--revision", "5"]);
let stdout = String::from_utf8(out.stdout).unwrap();
assert!(
!stdout.contains("\"error\""),
"plain output must stay plain, got: {}",
stdout
);
}
/// `--revision` was accepted and silently ignored in `--json` mode: the caller
/// asked to narrow the output and got everything. Same bug class as the one
/// above, so it is fixed with it.
#[test]
fn patch_show_json_filters_reviews_and_comments_by_revision() {
let (repo, id) = repo_with_one_revision_patch();
repo.run_ok(&[
"patch",
"review",
&id,
"-v",
"comment",
"-b",
"on r1",
"--revision",
"1",
]);
// Add a second revision, and a review anchored to it.
repo.git(&["checkout", "feat"]);
repo.commit_file("b.txt", "b", "more work");
repo.run_ok(&["patch", "revise", &id, "-B", "feat"]);
repo.git(&["checkout", "main"]);
repo.run_ok(&[
"patch",
"review",
&id,
"-v",
"comment",
"-b",
"on r2",
"--revision",
"2",
]);
let json = repo.run_ok(&["patch", "show", &id, "--json", "--revision", "1"]);
let v: serde_json::Value = serde_json::from_str(&json).unwrap();
let bodies: Vec<&str> = v["reviews"]
.as_array()
.unwrap()
.iter()
.map(|r| r["body"].as_str().unwrap())
.collect();
assert_eq!(
bodies,
vec!["on r1"],
"--json must honour --revision, not ignore it"
);
}
// ===========================================================================
// 62abe4b9 — `patch checkout` strands you on the created branch
// ===========================================================================
#[test]
fn patch_checkout_says_where_you_came_from_and_how_to_get_back() {
let (repo, id) = repo_with_one_revision_patch();
let out = repo.run_ok(&["patch", "checkout", &id]);
assert!(
out.contains("you were on main"),
"must name the branch you came from, got: {}",
out
);
assert!(
out.contains("git checkout main"),
"must name the command that gets you back, got: {}",
out
);
}
/// Re-checking-out the patch you are already on has nowhere to send you back
/// to. "you were on collab/x; return with `git checkout collab/x`" is noise, and
/// noise is what stops the useful lines from being read.
#[test]
fn patch_checkout_from_the_patchs_own_branch_offers_no_pointless_return() {
let (repo, id) = repo_with_one_revision_patch();
repo.run_ok(&["patch", "checkout", &id]);
let out = repo.run_ok(&["patch", "checkout", &id]);
assert!(
!out.contains("you were on"),
"must not report a move that did not happen, got: {}",
out
);
assert!(
!out.contains("Return with"),
"must not offer to return you where you already are, got: {}",
out
);
assert!(
out.contains(&format!("git branch -D collab/{}", id)),
"the cleanup hint is still worth having, got: {}",
out
);
}
#[test]
fn patch_checkout_says_the_branch_is_left_behind_and_how_to_remove_it() {
let (repo, id) = repo_with_one_revision_patch();
let out = repo.run_ok(&["patch", "checkout", &id]);
assert!(
out.contains(&format!("git branch -D collab/{}", id)),
"must name the cleanup command for the branch it left behind, got: {}",
out
);
}
#[test]
fn patch_checkout_from_a_detached_head_names_the_commit_to_return_to() {
let (repo, id) = repo_with_one_revision_patch();
let head = repo.git(&["rev-parse", "HEAD"]).trim().to_string();
repo.git(&["checkout", "--detach"]);
let out = repo.run_ok(&["patch", "checkout", &id]);
assert!(
out.contains("detached HEAD"),
"must say you were detached rather than invent a branch name, got: {}",
out
);
assert!(
out.contains(&head[..8]),
"must name the commit to return to, got: {}",
out
);
}
/// Checking the same patch out twice used to accumulate `collab/<id>-1`,
/// `collab/<id>-2`, ... The reporter's complaint was about the branches a review
/// session piles up, so a repeat checkout of an unchanged patch must reuse.
#[test]
fn patch_checkout_reuses_an_existing_branch_that_already_matches() {
let (repo, id) = repo_with_one_revision_patch();
repo.run_ok(&["patch", "checkout", &id]);
repo.git(&["checkout", "main"]);
let out = repo.run_ok(&["patch", "checkout", &id]);
assert!(
out.contains(&format!("on branch collab/{}", id)),
"must reuse the existing branch, got: {}",
out
);
let branches = repo.git(&["branch", "--list", "collab/*"]);
assert_eq!(
branches.lines().count(),
1,
"a repeat checkout must not pile up branches, got: {}",
branches
);
}