Re: [GSoC][PATCH v7 20/26] stash: add tests for `git stash push -q`

[Thomas Gummerer <t.gummerer@gmail.com>]

On 08/08, Paul-Sebastian Ungureanu wrote:
> This commit introduces more tests for the quiet option of
> `git stash push`.

I think this commit should be squashed into the previous one, so we
have implementation and tests in one commit.  That way it's easier to
see during review that there are tests for the change.  For more
discussion on that also see [1].

[1]: https://public-inbox.org/git/20180806144726.GB97564@aiede.svl.corp.google.com/

> ---
>  t/t3903-stash.sh | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 8d002a7f2..b78db74ae 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -1064,6 +1064,27 @@ test_expect_success 'push: <pathspec> not in the repository errors out' '
>  	test_path_is_file untracked
>  '
>  
> +test_expect_success 'push: -q is quiet with changes' '
> +	>foo &&
> +	git stash push -q >output 2>&1 &&

We create an untracked file here and then call 'git stash push', which
will not create a new stash, as we don't use the --include-untracked
option.  In fact, right now this test is doing the same thing as the
test below.  There should be a 'git add foo' above the 'git stash
push' call to test what we're claiming to test here.

> +	test_must_be_empty output
> +'
> +
> +test_expect_success 'push: -q is quiet with no changes' '
> +	git stash push -q >output 2>&1 &&
> +	test_must_be_empty output
> +'
> +
> +test_expect_success 'push: -q is quiet even if there is no initial commit' '
> +	git init foo_dir &&
> +	cd foo_dir &&
> +	touch bar &&

The typical style in the test suite for creating a new file is to use
'>bar', unless you care about the 'mtime' the file has.  We don't seem
to care about that in this test, so avoiding 'touch' would be better.

> +	test_must_fail git stash push -q >output 2>&1 &&
> +	test_must_be_empty output &&
> +	cd .. &&

The above should be in a subshell, i.e.

    (
        cd foo_dir &&
	touch bar &&
        test_must_fail git stash push -q >output 2>&1 &&
        test_must_be_empty output &&
    )

then you don't have to do the 'cd ..' in the end.  With the 'cd ..' in
the end, if one of the commands between the 'cd foo_dir' and 'cd ..'
fails, all subsequent tests will be run inside of 'foo_dir', which
puts them in a different environment than they expect.  That can cause
all kinds of weirdness.

If inside a subshell, the current working directory of the parent
shell is unaffected, so we don't have to worry about cd'ing back, and
subsequent tests will get the correct cwd even if things go wrong in
this test.
	

> +	rm -rf foo_dir

We'll want to run this cleanup to run even if the test fails.  To do
so, the 'test_when_finished' helper can be used.  Using that, this
would go at the top of the test, as 'test_when_finished rm -rf
foo_dir'.  Otherwise if any of the commands above fail, 'foo_dir' will
not be removed, and may interfere with subsequent tests.

> +'
> +
>  test_expect_success 'untracked files are left in place when -u is not given' '
>  	>file &&
>  	git add file &&
> -- 
> 2.18.0.573.g56500d98f
>