[10/27] t4138: stop losing return codes of git commands
diff mbox series

Message ID d617cbaaf1232e45b077786afaa9d3465a9bbd35.1573779465.git.liu.denton@gmail.com
State New
Headers show
Series
  • t: general test cleanup + `set -o pipefail`
Related show

Commit Message

Denton Liu Nov. 15, 2019, 1 a.m. UTC
In a pipe, only the return code of the last command is used. Thus, all
other commands will have their return codes masked. Rewrite pipes so
that there are no git commands upstream so that we will know if a
command fails.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4138-apply-ws-expansion.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Eric Sunshine Nov. 16, 2019, 9 a.m. UTC | #1
On Thu, Nov 14, 2019 at 8:01 PM Denton Liu <liu.denton@gmail.com> wrote:
> In a pipe, only the return code of the last command is used. Thus, all
> other commands will have their return codes masked. Rewrite pipes so
> that there are no git commands upstream so that we will know if a
> command fails.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t4138-apply-ws-expansion.sh b/t/t4138-apply-ws-expansion.sh
> @@ -17,8 +17,8 @@ test_expect_success setup '
> -       git diff --no-index before after |
> -               sed -e "s/before/test-1/" -e "s/after/test-1/" >patch1.patch &&
> +       test_must_fail git diff --no-index before after >patch1.patch.raw &&
> +       sed -e "s/before/test-1/" -e "s/after/test-1/" patch1.patch.raw >patch1.patch &&

I think this is a semantically incorrect use of test_must_fail(). What
you're interested here is that git-diff found differences (as opposed
to not finding any), so you want to test its exit code which reflects
whether or not the files were different. Hence, the following would be
more appropriate:

    test_expect_code 1 git diff --no-index before after >patch1.patch.raw &&

Same comment applies to remaining changes in this patch.

Patch
diff mbox series

diff --git a/t/t4138-apply-ws-expansion.sh b/t/t4138-apply-ws-expansion.sh
index 3b636a63a3..60435d6d41 100755
--- a/t/t4138-apply-ws-expansion.sh
+++ b/t/t4138-apply-ws-expansion.sh
@@ -17,8 +17,8 @@  test_expect_success setup '
 	printf "\t%s\n" 1 2 3 >after &&
 	printf "%64s\n" a b c >>after &&
 	printf "\t%s\n" 4 5 6 >>after &&
-	git diff --no-index before after |
-		sed -e "s/before/test-1/" -e "s/after/test-1/" >patch1.patch &&
+	test_must_fail git diff --no-index before after >patch1.patch.raw &&
+	sed -e "s/before/test-1/" -e "s/after/test-1/" patch1.patch.raw >patch1.patch &&
 	printf "%64s\n" 1 2 3 4 5 6 >test-1 &&
 	printf "%64s\n" 1 2 3 a b c 4 5 6 >expect-1 &&
 
@@ -33,8 +33,8 @@  test_expect_success setup '
 		x=$(( $x + 1 ))
 	done &&
 	printf "\t%s\n" d e f >>after &&
-	git diff --no-index before after |
-		sed -e "s/before/test-2/" -e "s/after/test-2/" >patch2.patch &&
+	test_must_fail git diff --no-index before after >patch2.patch.raw &&
+	sed -e "s/before/test-2/" -e "s/after/test-2/" patch2.patch.raw >patch2.patch &&
 	printf "%64s\n" a b c d e f >test-2 &&
 	printf "%64s\n" a b c >expect-2 &&
 	x=1 &&
@@ -56,8 +56,8 @@  test_expect_success setup '
 		x=$(( $x + 1 ))
 	done &&
 	printf "\t%s\n" d e f >>after &&
-	git diff --no-index before after |
-	sed -e "s/before/test-3/" -e "s/after/test-3/" >patch3.patch &&
+	test_must_fail git diff --no-index before after >patch3.patch.raw &&
+	sed -e "s/before/test-3/" -e "s/after/test-3/" patch3.patch.raw >patch3.patch &&
 	printf "%64s\n" a b c d e f >test-3 &&
 	printf "%64s\n" a b c >expect-3 &&
 	x=0 &&
@@ -84,8 +84,8 @@  test_expect_success setup '
 		printf "\t%02d\n" $x >>after
 		x=$(( $x + 1 ))
 	done &&
-	git diff --no-index before after |
-	sed -e "s/before/test-4/" -e "s/after/test-4/" >patch4.patch &&
+	test_must_fail git diff --no-index before after >patch4.patch.raw &&
+	sed -e "s/before/test-4/" -e "s/after/test-4/" patch4.patch.raw >patch4.patch &&
 	>test-4 &&
 	x=0 &&
 	while test $x -lt 50