Re: [PATCH 1/2] tests: use shell negation instead of test_must_fail for test_cmp

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 6, 2017, 20:21 UTC
Message-ID: <20171006202108.GX19555@aiede.mtv.corp.google.com>
In-Reply-To: <20171006190006.19623-1-sbeller@google.com>

Stefan Beller wrote:

Show 13 quoted lines

> The `test_must_fail` should only be used to indicate a git command is
> failing. `test_cmp` is not a git command, such that it doesn't need the
> special treatment of `test_must_fail` (which e.g. includes checking for
> segfault).
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  t/t3504-cherry-pick-rerere.sh         | 2 +-
>  t/t5512-ls-remote.sh                  | 2 +-
>  t/t5612-clone-refspec.sh              | 2 +-
>  t/t7508-status.sh                     | 2 +-
>  t/t9164-git-svn-dcommit-concurrent.sh | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)

Thanks. I agree that this is more readable, and it matches the advice in t/README:

   On the other hand, don't use test_must_fail for running regular
   platform commands; just use '! cmd'.  We are not in the business
   of verifying that the world given to us sanely works.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

I wonder if it would be useful to have a nod to that advice in the docstring in t/test-lib-functions.sh, too.