Re: [PATCH/RFCv2 1/2] format-patch: demonstrate that color.ui=always produces colorized patches

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 12, 2011, 18:07 UTC
Message-ID: <7vwrdd7hyp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1315849601-26497-2-git-send-email-pangyanhan@gmail.com>

Pang Yan Han <pangyanhan@gmail.com> writes:

> commit c9bfb953 (want_color: automatically fallback to color.ui,
> 2011-08-17) introduced a regression where format-patch produces colorized
> patches when color.ui is set to "always".

Thanks.

I'd very much prefer this "one setup, one test" folded into an existing test script, instead of wasting a new test number. Perhaps in t4014 whose title reads "various format-patch tests"?

Show 25 quoted lines

> diff --git a/t/t4051-format-patch-config.sh b/t/t4051-format-patch-config.sh
> new file mode 100755
> index 0000000..cea9c7d
> --- /dev/null
> +++ b/t/t4051-format-patch-config.sh
> @@ -0,0 +1,23 @@
> +#!/bin/sh
> +
> +test_description='check that format-patch does not respect porcelain config'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	echo foo >foo &&
> +	git add foo &&
> +	git commit -m "commit1" &&
> +	echo bar >foo &&
> +	git add foo &&
> +	git commit -m "commit2"
> +'
> +
> +test_expect_failure 'format patch with ui.color=always generates non colorized patch' '
> +	git config color.ui always &&
> +	git format-patch -1 &&
> +	mv 0001-commit2.patch actual &&

If you have a concrete output file, there is no point moving it to "actual", especially when you are not comparing it with "expect".

> +	test_must_fail grep "\[31m-" actual
> +'

Hmm, is this grep safe?

Perhaps adding something like this near the end of an existing test script might be simpler and sufficient, no?

test_expect_failure 'format patch ignores color.ui' '
	test_unconfig color.ui &&
        git format-patch --stdout -1 >expect &&
	test_config color.ui always &&
	git format-patch --stdout -1 >actual &&
        test_cmp expect actual
'