git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: "Samuel Čavoj" <samuel@cavoj.net>,
	git@vger.kernel.org,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand
Date: Tue, 13 Oct 2020 15:06:40 -0700	[thread overview]
Message-ID: <xmqqy2k9ixvz.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <d2491759-e761-647b-7e76-3515b8eeaf3f@gmail.com> (Phillip Wood's message of "Tue, 13 Oct 2020 11:02:38 +0100")

Phillip Wood <phillip.wood123@gmail.com> writes:

>>   +test_expect_success 'rebase -r, GPG and merge strategies' '
>> +	git reset --hard merged &&
>> +	git rebase -fr --gpg-sign -s resolve --root &&
>> +	git verify-commit HEAD
>> +'
>
> Unfortunately I've just noticed that the test above runs
>
> 	git config commit.gpgsign true
>
> So this test would pass anyway if merge picks up that config
> setting. The previous test needs to be changed to
>
> 	test_config commit.gpgsign true
>
> so that the config setting is cleared when that test finishes.

Thanks for a review, but I do not think that is a right way to go.

test_config has an inherent assumption that not having the config at
all is somehow the "natural" state, and if that holds true, that
would be OK.  But what is "natural" is subjective X-<.

The way each test is run by calling test_rebase_gpg_sign repeatedly
uses a different and more robust approach to ensure that previous
test does not affect the current one.  Each invocation of test
explicitly sets the configuration to the state the test wants to,
cancelling what the previous test did.

To blend in better with existing tests and match their robustness
expectations, the right fix is for this new test to explicitly use
"git config --set" or "git config --unset" to make the variable into
the desired state, regardless of what the previous tests did.  

If the test quoted at the beginning of this message wants to make
sure that --gpg-sign from the command line takes effect without
commit.gpgsign set, it should unset the variable explicitly.
Another combination worth testing is to ensure that --gpg-sign takes
effect when commit.gpgsign is explicitly set to false (not "left
unset").

Thanks.




  parent reply	other threads:[~2020-10-13 22:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-12 23:49 [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand Samuel Čavoj
2020-10-12 23:49 ` [PATCH v2 2/2] sequencer: pass explicit --no-gpg-sign to merge Samuel Čavoj
2020-10-13  4:46   ` Junio C Hamano
2020-10-13 10:03   ` Phillip Wood
2020-10-12 23:52 ` [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand Samuel Čavoj
2020-10-13  9:55 ` Phillip Wood
2020-10-13 10:43   ` Samuel Čavoj
2020-10-13 13:15     ` Phillip Wood
2020-10-13 10:02 ` Phillip Wood
2020-10-13 10:51   ` Samuel Čavoj
2020-10-13 13:28     ` Phillip Wood
2020-10-13 22:06   ` Junio C Hamano [this message]
2020-10-13 23:45     ` Samuel Čavoj
2020-10-14 15:31       ` Junio C Hamano
2020-10-16 13:40     ` Phillip Wood
2020-10-16 16:37       ` Junio C Hamano
2020-10-16 17:25         ` Phillip Wood
2020-10-16 20:15       ` 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=xmqqy2k9ixvz.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood123@gmail.com \
    --cc=samuel@cavoj.net \
    --cc=sandals@crustytoothpaste.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).