diff mbox series

[v2,1/8] t7601: test interaction of merge/rebase/fast-forward flags and options

Message ID 175609272110e4f8d71dce3e6c4e6ca010a8f366.1626831744.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Handle pull option precedence | expand

Commit Message

Elijah Newren July 21, 2021, 1:42 a.m. UTC
From: Elijah Newren <newren@gmail.com>

The interaction of rebase and merge flags and options was not well
tested.  Add several tests to check for correct behavior from the
following rules:
    * --ff-only vs. --[no-]rebase
      (and the related pull.ff=only vs. pull.rebase)
    * --rebase[=!false] vs. --no-ff and --ff
      (and the related pull.rebase=!false overrides pull.ff=!only)
    * command line flags take precedence over config, except:
      * --no-rebase heeds pull.ff=!only
      * pull.rebase=!false vs --no-ff and --ff

For more details behind these rules and a larger table of individual
cases, refer to https://lore.kernel.org/git/xmqqwnpqot4m.fsf@gitster.g/
and the links found therein.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t7601-merge-pull-config.sh | 182 +++++++++++++++++++++++++++++++++++
 1 file changed, 182 insertions(+)

Comments

Felipe Contreras July 21, 2021, 12:24 p.m. UTC | #1
Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> The interaction of rebase and merge flags and options was not well
> tested.  Add several tests to check for correct behavior from the
> following rules:
>     * --ff-only vs. --[no-]rebase
>       (and the related pull.ff=only vs. pull.rebase)
>     * --rebase[=!false] vs. --no-ff and --ff
>       (and the related pull.rebase=!false overrides pull.ff=!only)

This breaks current behavior.

>     * command line flags take precedence over config, except:
>       * --no-rebase heeds pull.ff=!only
>       * pull.rebase=!false vs --no-ff and --ff

This isn't explaind anywhere.

> --- a/t/t7601-merge-pull-config.sh
> +++ b/t/t7601-merge-pull-config.sh
> @@ -143,6 +143,188 @@

> +#
> +# Group 1: Interaction of --ff-only with --[no-]rebase
> +# (And related interaction of pull.ff=only with pull.rebase)
> +#
> +test_expect_failure '--ff-only overrides --rebase' '
> +	test_attempts_fast_forward pull --rebase --ff-only
> +'
> +
> +test_expect_failure '--ff-only overrides --rebase even if first' '
> +	test_attempts_fast_forward pull --ff-only --rebase
> +'
> +
> +test_expect_success '--ff-only overrides --no-rebase' '
> +	test_attempts_fast_forward pull --ff-only --no-rebase
> +'

These make absolutely no sense. Why does --rebase override pull.ff=only,
but not --ff-only?

> +test_expect_failure 'pull.ff=only overrides pull.rebase=true' '
> +	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=true pull
> +'
> +
> +test_expect_success 'pull.ff=only overrides pull.rebase=false' '
> +	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=false pull
> +'

This breaks existing behavior.

Users expect `-c pull.ff=only -c pull.rebase=true` to do a rebase.

> +test_expect_failure '--no-rebase takes precedence over pull.ff=only' '
> +	test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase
> +'

This breaks existing behavior.
diff mbox series

Patch

diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 52e8ccc933a..a44e6e69f2b 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -143,6 +143,188 @@  test_expect_success 'pull.rebase not set and --ff-only given (not-fast-forward)'
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
+test_does_rebase () {
+	git reset --hard c2 &&
+	git "$@" . c1 &&
+	# Check that we actually did a rebase
+	git rev-list --count HEAD >actual &&
+	git rev-list --merges --count HEAD >>actual &&
+	test_write_lines 3 0 >expect &&
+	test_cmp expect actual &&
+	rm actual expect
+}
+
+# Prefers merge over fast-forward
+test_does_merge_when_ff_possible () {
+	git reset --hard c0 &&
+	git "$@" . c1 &&
+	# Check that we actually did a merge
+	git rev-list --count HEAD >actual &&
+	git rev-list --merges --count HEAD >>actual &&
+	test_write_lines 3 1 >expect &&
+	test_cmp expect actual &&
+	rm actual expect
+}
+
+# Prefers fast-forward over merge or rebase
+test_does_fast_forward () {
+	git reset --hard c0 &&
+	git "$@" . c1 &&
+
+	# Check that we did not get any merges
+	git rev-list --count HEAD >actual &&
+	git rev-list --merges --count HEAD >>actual &&
+	test_write_lines 2 0 >expect &&
+	test_cmp expect actual &&
+
+	# Check that we ended up at c1
+	git rev-parse HEAD >actual &&
+	git rev-parse c1^{commit} >expect &&
+	test_cmp actual expect &&
+
+	# Remove temporary files
+	rm actual expect
+}
+
+# Doesn't fail when fast-forward not possible; does a merge
+test_falls_back_to_full_merge () {
+	git reset --hard c2 &&
+	git "$@" . c1 &&
+	# Check that we actually did a merge
+	git rev-list --count HEAD >actual &&
+	git rev-list --merges --count HEAD >>actual &&
+	test_write_lines 4 1 >expect &&
+	test_cmp expect actual &&
+	rm actual expect
+}
+
+# Attempts fast forward, which is impossible, and bails
+test_attempts_fast_forward () {
+	git reset --hard c2 &&
+	test_must_fail git "$@" . c1 2>err &&
+	test_i18ngrep "Not possible to fast-forward, aborting" err
+}
+
+#
+# Group 1: Interaction of --ff-only with --[no-]rebase
+# (And related interaction of pull.ff=only with pull.rebase)
+#
+test_expect_failure '--ff-only overrides --rebase' '
+	test_attempts_fast_forward pull --rebase --ff-only
+'
+
+test_expect_failure '--ff-only overrides --rebase even if first' '
+	test_attempts_fast_forward pull --ff-only --rebase
+'
+
+test_expect_success '--ff-only overrides --no-rebase' '
+	test_attempts_fast_forward pull --ff-only --no-rebase
+'
+
+test_expect_failure 'pull.ff=only overrides pull.rebase=true' '
+	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=true pull
+'
+
+test_expect_success 'pull.ff=only overrides pull.rebase=false' '
+	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=false pull
+'
+
+# Group 2: --rebase=[!false] overrides --no-ff and --ff
+# (And related interaction of pull.rebase=!false and pull.ff=!only)
+test_expect_success '--rebase overrides --no-ff' '
+	test_does_rebase pull --rebase --no-ff
+'
+
+test_expect_success '--rebase overrides --ff' '
+	test_does_rebase pull --rebase --ff
+'
+
+test_expect_success '--rebase fast-forwards when possible' '
+	test_does_fast_forward pull --rebase --ff
+'
+
+test_expect_success 'pull.rebase=true overrides pull.ff=false' '
+	test_does_rebase -c pull.rebase=true -c pull.ff=false pull
+'
+
+test_expect_success 'pull.rebase=true overrides pull.ff=true' '
+	test_does_rebase -c pull.rebase=true -c pull.ff=true pull
+'
+
+# Group 3: command line flags take precedence over config
+test_expect_failure '--ff-only takes precedence over pull.rebase=true' '
+	test_attempts_fast_forward -c pull.rebase=true pull --ff-only
+'
+
+test_expect_success '--ff-only takes precedence over pull.rebase=false' '
+	test_attempts_fast_forward -c pull.rebase=false pull --ff-only
+'
+
+test_expect_failure '--no-rebase takes precedence over pull.ff=only' '
+	test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase
+'
+
+test_expect_success '--rebase takes precedence over pull.ff=only' '
+	test_does_rebase -c pull.ff=only pull --rebase
+'
+
+test_expect_success '--rebase overrides pull.ff=true' '
+	test_does_rebase -c pull.ff=true pull --rebase
+'
+
+test_expect_success '--rebase overrides pull.ff=false' '
+	test_does_rebase -c pull.ff=false pull --rebase
+'
+
+test_expect_success '--rebase overrides pull.ff unset' '
+	test_does_rebase pull --rebase
+'
+
+# Group 4: --no-rebase heeds pull.ff=!only or explict --ff or --no-ff
+
+test_expect_success '--no-rebase works with --no-ff' '
+	test_does_merge_when_ff_possible pull --no-rebase --no-ff
+'
+
+test_expect_success '--no-rebase works with --ff' '
+	test_does_fast_forward pull --no-rebase --ff
+'
+
+test_expect_success '--no-rebase does ff if pull.ff unset' '
+	test_does_fast_forward pull --no-rebase
+'
+
+test_expect_success '--no-rebase heeds pull.ff=true' '
+	test_does_fast_forward -c pull.ff=true pull --no-rebase
+'
+
+test_expect_success '--no-rebase heeds pull.ff=false' '
+	test_does_merge_when_ff_possible -c pull.ff=false pull --no-rebase
+'
+
+# Group 5: pull.rebase=!false in combination with --no-ff or --ff
+test_expect_success 'pull.rebase=true and --no-ff' '
+	test_does_rebase -c pull.rebase=true pull --no-ff
+'
+
+test_expect_success 'pull.rebase=true and --ff' '
+	test_does_rebase -c pull.rebase=true pull --ff
+'
+
+test_expect_success 'pull.rebase=false and --no-ff' '
+	test_does_merge_when_ff_possible -c pull.rebase=false pull --no-ff
+'
+
+test_expect_success 'pull.rebase=false and --ff, ff possible' '
+	test_does_fast_forward -c pull.rebase=false pull --ff
+'
+
+test_expect_success 'pull.rebase=false and --ff, ff not possible' '
+	test_falls_back_to_full_merge -c pull.rebase=false pull --ff
+'
+
+# End of groupings for conflicting merge vs. rebase flags/options
+
 test_expect_success 'merge c1 with c2' '
 	git reset --hard c1 &&
 	test -f c0.c &&