test: add tests for the new behavior in the rebase engine

Author: slarse

crates/but-rebase/tests/fixtures/scenario/unsigned-commits-with-signing-key-setup-but-signing-disabled.sh

@@ -0,0 +1,23 @@
+#!/bin/bash
+#
+# Description: Scenario with unsigned commits and commit signing disabled, but
+# all the requisite configuration to sign commits is present.
+#
+# The scenario is designed to be used to test signing of commits in the absence
+# of other changes.
+
+set -eu -o pipefail
+
+git init
+
+ssh-keygen -t rsa -b 2048 -C "test@example.com" -N "" -f signature.key
+git config gpg.format ssh
+git config user.signingKey "$PWD/signature.key"
+echo "*.key*" >.gitignore
+
+echo "base" >base && git add . && git commit -m "base"
+git update-ref refs/heads/base $(git rev-parse HEAD)
+echo "mid" >mid && git add . && git commit -m "mid"
+git update-ref refs/heads/mid $(git rev-parse HEAD)
+echo "top" >top && git add . && git commit -m "top"
+git update-ref refs/heads/top $(git rev-parse HEAD)

crates/but-rebase/tests/rebase/graph_rebase/signing_preferences.rs

@@ -3,7 +3,10 @@ use std::fs;
 /// These tests cover the `sign_mode` property on the Step::Pick.
 use anyhow::Result;
 use but_graph::Graph;
-use but_rebase::graph_rebase::{Editor, Pick, Step, cherry_pick::PickSignMode};
+use but_rebase::graph_rebase::{
+    Editor, GraphEditorOptions, Pick, Step,
+    cherry_pick::{PickSignGuard, PickSignMode},
+};
 use but_testsupport::{cat_commit, visualize_commit_graph_all};
 
 use crate::utils::{fixture_writable_with_signing, standard_options};
@@ -185,3 +188,362 @@ fn when_cherry_picking_dont_resign_if_not_set() -> Result<()> {
 
     Ok(())
 }
