Re: [PATCH v2 2/4] rebase: add tests for --no-rebase-merges

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Alex

Thanks for adding some tests,

On 21/02/2023 05:58, Alex Henrie wrote:
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>   t/t3430-rebase-merges.sh | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> index fa2a06c19f..e0d910c229 100755
> --- a/t/t3430-rebase-merges.sh
> +++ b/t/t3430-rebase-merges.sh
> @@ -250,6 +250,31 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
>   	EOF
>   '
>   
> +test_expect_success 'do not rebase merges unless asked to' '
> +	git checkout -b rebase-merges-default E &&
> +	before="$(git rev-parse --verify HEAD)" &&
> +	test_tick &&
> +	git rebase --rebase-merges C &&

I don't quite follow what this part of the test is for

> +	test_cmp_rev HEAD $before &&
> +	test_tick &&
> +	git rebase C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'
> +
> +test_expect_success '--no-rebase-merges countermands --rebase-merges' '
> +	git checkout -b no-rebase-merges E &&
> +	git rebase --rebase-merges --no-rebase-merges C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'

This test looks good. I think we could probably have just this second 
test and squash this into the previous patch - improving the 
documentation and test coverage for --no-rebase-merges would make a nice 
self-contained commit.

Best Wishes

Phillip

>   test_expect_success 'do not rebase cousins unless asked for' '
>   	git checkout -b cousins main &&
>   	before="$(git rev-parse --verify HEAD)" &&