spectral/tests/patch_verbs.rs

//! The patch verbs, driven through the built binary against a fixture tree.

mod common;

use common::Fixture;

/// Every stub records how it was called, so a test can assert the arguments
/// spectral really passed and see the diff that came in on stdin.
const RECORD: &str = r#": "${FIXTURE_RECORD:=.checkpatch-call}"
{
  echo "argv: $*"
  echo "cwd: $(pwd)"
  cat
} >> "$FIXTURE_RECORD"
"#;

/// Build a checkpatch stand-in from the part that differs between tests.
fn stub(body: &str) -> String {
    format!("#!/bin/sh\n{RECORD}{body}")
}

/// Rewrites the patch's added line, the way a real fix pass does, then reports
/// clean.
const FIXER: &str = r#"if [ "$2" = "--fix-inplace" ]; then
  sed -i 's/return a + 1/return a + 2/' "$3"
fi
echo "total: 0 errors, 0 warnings, 9 lines checked"
exit 0
"#;

/// Leaves a mark in the patch file it is given, for the case where nothing has
/// to be applied to a tree afterwards.
const TOUCHER: &str = r#"if [ "$2" = "--fix-inplace" ]; then
  printf 'FIXED\n' >> "$3"
fi
echo "total: 0 errors, 0 warnings, 3 lines checked"
exit 0
"#;

/// A fix that is useless, for the path that has to undo one.
const RUINER: &str = r#"if [ "$2" = "--fix-inplace" ]; then
  printf 'this is not a patch at all\n' > "$3"
fi
echo "total: 1 errors, 0 warnings, 3 lines checked"
exit 1
"#;

