On Fri, Dec 02 2022, Eric Sunshine wrote: > On Fri, Dec 2, 2022 at 3:55 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: >> 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. That would be great. As a reminder I think (maybe it's not what you have in mind exactly?) that we had a brief discussion on this topic starting at [1]. I.e. I was hoping chainlint.pl could be extended to detect exactly these sort of "test...$(git" patterns (among other things). This topic doesn't get us there, but once we finally get rid of some of those patterns it would be nice to have assertions to ensure they don't come back. 1. https://lore.kernel.org/git/221024.865yg9ecsx.gmgdl@xxxxxxxxxxxxxxxxxxx/
