Re: [PATCH v2] cherry-pick: add failing test for out-of-order pick

[Jonathan Nieder <jrnieder@gmail.com>]

Ramkumar Ramachandra wrote:

>  Had some weird compulsion to conform to the style of the other tests
>  in the previous iteration.

The tests you're talking about were introduced in commit 7b53b92f to
check for a buglet that made --strategy suppress the progress
reporting ("Finished one cherry-pick.") output cherry-pick normally
would emit.  

So no inconsistency here --- those tests are _intending_ to check the
output format and that cherry-pick, unlike cherry-pick --ff, produces
new commits (though it would probably be clearer to put checks for
these behaviors in separate test assertions), while the new failing
test you are introducing is not about those things.

Striving for a consistent style is certainly not weird.

> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -59,6 +59,23 @@ test_expect_success 'cherry-pick first..fourth works' '
[...]
> +	git cherry-pick fourth second third &&
> +	{
> +		git rev-list --reverse HEAD |
> +		git diff-tree --stdin -s --format=%s
> +	} >actual &&
> +	cat >expect <<-\EOF &&
> +	fourth
> +	second
> +	third
> +	EOF
> +	test_cmp expect actual

This still feels more convoluted than expected (e.g., why --reverse?).
Something like

	printf "%s\n" third second fourth >expect &&
	...
	git log --format=%s >actual &&
	test_cmp expect actual

should be plenty.