All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Cc: git@vger.kernel.org,  ben.knoble@gmail.com,  mroik@delayed.space,
	quentin.bernet@bluewin.ch
Subject: Re: [PATCH v3] stash: infer "push" when push-specific options are given
Date: Thu, 09 Apr 2026 13:22:34 -0700	[thread overview]
Message-ID: <xmqqecknsx2t.fsf@gitster.g> (raw)
In-Reply-To: <20260405110953.3316-1-deveshigurgaon@gmail.com> (Deveshi Dwivedi's message of "Sun, 5 Apr 2026 11:09:53 +0000")

Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:

> +test_expect_success 'assume push when options imply push' '
> +	git reset --hard &&
> +	echo changed >file &&
> +	git add file &&
> +	git stash -m "implied push" file &&
> +	git stash pop &&
> +
> +	git add file &&
> +	git stash --staged file &&
> +	git stash pop &&
> +
> +	git add file &&
> +	git stash --keep-index file &&
> +	git stash pop &&
> +
> +	echo untracked >untracked-file &&
> +	git stash --include-untracked untracked-file &&
> +	test_path_is_missing untracked-file &&

A comment on these three lines.

> +	git stash pop &&
> +	rm -f untracked-file &&
> +	git reset --hard
> +'
> +

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.

>  test_expect_success 'stash --invalid-option' '
>  	echo bar5 >file &&
>  	echo bar6 >file2 &&
>
> base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0

  parent reply	other threads:[~2026-04-09 20:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
2026-04-04 15:19 ` Mirko Faina
2026-04-04 16:03 ` [PATCH v2] " Deveshi Dwivedi
2026-04-04 23:40   ` Mirko Faina
2026-04-05  7:02     ` Deveshi Dwivedi
2026-04-05 11:09 ` [PATCH v3] " Deveshi Dwivedi
2026-04-06 18:15   ` Mirko Faina
2026-04-07  9:36     ` Phillip Wood
2026-04-09 19:22       ` Deveshi Dwivedi
2026-04-09 19:37         ` Mirko Faina
2026-04-09 20:31       ` Junio C Hamano
2026-04-09 20:22   ` Junio C Hamano [this message]
2026-04-12 19:52 ` [PATCH v4] stash: infer "push" when command line starts with an option Deveshi Dwivedi
2026-04-13  9:08   ` Phillip Wood
2026-04-13 15:09   ` Junio C Hamano
2026-04-19 16:54 ` [PATCH v5] stash: assume " Deveshi Dwivedi
2026-04-21 15:28   ` Phillip Wood

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=xmqqecknsx2t.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=ben.knoble@gmail.com \
    --cc=deveshigurgaon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mroik@delayed.space \
    --cc=quentin.bernet@bluewin.ch \
    /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.