All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, derrickstolee@github.com,
	Victoria Dye <vdye@github.com>
Subject: Re: [PATCH v3 5/5] stash: make internal resets quiet and refresh index
Date: Tue, 15 Mar 2022 03:23:53 -0700	[thread overview]
Message-ID: <xmqqee33h5ty.fsf@gitster.g> (raw)
In-Reply-To: 4c45351a0c4b8b955a9101ed9445a763750ca3c3.1647308982.git.gitgitgadget@gmail.com

"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> 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)?

  reply	other threads:[~2022-03-15 10:23 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-12  0:08 [PATCH 0/5] Separate '--skip-refresh' from '--quiet' in 'reset', use '--quiet' internally in 'stash' Victoria Dye via GitGitGadget
2022-03-12  0:08 ` [PATCH 1/5] reset: revise index refresh advice Victoria Dye via GitGitGadget
2022-03-12  0:08 ` [PATCH 2/5] reset: introduce --[no-]refresh option to --mixed Victoria Dye via GitGitGadget
2022-03-14 15:05   ` Derrick Stolee
2022-03-14 15:13     ` Derrick Stolee
2022-03-14 15:55     ` Victoria Dye
2022-03-12  0:08 ` [PATCH 3/5] reset: replace '--quiet' with '--no-refresh' in performance advice Victoria Dye via GitGitGadget
2022-03-12  0:08 ` [PATCH 4/5] reset: suppress '--no-refresh' advice if logging is silenced Victoria Dye via GitGitGadget
2022-03-12  0:08 ` [PATCH 5/5] stash: make internal resets quiet and refresh index Victoria Dye via GitGitGadget
2022-03-14 15:10   ` Derrick Stolee
2022-03-14 15:56     ` Victoria Dye
2022-03-12 17:12 ` [PATCH 0/5] Separate '--skip-refresh' from '--quiet' in 'reset', use '--quiet' internally in 'stash' Victoria Dye
2022-03-14  6:22 ` Junio C Hamano
2022-03-14 15:13 ` Derrick Stolee
2022-03-14 16:10 ` [PATCH v2 0/5] Allow 'reset --quiet' to refresh the index, use 'reset --quiet' " Victoria Dye via GitGitGadget
2022-03-14 16:10   ` [PATCH v2 1/5] reset: revise index refresh advice Victoria Dye via GitGitGadget
2022-03-14 16:10   ` [PATCH v2 2/5] reset: introduce --[no-]refresh option to --mixed Victoria Dye via GitGitGadget
2022-03-14 19:27     ` Junio C Hamano
2022-03-14 23:48       ` Victoria Dye
2022-03-14 16:10   ` [PATCH v2 3/5] reset: replace '--quiet' with '--no-refresh' in performance advice Victoria Dye via GitGitGadget
2022-03-14 16:10   ` [PATCH v2 4/5] reset: suppress '--no-refresh' advice if logging is silenced Victoria Dye via GitGitGadget
2022-03-14 19:38     ` Junio C Hamano
2022-03-14 16:10   ` [PATCH v2 5/5] stash: make internal resets quiet and refresh index Victoria Dye via GitGitGadget
2022-03-14 19:42     ` Junio C Hamano
2022-03-14 23:54       ` Victoria Dye
2022-03-14 16:30   ` [PATCH v2 0/5] Allow 'reset --quiet' to refresh the index, use 'reset --quiet' in 'stash' Derrick Stolee
2022-03-14 23:17   ` Junio C Hamano
2022-03-15  1:49   ` [PATCH v3 " Victoria Dye via GitGitGadget
2022-03-15  1:49     ` [PATCH v3 1/5] reset: revise index refresh advice Victoria Dye via GitGitGadget
2022-03-15  1:49     ` [PATCH v3 2/5] reset: introduce --[no-]refresh option to --mixed Victoria Dye via GitGitGadget
2022-03-18 11:08       ` Phillip Wood
2022-03-18 17:17         ` Junio C Hamano
2022-03-18 19:19           ` Victoria Dye
2022-03-15  1:49     ` [PATCH v3 3/5] reset: replace '--quiet' with '--no-refresh' in performance advice Victoria Dye via GitGitGadget
2022-03-15  1:49     ` [PATCH v3 4/5] reset: suppress '--no-refresh' advice if logging is silenced Victoria Dye via GitGitGadget
2022-03-15  1:49     ` [PATCH v3 5/5] stash: make internal resets quiet and refresh index Victoria Dye via GitGitGadget
2022-03-15 10:23       ` Junio C Hamano [this message]
2022-03-16 20:07         ` Victoria Dye
2022-03-16 20:55           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqee33h5ty.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.