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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 12, 2012, 19:33 UTC
Message-ID: <20120112193312.GC6038@burratino>
In-Reply-To: <1326395132-18947-1-git-send-email-artagnon@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' '

[...]

Show 11 quoted lines

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