Re: [PATCH] t6302: drop unnecessary GPG requirement

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 1, 2016, 02:07 UTC
Message-ID: <xmqq4mdtxjxf.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1454271562-24291-1-git-send-email-sunshine@sunshineco.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 5 quoted lines

> These tests are concerned specifically with filtering, sorting,
> formatting behavior of git-for-each-ref, yet if GPG is not present, the
> entire test script is skipped even though none of the tests depend upon
> or care whether the tags are signed. This unnecessary dependency upon
> GPG may prevent these tests from being more widely run, so drop it.

It is conceivable, if not highly plausible, that a change to the code that does the filtering and formatting can become buggy because payload with GPG signature looks somewhat differently from what is in an annotated but not signed tag. Even if "these are specifically filtering..." is true, including tests for signed tags is valuable.

It seems that we are not currently testing with unsigned but annotated tags, and tests that use them would be good, too.

Would it make sense to introduce a helper function specific to this script to be used to prepare the expected output, to replace cat <<, that goes like this?

    test_prepare_expect () {
	if test_have_prereq GPG
        then
		cat
	else
        	sed -e '/signed/d'
	fi
    }

And then use it like this?

    test_expect_success 'check "%(contents:lines=99999)"' '
        test_prepare_expect <<-\EOF &&
	master |three
        ...
        signed-tag |A signed tag message
        ...
        EOF
	git for-each-ref --format=... >actual &&
        test_cmp expect actual
    '

The setup part would need to make GPG conditional, e.g.

test_expect_success 'setup some history and refs' '
	test_commit one &&
	test_commit two &&
	test_commit three &&
	git checkout -b side &&
	test_commit four &&
	git tag -m "An annotated tag" annotated-tag &&
        git tag -m "Annotated doubly" doubly-annotated-tag annotated-tag &&
	if test_have_prereq GPG
	then
                git tag -s -m "A signed tag message" signed-tag &&
                git tag -s -m "Annonated doubly" doubly-signed-tag signed-tag
	fi &&
	git checkout master &&
	git update-ref refs/odd/spot master
'

Perhaps?