Re: [PATCH] Add test for correct coloring of git log --decoration

[Junio C Hamano <gitster@pobox.com>]

Nazri Ramliy <ayiehere@gmail.com> writes:

> I've tried adding 
>
> 	log --decorate --all --oneline --color=always
>
> to t4013-diff-various.sh but it seems a bit out of place because my test only
> test for colors, while no other test in that file test for colors, hence the
> new test file (t4207-log-decoration-colors.sh).

It sounds fine to have these in a separate file.

> diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
> new file mode 100755
> index 0000000..260e71f
> --- /dev/null
> +++ b/t/t4207-log-decoration-colors.sh
> @@ -0,0 +1,70 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Nazri Ramliy
> +#
> +
> +test_description='Test for "git log --decorate" colors
> +'

Let's not expand a single-line description needlessly into a multi-line
one.

> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +  echo foo > foo.txt &&

Indent these with <TAB>, like:

	echo foo >foo.txt &&

> +  git add foo.txt &&
> +  test_tick &&
> +  git commit -m first &&
> +
> +  echo bar > bar.txt &&
> +  git add bar.txt &&
> +  test_tick &&
> +  git commit -m second &&
> +
> +  test_tick &&
> +  EDITOR=cat git tag v1.0 &&

I think "EDITOR=cat" is doubly wrong.  You are not annotating the tag
anyway, so it won't get called, but if you were, you will get something
like this:

	$ EDITOR=cat git tag -a v1.0

	#
        # Write a tag message
        #
        fatal: no tag message?

> +  git clone . local_clone &&
> +  cd local_clone &&

Do not chdir around inside test scripts without having that in a subshell,
as people typically write "cd .." at the very end of a && chain, which may
not be called when anything in between fails, throwing the later tests
into chaos.

In this case your excuse will be that you will run everything after this
point in that local-clone subdirectory, but still this is not a good style
we would want to keep around, risking to be copied by other people who do
not think carefully.

I think the set-up sequence for this test script should probably be
structured like this:

	get_color()
        {
        	git config ...
	}

	test_expect_success setup '
		git config diff.color.commit yellow &&
                ...
                git config color.decorate.HEAD cyan &&

                c_reset=$(get_color reset) &&
                ...
		c_HEAD=$(get_color cyan) &&

        	test_commit A &&
		git clone . other &&
                (
                	cd other &&
                        test_commit A1
		) &&
		git remote add -f other ./other &&
                test_commit B &&
		git tag v1.0 &&
                echo >>A &&
                git stash save Changes to A &&
	'

so that the main test is done inside the top-level directory (you wanted
the clone only because you wanted to have remote tracking branches, not
because you didn't want to touch the top-level directory).