Re: [PATCH 2/3] t: teach 'test_must_fail' to save the command's stderr to a file

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 9, 2018, 03:11 UTC
Message-ID: <CAPig+cRUqwOMuv60FsN7Xm1iqnkBBPRP8wW+JpCXTfi8vOHbew@mail.gmail.com>
In-Reply-To: <20180209024235.3431-3-szeder.dev@gmail.com>

On Thu, Feb 8, 2018 at 9:42 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:

Show 21 quoted lines

> To check that a git command fails and to inspect its error message we
> usually execute a command like this throughout our test suite:
>
>   test_must_fail git command --option 2>output.err
>
> Note that this command doesn't limit the redirection to the git
> command, but it redirects the standard error of the 'test_must_fail'
> helper function as well.  This is wrong for several reasons:
>
>   [...snip rationale...]
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
> diff --git a/t/README b/t/README
> @@ -430,6 +430,10 @@ Don't:
>     use 'test_must_fail git cmd'.  This will signal a failure if git
>     dies in an unexpected way (e.g. segfault).
>
> +   Don't redirect 'test_must_fail's standard error like
> +   'test_must_fail git cmd 2>err'.  Instead, run 'test_must_fail
> +   stderr=err git cmd'.

Perhaps not worth a re-roll, but it might be nice to cite a bit of the rationale from the commit message for the "don't redirect" rule since it's not necessarily obvious for readers of t/README why this limitation exists (and it seems unlikely they would check this commit message). The rationale here doesn't necessarily need to go into exquisite detail of the commit message, but could be perhaps as simple as:

    Don't redirect 'test_must_fail's standard error to a file (e.g.
    'test_must_fail git cmd 2>err') since error output from commands
    run internally by 'test_must_fail' may pollute file "err".
    Instead, run 'test_must_fail stderr=err git cmd'.