Re: [PATCH] t7406: Make a test_must_fail call fail for the right reason

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Aug 3, 2018 at 7:14 PM Elijah Newren <newren@gmail.com> wrote:
> A test making use of test_must_fail was failing like this:
>   fatal: ambiguous argument '|': unknown revision or path not in the working tree.
> when the intent was to verify that a specific string was not found
> in the output of the git diff command, i.e. that grep returned
> non-zero.  Fix the test to do that.
> ---
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> @@ -599,7 +599,7 @@ test_expect_success 'submodule update - update=none in .git/config but --checkou
>          git diff --raw | grep "        submodule" &&
>          git submodule update --checkout &&
> -        test_must_fail git diff --raw \| grep "        submodule" &&
> +        git diff --raw | test_must_fail grep " submodule" &&

Unfortunately, this is a mis-use of test_must_fail() which is intended
only for Git commands; it does extra checking to ensure that the Git
command failed in a sane way (say, by returning a failing exit status)
rather than by crashing. It's not intended for use with system
commands which are assumed to be bug-free.

Having a Git command upstream of a pipe is also discouraged since the
pipe swallows its exit status, which means we won't know if the Git
command actually crashed. So, what you really want is this:

    git diff --raw >out &&
    ! grep "<literal-tab>" out &&

(where <literal-tab> is a literal TAB)

Since this script has a number of instances of Git commands upstream
pipes, it may not make sense to fix just this one. So, either a
preparatory cleanup patch could fix them all at once, and then this
patch could come on top or, if you don't want to fix all the pipe
cases, you could do this instead:

    ! git diff --raw | grep "<literal-tab>" &&