Re: [PATCH v6] Add an explicit GIT_DIR to the list of excludes

From: Johannes Sixt <j6t@kdbg.org>
Date: Jun 12, 2014, 06:19 UTC
Message-ID: <53994667.90209@kdbg.org>
In-Reply-To: <1402529308-3940-1-git-send-email-pasha.bolokhov@gmail.com>

Am 12.06.2014 01:28, schrieb Pasha Bolokhov:

Show 18 quoted lines

> +test_expect_success "setup" '
> +	mkdir repo-inside/ &&
> +	(
> +		cd repo-inside/ &&
> +		for f in a b c d
> +		do
> +			echo "DATA" >"$f" || exit 1
> +		done &&
> +		mkdir dir1 dir1/meta &&
> +		mkdir dir1/ssubdir dir1/ssubdir/meta &&
> +		for f in e f g h
> +		do
> +			echo "MORE DATA" >"dir1/$f" || exit 1
> +		done &&
> +		echo "EVEN more Data" >dir1/meta/aa &&
> +		echo "Data and BAIT" >dir1/ssubdir/meta/aaa &&
> +		mkdir dir2
> +		echo "Not a Metadata File" >dir2/meta

&&-chain broken twice.

We already use 'mkdir -p' elsewhere; you can use it here as well to contract several mkdir invocations.

Show 25 quoted lines

> +		git --git-dir=meta init
> +	) &&
> +	mkdir repo-outside/ repo-outside/external repo-outside/tree &&
> +	(
> +		cd repo-outside/tree &&
> +		for f in n o p q
> +		do
> +			echo "Literal Data" >"$f" || exit 1
> +		done &&
> +		mkdir meta sub sub/meta &&
> +		echo "Sample data" >meta/bb &&
> +		echo "Stream of data" >sub/meta/bbb &&
> +		git --git-dir=../external/meta init
> +	)
> +'
> +
> +
> +#
> +# The first set of tests (the repository is inside the work tree)
> +#
> +test_expect_success "'git status' ignores the repository directory" '
> +	(
> +		cd repo-inside &&
> +		git --git-dir=meta --work-tree=. status --porcelain --untracked=all |
> +			grep meta | sort >status.actual.2 &&

Please do not place a git invocation in a pipline such that it is not the last command; its exist status would be ignored:

		git --git-dir=meta --work-tree=. status --porcelain --untracked=all

>status.actual.2+ &&

		grep meta status.actual.2+ | sort >status.actual.2 &&

There are more cases like this later in the patch.

Show 6 quoted lines

> +		cat >status.expect.2 <<-\EOF &&
> +		?? dir1/meta/aa
> +		?? dir1/ssubdir/meta/aaa
> +		?? dir2/meta
> +		EOF
> +		test_cmp status.actual.2 status.expect.2

It is customary to call the files 'expect' and 'actual'. Furthermore, swap the order so that in case of a failure the diff shows how the actual text was changed from the expected text:

		test_cmp status.expect.2 status.actual.2

Show 35 quoted lines

> +	)
> +'
> +
> +test_expect_success "'git add -A' ignores the repository directory" '
> +	(
> +		cd repo-inside &&
> +		git --git-dir=meta --work-tree=. add -A &&
> +		git --git-dir=meta --work-tree=. status --porcelain | grep meta | sort >status.actual.3 &&
> +		cat >status.expect.3 <<-\EOF &&
> +		A  dir1/meta/aa
> +		A  dir1/ssubdir/meta/aaa
> +		A  dir2/meta
> +		EOF
> +		test_cmp status.actual.3 status.expect.3
> +	)
> +'
> +
> +test_expect_success "'git grep --exclude-standard' ignores the repository directory" '
> +	(
> +		cd repo-inside &&
> +		test_might_fail git --git-dir=meta \
> +			grep --no-index --exclude-standard BAIT >grep.actual.4 &&
> +		cat >grep.expect.4 <<-\EOF &&
> +		dir1/ssubdir/meta/aaa:Data and BAIT
> +		EOF
> +		test_cmp grep.actual.4 grep.expect.4
> +	)
> +'
> +
> +#
> +# The second set of tests (the repository is outside of the work tree)
> +#
> +test_expect_success "'git status' acknowledges directories 'meta' \
> +if repo is not within work tree" '
> +	test_might_fail rm -rf meta/ &&

How might this fail? Only if permissions are wrong, and then we do want this to fail.

Moreover, test_*_fail helpers are intended to be used only with git commands; we don't expect system commands to fail in unexpected ways.

Show 29 quoted lines

> +	(
> +		cd repo-outside/tree &&
> +		git --git-dir=../external/meta init &&
> +		git --git-dir=../external/meta --work-tree=. status --porcelain --untracked=all |
> +			grep meta | sort >status.actual.5 &&
> +		cat >status.expect.5 <<-\EOF &&
> +		?? meta/bb
> +		?? sub/meta/bbb
> +		EOF
> +		test_cmp status.actual.5 status.expect.5
> +	)
> +'
> +
> +test_expect_success "'git add -A' adds 'meta' if the repo is outside the work tree" '
> +	(
> +		cd repo-outside/tree &&
> +		git --git-dir=../external/meta --work-tree=. add -A &&
> +		git --git-dir=../external/meta --work-tree=. status --porcelain --untracked=all |
> +			grep meta | sort >status.actual.6 &&
> +		cat >status.expect.6 <<-\EOF &&
> +		A  meta/bb
> +		A  sub/meta/bbb
> +		EOF
> +		test_cmp status.actual.6 status.expect.6
> +	)
> +'
> +
> +test_done
>