Re: [PATCH v3 3/9] tree: increase test coverage for tree.c

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

"Heather Lapointe via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/t/t1023-tree-read-tree-at.sh b/t/t1023-tree-read-tree-at.sh
> new file mode 100755
> index 00000000000..9e5ce3abb4b
> --- /dev/null
> +++ b/t/t1023-tree-read-tree-at.sh
> @@ -0,0 +1,65 @@
> +#!/bin/sh
> +
> +# tests for tree.c (not read-tree.c)
> +test_description='Test read_tree / read_tree_at'
> +. ./test-lib.sh
> +
> +test_expect_success 'read_tree basic' '
> +	rm -rf walk_tree_basic &&
> +	git init walk_tree_basic &&
> +	(
> +		cd walk_tree_basic &&
> +		set -x &&

Do we need this, when we have '-x' option alongside with '-v', '-i',
and '-d' available in the test harness already?

> +		mkdir -p dir1/dirA &&
> +		mkdir -p dir1/dirB &&
> +		mkdir -p dir2 &&

Can't we have these three done by the same single "mkdir -p" process?

> +		echo "file1" > file1.txt &&
> +		echo "file2" > file2.txt &&

Lose the SP between redirection operator ">" and its target, i.e.

		echo file1 >file1.txt

cf. Documentation/CodingGuidelines

Also you do not necessarily have to have dq around a single token.

> +		# uncommitted
> +		echo "file3" > file3.txt &&
> +
> +		echo "file1A1" > dir1/dirA/file1.txt &&
> +		git add file1.txt file2.txt dir1/dirA/file1.txt &&
> +		git commit -m "initial commit" &&
> +
> +		test-tool tree-read-tree-at . > walk1.txt &&
> +		grep " file1.txt" walk1.txt &&
> +		! grep " file3.txt" walk1.txt &&
> +		! grep " dir1/dirB" walk1.txt &&
> +		grep " dir1/dirA/file1.txt" walk1.txt
> +	)
> +'
> +
> +test_expect_success 'read_tree submodules' '
> +	rm -rf walk_tree_submodules &&

Curious why the above does not clean "submodule1", too.  After all,
all the "rm -rf" we saw in this script above are removing what its
earlier steps would never have created (but will create), just in
case.  Why not do the same?

If the pattern is to "remove what we will need to create immediately
before we actually try to create, just in case", then shouldn't the
above be removing "submodule1", and we should have another "rm -rf"
for "walk_tree_submodules" immediately before we do "git init" on
it several lines below?

> +	git init submodule1 &&
> +	(
> +		cd submodule1 &&
> +		mkdir -p dir1/dirA &&
> +		echo "dir2/sub1/file1.txt" > file1.txt &&
> +		echo "dir2/sub1/file1A1.txt" > dir1/dirA/file1.txt &&
> +		git add file1.txt dir1/dirA/file1.txt &&
> +		git commit -m "initial commit"
> +	) &&
> +	git init walk_tree_submodules &&
> +	(
> +		cd walk_tree_submodules &&
> +
> +		mkdir -p dir2 &&
> +		echo "file1" > file1.txt &&
> +		echo "dir2/file2" > dir2/file2.txt &&
> +		git add file1.txt dir2/file2.txt &&
> +		git commit -m "initial commit" &&
> +
> +		git submodule add ../submodule1 dir2/sub1 &&
> +		git commit -m "add submodule1" &&
> +
> +		test-tool tree-read-tree-at . > walk2.txt &&
> +		grep " file1.txt" walk2.txt &&
> +		grep " dir2/sub1/file1.txt" walk2.txt &&
> +		grep " dir2/sub1/dir1/dirA/file1.txt" walk2.txt
> +	)
> +'
> +
> +test_done