Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >>> Thus if we do: >>> >>> git something >expected && >>> test_cmp expected actual && >>> rm expected actual >> >> Isn't it a poor example to use to argue for your particular change, >> where $actual in the original is designed to be unique among tests, >> in order to ensure that $actual files left after test pieces fail >> would not interfere with the tests that come later? IOW, there is >> not a reason to remove $actual until the end of the test sequence, >> is there? > > Not really, because you needed to read the rest of the test file to come > to that conclusion. > > The point of using a helper that guarantees cleanup such as > test_when_finished or test_config over manual "git config" or "git rm" > isn't that we can prove that we need it because a later test needs the > cleanup, but that anyone can add new tests or functionality without > having to worry about cleaning up the existing trash directory. > > So yes, it's not needed here, but that's only because we know the rest > of the tests don't have e.g. a test that does a: In this particular case, $actual files are designed to be left behind for failed test pieces, so that the tester can come back and inspect them. I probably should have said it a bit more strongly than "there is not a reason to remove". You SHOULD NOT remove and that is why we had "check and then remove only upon success" there, instead of test_when_finished. We want them left for and only for failing test pieces. Please do not advocate for and encourage newbies who would be reading the discussion from sidelines to use test_when_finished out of dogmatic principle without thinking. Even though there are valid cases where test_when_finished is the perfect fit, in this particular case, use of it is a clear regression. Thanks.
