On Fri, 18 Jul 2025 at 02:31, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Meet Soni <meetsoni3017@xxxxxxxxx> writes:
>
> > +. ./test-lib.sh
> > +
> > +GIT_REFS_LIST_CMD='refs list'
> > +. "$TEST_DIRECTORY"/t6300-for-each-ref.sh
> > ...
> > - git for-each-ref refs/tags/fourth-signed \
> > + git ${GIT_REFS_LIST_CMD} refs/tags/fourth-signed \
>
> I know where your bias comes from ;-) but if this were
>
> > - git for-each-ref refs/tags/fourth-signed \
> > + $git_for_each_ref refs/tags/fourth-signed \
>
> it would have been easier to read the resulting test, as
> t6300-for-each-ref is and has been primarily about "git
> for-each-ref" command, and the new test script that overrides
>
> git_for_each_ref="git refs list"
>
> before including t6300 would be a good demonstration that the new
> "git ref list" command can stand in as its replacement.
>
> > +GIT_REFS_LIST_CMD='refs list'
> > +. "$TEST_DIRECTORY"/t6300-for-each-ref.sh
> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> > index ce9af79ab1..74a030371c 100755
> > --- a/t/t6300-for-each-ref.sh
> > +++ b/t/t6300-for-each-ref.sh
> > @@ -5,7 +5,9 @@
> >
> > test_description='for-each-ref test'
> >
> > -. ./test-lib.sh
> > +. "${TEST_DIRECTORY:-.}/test-lib.sh"
>
> This is probably wrong.
>
> Nobody promises that including test-lib.sh is and will forever be
> idempotent. While this patch may not give t6300 a serious
> regression right now, I am not sure what future subtle breakage we
> are looking at for t1461.
>
> Probably this should model itself after how 8752d11d (git-blame: Use
> the same tests for git-blame as for git-annotate, 2006-03-05) moved
> a lot from existing t8001 to annotate-tests and so that they can be
> shared with new t8002.
>
> Thanks.
Apologies for the delayed response, I was away last week.
Thanks for the review. The suggestions for the test suite make sense.
I'll refactor it accordingly and send out a v3.
Thanks.
