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

Reply via email to