git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Han Xin <chiyutianyi@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Han Xin <hanxin.hx@alibaba-inc.com>,
	Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: Re: [PATCH 1/2] t5534: new test case for atomic signed push
Date: Tue, 15 Sep 2020 13:31:38 -0700	[thread overview]
Message-ID: <xmqqv9gezsr9.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200915095827.52047-1-hanxin.hx@alibaba-inc.com> (Han Xin's message of "Tue, 15 Sep 2020 17:58:26 +0800")

Han Xin <chiyutianyi@gmail.com> writes:

> In order to test signed atomic push, add a new test case.
>
> Reviewed-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com>
> ---

Thanks, but nowhere in the above it does not say what is being
tested.  By looking at 2/2 (by the way, these should be a single
atomic patch, not a "failure turns into success", as it is not even
a bug fix), readers may be able to guess that you want to enforce
that with even broken implementation of GPG, an immediate failure to
push one of the refs will be noticed by looking at their refs, but
it is unclear why that is even desirable---if you combine the two
patches, you may have a better place to argue why it is a good idea,
but a test-only patch makes it even less clear why the new behavior
expected by this test is desirable.

>  t/t5534-push-signed.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
> index 030331f1c5..d0fcdc900e 100755
> --- a/t/t5534-push-signed.sh
> +++ b/t/t5534-push-signed.sh
> @@ -273,4 +273,21 @@ test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
>  	test_cmp expect dst/push-cert-status
>  '
>  
> +test_expect_failure GPG 'check atomic push before running GPG' '
> +	prepare_dst &&
> +	git -C dst config receive.certnonceseed sekrit &&
> +	write_script gpg <<-EOF &&
> +	echo >&2 "Fake gpg is called."
> +	exit 1
> +	EOF
> +	test_must_fail env PATH="$TRASH_DIRECTORY:$PATH" git push --signed --atomic \
> +			dst noop ff noff >out 2>&1 &&
> +	grep "^error:" out >actual &&
> +	cat >expect <<-EOF &&
> +	error: atomic push failed for ref refs/heads/noff. status: 2
> +	error: failed to push some refs to '"'"'dst'"'"'
> +	EOF
> +	test_i18ncmp expect actual
> +'
> +
>  test_done

  parent reply	other threads:[~2020-09-15 20:34 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15  9:58 Han Xin
2020-09-15  9:58 ` [PATCH 2/2] send-pack: check atomic push before running GPG Han Xin
2020-09-15 21:02   ` Junio C Hamano
2020-09-15 21:40     ` Junio C Hamano
2020-09-16  1:53     ` Jiang Xin
2020-09-16  4:37       ` Junio C Hamano
2020-09-16 11:49         ` Jiang Xin
2020-09-16 23:44           ` Junio C Hamano
2020-09-18  4:50             ` [PATCH v2] send-pack: run GPG after atomic push checking Han Xin
2020-09-19  0:02               ` Junio C Hamano
2020-09-19 14:47                 ` [PATCH v3] " Han Xin
2020-09-19 23:02                   ` Junio C Hamano
2020-09-20  6:20                     ` [PATCH v4] " Han Xin
2020-09-16 17:35         ` [PATCH 2/2] send-pack: check atomic push before running GPG 韩欣(炽天)
2020-09-15 20:31 ` Junio C Hamano [this message]
2020-09-16  0:34   ` [PATCH 1/2] t5534: new test case for atomic signed push brian m. carlson
2020-09-15 20:34 ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqv9gezsr9.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=chiyutianyi@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hanxin.hx@alibaba-inc.com \
    --cc=zhiyou.jx@alibaba-inc.com \
    --subject='Re: [PATCH 1/2] t5534: new test case for atomic signed push' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).