git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Oct 17, 2022, 13:34 UTC
Message-ID
<b8e2be95-0d42-aaae-4d9f-1825b62707ba@dunelm.org.uk>
In-Reply-To
<9a07c6932f4c7ef844df1fc4f5b6b9feb1810135.1665973401.git.gitgitgadget@gmail.com>
Hi Heather
On 17/10/2022 03:23, Heather Lapointe via GitGitGadget wrote:
> From: Heather Lapointe <alpha@alphaservcomputing.solutions>
> 
> This highlights some buggy behavior from read_tree for submodules that
> was not being executed.

The commit message should explain the reason behind the change being made. In this case it would be helpful to give an overview of what the bug is you're testing for. Given the description I was expecting to see some failing tests that are fixed by a later patch but that doesn't seem to be the case, so I'm wondering what these tests do.

> This introduces a test-tool tree-read-tree-at command
> (the complex name is because it is not related to the read-tree command).

It would also be helpful to explain why we cannot reproduce the bug with "git read-tree --recurse-submodules"

Best Wishes
Phillip
Show 164 quoted lines
> Signed-off-by: Heather Lapointe <alpha@alphaservcomputing.solutions>
> ---
>   Makefile                          |  1 +
>   t/helper/test-tool.c              |  1 +
>   t/helper/test-tool.h              |  1 +
>   t/helper/test-tree-read-tree-at.c | 40 +++++++++++++++++++
>   t/t1023-tree-read-tree-at.sh      | 65 +++++++++++++++++++++++++++++++
>   5 files changed, 108 insertions(+)
>   create mode 100644 t/helper/test-tree-read-tree-at.c
>   create mode 100755 t/t1023-tree-read-tree-at.sh
> 
> diff --git a/Makefile b/Makefile
> index 6bfb62cbe94..52d17ca7276 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -788,6 +788,7 @@ TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
>   TEST_BUILTINS_OBJS += test-submodule.o
>   TEST_BUILTINS_OBJS += test-subprocess.o
>   TEST_BUILTINS_OBJS += test-trace2.o
> +TEST_BUILTINS_OBJS += test-tree-read-tree-at.o
>   TEST_BUILTINS_OBJS += test-urlmatch-normalization.o
>   TEST_BUILTINS_OBJS += test-userdiff.o
>   TEST_BUILTINS_OBJS += test-wildmatch.o
> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> index d1d013bcd92..a8a9bedec5f 100644
> --- a/t/helper/test-tool.c
> +++ b/t/helper/test-tool.c
> @@ -82,6 +82,7 @@ static struct test_cmd cmds[] = {
>   	{ "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
>   	{ "subprocess", cmd__subprocess },
>   	{ "trace2", cmd__trace2 },
> +	{ "tree-read-tree-at", cmd__tree_read_tree_at },
>   	{ "userdiff", cmd__userdiff },
>   	{ "urlmatch-normalization", cmd__urlmatch_normalization },
>   	{ "xml-encode", cmd__xml_encode },
> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
> index 6b46b6444b6..409fddfaeb8 100644
> --- a/t/helper/test-tool.h
> +++ b/t/helper/test-tool.h
> @@ -76,6 +76,7 @@ int cmd__submodule_config(int argc, const char **argv);
>   int cmd__submodule_nested_repo_config(int argc, const char **argv);
>   int cmd__subprocess(int argc, const char **argv);
>   int cmd__trace2(int argc, const char **argv);
> +int cmd__tree_read_tree_at(int argc, const char **argv);
>   int cmd__userdiff(int argc, const char **argv);
>   int cmd__urlmatch_normalization(int argc, const char **argv);
>   int cmd__xml_encode(int argc, const char **argv);
> diff --git a/t/helper/test-tree-read-tree-at.c b/t/helper/test-tree-read-tree-at.c
> new file mode 100644
> index 00000000000..bba759bb264
> --- /dev/null
> +++ b/t/helper/test-tree-read-tree-at.c
> @@ -0,0 +1,40 @@
> +/* This tests tree.c's read_tree / read_tree_at.
> +We call it tree-read-tree-at to disambiguate with the read-tree tool.
> +*/
> +#include "cache.h"
> +#include "pathspec.h"
> +#include "test-tool.h"
> +#include "tree.h"
> +
> +static int test_handle_entry(const struct object_id *oid,
> +		struct strbuf *base, const char *filename,
> +		unsigned mode, void *context UNUSED) {
> +	printf("%i %s %s%s\n", mode, oid_to_hex(oid), base->buf, filename);
> +	if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
> +		return READ_TREE_RECURSIVE;
> +	}
> +	return 0;
> +}
> +
> +int cmd__tree_read_tree_at(int argc UNUSED, const char **argv)
> +{
> +	struct pathspec pathspec;
> +	struct tree *tree;
> +	struct repository *repo;
> +	struct object_id oid;
> +
> +	setup_git_directory();
> +	repo = the_repository;
> +	assert(repo);
> +
> +	parse_pathspec(&pathspec, 0,
> +		       PATHSPEC_PREFER_FULL,
> +		       "", argv);
> +
> +	assert(repo_get_oid(repo, "HEAD", &oid) == 0);
> +	tree = repo_parse_tree_indirect(repo, &oid);
> +	assert(tree);
> +	pathspec.recurse_submodules = 1;
> +	read_tree(repo, tree, &pathspec, test_handle_entry, NULL);
> +	return 0;
> +}
> 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 &&
> +
> +		mkdir -p dir1/dirA &&
> +		mkdir -p dir1/dirB &&
> +		mkdir -p dir2 &&
> +		echo "file1" > file1.txt &&
> +		echo "file2" > file2.txt &&
> +		# 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 &&
> +	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
Previous: Heather Lapointe via GitGitGadgetNext: Junio C Hamano
Message 18 of 48 in “archive: add --recurse-submodules to git-archive command”
  1. archive: add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 12, 2022
  2. 0/2 archive: Add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 13, 2022
  3. 2/2 archive: fix a case of submodule in submodule traversalAlphadelta14 via GitGitGadget, Oct 13, 2022
  4. 1/2 archive: add --recurse-submodules to git-archive commandAlphadelta14 via GitGitGadget, Oct 13, 2022
  5. René ScharfeOct 13, 2022
  6. Heather LapointeOct 13, 2022
  7. René ScharfeOct 13, 2022
  8. Heather LapointeOct 13, 2022
  9. René ScharfeOct 14, 2022
  10. 0/9 archive: Add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 17, 2022
  11. 1/9 tree: do not use the_repository for tree traversal methods.Alphadelta14 via GitGitGadget, Oct 17, 2022
  12. Junio C HamanoOct 17, 2022
  13. Glen ChooOct 26, 2022
  14. Jonathan TanOct 27, 2022
  15. Junio C HamanoOct 27, 2022
  16. 2/9 tree: update cases to use repo_ tree methodsHeather Lapointe via GitGitGadget, Oct 17, 2022
  17. 3/9 tree: increase test coverage for tree.cHeather Lapointe via GitGitGadget, Oct 17, 2022
  18. Phillip WoodOct 17, 2022
  19. Junio C HamanoOct 17, 2022
  20. Jonathan TanOct 27, 2022
  21. 4/9 tree: handle submodule case for read_tree_at properlyHeather Lapointe via GitGitGadget, Oct 17, 2022
  22. Phillip WoodOct 17, 2022
  23. Junio C HamanoOct 17, 2022
  24. Glen ChooOct 26, 2022
  25. Jonathan TanOct 27, 2022
  26. 5/9 tree: add repository parameter to read_tree_fn_tHeather Lapointe via GitGitGadget, Oct 17, 2022
  27. 6/9 archive: pass repo objects to write_archive handlersHeather Lapointe via GitGitGadget, Oct 17, 2022
  28. Phillip WoodOct 17, 2022
  29. 7/9 archive: remove global repository from archive_argsHeather Lapointe via GitGitGadget, Oct 17, 2022
  30. 9/9 archive: add tests for git archive --recurse-submodulesHeather Lapointe via GitGitGadget, Oct 17, 2022
  31. Jonathan TanOct 27, 2022
  32. Glen ChooOct 27, 2022
  33. Ævar Arnfjörð BjarmasonOct 28, 2022
  34. 8/9 archive: add --recurse-submodules to git-archive commandHeather Lapointe via GitGitGadget, Oct 17, 2022
  35. Glen ChooOct 26, 2022
  36. René ScharfeOct 27, 2022
  37. Glen ChooOct 27, 2022
  38. Glen ChooOct 27, 2022
  39. Glen ChooOct 27, 2022
  40. Phillip WoodOct 17, 2022
  41. Junio C HamanoOct 18, 2022
  42. Heather LapointeOct 18, 2022
  43. Junio C HamanoOct 19, 2022
  44. Junio C HamanoOct 19, 2022
  45. Junio C HamanoOct 20, 2022
  46. Junio C HamanoOct 21, 2022
  47. Glen ChooOct 26, 2022
  48. Heather LapointeOct 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.