Subject: Re: [PATCH v3 5/5] stash: make internal resets quiet and refresh index

From: Junio C Hamano <[email-masked]>
Date: Tue, 15 Mar 2022 03:23:53 -0700

"Victoria Dye via GitGitGadget" <[email-masked]> writes:

> +test_expect_success 'apply --index -q is quiet' '

Hmph.  So being quiet and refreshing index are tested separately in
different tests, and this one is only about being quiet?

I wonder if a single test that checks chattiness and refreshing of
"git stash -q" and "git apply --index -q" (that's 2x2 which is 4)
would be sufficient?

> +	# Added file, deleted file, modified file all staged for commit
> +	echo foo >new-file &&
> +	echo test >file &&
> +	git add new-file file &&
> +	git rm other-file &&
> +
> +	git stash &&

As this is only about chattiness about "apply --index -q", this
command goes unchecked (4 - 2 = 2).

> +	git stash apply --index -q >output.out 2>&1 &&

And this is only about chattiness so we do not test if the index
gets refreshed after this operation (2 - 1 = 1).

> +	test_must_be_empty output.out

This ensures that "git stash apply --index -q" is silent, as that is
the main objective of this step: make sure reset won't chatter,
especially when stash is told to be --quiet, which is good.

But with a few more lines, this set-up can also test the other three
with minimum additional effort, no?

> +'
> +
>  test_expect_success 'save -q is quiet' '
>  	git stash save --quiet >output.out 2>&1 &&
>  	test_must_be_empty output.out
> @@ -291,6 +303,27 @@ test_expect_success 'drop -q is quiet' '
>  	test_must_be_empty output.out
>  '
>  
> +test_expect_success 'stash push -q --staged refreshes the index' '
> +	git reset --hard &&
> +	echo test >file &&
> +	git add file &&
> +	git stash push -q --staged &&

"git stash" and "git stash push -q --staged" may do different
things, so leaving the plain "git stash" untested for refreshing in
an earlier test, and "git stash" with different options being tested
for refreshing here, makes me wonder about a gap in test coverage.

The overall theme of the whole topic was that chatty output from
"git reset" run as an implementation detail seeps through from "git
stash", IIUC.  So, making sure that our index is refreshed after the
operation is good, but at the same time, wouldn't we want to see
what the output of this command says (or be silent)?