On 15 Feb 2016, at 19:05, Jeff King <peff@xxxxxxxx> wrote: > On Mon, Feb 15, 2016 at 11:17:43AM +0100, larsxschneider@xxxxxxxxx wrote: > >> I like the idea of a "test set up block" within a test script. In order >> to clean up nicely before any subsequent tests I would like to propose >> a "tear down" block. Would that work as a compromise in our "test cases >> depend on earlier test cases" discussion? > > I don't have any real problem with what you've written in the final > patch, but I also don't think it's accomplishing much (and is more lines > of code, and more running processes). > > If you want to run test N without having run all of 1..N-1, what you > really want is some known, reliable state when that test starts. But the > tests before it do not necessarily know what that state is. The best > they can do is roughly restore the original state before they ran. But: > > 1. What does the state consist of? Which files (and their contents) > are important to the test? > > In your tear-down you get rid of $INCLUDE_DIR, and you zero-out the > config files. But you leave expect, output, output.raw, and the > oddly named $CUSTOM_CONFIG_FILE. Nor do you clean up the > environment variables. Good argument - I can't disagree. > > To be clear, I think it's perfectly fine to leave those. But you > are still making assumptions about what the next test relies on. > > 2. We may create a clean slate, but that is probably not what the next > test wants. It will want to do its own setup. I.e., it will > probably not want a blank .git/config, and will create it itself, > just as you did in your setup step. > > So rather than tearing down, I think we are better off trying to make > tests themselves (or blocks of them) set up their own assumptions. E.g., > by overwriting files rather than appending to them. By using unique > filenames, commit messages, etc for their tests. That's less of a big > deal here, but in many tests that create commits, "test_commit foo" > would fail a second time, because there are no changes to "foo". Doing > "test_commit subdir/check-diff-in-subdir" is less likely to clash > without another test. > > Sometimes we _are_ better off with a teardown step, because subsequent > tests would not reasonably think to clear some state we've set (e.g., in > non-config tests, if we set some random config variable, we use > test_config to tear it down afterwards rather than have each test clean > out all of the config). So there's definitely a subjective judgement > call on what is "reasonable" there. But I find it unlikely that your > tear-down will help anybody in this case. Further tests will not care > about $INCLUDE_DIR unless they reference it, and any further tests would > set up their own .git/config, etc. OK, I will remove the block in the next roll. Thanks for explaining your thoughts on this. - Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html