+
+/// Picking with [`PickSignMode::Force`] should cause the pick to be cherry-picked even in
+/// absence of other changes, and the resulting commit should be signed.
+#[test]
+fn commit_picked_with_force_sign_is_signed_when_otherwise_unchanged_and_signing_config_is_not_enabled()
+-> Result<()> {
+    let (repo, _tmpdir, mut meta) = fixture_writable_with_signing(
+        "unsigned-commits-with-signing-key-setup-but-signing-disabled",
+    )?;
+
+    let before = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(before, @"
+    * ea8caac (HEAD -> main, top) top
+    * 135e6ba (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    let mut ws = graph.into_workspace()?;
+    let mut editor = Editor::create_with_opts(
+        &mut ws,
+        &mut *meta,
+        &repo,
+        &GraphEditorOptions {
+            sign_mode: PickSignMode::Never,
+        },
+    )?;
+
+    // Force sign the top commit
+    let top_commit_id = repo.rev_parse_single("top")?.detach();
+    let top_commit_sel = editor.select_commit(top_commit_id)?;
+    let mut pick = Pick::new_pick(top_commit_id);
+    pick.sign_mode = PickSignMode::Force;
+    editor.replace(top_commit_sel, Step::Pick(pick))?;
+
+    let outcome = editor.rebase()?;
+    let materialize_outcome = outcome.materialize()?;
+
+    let after = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(after, @"
+    * 94b4c45 (HEAD -> main, top) top
+    * 135e6ba (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let commit_mappings = materialize_outcome.history.commit_mappings();
+    assert_eq!(
+        commit_mappings.len(),
+        1,
+        "expected 1 commit to be cherry-picked"
+    );
+    let new_commit_id = commit_mappings
+        .get(&top_commit_id)
+        .expect("the force-signed commit should be in the commit mappings");
+
+    let new_commit = repo.find_commit(*new_commit_id)?;
+    assert!(
+        new_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_some(),
+        "expected the force-signed commit to be signed"
+    );
+
+    Ok(())
+}
+
+/// Picking with [`PickSignMode::Force`] should _not_ cause a cascade of signatures on
+/// descendants that are picked with [`PickSignMode::Never`].
+#[test]
+fn force_signing_ancestor_does_not_sign_descendants_that_are_picked_with_sign_mode_never()
+-> Result<()> {
+    let (repo, _tmpdir, mut meta) = fixture_writable_with_signing(
+        "unsigned-commits-with-signing-key-setup-but-signing-disabled",
+    )?;
+
+    let before = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(before, @"
+    * ea8caac (HEAD -> main, top) top
+    * 135e6ba (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    let mut ws = graph.into_workspace()?;
+    let mut editor = Editor::create_with_opts(
+        &mut ws,
+        &mut *meta,
+        &repo,
+        &GraphEditorOptions {
+            sign_mode: PickSignMode::Never,
+        },
+    )?;
+
+    let top_commit_id = repo.rev_parse_single("top")?.detach();
+    let mid_commit_id = repo.rev_parse_single("mid")?.detach();
+
+    // We pick the mid commit with force. This should cause it to be signed, but its descendant
+    // top commit should _not_ get signed as it was picked with PickSignMode::Never.
+    let mid_sel = editor.select_commit(mid_commit_id)?;
+    let mut pick = Pick::new_pick(mid_commit_id);
+    pick.sign_mode = PickSignMode::Force;
+    editor.replace(mid_sel, Step::Pick(pick))?;
+
+    let outcome = editor.rebase()?;
+    let materialize_outcome = outcome.materialize()?;
+
+    let after = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(after, @"
+    * 5f964cc (HEAD -> main, top) top
+    * 9320e55 (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let commit_mappings = materialize_outcome.history.commit_mappings();
+    assert_eq!(
+        commit_mappings.len(),
+        2,
+        "expected 2 commits to be cherry-picked"
+    );
+    let new_mid_commit_id = commit_mappings
+        .get(&mid_commit_id)
+        .expect("the force-signed commit should be in the commit mappings");
+    let new_top_commit_id = commit_mappings
+        .get(&top_commit_id)
+        .expect("the head commit should be in the commit mappings");
+
+    let new_top_commit = repo.find_commit(*new_top_commit_id)?;
+    let new_mid_commit = repo.find_commit(*new_mid_commit_id)?;
+    assert!(
+        new_top_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_none(),
+        "top commit should not have been cascade-signed"
+    );
+    assert!(
+        new_mid_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_some(),
+        "mid commit should have been force-signed"
+    );
+
+    Ok(())
+}
+
+/// Picking with with [`PickSignMode::Force`] _should_ cause a cascade of signatures when
+/// descendants are picked with an unguarded [`PickSignMode::IfChanged`].
+///
+/// This is the primary mechanism by which we can programmatically sign/re-sign a branch
+/// independently of Git-compatible configuration.
+#[test]
+fn force_signing_ancestor_triggers_cascading_signatures_of_descendants_that_are_picked_with_unguarded_sign_mode_ifchanged()
+-> Result<()> {
+    let (repo, _tmpdir, mut meta) = fixture_writable_with_signing(
+        "unsigned-commits-with-signing-key-setup-but-signing-disabled",
+    )?;
+
+    let before = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(before, @"
+    * ea8caac (HEAD -> main, top) top
+    * 135e6ba (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    let mut ws = graph.into_workspace()?;
+    let mut editor = Editor::create_with_opts(
+        &mut ws,
+        &mut *meta,
+        &repo,
+        &GraphEditorOptions {
+            sign_mode: PickSignMode::IfChanged(PickSignGuard::None),
+        },
+    )?;
+
+    let top_commit_id = repo.rev_parse_single("top")?.detach();
+    let mid_commit_id = repo.rev_parse_single("mid")?.detach();
+
+    // We pick the mid commit with force. This should cause it to be signed, and its descendant
+    // top commit should get signed through the cascading rewrites.
+    let mid_sel = editor.select_commit(mid_commit_id)?;
+    let mut pick = Pick::new_pick(mid_commit_id);
+    pick.sign_mode = PickSignMode::Force;
+    editor.replace(mid_sel, Step::Pick(pick))?;
+
+    let outcome = editor.rebase()?;
+    let materialize_outcome = outcome.materialize()?;
+
+    let after = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(after, @"
+    * 4d0bf76 (HEAD -> main, top) top
+    * 9320e55 (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let commit_mappings = materialize_outcome.history.commit_mappings();
+    assert_eq!(
+        commit_mappings.len(),
+        2,
+        "expected 2 commits to be cherry-picked"
+    );
+    let new_mid_commit_id = commit_mappings
+        .get(&mid_commit_id)
+        .expect("the force-signed commit should be in the commit mappings");
+    let new_top_commit_id = commit_mappings
+        .get(&top_commit_id)
+        .expect("the head commit should be in the commit mappings");
+
+    let new_top_commit = repo.find_commit(*new_top_commit_id)?;
+    let new_mid_commit = repo.find_commit(*new_mid_commit_id)?;
+    assert!(
+        new_mid_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_some(),
+        "mid commit should be signed"
+    );
+    assert!(
+        new_top_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_some(),
+        "top commit should be signed"
+    );
+
+    Ok(())
+}
+
+/// A commit should not be signed in the event it gets picked with a
+/// [`PickSignGuard::IfSignCommitsEnabled`]-guarded [`PickSignMode::IfChanged`], and Git-compatible
+/// signing is not enabled in the config.
+#[test]
+fn commit_picked_with_ifchange_and_configuration_guard_is_not_signed_when_signing_config_is_not_enabled()
+-> Result<()> {
+    let (repo, _tmpdir, mut meta) = fixture_writable_with_signing(
+        "unsigned-commits-with-signing-key-setup-but-signing-disabled",
+    )?;
+
+    let before = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(before, @"
+    * ea8caac (HEAD -> main, top) top
+    * 135e6ba (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    let mut ws = graph.into_workspace()?;
+
+    let mut editor = Editor::create_with_opts(
+        &mut ws,
+        &mut *meta,
+        &repo,
+        &GraphEditorOptions {
+            sign_mode: PickSignMode::IfChanged(PickSignGuard::IfSignCommitsEnabled),
+        },
+    )?;
+
+    let top_commit_id = repo.rev_parse_single("top")?.detach();
+    let mid_commit_id = repo.rev_parse_single("mid")?.detach();
+
+    // Delete the mid commit so the top commit gets picked. The top commit should _NOT_ get signed
+    // as signing config is not enabled, and there is a sign guard in place on the pick.
+    let mid_sel = editor.select_commit(mid_commit_id)?;
+    editor.replace(mid_sel, Step::None)?;
+
+    let outcome = editor.rebase()?;
+    let materialize_outcome = outcome.materialize()?;
+
+    let after = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(after, @"
+    * f923739 (HEAD -> main, top) top
+    * 7a5aacf (mid, base) base
+    ");
+
+    let commit_mappings = materialize_outcome.history.commit_mappings();
+    assert_eq!(
+        commit_mappings.len(),
+        1,
+        "expected 1 commit to be cherry-picked"
+    );
+    let new_top_commit_id = commit_mappings
+        .get(&top_commit_id)
+        .expect("the head commit should be in the commit mappings");
+
+    let new_commit = repo.find_commit(*new_top_commit_id)?;
+    assert!(
+        new_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_none(),
+        "the cherry-picked top commit should not be signed due to the sign guard"
+    );
+
+    Ok(())
+}
+
+/// Test for an edge case where a parent-less commit would not be signed even when picked with
+/// [`PickSignMode::Force`].
+#[test]
+fn parentless_commit_picked_with_force_sign_is_signed() -> Result<()> {
+    let (repo, _tmpdir, mut meta) = fixture_writable_with_signing(
+        "unsigned-commits-with-signing-key-setup-but-signing-disabled",
+    )?;
+
+    let before = visualize_commit_graph_all(&repo)?;
+    insta::assert_snapshot!(before, @"
+    * ea8caac (HEAD -> main, top) top
+    * 135e6ba (mid) mid
+    * 7a5aacf (base) base
+    ");
+
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    let mut ws = graph.into_workspace()?;
+
+    let mut editor = Editor::create_with_opts(
+        &mut ws,
+        &mut *meta,
+        &repo,
+        &GraphEditorOptions {
+            sign_mode: PickSignMode::IfChanged(PickSignGuard::IfSignCommitsEnabled),
+        },
+    )?;
+
+    let base_commit_id = repo.rev_parse_single("base")?.detach();
+
+    // We pick the base commit with force, which should cause it to get signed.
+    let base_sel = editor.select_commit(base_commit_id)?;
+    let mut pick = Pick::new_pick(base_commit_id);
+    pick.sign_mode = PickSignMode::Force;
+    editor.replace(base_sel, Step::Pick(pick))?;
+
+    let outcome = editor.rebase()?;
+    let materialize_outcome = outcome.materialize()?;
+
+    let commit_mappings = materialize_outcome.history.commit_mappings();
+    let new_base_commit_id = commit_mappings
+        .get(&base_commit_id)
+        .expect("the base commit should be in the commit mappings");
+
+    let new_base_commit = repo.find_commit(*new_base_commit_id)?;
+    assert!(
+        new_base_commit
+            .decode()?
+            .extra_headers()
+            .pgp_signature()
+            .is_some(),
+        "the cherry-picked base commit should be signed"
+    );
+
+    Ok(())
+}