Re: [PATCH v2 2/2] log: add log.showSignature configuration variable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jun 18, 2016 at 8:25 AM, Mehul Jain <mehul.jain2029@xxxxxxxxx> wrote:
> Users may want to always use "--show-signature" while using git-log and
> related commands.
>
> When log.showSignature is set to true, git-log and related commands will
> behave as if "--show-signature" was given to them.
>
> Note that this config variable is meant to affect git-log, git-show,
> git-whatchanged and git-reflog. Other commands like git-format-patch,
> git-rev-list are not to be affected by this config variable.
>
> Signed-off-by: Mehul Jain <mehul.jain2029@xxxxxxxxx>
> ---
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> @@ -900,6 +900,31 @@ test_expect_success GPG '--no-show-signature overrides --show-signature' '
> +test_expect_success GPG 'log.showsignature=true behaves like --show-signature' '
> +       git checkout -b test_sign master &&

It appears that you copied the bulk of this test from the 'log --graph
--show-signature' test and changed it to create a new branch named
'test_sign' rather than 'signed', however...

> +       echo foo >foo &&
> +       git add foo &&
> +       git commit -S -m signed_commit &&
> +       test_config log.showsignature true &&
> +       git log -1 signed >actual &&

... you're incorrectly testing against the 'signed' branch rather than
the 'test_sign' created specifically for this test.

> +       grep "gpg: Signature made" actual &&
> +       grep "gpg: Good signature" actual
> +'
> +
> +test_expect_success GPG '--no-show-signature overrides log.showsignature=true' '
> +       test_config log.showsignature true &&
> +       git log -1 --no-show-signature signed >actual &&
> +       ! grep "^gpg:" actual
> +'
> +
> +test_expect_success GPG '--show-signature overrides log.showsignature=false' '
> +       test_when_finished "git reset --hard && git checkout master" &&

So, in the first of these three new tests, you're setting up some
state by creating and checking out a new branch named 'test_sign', and
leaving that branch checked out while these three tests run, and
finally use test_when_finished() in the last of the three tests to
restore sanity (by returning to the 'master' branch) when that test
exits.

This is ugly and couples these three tests too tightly. It would be
better to make each test more self-contained, not relying upon state
left over from previous tests.

> +       test_config log.showsignature false &&
> +       git log -1 --show-signature signed >actual &&
> +       grep "gpg: Signature made" actual &&
> +       grep "gpg: Good signature" actual
> +'

In fact, the original 'log --graph --show-signature' test which
created the 'signed' branch, the new --no-show-signature test added in
patch 1/2, and the three new tests here could all just work against
the same 'signed' branch. There is no need to create a new 'test_sign'
branch for these three tests, or a 'nosign' branch for the patch 1/2
test.

Therefore, it probably would make more sense to add a new distinct
'setup signed' test (or just enhance the existing 'setup' test) which
creates the 'signed' branch and update the original 'log --graph
--show-signature' to take advantage of it, as well as each of the new
tests introduced by this patch series. And since each test would
mention 'signed' explicitly in its git-log invocation, there's no need
to leave that branch checked out, so the setup test itself only needs
test_when_finished() to ensure that the current branch is restored to
'master'.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]