On Mon, Mar 24, 2025 at 12:07:52PM -0400, Eric Sunshine wrote: > On Mon, Mar 24, 2025 at 8:46 AM Patrick Steinhardt <ps@xxxxxx> 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 <ps@xxxxxx> 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
