Re: [PATCH v3] stash: infer "push" when push-specific options are given

Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:

A comment on these three lines.

I suspect that they are meant to be "clean-up after we are done with the test, to avoid interfering with the next test", but if so, "clean-up at the very end" is not a very effective strategy to do so. Imagine that one of the previous steps fails, breaking all later commands in the &&- cascade. Sitting at the very end, your clean-up sequence will not run. Unless the tester is running this test script with the "-i" option, the test will move on to the next piece. Installing clean-up handler with test_when_finished may be a cleaner approach.

	test_expect_success 'do this test' '
		test_when_finished "git stash clear; git reset --hard" &&
		git reset --hard &&
		... do all the dirty things in the working tree ...

		test_when_finished "rm -f untracked-file" &&
                echo untracked >untracked-file &&
                git stash --include-untracked untracked-file &&
		test_path_is_missing untracked-file
	'

You can use more than one test_when_finished in a single test.  

It is often done to add an upfront blunt hammer at the beginning to do a clean-up without worrying too much about where exactly in the command sequence a breakage may happen (e.g., we may fail before we run our first "git add", or "git stash", and "git reset --hard" or "git stash clear" may be an unnecessary no-op, but we do not worry too much about the clean-up step doing potentially unnecessary things.

Or you would set up a clean-up handler immediately before you create a thing that you want to make sure you clean up. If the command sequence fails before you echo the string into untracked-file to create it, there is no point preparing to remove it when you are done.

Both approaches are commmonly used.