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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jul 21, 2021, 12:24 UTC
Message-ID: <60f811ffd40b8_1305dc2082a@natae.notmuch>
In-Reply-To: <175609272110e4f8d71dce3e6c4e6ca010a8f366.1626831744.git.gitgitgadget@gmail.com>

Elijah Newren via GitGitGadget wrote:

Show 9 quoted lines

> 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 @@

Show 15 quoted lines

> +#
> +# 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?

Show 7 quoted lines

> +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.

-- 
Felipe Contreras