Re: [PATCH v4 12/20] t: refactor tests depending on Perl to print data

[Patrick Steinhardt <ps@pks.im>]

On Tue, Jun 10, 2025 at 02:31:34PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
> >> @@ -241,8 +234,7 @@ test_expect_success 'pseudo-merge pattern with capture groups' '
> >>  			test_commit_bulk 16 &&
> >>  
> >>  			git rev-list HEAD~16.. >in &&
> >> -
> >> -			perl -lne "print \"create refs/remotes/$r/tags/\$. \$_\"" <in |
> >> +			sed "s|\(.*\)|create refs/remotes/$r/tags/\1 \1" in |
> >
> > This conversion results in the error:
> >
> >   sed: -e expression #1, char 41: unterminated `s' command
> 
> This not just misses the terminating "|", but the conversion seems
> not very faithful to the original.  It used to create tags 1 2 3 4
> 5... but now the tags it tries to create (unsuccessfully) are the
> names of tagged objects in full hexadecimal glory.

Yup, that change was intentional. Anything past "refs/remotes/$r/tags"
doesn't matter: the pseudo-merge patterns only match on
"refs/remotes/([0-9]+)/tags/", so the final part of the tag name does
not matter. It was simply easier to slightly change the format than to
faithfully retain the original refnames.

> > I find it suspicious that the test still succeeds...
> 
> That is because the downstream "update-ref --stdin" does not notice
> anything wrong in its input, which is empty.

Ugh, indeed.

> >>  			git update-ref --stdin || return 1
> >>  		done &&
> 
> And the step after this, which is not touched by this patch, may not
> be testing what it wants to test.  test_pseudo_merges produces no
> lines, and iterating over the lines in that file produces an empty
> result in "remotes" below ...

Yup. This is a consequence of us not having created the tags though. The
pseudo-merge patterns we have configured don't match anything, and
because of that the test doesn't do anything.

> >> @@ -258,7 +250,7 @@ test_expect_success 'pseudo-merge pattern with capture groups' '
> >>  		do
> >>  			test_pseudo_merge_commits $m >oids &&
> >>  			grep -f oids refs |
> >> -			perl -lne "print \$1 if /refs\/remotes\/([0-9]+)/" |
> >> +			sed -n "s|refs/remotes/\([0-9][0-9]*\)/|\1|p" &&
> >>  			sort -u || return 1
> >>  		done >remotes &&
> 
> ... and then it checks remotes has no duplicated lines with
> 
> 		test $(wc -l <remotes) -eq $(sort -u <remotes | wc -l)
> 
> No wonder it passes, as remotes is an empty file ;-)

True.

But in any case, the test does what it's intended to do again if we
append the missing "|" terminator.

I'll send a patch in a bit, thanks!

Patrick