[14/22] t/t9*: avoid redundant uses of cat

Message ID	20240305212533.12947-15-dev+git@drbeat.li (mailing list archive)
State	Superseded
Headers	show Received: from mail-gateway-shared15.cyon.net (mail-gateway-shared15.cyon.net [194.126.200.68]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1447912D778 for <git@vger.kernel.org>; Tue, 5 Mar 2024 21:26:43 +0000 (UTC) From: "Beat Bolli" <bb@drbeat.li> To: git@vger.kernel.org Cc: Junio C Hamano <gitster@pobox.com>, Beat Bolli <dev+git@drbeat.li> Subject: [PATCH 14/22] t/t9*: avoid redundant uses of cat Date: Tue, 5 Mar 2024 22:25:13 +0100 Message-ID: <20240305212533.12947-15-dev+git@drbeat.li> In-Reply-To: <20240305212533.12947-1-dev+git@drbeat.li> References: <20240305212533.12947-1-dev+git@drbeat.li> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	avoid redundant pipelines \| expand [00/22] avoid redundant pipelines [01/22] doc: avoid redundant use of cat [02/22] contrib/subtree/t: avoid redundant use of cat [03/22] t/lib-cvs.sh: avoid redundant use of cat [04/22] t/annotate-tests.sh: avoid redundant use of cat [05/22] t/perf: avoid redundant use of cat [06/22] t/t0: avoid redundant uses of cat [07/22] t/t1: avoid redundant uses of cat [08/22] t/t3: avoid redundant uses of cat [09/22] t/t4: avoid redundant uses of cat [10/22] t/t5: avoid redundant uses of cat [11/22] t/t6: avoid redundant uses of cat [12/22] t/t7: avoid redundant use of cat [13/22] t/t8: avoid redundant use of cat [14/22] t/t9: avoid redundant uses of cat [15/22] t/t1: merge a "grep \| sed" pipeline [16/22] t/t3: merge a "grep \| awk" pipeline [17/22] t/t4: merge a "grep \| sed" pipeline [18/22] t/t5: merge a "grep \| sed" pipeline [19/22] t/t8: merge "grep \| sed" pipelines [20/22] t/t9*: merge "grep \| sed" pipelines [21/22] contrib/coverage-diff: avoid redundant pipelines [22/22] git-quiltimport: avoid an unnecessary subshell

Message ID

20240305212533.12947-15-dev+git@drbeat.li (mailing list archive)

State

Superseded

Headers

From: "Beat Bolli" <bb@drbeat.li>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Beat Bolli <dev+git@drbeat.li>
Subject: [PATCH 14/22] t/t9*: avoid redundant uses of cat
Date: Tue,  5 Mar 2024 22:25:13 +0100
Message-ID: <20240305212533.12947-15-dev+git@drbeat.li>
In-Reply-To: <20240305212533.12947-1-dev+git@drbeat.li>
References: <20240305212533.12947-1-dev+git@drbeat.li>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Series

avoid redundant pipelines | expand

Commit Message

Beat Bolli March 5, 2024, 9:25 p.m. UTC

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 t/t9300-fast-import.sh          | 10 +++++-----
 t/t9350-fast-export.sh          |  2 +-
 t/t9400-git-cvsserver-server.sh | 35 +++++++++++++++------------------
 t/t9802-git-p4-filetype.sh      |  2 +-
 t/t9807-git-p4-submit.sh        |  2 +-
 t/t9824-git-p4-git-lfs.sh       |  4 ++--
 6 files changed, 26 insertions(+), 29 deletions(-)

Comments

Rubén Justo March 5, 2024, 10:52 p.m. UTC | #1

On Tue, Mar 05, 2024 at 10:25:13PM +0100, Beat Bolli wrote:
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>  t/t9300-fast-import.sh          | 10 +++++-----
>  t/t9350-fast-export.sh          |  2 +-
>  t/t9400-git-cvsserver-server.sh | 35 +++++++++++++++------------------
>  t/t9802-git-p4-filetype.sh      |  2 +-
>  t/t9807-git-p4-submit.sh        |  2 +-
>  t/t9824-git-p4-git-lfs.sh       |  4 ++--
>  6 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index dbb5042b0b8f..c03adbdd145f 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -986,7 +986,7 @@ test_expect_success 'L: nested tree copy does not corrupt deltas' '
>  	test_when_finished "git update-ref -d refs/heads/L2" &&
>  	git fast-import <input &&
>  	git ls-tree L2 g/b/ >tmp &&
> -	cat tmp | cut -f 2 >actual &&
> +	cut -f 2 <tmp >actual &&
>  	test_cmp expect actual &&

Nit: Maybe we can avoid tmp.

>  	git fsck $(git rev-parse L2)
>  '
> @@ -2012,7 +2012,7 @@ test_expect_success 'Q: verify first notes tree' '
>  	100644 blob $commit2
>  	100644 blob $commit3
>  	EOF
> -	cat expect.unsorted | sort >expect &&
> +	sort expect.unsorted >expect &&

Nit: I wonder if we can also avoid the cat that just precedes this hunk.

>  '
> @@ -2053,7 +2053,7 @@ test_expect_success 'Q: verify second notes tree' '
>  	100644 blob $commit2
>  	100644 blob $commit3
>  	EOF
> -	cat expect.unsorted | sort >expect &&
> +	sort expect.unsorted >expect &&

Ditto.

>  	git cat-file -p refs/notes/foobar^^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
>  	test_cmp expect actual
>  '
> @@ -2091,7 +2091,7 @@ test_expect_success 'Q: verify third notes tree' '
>  	cat >expect.unsorted <<-EOF &&
>  	100644 blob $commit1
>  	EOF
> -	cat expect.unsorted | sort >expect &&
> +	sort expect.unsorted >expect &&

Ditto.

>  	git cat-file -p refs/notes/foobar2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
>  	test_cmp expect actual
>  '
> @@ -2118,7 +2118,7 @@ test_expect_success 'Q: verify fourth notes tree' '
>  	cat >expect.unsorted <<-EOF &&
>  	100644 blob $commit2
>  	EOF
> -	cat expect.unsorted | sort >expect &&
> +	sort expect.unsorted >expect &&

Ditto.

Junio C Hamano March 6, 2024, 12:43 a.m. UTC | #2

Rubén Justo <rjusto@gmail.com> writes:

>>  	test_when_finished "git update-ref -d refs/heads/L2" &&
>>  	git fast-import <input &&
>>  	git ls-tree L2 g/b/ >tmp &&
>> -	cat tmp | cut -f 2 >actual &&
>> +	cut -f 2 <tmp >actual &&
>>  	test_cmp expect actual &&
>
> Nit: Maybe we can avoid tmp.

Piping "git ls-tree" output to "cut" would hide the exit status of
"git ls-tree" if it fails, which is not a good idea, so I do not
think of a way to avoid tmp so easily.

>
>>  	git fsck $(git rev-parse L2)
>>  '
>> @@ -2012,7 +2012,7 @@ test_expect_success 'Q: verify first notes tree' '
>>  	100644 blob $commit2
>>  	100644 blob $commit3
>>  	EOF
>> -	cat expect.unsorted | sort >expect &&
>> +	sort expect.unsorted >expect &&
>
> Nit: I wonder if we can also avoid the cat that just precedes this hunk.

The whole thing reads like this:

test_expect_success 'Q: verify first notes tree' '
	cat >expect.unsorted <<-EOF &&
	100644 blob $commit1
	100644 blob $commit2
	100644 blob $commit3
	EOF
	cat expect.unsorted | sort >expect &&
	git cat-file -p refs/notes/foobar~2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
	test_cmp expect actual
'

As we are not in the business of debugging system-provided "sort",
I agree that

	sort >expect <<-EOF &&
	100644 blob $commit1
	100644 blob $commit2
	100644 blob $commit3
	EOF

without having to use expect.unsorted would probably make sense.
Well spotted.

This is outside the topic, but this test has different clean-up
opportunities that are not related to the "do not run cat a single
file and send its output into a pipe" pattern.  The expected output
we see here implicitly depends on the fact that the notes tree is so
small that it hasn't been reorganized using fan-out levels.  If the
algorithm to decide when to start using fan-out directories changes,
this test can break.  To avoid that, we may need to do something
like

	git ls-tree -r refs/notes/foobar~2 >ls-tree &&
	sed -e "s/ [0-9a-f]*               / /" -e "s|/||g" >actual &&

so that we will get the commit object name without the slashes that
show the fan-out directories.  Also, the current "actual" generation
hides the exit status from "git cat-file".

Rubén Justo March 6, 2024, 1:10 a.m. UTC | #3

On Tue, Mar 05, 2024 at 04:43:33PM -0800, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
> 
> >>  	git ls-tree L2 g/b/ >tmp &&
> >> -	cat tmp | cut -f 2 >actual &&
> >> +	cut -f 2 <tmp >actual &&
> >>  	test_cmp expect actual &&
> >
> > Nit: Maybe we can avoid tmp.
> 
> Piping "git ls-tree" output to "cut" would hide the exit status of
> "git ls-tree" if it fails, which is not a good idea, so I do not
> think of a way to avoid tmp so easily.

Right.  Thanks for pointing that out.

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index dbb5042b0b8f..c03adbdd145f 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -986,7 +986,7 @@  test_expect_success 'L: nested tree copy does not corrupt deltas' '
 	test_when_finished "git update-ref -d refs/heads/L2" &&
 	git fast-import <input &&
 	git ls-tree L2 g/b/ >tmp &&
-	cat tmp | cut -f 2 >actual &&
+	cut -f 2 <tmp >actual &&
 	test_cmp expect actual &&
 	git fsck $(git rev-parse L2)
 '
@@ -2012,7 +2012,7 @@  test_expect_success 'Q: verify first notes tree' '
 	100644 blob $commit2
 	100644 blob $commit3
 	EOF
-	cat expect.unsorted | sort >expect &&
+	sort expect.unsorted >expect &&
 	git cat-file -p refs/notes/foobar~2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	test_cmp expect actual
 '
@@ -2053,7 +2053,7 @@  test_expect_success 'Q: verify second notes tree' '
 	100644 blob $commit2
 	100644 blob $commit3
 	EOF
-	cat expect.unsorted | sort >expect &&
+	sort expect.unsorted >expect &&
 	git cat-file -p refs/notes/foobar^^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	test_cmp expect actual
 '
@@ -2091,7 +2091,7 @@  test_expect_success 'Q: verify third notes tree' '
 	cat >expect.unsorted <<-EOF &&
 	100644 blob $commit1
 	EOF
-	cat expect.unsorted | sort >expect &&
+	sort expect.unsorted >expect &&
 	git cat-file -p refs/notes/foobar2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	test_cmp expect actual
 '
@@ -2118,7 +2118,7 @@  test_expect_success 'Q: verify fourth notes tree' '
 	cat >expect.unsorted <<-EOF &&
 	100644 blob $commit2
 	EOF
-	cat expect.unsorted | sort >expect &&
+	sort expect.unsorted >expect &&
 	git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index e9a12c18bbd3..d86d07a79d4f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -236,7 +236,7 @@  EOF
 
 test_expect_success 'set up faked signed tag' '
 
-	cat signed-tag-import | git fast-import
+	git fast-import <signed-tag-import
 
 '
 
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 003c0b61d0ff..e499c7f95512 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -117,12 +117,12 @@  END VERIFICATION REQUEST
 EOF
 
 test_expect_success 'pserver authentication' '
-	cat request-anonymous | git-cvsserver pserver >log 2>&1 &&
+	git-cvsserver pserver <request-anonymous >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'pserver authentication failure (non-anonymous user)' '
-	if cat request-git | git-cvsserver pserver >log 2>&1
+	if git-cvsserver pserver <request-git >log 2>&1
 	then
 	    false
 	else
@@ -132,17 +132,17 @@  test_expect_success 'pserver authentication failure (non-anonymous user)' '
 '
 
 test_expect_success 'pserver authentication success (non-anonymous user with password)' '
-	cat login-git-ok | git-cvsserver pserver >log 2>&1 &&
+	git-cvsserver pserver <login-git-ok >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'pserver authentication (login)' '
-	cat login-anonymous | git-cvsserver pserver >log 2>&1 &&
+	git-cvsserver pserver <login-anonymous >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'pserver authentication failure (login/non-anonymous user)' '
-	if cat login-git | git-cvsserver pserver >log 2>&1
+	if git-cvsserver pserver <login-git >log 2>&1
 	then
 	    false
 	else
@@ -172,7 +172,7 @@  Root $WORKDIR
 EOF
 
 test_expect_success 'req_Root failure (relative pathname)' '
-	if cat request-relative | git-cvsserver pserver >log 2>&1
+	if git-cvsserver pserver <request-relative >log 2>&1
 	then
 		echo unexpected success
 		false
@@ -183,28 +183,26 @@  test_expect_success 'req_Root failure (relative pathname)' '
 '
 
 test_expect_success 'req_Root failure (conflicting roots)' '
-	cat request-conflict | git-cvsserver pserver >log 2>&1 &&
+	git-cvsserver pserver <request-conflict >log 2>&1 &&
 	tail log | grep "^error 1 Conflicting roots specified$"
 '
 
 test_expect_success 'req_Root (strict paths)' '
-	cat request-anonymous | git-cvsserver --strict-paths pserver "$SERVERDIR" >log 2>&1 &&
+	git-cvsserver --strict-paths pserver "$SERVERDIR" <request-anonymous >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'req_Root failure (strict-paths)' '
-	! cat request-anonymous |
-	git-cvsserver --strict-paths pserver "$WORKDIR" >log 2>&1
+	! git-cvsserver --strict-paths pserver "$WORKDIR" <request-anonymous >log 2>&1
 '
 
 test_expect_success 'req_Root (w/o strict-paths)' '
-	cat request-anonymous | git-cvsserver pserver "$WORKDIR/" >log 2>&1 &&
+	git-cvsserver pserver "$WORKDIR/" <request-anonymous >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'req_Root failure (w/o strict-paths)' '
-	! cat request-anonymous |
-	git-cvsserver pserver "$WORKDIR/gitcvs" >log 2>&1
+	! git-cvsserver pserver "$WORKDIR/gitcvs" <request-anonymous >log 2>&1
 '
 
 cat >request-base  <<EOF
@@ -217,27 +215,26 @@  Root /gitcvs.git
 EOF
 
 test_expect_success 'req_Root (base-path)' '
-	cat request-base | git-cvsserver --strict-paths --base-path "$WORKDIR/" pserver "$SERVERDIR" >log 2>&1 &&
+	git-cvsserver --strict-paths --base-path "$WORKDIR/" pserver "$SERVERDIR" <request-base >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'req_Root failure (base-path)' '
-	! cat request-anonymous |
-	git-cvsserver --strict-paths --base-path "$WORKDIR" pserver "$SERVERDIR" >log 2>&1
+	! git-cvsserver --strict-paths --base-path "$WORKDIR" pserver "$SERVERDIR" <request-anonymous >log 2>&1
 '
 
 GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled false || exit 1
 
 test_expect_success 'req_Root (export-all)' '
-	cat request-anonymous | git-cvsserver --export-all pserver "$WORKDIR" >log 2>&1 &&
+	git-cvsserver --export-all pserver "$WORKDIR" <request-anonymous >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
 test_expect_success 'req_Root failure (export-all w/o directory list)' '
-	! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'
+	! (git-cvsserver --export-all pserver <request-anonymous >log 2>&1 || false)'
 
 test_expect_success 'req_Root (everything together)' '
-	cat request-base | git-cvsserver --export-all --strict-paths --base-path "$WORKDIR/" pserver "$SERVERDIR" >log 2>&1 &&
+	git-cvsserver --export-all --strict-paths --base-path "$WORKDIR/" pserver "$SERVERDIR" <request-base >log 2>&1 &&
 	sed -ne \$p log | grep "^I LOVE YOU\$"
 '
 
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 2a6ee2a46787..bb236cd2b57a 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -175,7 +175,7 @@  test_expect_success 'keyword file create' '
 		cp k-text-k k-text-ko &&
 		p4 add -t text+ko k-text-ko &&
 
-		cat k-text-k | iconv -f ascii -t utf-16 >k-utf16-k &&
+		iconv -f ascii -t utf-16 <k-text-k >k-utf16-k &&
 		p4 add -t utf16+k k-utf16-k &&
 
 		cp k-utf16-k k-utf16-ko &&
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index af4b286f9d51..6ae7ced51be1 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -418,7 +418,7 @@  test_expect_success 'description with Jobs and values on separate lines' '
 			marshal_dump job0 <change &&
 			marshal_dump job1 <change
 		) | sort >jobs &&
