Re: [PATCH v3 1/5] pretty: add conditional %C?colorname placeholders

Will Palmer [off-list ref] writes:

Hmm...

+test_description='Test pretty formats'
+. ./test-lib.sh
+
+test_expect_success "set up basic repos" \
+	">foo &&
+	>bar &&
+	git add foo &&
+	test_tick &&
+	git commit -m initial &&
+	git add bar &&
+	test_tick &&
+	git commit -m 'add bar'"

Just a style thing, but it is easier to read to write:

        test_expect_success 'set up basic repos' '
                >foo &&
                >bar &&
                ...
        '

+for flag in false true always; do
+for color in red green blue reset; do
+
+	make_expected="git config --get-color no.such.slot $color >expected"
+	test_expect_success "%C$color with color.ui $flag" \
+		"$make_expected &&
+		git config color.ui $flag &&
+		git log -1 --pretty=format:'%C$color' > actual &&
+		test_cmp expected actual"

I would really prefer to see a test that checks output with real contents,
not just "switch to color".  E.g. --format="Title: %C${color}%s%C(normal)"
or something like that.

Also you probably would want to make this one round into a shell function
e.g.

	make_expected ()
        {
        	git config --get-color no.such.slot "$1"
	}

	run_format_test ()
        {
		flag=$1 color=$2
                test_expect_success "%C$color with ..." '
			make_expected $color >expected
                        git config color.ui $flag &&
                        git log -1 --pretty=format:'%C($color)' >actual &&
                        test_cmp expected actual"
                '
                ...
	}

and then run this loop:

	for flag in flase true always
        do
        	for color in red green blue reset
                do
                	run_format_test $flag $color
		done
	done