On Tue, Dec 20, 2016 at 11:55:45PM -0800, Jacob Keller wrote: > > I do wonder if in general it should be the responsibility of skippable > > tests to make sure we end up with the same state whether they are run or > > not. That might manage the complexity more. But I certainly don't mind > > tests being defensive like you have here. > > > > -Peff > > That seems like a good idea, but I'm not sure how you would implement > it in practice? Would we just "rely" on a skipable test having a "do > this if we skip, instead" block? That would be easier to spot but I > think still relies on the skip-able tests being careful? Yes, it definitely means the skip-able tests would have to be careful. But it's putting the onus on them, rather than on all the other tests. If the rule is "the on-disk state must be the same whether or not the test runs", then I suspect many tests could get by with a test_when_finished, like: test_expect_success FOO 'test --foo knob' ' git commit -m "new commit for test" && test_when_finished "git reset --hard HEAD^" && git log --foo >actual && test_cmp expect actual ' It gets harder if you have multiple such tests that rely on intermediate state. Probably you'd have: test_expect_success FOO 'clean up FOO state' ' git reset --hard HEAD^ ' at the end of the sequence. I dunno. It is unclear to me whether such a rule is worth it. Preemptively fighting these state-based bugs before they occur is a nice thought, but I think it may end up being more work to write the tests that way than it is to simply find and fix the bugs when they occur. Of course it also changes where the work falls (the test writers versus the bug hunters, and given that !MINGW is our most common prereq, I think Windows devs are over-represented in the latter case). -Peff