-		cat jobname1 jobname2 | sort >expected &&
+		sort jobname1 jobname2 >expected &&
 		test_cmp expected jobs
 	)
 '
diff --git a/t/t9824-git-p4-git-lfs.sh b/t/t9824-git-p4-git-lfs.sh
index a28dbbdd566c..fd430403d716 100755
--- a/t/t9824-git-p4-git-lfs.sh
+++ b/t/t9824-git-p4-git-lfs.sh
@@ -17,8 +17,8 @@  test_file_in_lfs () {
 	sed -n '2,2 p' "$FILE" | grep "^oid " &&
 	sed -n '3,3 p' "$FILE" | grep "^size " &&
 	test_line_count = 3 "$FILE" &&
-	cat "$FILE" | grep "size $SIZE" &&
-	HASH=$(cat "$FILE" | grep "oid sha256:" | sed -e "s/oid sha256://g") &&
+	grep "size $SIZE" "$FILE" &&
+	HASH=$(grep "oid sha256:" "$FILE" | sed -e "s/oid sha256://g") &&
 	LFS_FILE=".git/lfs/objects/$(echo "$HASH" | cut -c1-2)/$(echo "$HASH" | cut -c3-4)/$HASH" &&
 	echo $EXPECTED_CONTENT >expect &&
 	test_path_is_file "$FILE" &&

[14/22] t/t9*: avoid redundant uses of cat

Commit Message

Comments

Patch