On Thu, Feb 8, 2018 at 9:42 PM, SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
> ---
> 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'.
