Re: [PATCH] t6302: simplify non-gpg cases

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 9, 2016, 17:47 UTC
Message-ID: <CAPig+cQPbwM0+6yruK0VKKq2ujFLoCLogS7eQNN7WWgRjG5V0w@mail.gmail.com>
In-Reply-To: <20160509164859.GA8231@sigill.intra.peff.net>

On Mon, May 9, 2016 at 12:49 PM, Jeff King <peff@peff.net> wrote:

> On Mon, May 09, 2016 at 12:30:43PM -0400, Eric Sunshine wrote:
> Since you as the author of 618310a seem to agree with this direction,
> here it is as a real patch.

Thanks for working on this.

> Subject: [PATCH] t6302: simplify non-gpg cases
>
> When commit 618310a taught t6302 to run without the GPG

618310a (t6302: skip only signed tags rather than all tests when GPG is missing, 2016-03-06)

Show 7 quoted lines

> prerequisite, it did so by conditionally creating the signed
> tags only when gpg is available. As a result, further tests
> need to take this into account, which they can do with the
> test_prepare_expect helper. This is a minor hassle, though,
> as the helper cannot easily cover all cases (it just matches
> "signed" in the output, so all output must include the
> actual refname).

Should we cite bc9acea (ref-filter: implement %(if), %(then), and %(else) atoms, 2016-04-25) here as an example of a commit for which this was problematic (and which indeed broke the tests when GPG is unavailable)?

Show 8 quoted lines

> Instead, let's take a different approach. We'll always
> create the tags, and only conditionally sign them. This does
> mean our tag-names are a minor lie, but it lets the tests
> which do not care about signing easily behave the same in
> all settings. We'll include a comment to document our lie
> and avoid confusing further test-writers.
>
> Signed-off-by: Jeff King <peff@peff.net>

Looks good. With or without the minor change below, this patch is:

    Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Show 5 quoted lines

> ---
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
>  test_expect_success 'check signed tags with --points-at' '
> -       test_prepare_expect <<-\EOF | sed -e "s/Z$//" >expect &&
> +       cat <<-\EOF | sed -e "s/Z$//" >expect &&

To make this as close to a reversion as possible, this could be restored to the original (sans 'cat'):

    sed -e "s/Z$//" >expect <<-\EOF &&

>         refs/heads/side Z
>         refs/tags/annotated-tag four
>         refs/tags/four Z