Re: [PATCH v7 09/17] ref-filter: make "%(symref)" atom work with the ':short' modifier

From: Jakub Narębski <jnareb@gmail.com>
Date: Nov 18, 2016, 21:34 UTC
Message-ID: <ce2862d5-874b-f244-f9b3-f74e18f7ad42@gmail.com>
In-Reply-To: <20161108201211.25213-10-Karthik.188@gmail.com>

W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
[...]

> Add tests for %(symref) and %(symref:short) while we're here.

That's nice.

> 
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
> ---

[...]

Show 7 quoted lines

> +test_expect_success 'Add symbolic ref for the following tests' '
> +	git symbolic-ref refs/heads/sym refs/heads/master
> +'
> +
> +cat >expected <<EOF
> +refs/heads/master
> +EOF

This should be inside the relevant test, not outside. In other patches in this series you are putting setup together with the rest of test, by using "cat >expected <<-\EOF".

> +
> +test_expect_success 'Verify usage of %(symref) atom' '
> +	git for-each-ref --format="%(symref)" refs/heads/sym > actual &&

This should be spelled " >actual", rather than " > actual"; there should be no space between redirection and file name.

Show 11 quoted lines

> +	test_cmp expected actual
> +'
> +
> +cat >expected <<EOF
> +heads/master
> +EOF
> +
> +test_expect_success 'Verify usage of %(symref:short) atom' '
> +	git for-each-ref --format="%(symref:short)" refs/heads/sym > actual &&
> +	test_cmp expected actual
> +'

Same here.

> +
>  test_done
>

-- 
Jakub Narębski