diff mbox series

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

Message ID 20240305212533.12947-10-dev+git@drbeat.li (mailing list archive)
State Superseded
Headers show
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/t4020-diff-external.sh         | 2 +-
 t/t4150-am.sh                    | 2 +-
 t/t4205-log-pretty-formats.sh    | 2 +-
 t/t4301-merge-tree-write-tree.sh | 8 ++++----
 4 files changed, 7 insertions(+), 7 deletions(-)

Comments

Junio C Hamano March 6, 2024, 12:49 a.m. UTC | #1
"Beat Bolli" <bb@drbeat.li> writes:

> @@ -786,7 +786,7 @@ test_expect_success 'am takes patches from a Pine mailbox' '
>  	rm -fr .git/rebase-apply &&
>  	git reset --hard &&
>  	git checkout first &&
> -	cat pine patch1 | git am &&
> +	git am pine patch1 &&
>  	test_path_is_missing .git/rebase-apply &&
>  	git diff --exit-code main^..HEAD
>  '

I am not so certain about this one.

We can say "sed can read from the file listed on the command line,
or it can read from its standard input, so we can use whichever is
convenient for us", as we are not in the business of testing "sed"
that is supplied by the system.

On the other hand, the ability of "git am" to read either from the
files listed on the command line or from the standard input is not a
given.  It is one of the many aspects of how "git am" behaves that
we are testing.  By changing a test that feeds the contents of the
mailboxes from the standard input to instead have the command read
these mailbox files listed on the command line, this changes what
gets tested.

All other changes in the file look good to me.

Thanks.
Eric Sunshine March 6, 2024, 1:08 a.m. UTC | #2
On Tue, Mar 5, 2024 at 4:31 PM Beat Bolli <bb@drbeat.li> wrote:
> diff --git 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)
>  '

Could also fix the style problem (drop whitespace after existing `<`
operator) while here, but not at all worth a reroll.

> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
> @@ -786,7 +786,7 @@ test_expect_success 'am takes patches from a Pine mailbox' '
>         git checkout first &&
> -       cat pine patch1 | git am &&
> +       git am pine patch1 &&

As with Junio, the semantic change made here concerned me.
Beat Bolli March 6, 2024, 8:58 p.m. UTC | #3
On 06.03.24 02:08, Eric Sunshine wrote:
> On Tue, Mar 5, 2024 at 4:31 PM Beat Bolli <bb@drbeat.li> wrote:
>> diff --git 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)
>>   '
> 
> Could also fix the style problem (drop whitespace after existing `<`
> operator) while here, but not at all worth a reroll.
> 
>> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
>> @@ -786,7 +786,7 @@ test_expect_success 'am takes patches from a Pine mailbox' '
>>          git checkout first &&
>> -       cat pine patch1 | git am &&
>> +       git am pine patch1 &&
> 
> As with Junio, the semantic change made here concerned me.

I was even more on the fence about this hunk than the others, but then 
the test was about 'am takes patches from a Pine mailbox', not 
specifically about reading a Pine mailbox from stdin. But I can drop 
this hunk in v2.

Cheers, Beat
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/t4150-am.sh b/t/t4150-am.sh
index 3b125762694e..080a07e9d414 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -786,7 +786,7 @@  test_expect_success 'am takes patches from a Pine mailbox' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
 	git checkout first &&
-	cat pine patch1 | git am &&
+	git am pine patch1 &&
 	test_path_is_missing .git/rebase-apply &&
 	git diff --exit-code main^..HEAD
 '
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 &&