Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Heather Lapointe via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: "René Scharfe" <l.s.r@web.de>,
	"Heather Lapointe" <alpha@alphaservcomputing.solutions>
Subject: Re: [PATCH v3 3/9] tree: increase test coverage for tree.c
Date: Mon, 17 Oct 2022 14:34:22 +0100	[thread overview]
Message-ID: <b8e2be95-0d42-aaae-4d9f-1825b62707ba@dunelm.org.uk> (raw)
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

> 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

  reply	other threads:[~2022-10-17 13:34 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-12 17:52 [PATCH] archive: add --recurse-submodules to git-archive command Heather Lapointe via GitGitGadget
2022-10-13 11:35 ` [PATCH v2 0/2] archive: Add " Heather Lapointe via GitGitGadget
2022-10-13 11:35   ` [PATCH v2 1/2] archive: add " Alphadelta14 via GitGitGadget
2022-10-13 17:53     ` René Scharfe
2022-10-13 21:37       ` Heather Lapointe
2022-10-13 11:36   ` [PATCH v2 2/2] archive: fix a case of submodule in submodule traversal Alphadelta14 via GitGitGadget
2022-10-13 17:53   ` [PATCH v2 0/2] archive: Add --recurse-submodules to git-archive command René Scharfe
2022-10-13 21:23     ` Heather Lapointe
2022-10-14  9:47       ` René Scharfe
2022-10-17  2:23   ` [PATCH v3 0/9] " Heather Lapointe via GitGitGadget
2022-10-17  2:23     ` [PATCH v3 1/9] tree: do not use the_repository for tree traversal methods Alphadelta14 via GitGitGadget
2022-10-17 13:26       ` Junio C Hamano
2022-10-26 22:33       ` Glen Choo
2022-10-27 18:09       ` Jonathan Tan
2022-10-27 18:50         ` Junio C Hamano
2022-10-17  2:23     ` [PATCH v3 2/9] tree: update cases to use repo_ tree methods Heather Lapointe via GitGitGadget
2022-10-17  2:23     ` [PATCH v3 3/9] tree: increase test coverage for tree.c Heather Lapointe via GitGitGadget
2022-10-17 13:34       ` Phillip Wood [this message]
2022-10-17 13:36       ` Junio C Hamano
2022-10-27 18:28       ` Jonathan Tan
2022-10-17  2:23     ` [PATCH v3 4/9] tree: handle submodule case for read_tree_at properly Heather Lapointe via GitGitGadget
2022-10-17 13:48       ` Phillip Wood
2022-10-17 13:56       ` Junio C Hamano
2022-10-26 22:48       ` Glen Choo
2022-10-27 18:43       ` Jonathan Tan
2022-10-17  2:23     ` [PATCH v3 5/9] tree: add repository parameter to read_tree_fn_t Heather Lapointe via GitGitGadget
2022-10-17  2:23     ` [PATCH v3 6/9] archive: pass repo objects to write_archive handlers Heather Lapointe via GitGitGadget
2022-10-17 13:50       ` Phillip Wood
2022-10-17  2:23     ` [PATCH v3 7/9] archive: remove global repository from archive_args Heather Lapointe via GitGitGadget
2022-10-17  2:23     ` [PATCH v3 8/9] archive: add --recurse-submodules to git-archive command Heather Lapointe via GitGitGadget
2022-10-26 23:34       ` Glen Choo
2022-10-27  7:09         ` René Scharfe
2022-10-27 17:29           ` Glen Choo
2022-10-27 17:30           ` Glen Choo
2022-10-27 17:33           ` Glen Choo
2022-10-17  2:23     ` [PATCH v3 9/9] archive: add tests for git archive --recurse-submodules Heather Lapointe via GitGitGadget
2022-10-27 18:54       ` Jonathan Tan
2022-10-27 23:30         ` Glen Choo
2022-10-28  0:17       ` Ævar Arnfjörð Bjarmason
2022-10-17 13:57     ` [PATCH v3 0/9] archive: Add --recurse-submodules to git-archive command Phillip Wood
2022-10-18 18:34     ` Junio C Hamano
2022-10-18 18:48       ` Heather Lapointe
2022-10-19 16:16         ` Junio C Hamano
2022-10-19 20:44           ` Junio C Hamano
2022-10-20  1:21             ` Junio C Hamano
2022-10-21  1:43               ` Junio C Hamano
2022-10-26 22:14     ` Glen Choo
2022-10-28 18:18       ` Heather Lapointe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8e2be95-0d42-aaae-4d9f-1825b62707ba@dunelm.org.uk \
    --to=phillip.wood123@gmail.com \
    --cc=alpha@alphaservcomputing.solutions \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=l.s.r@web.de \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).