diff mbox series

[v2,1/2] t0001-t0028: avoid pipes with Git on LHS

Message ID 20220227122453.25278-2-shivam828787@gmail.com (mailing list archive)
State Superseded
Headers show
Series avoid pipes with Git on LHS | expand

Commit Message

Shubham Mishra Feb. 27, 2022, 12:24 p.m. UTC 
Pipes ignore error codes of LHS command and thus we should not use
them with Git in tests. As an alternative, use a 'tmp' file to write
the Git output so we can test the exit code.

Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
---
 t/t0000-basic.sh            | 10 ++++++----
 t/t0022-crlf-rename.sh      |  4 ++--
 t/t0025-crlf-renormalize.sh |  4 ++--
 t/t0027-auto-crlf.sh        | 12 ++++++------
 4 files changed, 16 insertions(+), 14 deletions(-)

Comments

Johannes Sixt Feb. 27, 2022, 5:43 p.m. UTC | #1 
Am 27.02.22 um 13:24 schrieb Shubham Mishra:
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index b007f0efef..b5fa76059b 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh

> @@ -1104,12 +1105,13 @@ test_expect_success 'very long name in the index handled sanely' '
>  	>path4 &&
>  	git update-index --add path4 &&
>  	(
> -		git ls-files -s path4 |
> -		sed -e "s/	.*/	/" |
> +		git ls-files -s path4 >tmp &&
> +		sed -e "s/	.*/	/" tmp |
>  		tr -d "\012" &&
>  		echo "$a"
>  	) | git update-index --index-info &&

We see a pipe here, and in the upstream of that pipe is a git
invocation. That should be fixed, too. After the rewrite that you
already did here, it should be sufficient to move the git invocation
before the parentheses.

> -	len=$(git ls-files "a*" | wc -c) &&
> +	git ls-files "a*" >tmp &&
> +	len=$(wc -c <tmp) &&
>  	test $len = 4098
>  '

-- Hannes
Shubham Mishra March 3, 2022, 12:48 p.m. UTC | #2 
On Sun, Feb 27, 2022 at 11:13 PM Johannes Sixt <j6t@kdbg.org> wrote:


> We see a pipe here, and in the upstream of that pipe is a git
> invocation. That should be fixed, too. After the rewrite that you
> already did here, it should be sufficient to move the git invocation
> before the parentheses.

I missed that pipe, Thanks for pointing it out and reviewing.

Sorry for responding late, I am in the middle of my exams, so I am
hardly checking mails.

Thanks,
Shubham
diff mbox series

Patch

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index b007f0efef..b5fa76059b 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1089,7 +1089,8 @@  test_expect_success 'update-index D/F conflict' '
 	mv path2 path0 &&
 	mv tmp path2 &&
 	git update-index --add --replace path2 path0/file2 &&
-	numpath0=$(git ls-files path0 | wc -l) &&
+	git ls-files path0 >tmp &&
+	numpath0=$(wc -l <tmp) &&
 	test $numpath0 = 1
 '
 
@@ -1104,12 +1105,13 @@  test_expect_success 'very long name in the index handled sanely' '
 	>path4 &&
 	git update-index --add path4 &&
 	(
-		git ls-files -s path4 |
-		sed -e "s/	.*/	/" |
+		git ls-files -s path4 >tmp &&
+		sed -e "s/	.*/	/" tmp |
 		tr -d "\012" &&
 		echo "$a"
 	) | git update-index --index-info &&
-	len=$(git ls-files "a*" | wc -c) &&
+	git ls-files "a*" >tmp &&
+	len=$(wc -c <tmp) &&
 	test $len = 4098
 '
 
diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh
index c1a331e9e9..9fe9891251 100755
--- a/t/t0022-crlf-rename.sh
+++ b/t/t0022-crlf-rename.sh
@@ -24,8 +24,8 @@  test_expect_success setup '
 
 test_expect_success 'diff -M' '
 
-	git diff-tree -M -r --name-status HEAD^ HEAD |
-	sed -e "s/R[0-9]*/RNUM/" >actual &&
+	git diff-tree -M -r --name-status HEAD^ HEAD >tmp &&
+	sed -e "s/R[0-9]*/RNUM/" tmp >actual &&
 	echo "RNUM	sample	elpmas" >expect &&
 	test_cmp expect actual
 
diff --git a/t/t0025-crlf-renormalize.sh b/t/t0025-crlf-renormalize.sh
index 81447978b7..f7202c192e 100755
--- a/t/t0025-crlf-renormalize.sh
+++ b/t/t0025-crlf-renormalize.sh
@@ -22,8 +22,8 @@  test_expect_success 'renormalize CRLF in repo' '
 	i/lf w/lf attr/text=auto LF.txt
 	i/lf w/mixed attr/text=auto CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol |
-	sed -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol >tmp &&
+	sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index c5f7ac63b0..0feb41a23f 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -311,8 +311,8 @@  checkout_files () {
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__CRLF_nul.txt
 		i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr $aeol) crlf_false_attr__LF_nul.txt
 		EOF
-		git ls-files --eol crlf_false_attr__* |
-		sed -e "s/	/ /g" -e "s/  */ /g" |
+		git ls-files --eol crlf_false_attr__* >tmp &&
+		sed -e "s/	/ /g" -e "s/  */ /g" tmp |
 		sort >actual &&
 		test_cmp expect actual
 	'
@@ -359,12 +359,12 @@  test_expect_success 'ls-files --eol -o Text/Binary' '
 	i/ w/crlf TeBi_126_CL
 	i/ w/-text TeBi_126_CLC
 	EOF
-	git ls-files --eol -o |
+	git ls-files --eol -o >tmp &&
 	sed -n -e "/TeBi_/{s!attr/[	]*!!g
 	s!	! !g
 	s!  *! !g
 	p
-	}" | sort >actual &&
+	}" tmp | sort >actual &&
 	test_cmp expect actual
 '
 
@@ -617,8 +617,8 @@  test_expect_success 'ls-files --eol -d -z' '
 	i/lf w/ crlf_false_attr__LF.txt
 	i/mixed w/ crlf_false_attr__CRLF_mix_LF.txt
 	EOF
-	git ls-files --eol -d |
-	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" |
+	git ls-files --eol -d >tmp &&
+	sed -e "s!attr/[^	]*!!g" -e "s/	/ /g" -e "s/  */ /g" tmp |
 	sort >actual &&
 	test_cmp expect actual
 '