#[test]
fn commit_signs_off_and_keeps_the_message() {
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.git(&["add", "-A"]);

    let output = fixture.cli(&["patch", "commit", "foo: return a + 1", "--signoff"]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    let message = fixture.git(&["log", "-1", "--pretty=%B"]);
    assert!(message.contains("foo: return a + 1"), "{message}");
    assert!(
        message.contains("Signed-off-by: Fixture <[email protected]>"),
        "{message}"
    );
}

#[test]
fn commit_amend_replaces_the_previous_commit() {
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.git(&["add", "-A"]);

    let output = fixture.cli(&["patch", "commit", "foo: reworded", "--amend"]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    assert_eq!(fixture.git(&["rev-list", "--count", "HEAD"]), "1");
    assert!(
        fixture
            .git(&["log", "-1", "--pretty=%s"])
            .contains("reworded")
    );
}

#[test]
fn create_writes_a_patch_that_send_email_accepts() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");

    let output = fixture.cli(&["patch", "create", "000-foo"]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    let printed = String::from_utf8_lossy(&output.stdout).trim().to_owned();
    assert_eq!(
        printed,
        fixture
            .home()
            .join(".spectral/patches/000-foo.patch")
            .display()
            .to_string()
    );

    let patch = std::fs::read_to_string(&printed).expect("the patch create wrote");
    assert!(patch.starts_with("From "), "{patch}");
    assert!(
        patch.contains("Subject: [PATCH] foo: return a + 1"),
        "{patch}"
    );
    assert!(patch.contains("return a + 1;"), "{patch}");

    let sent = fixture.git(&[
        "send-email",
        "--dry-run",
        "--confirm=never",
        "--to",
        "[email protected]",
        &printed,
    ]);
    assert!(sent.contains("Dry-OK"), "{sent}");
}

#[test]
fn a_bare_diff_is_refused_by_send_email() {
    // This is why `create` goes through format-patch, so the failure is pinned
    // rather than rediscovered.
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    let diff = fixture.git(&["diff"]);
    let path = fixture.home().join("bare.patch");
    std::fs::write(&path, format!("{diff}\n")).expect("write the bare diff");

    let stderr = fixture.git_must_fail(&[
        "send-email",
        "--dry-run",
        "--confirm=never",
        "--to",
        "[email protected]",
        path.to_str().expect("the patch path"),
    ]);

    assert!(stderr.contains("No subject line"), "{stderr}");
}

#[test]
fn a_commit_with_nothing_staged_says_what_git_said() {
    // git explains "nothing staged" on stdout, so an error path that only
    // reads stderr prints a failure with nothing after it.
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);

    let output = fixture.cli(&["patch", "commit", "foo: nothing staged"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("failed (exit 1)"), "{stderr}");
    assert!(
        stderr.contains("no changes added to commit") || stderr.contains("not staged"),
        "git's own explanation was dropped: {stderr}"
    );
}

#[test]
fn create_names_a_base_that_does_not_resolve() {
    let fixture = Fixture::kernel_tree();

    let output = fixture.cli(&["patch", "create", "000-foo", "--base", "not-a-branch"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("no revision `not-a-branch`"), "{stderr}");
}

#[test]
fn create_refuses_a_range_that_holds_more_than_one_commit() {
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    fixture.write(
        "drivers/foo/bar.c",
        "static int foo(int a)\n{\n\treturn a + 2;\n}\n",
    );
    fixture.commit_all("foo: return a + 2");

    let output = fixture.cli(&["patch", "create", "000-foo", "--base", "HEAD~2"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("holds 2 commits"), "{stderr}");
    assert!(
        stderr.contains("--range"),
        "the refusal has to say where a set belongs: {stderr}"
    );
}

#[test]
fn create_refuses_when_there_is_nothing_to_send() {
    let fixture = Fixture::kernel_tree();

    let output = fixture.cli(&["patch", "create", "000-foo", "--base", "HEAD"]);

    assert_eq!(output.status.code(), Some(1));
    assert!(String::from_utf8_lossy(&output.stderr).contains("nothing to send"));
}

#[test]
fn check_pipes_the_working_tree_diff_into_checkpatch_and_fails_on_errors() {
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);

    let output = fixture.cli_env(
        &["patch", "check"],
        &[
            (
                "FIXTURE_SUMMARY",
                "total: 4 errors, 2 warnings, 9 lines checked",
            ),
            ("FIXTURE_EXIT", "1"),
        ],
    );

    assert_eq!(output.status.code(), Some(1));
    let stdout = String::from_utf8_lossy(&output.stdout);
    assert!(
        stdout.contains("total: 4 errors, 2 warnings, 9 lines checked"),
        "{stdout}"
    );

    // The diff really went in, rather than a revision being named.
    let call = fixture.read(".checkpatch-call");
    assert!(call.contains("argv: --no-tree -"), "{call}");
    assert!(call.contains("+	return a + 1;"), "{call}");
    assert!(
        call.contains(&format!("cwd: {}", fixture.tree().display())),
        "{call}"
    );
}

#[test]
fn checkpatch_warnings_alone_do_not_fail_the_command() {
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);

    let output = fixture.cli_env(
        &["patch", "check"],
        &[(
            "FIXTURE_SUMMARY",
            "total: 0 errors, 3 warnings, 9 lines checked",
        )],
    );

    assert_eq!(output.status.code(), Some(0));
    assert!(String::from_utf8_lossy(&output.stdout).contains("0 errors, 3 warnings"));
}

#[test]
fn checkpatch_counts_a_run_with_no_total_line_as_clean() {
    // What a clean run looks like under checkpatch's own --terse, where the
    // summary line is suppressed entirely.
    let fixture = Fixture::kernel_tree();
    fixture.write("drivers/foo/bar.c", common::EDITED);

    let output = fixture.cli(&["patch", "check"]);

    assert_eq!(output.status.code(), Some(0));
    assert!(String::from_utf8_lossy(&output.stdout).trim().is_empty());
}

#[test]
fn a_patch_file_and_a_revision_go_in_as_themselves() {
    let fixture = Fixture::kernel_tree();
    let patch = fixture.home().join("thing.patch");
    std::fs::write(&patch, "--- a/drivers/foo/bar.c\n+++ b/drivers/foo/bar.c\n").expect("write");

    let by_file = fixture.cli(&[
        "patch",
        "check",
        patch.to_str().expect("the patch path"),
        "--strict",
    ]);
    assert!(by_file.status.success());
    let call = fixture.read(".checkpatch-call");
    assert!(call.contains("--strict"), "{call}");
    assert!(
        !call.contains("--file"),
        "checkpatch reads --file as a source file, not as a patch: {call}"
    );
    assert!(call.contains("thing.patch"), "{call}");

    let by_rev = fixture.cli(&["patch", "check", "--rev", "HEAD"]);
    assert!(by_rev.status.success());
    let call = fixture.read(".checkpatch-call");
    assert!(call.contains("--git HEAD"), "{call}");
}

#[test]
fn a_patch_file_that_is_not_there_is_named_as_such() {
    let fixture = Fixture::kernel_tree();

    let output = fixture.cli(&["patch", "check", "nope.patch"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("no patch named `nope.patch`"), "{stderr}");
}

#[test]
fn a_missing_tree_names_the_variable_that_would_have_set_it() {
    let fixture = Fixture::bare();

    let output = fixture.cli_env(
        &["patch", "check"],
        &[("SPECTRAL_KERNEL", "/nonexistent/linux")],
    );

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(
        stderr.contains("no kernel tree at `/nonexistent/linux`"),
        "{stderr}"
    );
    assert!(stderr.contains("SPECTRAL_KERNEL"), "{stderr}");
}

#[test]
fn format_rewrites_a_patch_file_in_place_and_checks_it_again() {
    let fixture = Fixture::kernel_tree();
    fixture.write_script("scripts/checkpatch.pl", &stub(TOUCHER));
    let patch = fixture.home().join("thing.patch");
    std::fs::write(&patch, "--- a/x\n+++ b/x\n").expect("write the patch");

    let output = fixture.cli(&["patch", "format", patch.to_str().expect("the patch path")]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    assert!(
        std::fs::read_to_string(&patch)
            .expect("the patch")
            .ends_with("FIXED\n")
    );

    let call = fixture.read(".checkpatch-call");
    assert!(call.contains("argv: --no-tree --fix-inplace "), "{call}");
    assert!(call.contains("thing.patch"), "{call}");
    assert!(!call.contains("--file"), "{call}");
}

#[test]
fn format_applies_the_fix_to_the_working_tree() {
    let fixture = Fixture::kernel_tree();
    fixture.write_script("scripts/checkpatch.pl", &stub(FIXER));
    fixture.commit_all("fixture: use the fixing checkpatch");
    fixture.write("drivers/foo/bar.c", common::EDITED);

    let output = fixture.cli(&["patch", "format"]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    let file = fixture.read("drivers/foo/bar.c");
    assert!(
        file.contains("return a + 2;"),
        "the fix did not land: {file}"
    );
    assert!(fixture.git(&["diff"]).contains("+	return a + 2;"));

    let call = fixture.read(".checkpatch-call");
    assert!(call.contains("argv: --no-tree --fix-inplace "), "{call}");
    assert!(!call.contains("--file"), "{call}");
    assert!(call.contains("argv: --no-tree -"), "{call}");
}

#[test]
fn a_fix_that_does_not_apply_leaves_the_tree_alone() {
    let fixture = Fixture::kernel_tree();
    fixture.write_script("scripts/checkpatch.pl", &stub(RUINER));
    fixture.commit_all("fixture: use the useless checkpatch");
    fixture.write("drivers/foo/bar.c", common::EDITED);

    let output = fixture.cli(&["patch", "format"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("does not apply"), "{stderr}");
    assert!(
        fixture.read("drivers/foo/bar.c").contains("return a + 1;"),
        "the uncommitted change was not put back"
    );
}

#[test]
fn submit_dry_run_shows_the_recipients_and_the_command() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    let created = fixture.cli(&["patch", "create", "000-foo"]);
    assert!(
        created.status.success(),
        "{}",
        String::from_utf8_lossy(&created.stderr)
    );
    let patch = fixture.home().join(".spectral/patches/000-foo.patch");

    let output = fixture.cli(&[
        "patch",
        "submit",
        patch.to_str().expect("the patch path"),
        "--dry-run",
    ]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    let stdout = String::from_utf8_lossy(&output.stdout);
    assert!(
        stdout.contains("Files: drivers/foo/bar.c"),
        "the lookup should say what it ran against: {stdout}"
    );
    assert!(
        stdout.contains("To: Fixture Maintainer <[email protected]>"),
        "{stdout}"
    );
    assert!(stdout.contains("To: [email protected]"), "{stdout}");
    assert!(
        stdout.contains("Cc: Fixture Reviewer <[email protected]>"),
        "{stdout}"
    );
    assert!(stdout.contains("Cc: [email protected]"), "{stdout}");
    assert!(
        !stdout.contains("Cc: Fixture Maintainer"),
        "an address on the To line needs no courtesy copy: {stdout}"
    );

    // The command line is the thing the README tells a reader to look at.
    assert!(
        stdout.contains("send-email --dry-run --confirm=never"),
        "{stdout}"
    );
    assert!(
        stdout.contains("--to 'Fixture Maintainer <[email protected]>'"),
        "{stdout}"
    );
    assert!(stdout.contains("Dry-OK"), "send-email's own plan: {stdout}");

    // And the recipients came from the tree's own script, asked the way the
    // fixture tree needs to be asked.
    let call = fixture.read(".getmaintainer-call");
    assert!(
        call.contains("--no-tree --git --roles --no-rolestats --pattern-depth=0"),
        "{call}"
    );
    assert!(call.contains("000-foo.patch"), "{call}");
}

#[test]
fn submit_puts_extra_ccs_on_the_cc_line_once_and_skips_anyone_on_to() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    let created = fixture.cli(&["patch", "create", "000-foo"]);
    assert!(created.status.success());
    let patch = fixture.home().join(".spectral/patches/000-foo.patch");

    let output = fixture.cli(&[
        "patch",
        "submit",
        patch.to_str().expect("the patch path"),
        "--dry-run",
        "--cc",
        "[email protected]",
        "--cc",
        "[email protected]",
        "--cc",
        "[email protected]",
    ]);

    assert!(output.status.success());
    let stdout = String::from_utf8_lossy(&output.stdout);

    // Only the block above the command line is ours; send-email echoes the
    // same addresses back in its own plan.
    let ours: Vec<&str> = stdout
        .lines()
        .take_while(|line| !line.contains("send-email --dry-run"))
        .collect();
    let cc_lines: Vec<&str> = ours
        .iter()
        .copied()
        .filter(|line| line.starts_with("Cc: "))
        .collect();
    assert_eq!(cc_lines.len(), 3, "{stdout}");
    assert_eq!(cc_lines[2], "Cc: [email protected]");
    assert_eq!(
        stdout.matches("--cc [email protected]").count(),
        1,
        "{stdout}"
    );
    assert!(
        !stdout.contains("Cc: [email protected]"),
        "the list is already on the To line: {stdout}"
    );
}

#[test]
fn submit_threads_a_reroll_off_the_patch_it_replaces() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    let created = fixture.cli(&["patch", "create", "000-foo"]);
    assert!(created.status.success());
    let patch = fixture.home().join(".spectral/patches/000-foo.patch");

    let output = fixture.cli(&[
        "patch",
        "submit",
        patch.to_str().expect("the patch path"),
        "--dry-run",
        "--in-reply-to",
        "<[email protected]>",
    ]);

    assert!(output.status.success());
    let stdout = String::from_utf8_lossy(&output.stdout);
    assert!(
        stdout.contains("--in-reply-to '<[email protected]>'"),
        "an unquoted <...> is a redirection when pasted: {stdout}"
    );
    assert!(stdout.contains("In-Reply-To: <[email protected]>"), "{stdout}");
}

#[test]
fn a_tree_that_is_not_a_repository_says_so_rather_than_blaming_a_revision() {
    // `git rev-parse --verify --quiet master` fails both for a revision that is
    // not there and for a directory that is not a repository. They want
    // different fixes, so they get different messages.
    let fixture = Fixture::bare();
    fixture.write_script("scripts/checkpatch.pl", "#!/bin/sh\n");
    fixture.write_script("scripts/get_maintainer.pl", "#!/bin/sh\n");

    let output = fixture.cli(&["patch", "create", "000-foo"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("not a git repository"), "{stderr}");
    assert!(
        !stderr.contains("no revision"),
        "a missing repository is not a missing revision: {stderr}"
    );
}

#[test]
fn submit_refuses_a_template_cover_letter_instead_of_swallowing_it() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    fixture.git(&[
        "format-patch",
        "--cover-letter",
        "-o",
        "out",
        "master..HEAD",
    ]);
    let cover = fixture.join("out/0000-cover-letter.patch");

    let output = fixture.cli(&[
        "patch",
        "submit",
        cover.to_str().expect("the cover letter path"),
        "--dry-run",
    ]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("Refusing to send"), "{stderr}");
    assert!(stderr.contains("exit 25"), "{stderr}");
}

#[test]
fn submit_names_a_patch_that_is_not_there() {
    let fixture = Fixture::kernel_tree();

    let output = fixture.cli(&["patch", "submit", "nope.patch", "--dry-run"]);

    assert_eq!(output.status.code(), Some(1));
    assert!(String::from_utf8_lossy(&output.stderr).contains("no patch named `nope.patch`"));
}

#[test]
fn update_renames_a_patch_to_the_next_revision_without_rewriting_it() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    assert!(
        fixture
            .cli(&["patch", "create", "000-foo"])
            .status
            .success()
    );
    let patches = fixture.home().join(".spectral/patches");
    let before = std::fs::read(patches.join("000-foo.patch")).expect("the created patch");

    // Named the way the README's examples name it: no path in front.
    let output = fixture.cli(&["patch", "update", "000-foo.patch"]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    let printed = String::from_utf8_lossy(&output.stdout).trim().to_owned();
    assert_eq!(
        printed,
        patches.join("v1-000-foo.patch").display().to_string()
    );
    assert_eq!(
        std::fs::read(patches.join("v1-000-foo.patch")).expect("the renamed patch"),
        before,
        "a re-roll moves the file, it does not rewrite it"
    );
    assert!(!patches.join("000-foo.patch").exists());
}

#[test]
fn update_infers_the_next_revision_and_does_nothing_at_the_same_one() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    assert!(
        fixture
            .cli(&["patch", "create", "000-foo"])
            .status
            .success()
    );
    let patches = fixture.home().join(".spectral/patches");

    assert!(
        fixture
            .cli(&["patch", "update", "000-foo.patch"])
            .status
            .success()
    );
    let to_v2 = fixture.cli(&["patch", "update", "v1-000-foo.patch"]);
    assert!(to_v2.status.success());
    assert_eq!(
        String::from_utf8_lossy(&to_v2.stdout).trim(),
        patches.join("v2-000-foo.patch").display().to_string()
    );

    let before = std::fs::read(patches.join("v2-000-foo.patch")).expect("the v2 patch");
    let again = fixture.cli(&["patch", "update", "v2-000-foo.patch", "-v", "2"]);

    assert!(again.status.success());
    assert_eq!(
        String::from_utf8_lossy(&again.stdout).trim(),
        patches.join("v2-000-foo.patch").display().to_string()
    );
    assert_eq!(
        std::fs::read(patches.join("v2-000-foo.patch")).expect("the v2 patch"),
        before
    );
    assert!(!patches.join("v3-000-foo.patch").exists());
}

#[test]
fn update_refuses_to_land_on_a_patch_that_is_already_there() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    assert!(
        fixture
            .cli(&["patch", "create", "000-foo"])
            .status
            .success()
    );
    assert!(
        fixture
            .cli(&["patch", "update", "000-foo.patch", "-v", "1"])
            .status
            .success()
    );
    assert!(
        fixture
            .cli(&["patch", "create", "000-foo"])
            .status
            .success()
    );

    let output = fixture.cli(&["patch", "update", "000-foo.patch", "-v", "1"]);

    assert_eq!(output.status.code(), Some(1));
    let stderr = String::from_utf8_lossy(&output.stderr);
    assert!(stderr.contains("already exists"), "{stderr}");
}

#[test]
fn a_patch_name_resolves_without_its_path() {
    let fixture = Fixture::kernel_tree();
    fixture.work_on_a_branch();
    fixture.write("drivers/foo/bar.c", common::EDITED);
    fixture.commit_all("foo: return a + 1");
    assert!(
        fixture
            .cli(&["patch", "create", "000-foo"])
            .status
            .success()
    );

    // Straight from `create` into `submit`, by name, as the README shows.
    let output = fixture.cli(&["patch", "submit", "000-foo.patch", "--dry-run"]);

    assert!(
        output.status.success(),
        "{}",
        String::from_utf8_lossy(&output.stderr)
    );
    let stdout = String::from_utf8_lossy(&output.stdout);
    assert!(
        stdout.contains("To: Fixture Maintainer <[email protected]>"),
        "{stdout}"
    );
    assert!(stdout.contains("Dry-OK"), "{stdout}");
}