diff mbox series

[v3,11/20] t: refactor tests depending on Perl substitution operator

Message ID 20250327-b4-pks-t-perlless-v3-11-b436de9da1b8@pks.im (mailing list archive)
State New
Headers show
Series t: drop Perl as a mandatory prerequisite | expand

Commit Message

Patrick Steinhardt March 27, 2025, 10:37 a.m. UTC
We have a bunch of tests that use Perl to perform substitution via the
"s/" operator. These usecases can be trivially replaced with sed(1).

Refactor the tests accordingly so that we can drop a couple of
PERL_TEST_HELPERS prerequisites.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0008-ignores.sh                        | 10 ++--------
 t/t4029-diff-trailing-space.sh            |  5 +++--
 t/t4200-rerere.sh                         | 12 +++---------
 t/t5303-pack-corruption-resilience.sh     | 10 ++++++----
 t/t5310-pack-bitmaps.sh                   |  4 ++--
 t/t5534-push-signed.sh                    |  4 ++--
 t/t6011-rev-list-with-bad-commit.sh       | 20 +++++++++-----------
 t/t7416-submodule-dash-url.sh             |  9 ++-------
 t/t7508-status.sh                         |  4 ++--
 t/t8006-blame-textconv.sh                 |  8 +-------
 t/t9137-git-svn-dcommit-clobber-series.sh | 14 ++++++++------
 11 files changed, 40 insertions(+), 60 deletions(-)

Comments

Johannes Schindelin April 1, 2025, 6:32 p.m. UTC | #1
Hi Patrick,

On Thu, 27 Mar 2025, Patrick Steinhardt wrote:

> We have a bunch of tests that use Perl to perform substitution via the
> "s/" operator. These usecases can be trivially replaced with sed(1).

... and sometimes `tr`.

In fact, it looks like...

> diff --git a/t/t7508-status.sh b/t/t7508-status.sh
> index 14c41b2cb7c..cdc1d6fcc78 100755
> --- a/t/t7508-status.sh
> +++ b/t/t7508-status.sh
> @@ -1064,9 +1064,9 @@ test_expect_success 'status -s submodule summary (clean submodule)' '
>  	test_cmp expect output
>  '
>
> -test_expect_success PERL_TEST_HELPERS 'status -z implies porcelain' '
> +test_expect_success 'status -z implies porcelain' '
>  	git status --porcelain |
> -	perl -pe "s/\012/\000/g" >expect &&
> +	tr "\012" "\000" >expect &&
>  	git status -z >output &&
>  	test_cmp expect output
>  '

... this change is not about `sed` at all, but only about `tr`.
_Technically_, this hunk would therefore feel more at home in the previous
patch. But practically, I actually do not mind it being here at all.

Thank you,
Johannes
Patrick Steinhardt April 2, 2025, 7:16 a.m. UTC | #2
On Tue, Apr 01, 2025 at 08:32:02PM +0200, Johannes Schindelin wrote:
> Hi Patrick,
> 
> On Thu, 27 Mar 2025, Patrick Steinhardt wrote:
> 
> > We have a bunch of tests that use Perl to perform substitution via the
> > "s/" operator. These usecases can be trivially replaced with sed(1).
> 
> ... and sometimes `tr`.
> 
> In fact, it looks like...
> 
> > diff --git a/t/t7508-status.sh b/t/t7508-status.sh
> > index 14c41b2cb7c..cdc1d6fcc78 100755
> > --- a/t/t7508-status.sh
> > +++ b/t/t7508-status.sh
> > @@ -1064,9 +1064,9 @@ test_expect_success 'status -s submodule summary (clean submodule)' '
> >  	test_cmp expect output
> >  '
> >
> > -test_expect_success PERL_TEST_HELPERS 'status -z implies porcelain' '
> > +test_expect_success 'status -z implies porcelain' '
> >  	git status --porcelain |
> > -	perl -pe "s/\012/\000/g" >expect &&
> > +	tr "\012" "\000" >expect &&
> >  	git status -z >output &&
> >  	test_cmp expect output
> >  '
> 
> ... this change is not about `sed` at all, but only about `tr`.
> _Technically_, this hunk would therefore feel more at home in the previous
> patch. But practically, I actually do not mind it being here at all.

Yeah, the boundaries between commits are fuzzy at times. I mostly wanted
to split up similar changes so that the review load is somewhat
reasonable and not have a single patch that does it all at once. And
this site here did use Perl's substitution operator, so it's not wrong
per se that we do the conversion as part of this patch. But it's of
course wrong that I only mention sed(1) in the commit message.

