Re: [PATCH] t/perf: fix test_export() failure with BSD `sed`

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 18, 2020, 06:15 UTC
Message-ID: <CAPig+cQvaOBo=zx=f2ZsPy7QnPXgcdc0SUZyGWaZPZ31FUUwZg@mail.gmail.com>
In-Reply-To: <X9xBRXW7/tXsqLT5@coredump.intra.peff.net>

On Fri, Dec 18, 2020 at 12:44 AM Jeff King <peff@peff.net> wrote:

Show 11 quoted lines

> On Wed, Dec 16, 2020 at 02:29:26PM -0500, Eric Sunshine wrote:
> > Perhaps a test_unexport() might be handy in the distant future, but
> > presently there is only a single call to test_export() in the entire
> > suite, so it's probably not worth worrying about now.
>
> I actually wonder if we could drop test_export entirely. I assume you
> mean the call in p0001. It is inside a test_expect_success block, where
> we don't need to do anything fancier than just "export". It is already
> running in the main script's environment, just like a normal test. If it
> were in a test_perf, then we would need to take special care to get it
> back into the main script.

Considering that test_export() hasn't seen much use since its introduction nine years ago and that the one and only existing call doesn't even need the special subprocess magic, retiring the function is certainly an option. On the other hand, aside from this one minor portability fix, it hasn't been a maintenance burden and may actually come in handy someday if people start writing more "perf" tests. So, I don't feel strongly one way or the other, though I lean somewhat toward keeping it around.