Web lists-archives.com

Re: [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually




Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

>  test_expect_success 'merge my-side@{u} records the correct name' '
>  (
> -	cd clone || exit
> -	git checkout master || exit
> -	git branch -D new ;# can fail but is ok
> +	cd clone &&
> +	git checkout master &&
> +	test_might_fail git branch -D new &&

I wonder why we had these "|| exit" in the first place, but these
are obviously good changes.

> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index 1e81b33b2e..39133bcbc8 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -435,7 +435,7 @@ test_expect_success 'writing split index with null sha1 does not write cache tre
>  	commit=$(git commit-tree $tree -p HEAD <msg) &&
>  	git update-ref HEAD "$commit" &&
>  	GIT_ALLOW_NULL_SHA1=1 git reset --hard &&
> -	(test-tool dump-cache-tree >cache-tree.out || true) &&
> +	test_might_fail test-tool dump-cache-tree >cache-tree.out &&
>  	test_line_count = 0 cache-tree.out
>  '

I wonder where the unstable expectation for the exit status comes
from.  4bddd983 ("split-index: don't write cache tree with null oid
entries", 2018-01-07) is not exactly illuminating, either X-<.

> diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
> index 0a8af76aab..6579c81216 100755
> --- a/t/t4012-diff-binary.sh
> +++ b/t/t4012-diff-binary.sh
> @@ -102,10 +102,8 @@ test_expect_success 'apply binary patch' '
>  
>  test_expect_success 'diff --no-index with binary creation' '
>  	echo Q | q_to_nul >binary &&
> -	(: hide error code from diff, which just indicates differences
> -	 git diff --binary --no-index /dev/null binary >current ||
> -	 true
> -	) &&
> +	# hide error code from diff, which just indicates differences
> +	test_might_fail git diff --binary --no-index /dev/null binary >current &&

As you said in downthread, we only care about the correct output in
the "current" file, which is verified by using it as input to the
"apply --binary", so might-fail is fine here.  But we probably want
to make sure "diff --binary --no-index" to *notice* that there is a
difference and report that with its exit status.  I wouldn't say so
if this was about testing "git apply", but the test title tells us
that it is about "diff --no-index", so...