Re: [PATCH v2 1/8] log tests: don't use "exit 1" outside a sub-shell

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 2, 2022, 09:03 UTC
Message-ID: <CAPig+cQvqPgo1UKbAAtyz8UVJZiwrG7j+kAokMEimv1E62pR0A@mail.gmail.com>
In-Reply-To: <221202.865yeugp7e.gmgdl@evledraar.gmail.com>

On Fri, Dec 2, 2022 at 3:55 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 15 quoted lines

> On Fri, Dec 02 2022, Junio C Hamano wrote:
> > "return 1" is obvious and safe correction.  I have to wonder if
> > test_expect_success can be taught to be smarter to intercept "exit"
> > so we do not have to bo so careful, but that would be a much more
> > involved change to the lower-level of test framework.
>
> I can't think of a way to do so that wouldn't involve running the test
> in a sub-shell, which I think would bring us to the state management
> problems noted in [1] for Phillip's "test_todo" series, except in this
> case we'd have those issues trying to pass state back from the
> "test_expect_success".
>
> It's possible, but we'd need to change a lot of code that's expecting to
> talk to itself via variables in the same shell to use IPC between
> shells, wouldn't we?

It might make more sense to turn this on its head and make it a linting issue and simply throw a "?!FOO?!" as is done for other suspect shell code. In fact, I already have local chainlint.pl patches which detect whether a subshell is active so that the linter can complain if it sees `cd` outside of a subshell. I would think that warning about misuse of `exit 1` outside a subshell (and perhaps `return 1` inside a subshell) should be possible, though I haven't thought through all the possibilities.