On Mon, Aug 22, 2022 at 12:31:44PM -0700, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > > >> > + git init repo && > >> > + test_when_finished "rm -fr repo" && > >> > >> nit: test_when_finished should be the first line of the test. > > > > The "rm-then-init-then-test_when_finished" is an (unfortunate) pattern > > extended throughout t5326, mostly that some tests don't clean up "repo" > > after deleting and recreating it. > > I do not think it is so bad to be defensive to "prepare it cleanly > enough so that I would not be affected". So "rm -fr repo && git > init repo" I would fully support. "init && test_when_finished" is > totally indefensible. It should be the other way around. Agreed. We should fix those up, probably once Abhradeep's topic has landed, since doing so now would create an avoidable amount of merge conflicts. > > But it's easy enough to just use a separate repository, and avoid > > removing it altogether. Thanks for the suggestion! > > Those who run tests in a batch without "-i" would have more material > to study and find breakages if you did so. I agree that is probably > something worth doing (unless in narrow corner cases where each test > repository consumes unusual amount of storage or somethinglike that). Agreed here, too. It's much handier to get a broken state immediately, instead of realizing that a test was broken, commenting out the "test_when_finished" bits, and rerunning it again (not to mention hoping that the failure is deterministic). Luckily these repositories are small in size, so it doesn't hurt to keep them around. Thanks, Taylor