diff mbox series

[v2,09/22] t/t4*: avoid redundant uses of cat

Message ID 20240315194620.10713-10-dev+git@drbeat.li (mailing list archive)
State Accepted
Commit 237ce762ef9ee360ae242cb47af808fc617c1d0a
Headers show
Series avoid redundant pipelines | expand

Commit Message

Beat Bolli March 15, 2024, 7:46 p.m. UTC
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 t/t4020-diff-external.sh         | 2 +-
 t/t4205-log-pretty-formats.sh    | 2 +-
 t/t4301-merge-tree-write-tree.sh | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

Comments

Taylor Blau March 16, 2024, 1:34 a.m. UTC | #1
On Fri, Mar 15, 2024 at 08:46:06PM +0100, Beat Bolli wrote:
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> index 12ac43687366..578641467753 100755
> --- a/t/t4301-merge-tree-write-tree.sh
> +++ b/t/t4301-merge-tree-write-tree.sh
> @@ -313,7 +313,7 @@ test_expect_success 'rename/add handling' '
>  		# First, check that the bar that appears at stage 3 does not
>  		# correspond to an individual blob anywhere in history
>  		#
> -		hash=$(cat out | tr "\0" "\n" | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
> +		hash=$(tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
>  		git rev-list --objects --all >all_blobs &&
>  		! grep $hash all_blobs &&

This and the two similar transformations below it look good to me. This
obviously isn't the fault of your patch (nor should it necessarily be
its aim to fix), but I wonder if it would be worthwhile to extract the

    tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " "

pattern into a helper function, since it's used in a few places in this
test script.

That's just a suggestion, and shouldn't hold up this patch/series. Maybe
just some #leftoverbits :-).

Thanks,
Taylor
Junio C Hamano March 16, 2024, 6:04 a.m. UTC | #2
Taylor Blau <me@ttaylorr.com> writes:

> This and the two similar transformations below it look good to me. This
> obviously isn't the fault of your patch (nor should it necessarily be
> its aim to fix), but I wonder if it would be worthwhile to extract the
>
>     tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " "
>
> pattern into a helper function, since it's used in a few places in this
> test script.

I somehow thought that the theme of the topic is to reduce the depth
of the pipeline.  "head -n 3" piped into "grep" sounds something a
single sed script can do, e.g.

	tr '\000' '\012' <out |	sed -n -e '1,3s/3.bar/&/p'

> That's just a suggestion, and shouldn't hold up this patch/series. Maybe
> just some #leftoverbits :-).

Ditto.  The whole thing can be turned into a Perl scriptlet, which
may be even easier to read (the cost to spin up one Perl interpreter
might be greater than constructing 4 process pipeline on certain
systems, though).

Thanks.

P.S. It is nice to hear from you Taylor.  It's been a while.
diff mbox series

Patch

diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index c1ac09ecc714..fdd865f7c38d 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -232,7 +232,7 @@  keep_only_cr () {
 test_expect_success 'external diff with autocrlf = true' '
 	test_config core.autocrlf true &&
 	GIT_EXTERNAL_DIFF=./fake-diff.sh git diff &&
-	test $(wc -l < crlfed.txt) = $(cat crlfed.txt | keep_only_cr | wc -c)
+	test $(wc -l <crlfed.txt) = $(keep_only_cr <crlfed.txt | wc -c)
 '
 
 test_expect_success 'diff --cached' '
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index e3d655e6b8b5..1409eebcd855 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -156,7 +156,7 @@  test_expect_success 'NUL termination with --reflog --pretty=oneline' '
 	for r in $revs
 	do
 		git show -s --pretty=oneline "$r" >raw &&
-		cat raw | lf_to_nul || return 1
+		lf_to_nul <raw || return 1
 	done >expect &&
 	# the trailing NUL is already produced so we do not need to
 	# output another one
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 12ac43687366..578641467753 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -313,7 +313,7 @@  test_expect_success 'rename/add handling' '
 		# First, check that the bar that appears at stage 3 does not
 		# correspond to an individual blob anywhere in history
 		#
-		hash=$(cat out | tr "\0" "\n" | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
+		hash=$(tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
 		git rev-list --objects --all >all_blobs &&
 		! grep $hash all_blobs &&
 
@@ -380,7 +380,7 @@  test_expect_success SYMLINKS 'rename/add, where add is a mode conflict' '
 		# First, check that the bar that appears at stage 3 does not
 		# correspond to an individual blob anywhere in history
 		#
-		hash=$(cat out | tr "\0" "\n" | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
+		hash=$(tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
 		git rev-list --objects --all >all_blobs &&
 		! grep $hash all_blobs &&
 
@@ -630,8 +630,8 @@  test_expect_success 'mod6: chains of rename/rename(1to2) and add/add via collidi
 		# conflict entries do not appear as individual blobs anywhere
 		# in history.
 		#
-		hash1=$(cat out | tr "\0" "\n" | head | grep 2.four | cut -f 2 -d " ") &&
-		hash2=$(cat out | tr "\0" "\n" | head | grep 3.two | cut -f 2 -d " ") &&
+		hash1=$(tr "\0" "\n" <out | head | grep 2.four | cut -f 2 -d " ") &&
+		hash2=$(tr "\0" "\n" <out | head | grep 3.two | cut -f 2 -d " ") &&
 		git rev-list --objects --all >all_blobs &&
 		! grep $hash1 all_blobs &&
 		! grep $hash2 all_blobs &&