Re: [PATCH] chainlint.pl: recognize test bodies defined via heredoc

From: Jeff King <peff@peff.net>
Date: Jul 8, 2024, 09:08 UTC
Message-ID: <20240708090837.GD819809@coredump.intra.peff.net>
In-Reply-To: <CAPig+cTv-DaGRmwWWCk8b33MKzV25vfP2zPKd2VOAEOtz4FZ2A@mail.gmail.com>

On Sun, Jul 07, 2024 at 11:51:15PM -0400, Eric Sunshine wrote:

Show 17 quoted lines

> >     I did manually verify that it is OK after my fix, but do we want
> >     that to be part of the chainlint tests? Just leaving the line
> >     numbers in is a maintenance nightmare, since it depends on the order
> >     of concatenating all of the tests together (so our "expect" files
> >     would depend on all of the previous tests). But if we wanted to get
> >     fancy, we could perhaps store relative offsets in the expect file. I
> >     think it gets pretty complicated, though, since we print only
> >     problematic lines.
> 
> Given the way the Makefile currently concatenates all the self-tests,
> it would indeed be a nightmare to retain the line numbers. In the long
> run, we probably ought someday to adopt Ævar's idea of checking the
> self-test files individually[*] rather than en masse. With that
> approach, it may make sense to revisit whether or not line numbers
> should be present in the "expected" files.
> 
> [*] https://lore.kernel.org/git/CAPig+cSBjsosRqoAafYN94Cco8+7SdUt0ND_jHS+jVPoM4K0JA@mail.gmail.com/

I took a look at running each test individually. It's surprisingly quite a bit slower! About 4s instead of 200ms. There's a bit of low-hanging fruit to get it down to ~1.7s (which I'll include in my series). But in the end I punted on that for now, but did add line-number checks. Each expect file just knows its own numbers, and I use a bit of perl to handle the running offset.

-Peff