Patrick
diff mbox series

Patch

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 1aaa6bf5ae8..273d71411fe 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -5,12 +5,6 @@  test_description=check-ignore
 TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
-if ! test_have_prereq PERL_TEST_HELPERS
-then
-	skip_all='skipping ignores tests; Perl not available'
-	test_done
-fi
-
 init_vars () {
 	global_excludes="global-excludes"
 }
@@ -45,11 +39,11 @@  test_stderr () {
 }
 
 broken_c_unquote () {
-	"$PERL_PATH" -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+	sed -e 's/^"//' -e 's/\\//' -e 's/"$//' "$1" | tr '\n' '\0'
 }
 
 broken_c_unquote_verbose () {
-	"$PERL_PATH" -pe 's/	"/	/; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+	sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' "$1" | tr ':\t\n' '\000'
 }
 
 stderr_contains () {
diff --git a/t/t4029-diff-trailing-space.sh b/t/t4029-diff-trailing-space.sh
index a92a42990b1..90cdde88d8b 100755
--- a/t/t4029-diff-trailing-space.sh
+++ b/t/t4029-diff-trailing-space.sh
@@ -18,7 +18,7 @@  index 5f6a263..8cb8bae 100644
 EOF
 exit 1
 
-test_expect_success PERL_TEST_HELPERS "$test_description" '
+test_expect_success "$test_description" '
 	printf "\nx\n" > f &&
 	before=$(git hash-object f) &&
 	before=$(git rev-parse --short $before) &&
@@ -31,7 +31,8 @@  test_expect_success PERL_TEST_HELPERS "$test_description" '
 	git config --bool diff.suppressBlankEmpty true &&
 	git diff f > actual &&
 	test_cmp exp actual &&
-	perl -i.bak -p -e "s/^\$/ /" exp &&
+	sed "s/^\$/ /" exp >exp.munged &&
+	mv exp.munged exp &&
 	git config --bool diff.suppressBlankEmpty false &&
 	git diff f > actual &&
 	test_cmp exp actual &&
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 7fcca9ddad5..204325f4d53 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -27,12 +27,6 @@  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
-if ! test_have_prereq PERL_TEST_HELPERS
-then
-	skip_all='skipping rerere tests; Perl not available'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	cat >a1 <<-\EOF &&
 	Some title
@@ -87,7 +81,7 @@  test_expect_success 'activate rerere, old style (conflicting merge)' '
 	test_might_fail git config --unset rerere.enabled &&
 	test_must_fail git merge first &&
 
-	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
+	sha1=$(sed "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1 &&
 	grep "^=======\$" $rr/preimage &&
 	! test -f $rr/postimage &&
@@ -100,7 +94,7 @@  test_expect_success 'rerere.enabled works, too' '
 	git reset --hard &&
 	test_must_fail git merge first &&
 
-	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
+	sha1=$(sed "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1 &&
 	grep ^=======$ $rr/preimage
 '
@@ -110,7 +104,7 @@  test_expect_success 'set up rr-cache' '
 	git config rerere.enabled true &&
 	git reset --hard &&
 	test_must_fail git merge first &&
-	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
+	sha1=$(sed "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1
 '
 
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index ac5e370e1e4..ab99c8b6850 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -99,11 +99,12 @@  test_expect_success '... and loose copy of first delta allows for partial recove
 	git cat-file blob $blob_3 > /dev/null
 '
 
-test_expect_success PERL_TEST_HELPERS 'create corruption in data of first object' '
+test_expect_success 'create corruption in data of first object' '
 	create_new_pack &&
 	git prune-packed &&
 	chmod +w ${pack}.pack &&
-	perl -i.bak -pe "s/ base /abcdef/" ${pack}.pack &&
+	sed "s/ base /abcdef/" ${pack}.pack >${pack}.pack.munged &&
+	mv ${pack}.pack.munged ${pack}.pack &&
 	test_must_fail git cat-file blob $blob_1 > /dev/null &&
 	test_must_fail git cat-file blob $blob_2 > /dev/null &&
 	test_must_fail git cat-file blob $blob_3 > /dev/null
@@ -156,11 +157,12 @@  test_expect_success '... and then a repack "clears" the corruption' '
 	git cat-file blob $blob_3 > /dev/null
 '
 
-test_expect_success PERL_TEST_HELPERS 'create corruption in data of first delta' '
+test_expect_success 'create corruption in data of first delta' '
 	create_new_pack &&
 	git prune-packed &&
 	chmod +w ${pack}.pack &&
-	perl -i.bak -pe "s/ delta1 /abcdefgh/" ${pack}.pack &&
+	sed "s/ delta1 /abcdefgh/" ${pack}.pack >${pack}.pack.munged &&
+	mv ${pack}.pack.munged ${pack}.pack &&
 	git cat-file blob $blob_1 > /dev/null &&
 	test_must_fail git cat-file blob $blob_2 > /dev/null &&
 	test_must_fail git cat-file blob $blob_3 > /dev/null
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index 81987296235..a62b463eaf0 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -395,7 +395,7 @@  test_bitmap_cases () {
 		)
 	'
 
-	test_expect_success PERL_TEST_HELPERS 'pack.preferBitmapTips' '
+	test_expect_success 'pack.preferBitmapTips' '
 		git init repo &&
 		test_when_finished "rm -fr repo" &&
 		(
@@ -421,7 +421,7 @@  test_bitmap_cases () {
 
 			# mark the commits which did not receive bitmaps as preferred,
 			# and generate the bitmap again
-			perl -pe "s{^}{create refs/tags/include/$. }" <before |
+			sed "s|\(.*\)|create refs/tags/include/\1 \1|" before |
 				git update-ref --stdin &&
 			git -c pack.preferBitmapTips=refs/tags/include repack -adb &&
 
diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
index 342d0423c92..2a782214ee1 100755
--- a/t/t5534-push-signed.sh
+++ b/t/t5534-push-signed.sh
@@ -177,7 +177,7 @@  test_expect_success GPGSSH 'ssh signed push sends push certificate' '
 	test_cmp expect dst/push-cert-status
 '
 
-test_expect_success GPG,PERL_TEST_HELPERS 'inconsistent push options in signed push not allowed' '
+test_expect_success GPG 'inconsistent push options in signed push not allowed' '
 	# First, invoke receive-pack with dummy input to obtain its preamble.
 	prepare_dst &&
 	git -C dst config receive.certnonceseed sekrit &&
@@ -205,7 +205,7 @@  test_expect_success GPG,PERL_TEST_HELPERS 'inconsistent push options in signed p
 	# Tweak the push output to make the push option outside the cert
 	# different, then replay it on a fresh dst, checking that ff is not
 	# deleted.
-	perl -pe "s/([^ ])bar/\$1baz/" push >push.tweak &&
+	sed "s/\([^ ]\)bar/\1baz/" push >push.tweak &&
 	prepare_dst &&
 	git -C dst config receive.certnonceseed sekrit &&
 	git -C dst config receive.advertisepushoptions 1 &&
diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh
index 6131c361094..b6f3344dbfb 100755
--- a/t/t6011-rev-list-with-bad-commit.sh
+++ b/t/t6011-rev-list-with-bad-commit.sh
@@ -4,12 +4,6 @@  test_description='git rev-list should notice bad commits'
 
 . ./test-lib.sh
 
-if ! test_have_prereq PERL_TEST_HELPERS
-then
-	skip_all='skipping rev-list with bad commit tests; Perl not available'
-	test_done
-fi
-
 # Note:
 # - compression level is set to zero to make "corruptions" easier to perform
 # - reflog is disabled to avoid extra references which would twart the test
@@ -41,11 +35,15 @@  test_expect_success 'verify number of revisions' \
    first_commit=$(git rev-parse HEAD~3)
    '
 
-test_expect_success 'corrupt second commit object' \
-   '
-   perl -i.bak -pe "s/second commit/socond commit/" .git/objects/pack/*.pack &&
-   test_must_fail git fsck --full
-   '
+test_expect_success 'corrupt second commit object' '
+	for p in .git/objects/pack/*.pack
+	do
+		sed "s/second commit/socond commit/" "$p" >"$p.munged" &&
+		mv "$p.munged" "$p" ||
+		return 1
+	done &&
+	test_must_fail git fsck --full
+'
 
 test_expect_success 'rev-list should fail' '
 	test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git -c core.commitGraph=false rev-list --all > /dev/null
diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh
index 14069600a2f..3d944a00e0d 100755
--- a/t/t7416-submodule-dash-url.sh
+++ b/t/t7416-submodule-dash-url.sh
@@ -4,12 +4,6 @@  test_description='check handling of disallowed .gitmodule urls'
 
 . ./test-lib.sh
 
-if ! test_have_prereq PERL_TEST_HELPERS
-then
-	skip_all='skipping submodule dash URL tests; Perl not available'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	git config --global protocol.file.allow always
 '
@@ -39,7 +33,8 @@  test_expect_success 'fsck accepts protected dash' '
 '
 
 test_expect_success 'remove ./ protection from .gitmodules url' '
-	perl -i -pe "s{\./}{}" .gitmodules &&
+	sed "s|\./||" .gitmodules >.gitmodules.munged &&
+	mv .gitmodules.munged .gitmodules &&
 	git commit -am "drop protection"
 '
 
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 14c41b2cb7c..cdc1d6fcc78 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1064,9 +1064,9 @@  test_expect_success 'status -s submodule summary (clean submodule)' '
 	test_cmp expect output
 '
 
-test_expect_success PERL_TEST_HELPERS 'status -z implies porcelain' '
+test_expect_success 'status -z implies porcelain' '
 	git status --porcelain |
-	perl -pe "s/\012/\000/g" >expect &&
+	tr "\012" "\000" >expect &&
 	git status -z >output &&
 	test_cmp expect output
 '
diff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh
index 5cb16872081..db1e2afb2ca 100755
--- a/t/t8006-blame-textconv.sh
+++ b/t/t8006-blame-textconv.sh
@@ -4,12 +4,6 @@  test_description='git blame textconv support'
 
 . ./test-lib.sh
 
-if ! test_have_prereq PERL_TEST_HELPERS
-then
-	skip_all='skipping blame textconv tests; Perl not available'
-	test_done
-fi
-
 find_blame() {
 	sed -e 's/^[^(]*//'
 }
@@ -17,7 +11,7 @@  find_blame() {
 cat >helper <<'EOF'
 #!/bin/sh
 grep -q '^bin: ' "$1" || { echo "E: $1 is not \"binary\" file" 1>&2; exit 1; }
-"$PERL_PATH" -p -e 's/^bin: /converted: /' "$1"
+sed 's/^bin: /converted: /' "$1"
 EOF
 chmod +x helper
 
diff --git a/t/t9137-git-svn-dcommit-clobber-series.sh b/t/t9137-git-svn-dcommit-clobber-series.sh
index a9d38be997c..b57a362bb98 100755
--- a/t/t9137-git-svn-dcommit-clobber-series.sh
+++ b/t/t9137-git-svn-dcommit-clobber-series.sh
@@ -15,13 +15,13 @@  test_expect_success 'initialize repo' '
 	test -e file
 	'
 
-test_expect_success PERL_TEST_HELPERS '(supposedly) non-conflicting change from SVN' '
+test_expect_success '(supposedly) non-conflicting change from SVN' '
 	test x"$(sed -n -e 58p < file)" = x58 &&
 	test x"$(sed -n -e 61p < file)" = x61 &&
 	svn_cmd co "$svnrepo" tmp &&
 	(cd tmp &&
-		perl -i.bak -p -e "s/^58$/5588/" file &&
-		perl -i.bak -p -e "s/^61$/6611/" file &&
+		sed -e "s/^58$/5588/" -e "s/^61$/6611/" file >file.munged &&
+		mv file.munged file &&
 		poke file &&
 		test x"$(sed -n -e 58p < file)" = x5588 &&
 		test x"$(sed -n -e 61p < file)" = x6611 &&
@@ -37,11 +37,13 @@  test_expect_success 'some unrelated changes to git' "
 	git commit -m bye-life life
 	"
 
-test_expect_success PERL_TEST_HELPERS 'change file but in unrelated area' "
+test_expect_success 'change file but in unrelated area' "
 	test x\"\$(sed -n -e 4p < file)\" = x4 &&
 	test x\"\$(sed -n -e 7p < file)\" = x7 &&
-	perl -i.bak -p -e 's/^4\$/4444/' file &&
-	perl -i.bak -p -e 's/^7\$/7777/' file &&
+	sed -e 's/^4\$/4444/' \
+	    -e 's/^7\$/7777/' \
+		file >file.munged &&
+	mv file.munged file &&
 	test x\"\$(sed -n -e 4p < file)\" = x4444 &&
 	test x\"\$(sed -n -e 7p < file)\" = x7777 &&
 	git commit -m '4 => 4444, 7 => 7777' file &&