Re: [PATCH 2/5] name-rev: extend --refs to accept multiple patterns

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Jake,

On Wed, 11 Jan 2017, Jacob Keller wrote:

> diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
> index 1408b608eb03..d072ec43b016 100755
> --- a/t/t6007-rev-list-cherry-pick-file.sh
> +++ b/t/t6007-rev-list-cherry-pick-file.sh
> @@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '
>  	test_cmp actual.named expect
>  '
>  
> +test_expect_success 'name-rev multiple --refs combine inclusive' '
> +	git rev-list --left-right --cherry-pick F...E -- bar > actual &&

Our current coding style seems to skip the space between `>` and `actual`
(this applies to all redirections added in this patch).

> +	git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
> +		< actual > actual.named &&
> +	test_cmp actual.named expect
> +'
> +
> +cat >expect <<EOF
> +<tags/F
> +$(git rev-list --left-right --right-only --cherry-pick F...E -- bar)
> +EOF

In the current revision of t6007, we seem to list the expected output
explicitly, i.e. *not* generating it dynamically.

If you *do* insist to generate the `expect` file dynamically, a better way
would be to include that generation in the `test_expect_success` code so
that errors in the call can be caught, too:

test_expect_success 'name-rev --refs excludes non-matched patterns' '
	echo "<tags/F" >expect &&
	git rev-list --left-right --right-only --cherry-pick F...E -- \
		bar >>expect &&
	[...]

However, if I was asked for my preference, I would suggest to specify the
`expect` contents explicitly, to document the expectation as of time of
writing. The reason: I debugged my share of test breakages and these
dynamically-generated `expect` files are the worst. When things break, you
have to dig *real* deep to figure out what is going wrong, as sometimes
the *generation of the `expect` file* regresses.

Ciao,
Dscho