Subject: Re: [PATCH 13/20] t: refactor tests depending on Perl for textconv scripts

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 25 Mar 2025 13:42:45 +0100

On Mon, Mar 24, 2025 at 12:07:52PM -0400, Eric Sunshine wrote:
> On Mon, Mar 24, 2025 at 8:46 AM Patrick Steinhardt <[email-masked]> wrote:
> > On Thu, Mar 20, 2025 at 03:37:08PM -0400, Eric Sunshine wrote:
> > > On Thu, Mar 20, 2025 at 5:37 AM Patrick Steinhardt <[email-masked]> wrote:
> > > > -test_expect_success PERL_TEST_HELPERS 'rewrite diff respects textconv' '
> > > > +test_expect_success 'rewrite diff respects textconv' '
> > > >         git diff -B >diff &&
> > > > -       grep "dissimilarity index" diff &&
> > > > -       grep "^-61" diff &&
> > > > -       grep "^-0" diff
> > > > +       test_grep "dissimilarity index" diff &&
> > > > +       test_grep "^-3d 0a 00" diff &&
> > > > +       test_grep "^+3d 0a 01" diff
> > > >  '
> > >
> > > This change seems unrelated to the stated purpose (`textconv`) of this patch(?).
> >
> > Not quite. The test previously didn't run because it depends on the
> > Perl-based textconv script. Now that this textconv script was adapted
> > to use shell scripting instead it can run, but as explained in the
> > commit message the output of the textconv script changed. We don't
> > really care for the exact output at all, we only care that textconv did
> > its thing. But we do have to adapt the test accordingly.
> 
> Okay, I see that now that I have read your response and examined the
> change more closely. The unrelated `grep` to `test_grep` change
> visually overwhelms the diff, so much so that I overlooked the other
> smaller necessary changes. Perhaps it would make sense to mention the
> unrelated change in the commit message but is not itself worth a
> reroll.

I already tried to describe this in the commit message, but obviously I
seem to have failed :) I'll add another sentence to mention that tests
have to be adapted accordingly.

Patrick