[PATCH 0/3] t: rework tests for --pathspec-from-file

[PATCH 0/3] t: rework tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

Please refer to commit messages for rationale.

This branch is a follow-up for [1] where part of branch was merged into `master` via [2].

Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.

Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.

[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/

Alexandr Miloslavskiy (3):
  t: fix quotes tests for --pathspec-from-file
  t: directly test parse_pathspec_file()
  t: drop copy&pasted tests for --pathspec-from-file

 Makefile                            |  1 +
 t/helper/test-parse-pathspec-file.c | 34 +++++++++++
 t/helper/test-tool.c                |  1 +
 t/helper/test-tool.h                |  1 +
 t/t0067-parse_pathspec_file.sh      | 89 +++++++++++++++++++++++++++++
 t/t2026-checkout-pathspec-file.sh   | 70 +----------------------
 t/t2072-restore-pathspec-file.sh    | 70 +----------------------
 t/t3704-add-pathspec-file.sh        | 70 +----------------------
 t/t7107-reset-pathspec-file.sh      | 79 +++----------------------
 t/t7526-commit-pathspec-file.sh     | 70 +----------------------
 10 files changed, 142 insertions(+), 343 deletions(-)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh


base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/503
-- 
gitgitgadget

[PATCH 1/3] t: fix quotes tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.

Fix this by properly escaping one more time.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 9 +++++++--
 t/t2072-restore-pathspec-file.sh  | 9 +++++++--
 t/t3704-add-pathspec-file.sh      | 9 +++++++--
 t/t7107-reset-pathspec-file.sh    | 9 +++++++--
 t/t7526-commit-pathspec-file.sh   | 9 +++++++--
 5 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..2dc8901bca 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	M  fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
 '
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..70e95ef3b6 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	 M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
 '
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..2e0141fcce 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
 	A  fileA.t
@@ -108,7 +111,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
 '
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..52a44f033d 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
 	git rm fileA.t &&
-	printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+	printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
 	 D fileA.t
@@ -117,8 +120,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
 	git rm fileA.t &&
-	printf "\"file\\101.t\"" >list &&
+	printf "\"file\\\\101.t\"" >list &&
 	# Note: "git reset" has not yet learned to fail on wrong pathspecs
 	git reset --pathspec-from-file=list --pathspec-file-nul &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..e7dc2ff8b1 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
 
 	cat >expect <<-\EOF &&
 	A	fileA.t
@@ -111,7 +114,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
 '
 
-- 
gitgitgadget

[PATCH 2/3] t: directly test parse_pathspec_file()

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.

Introduce direct tests for `parse_pathspec_file()`.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 Makefile                            |  1 +
 t/helper/test-parse-pathspec-file.c | 34 +++++++++++
 t/helper/test-tool.c                |  1 +
 t/helper/test-tool.h                |  1 +
 t/t0067-parse_pathspec_file.sh      | 89 +++++++++++++++++++++++++++++
 5 files changed, 126 insertions(+)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh

diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
 TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
 TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-pkt-line.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..e7f525feb9
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,34 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+	struct pathspec pathspec;
+	const char *pathspec_from_file = 0;
+	int pathspec_file_nul = 0, i;
+
+	static const char *const usage[] = {
+		"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+		NULL
+	};
+
+	struct option options[] = {
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+		OPT_END()
+	};
+
+	parse_options(argc, argv, 0, options, usage, 0);
+
+	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+			    pathspec_file_nul);
+
+	for (i = 0; i < pathspec.nr; i++) {
+		printf("%s\n", pathspec.items[i].original);
+	}
+
+	clear_pathspec(&pathspec);
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
 	{ "oidmap", cmd__oidmap },
 	{ "online-cpus", cmd__online_cpus },
 	{ "parse-options", cmd__parse_options },
+	{ "parse-pathspec-file", cmd__parse_pathspec_file },
 	{ "path-utils", cmd__path_utils },
 	{ "pkt-line", cmd__pkt_line },
 	{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
 int cmd__oidmap(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__pkt_line(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..df7b319713
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,89 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+	echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+	echo fileA.t >list &&
+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+	printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+	printf "fileA.t\nfileB.t\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+	printf "fileA.t\nfileB.t" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+	printf "fileA.t\r\nfileB.t\r\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+	cat >expect <<-\EOF &&
+	"file\101.t"
+	EOF
+	test_cmp expect actual
+'
+
+test_done
-- 
gitgitgadget

Re: [PATCH 2/3] t: directly test parse_pathspec_file()

[Junio C Hamano]

"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
> new file mode 100644
> index 0000000000..e7f525feb9
> --- /dev/null
> +++ b/t/helper/test-parse-pathspec-file.c
> @@ -0,0 +1,34 @@
> +#include "test-tool.h"
> +#include "parse-options.h"
> +#include "pathspec.h"
> +#include "gettext.h"
> + ...
> +	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
> +			    pathspec_file_nul);
> +
> +	for (i = 0; i < pathspec.nr; i++) {
> +		printf("%s\n", pathspec.items[i].original);
> +	}

No need for {} around a single statement block.

> diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
> new file mode 100755
> index 0000000000..df7b319713
> --- /dev/null
> +++ b/t/t0067-parse_pathspec_file.sh
> @@ -0,0 +1,89 @@
> +#!/bin/sh
> +
> +test_description='Test parse_pathspec_file()'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'one item from stdin' '
> +	echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
> +
> +	cat >expect <<-\EOF &&
> +	fileA.t
> +	EOF
> +	test_cmp expect actual
> +'

The use of the blank lines are somewhat inconsistent here.

> + ...
> +test_expect_success 'NUL delimiters' '
> +	printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&

Fold line immediately after the pipe (same for the earlier and later ones).

> +	cat >expect <<-\EOF &&
> +	fileA.t
> +	fileB.t
> +	EOF
> +	test_cmp expect actual
> +'

If you want to have a gap between the steps, i.e. "capturing the
actual output", "creating the ideal output", and "seeing how they
differ", using blank like this is OK:

	printf "fileA.t\0fileB.t\0" |
	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&

	cat >expect <<-\EOF &&
	fileA.t
	fileB.t
	EOF

	test_cmp expect actual

I thought we typically prepare the ideal output sample before
capturing the actual output, so if we follow that convention, the
above becomes

	cat >expect <<-\EOF &&
	fileA.t
	fileB.t
	EOF

	printf "fileA.t\0fileB.t\0" |
	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&

	test_cmp expect actual


> +test_expect_success 'quotes' '
> +	# shell  takes \\\\101 and spits \\101
> +	# printf takes   \\101 and spits  \101
> +	# git    takes    \101 and spits     A
> +	printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
> +
> +	cat >expect <<-\EOF &&
> +	fileA.t
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--pathspec-file-nul takes quotes literally' '
> +	# shell  takes \\\\101 and spits \\101
> +	# printf takes   \\101 and spits  \101
> +	printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
> +
> +	cat >expect <<-\EOF &&
> +	"file\101.t"
> +	EOF
> +	test_cmp expect actual
> +'

Testing low level machinery like this is of course a good idea, in
addition to the end-to-end tests that make sure that the machinery
is called correctly from the higher layer.

Thanks.

Re: [PATCH 2/3] t: directly test parse_pathspec_file()

[Alexandr Miloslavskiy]

Thanks for having a look! I think I have fixed all mentioned issues in V2.

[PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).

I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 75 +--------------------------
 t/t2072-restore-pathspec-file.sh  | 75 +--------------------------
 t/t3704-add-pathspec-file.sh      | 75 +--------------------------
 t/t7107-reset-pathspec-file.sh    | 84 +++----------------------------
 t/t7526-commit-pathspec-file.sh   | 75 +--------------------------
 5 files changed, 16 insertions(+), 368 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index 2dc8901bca..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git checkout --pathspec-from-file=list HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index 70e95ef3b6..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git restore --pathspec-from-file=list --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 2e0141fcce..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git add --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 52a44f033d..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	git rm fileA.t &&
-	echo fileA.t >list &&
-	git reset --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	git rm fileA.t fileB.t &&
@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+	git rm fileA.t fileB.t fileC.t fileD.t &&
+	printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
-	 D fileA.t
+	D  fileA.t
 	 D fileB.t
+	 D fileC.t
+	D  fileD.t
 	EOF
 	verify_expect
 '
 
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	git rm fileA.t &&
-	printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	git rm fileA.t &&
-	printf "\"file\\\\101.t\"" >list &&
-	# Note: "git reset" has not yet learned to fail on wrong pathspecs
-	git reset --pathspec-from-file=list --pathspec-file-nul &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	test_must_fail verify_expect
-'
-
 test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
 	restore_checkpoint &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index e7dc2ff8b1..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git commit --pathspec-from-file=list -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-- 
gitgitgadget

[PATCH v2 0/3] t: rework tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

Please refer to commit messages for rationale.

This branch is a follow-up for [1] where part of branch was merged into `master` via [2].

Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.

Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.

[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/

Changes since V1
----------------
Small code formatting changes suggested in V1.

Alexandr Miloslavskiy (3):
  t: fix quotes tests for --pathspec-from-file
  t: directly test parse_pathspec_file()
  t: drop copy&pasted tests for --pathspec-from-file

 Makefile                            |   1 +
 t/helper/test-parse-pathspec-file.c |  33 +++++++++
 t/helper/test-tool.c                |   1 +
 t/helper/test-tool.h                |   1 +
 t/t0067-parse_pathspec_file.sh      | 104 ++++++++++++++++++++++++++++
 t/t2026-checkout-pathspec-file.sh   |  70 +------------------
 t/t2072-restore-pathspec-file.sh    |  70 +------------------
 t/t3704-add-pathspec-file.sh        |  70 +------------------
 t/t7107-reset-pathspec-file.sh      |  79 +++------------------
 t/t7526-commit-pathspec-file.sh     |  70 +------------------
 10 files changed, 156 insertions(+), 343 deletions(-)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh


base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/503

Range-diff vs v1:

 1:  ab9519298d = 1:  6193dc7396 t: fix quotes tests for --pathspec-from-file
 2:  27383a5b08 ! 2:  ab449ac15a t: directly test parse_pathspec_file()
     @@ -55,9 +55,8 @@
      +	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
      +			    pathspec_file_nul);
      +
     -+	for (i = 0; i < pathspec.nr; i++) {
     ++	for (i = 0; i < pathspec.nr; i++)
      +		printf("%s\n", pathspec.items[i].original);
     -+	}
      +
      +	clear_pathspec(&pathspec);
      +	return 0;
     @@ -99,84 +98,99 @@
      +. ./test-lib.sh
      +
      +test_expect_success 'one item from stdin' '
     -+	echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	fileA.t
      +	EOF
     ++
     ++	echo fileA.t |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
      +test_expect_success 'one item from file' '
     -+	echo fileA.t >list &&
     -+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	fileA.t
      +	EOF
     ++
     ++	echo fileA.t >list &&
     ++	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
      +test_expect_success 'NUL delimiters' '
     -+	printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	fileA.t
      +	fileB.t
      +	EOF
     ++
     ++	printf "fileA.t\0fileB.t\0" |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
      +test_expect_success 'LF delimiters' '
     -+	printf "fileA.t\nfileB.t\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	fileA.t
      +	fileB.t
      +	EOF
     ++
     ++	printf "fileA.t\nfileB.t\n" |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
      +test_expect_success 'no trailing delimiter' '
     -+	printf "fileA.t\nfileB.t" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	fileA.t
      +	fileB.t
      +	EOF
     ++
     ++	printf "fileA.t\nfileB.t" |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
      +test_expect_success 'CRLF delimiters' '
     -+	printf "fileA.t\r\nfileB.t\r\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	fileA.t
      +	fileB.t
      +	EOF
     ++
     ++	printf "fileA.t\r\nfileB.t\r\n" |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
      +test_expect_success 'quotes' '
     ++	cat >expect <<-\EOF &&
     ++	fileA.t
     ++	EOF
     ++
      +	# shell  takes \\\\101 and spits \\101
      +	# printf takes   \\101 and spits  \101
      +	# git    takes    \101 and spits     A
     -+	printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     ++	printf "\"file\\\\101.t\"" |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
      +
     -+	cat >expect <<-\EOF &&
     -+	fileA.t
     -+	EOF
      +	test_cmp expect actual
      +'
      +
      +test_expect_success '--pathspec-file-nul takes quotes literally' '
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
     -+
      +	cat >expect <<-\EOF &&
      +	"file\101.t"
      +	EOF
     ++
     ++	# shell  takes \\\\101 and spits \\101
     ++	# printf takes   \\101 and spits  \101
     ++	printf "\"file\\\\101.t\"" |
     ++	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
     ++
      +	test_cmp expect actual
      +'
      +
 3:  daef256db3 = 3:  88086cebce t: drop copy&pasted tests for --pathspec-from-file

-- 
gitgitgadget

[PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.

Fix this by properly escaping one more time.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 9 +++++++--
 t/t2072-restore-pathspec-file.sh  | 9 +++++++--
 t/t3704-add-pathspec-file.sh      | 9 +++++++--
 t/t7107-reset-pathspec-file.sh    | 9 +++++++--
 t/t7526-commit-pathspec-file.sh   | 9 +++++++--
 5 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..2dc8901bca 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	M  fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
 '
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..70e95ef3b6 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	 M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
 '
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..2e0141fcce 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
 	A  fileA.t
@@ -108,7 +111,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
 '
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..52a44f033d 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
 	git rm fileA.t &&
-	printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+	printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
 	 D fileA.t
@@ -117,8 +120,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
 	git rm fileA.t &&
-	printf "\"file\\101.t\"" >list &&
+	printf "\"file\\\\101.t\"" >list &&
 	# Note: "git reset" has not yet learned to fail on wrong pathspecs
 	git reset --pathspec-from-file=list --pathspec-file-nul &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..e7dc2ff8b1 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
 
 	cat >expect <<-\EOF &&
 	A	fileA.t
@@ -111,7 +114,9 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" >list &&
 	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
 '
 
-- 
gitgitgadget

Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file

[Eric Sunshine]

On Mon, Dec 30, 2019 at 2:15 PM Alexandr Miloslavskiy via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> While working on the next patch, I also noticed that quotes testing via
> `"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
> I had to escape it two times! Tests still worked due to `"` being
> preserved which in turn prevented pathspec from matching files.
>
> Fix this by properly escaping one more time.
>
> Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
> ---
> diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
> @@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
> -       printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
> +       # shell  takes \\\\101 and spits \\101
> +       # printf takes   \\101 and spits  \101
> +       # git    takes    \101 and spits     A
> +       printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&

So, you want git-checkout to receive the following, quotes, backslash,
and no newline, on its standard input?

    "file\101.t"

If so, another way to achieve the same without taxing the brain of the
reader or the next person who works on this code would be:

    tr -d "\012" | git checkout --pathspec-from-file=- HEAD^1 <<-\EOF &&
    "file\101.t"
    EOF

Although it's three lines long, the body of the here-doc is the
literal text you want sent to the Git command, so no counting
backslashes, and no need for a lengthy in-code comment.

But is the "no newline" bit indeed intentional? If not, then a simple
echo would be even easier (though with a bit more escaping):

    echo "\"file\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&

Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file

[Jonathan Nieder]

Eric Sunshine wrote:

> But is the "no newline" bit indeed intentional? If not, then a simple
> echo would be even easier (though with a bit more escaping):
>
>     echo "\"file\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&

For portability, that would be

	printf "%s\n" "\"file\101.t\"" | ...

because some implementations of echo interpret escapes by default.

Thanks,
Jonathan

Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file

[Alexandr Miloslavskiy]

Luckily, trailing \n didn't matter much, and I could also send input via 
commandline argument instead of stdin, so here-doc is really the most 
readable solution here.

Fixed it in V3, thanks for your suggestion!

On 30.12.2019 22:55, Eric Sunshine wrote:
 > So, you want git-checkout to receive the following, quotes, backslash,
 > and no newline, on its standard input?
 >
 >      "file\101.t"
 >
 > If so, another way to achieve the same without taxing the brain of the
 > reader or the next person who works on this code would be:
 >
 >      tr -d "\012" | git checkout --pathspec-from-file=- HEAD^1 <<-\EOF &&
 >      "file\101.t"
 >      EOF
 >
 > Although it's three lines long, the body of the here-doc is the
 > literal text you want sent to the Git command, so no counting
 > backslashes, and no need for a lengthy in-code comment.
 >
 > But is the "no newline" bit indeed intentional? If not, then a simple
 > echo would be even easier (though with a bit more escaping):
 >
 >      echo "\"file\101.t\"" | git checkout --pathspec-from-file=- 
HEAD^1 &&
 >

On 31.12.2019 1:26, Jonathan Nieder wrote:
>> But is the "no newline" bit indeed intentional? If not, then a simple
>> echo would be even easier (though with a bit more escaping):
>>
>>      echo "\"file\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
> 
> For portability, that would be
> 
> 	printf "%s\n" "\"file\101.t\"" | ...
> 
> because some implementations of echo interpret escapes by default.

[PATCH v2 2/3] t: directly test parse_pathspec_file()

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.

Introduce direct tests for `parse_pathspec_file()`.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 Makefile                            |   1 +
 t/helper/test-parse-pathspec-file.c |  33 +++++++++
 t/helper/test-tool.c                |   1 +
 t/helper/test-tool.h                |   1 +
 t/t0067-parse_pathspec_file.sh      | 104 ++++++++++++++++++++++++++++
 5 files changed, 140 insertions(+)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh

diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
 TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
 TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-pkt-line.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..02f4ccfd2a
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,33 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+	struct pathspec pathspec;
+	const char *pathspec_from_file = 0;
+	int pathspec_file_nul = 0, i;
+
+	static const char *const usage[] = {
+		"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+		NULL
+	};
+
+	struct option options[] = {
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+		OPT_END()
+	};
+
+	parse_options(argc, argv, 0, options, usage, 0);
+
+	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+			    pathspec_file_nul);
+
+	for (i = 0; i < pathspec.nr; i++)
+		printf("%s\n", pathspec.items[i].original);
+
+	clear_pathspec(&pathspec);
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
 	{ "oidmap", cmd__oidmap },
 	{ "online-cpus", cmd__online_cpus },
 	{ "parse-options", cmd__parse_options },
+	{ "parse-pathspec-file", cmd__parse_pathspec_file },
 	{ "path-utils", cmd__path_utils },
 	{ "pkt-line", cmd__pkt_line },
 	{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
 int cmd__oidmap(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__pkt_line(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..77b44f6702
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	echo fileA.t |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	echo fileA.t >list &&
+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\0fileB.t\0" |
+	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\nfileB.t\n" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\nfileB.t" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\r\nfileB.t\r\n" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	# git    takes    \101 and spits     A
+	printf "\"file\\\\101.t\"" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+	cat >expect <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	# shell  takes \\\\101 and spits \\101
+	# printf takes   \\101 and spits  \101
+	printf "\"file\\\\101.t\"" |
+	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+	test_cmp expect actual
+'
+
+test_done
-- 
gitgitgadget

[PATCH v2 3/3] t: drop copy&pasted tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).

I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 75 +--------------------------
 t/t2072-restore-pathspec-file.sh  | 75 +--------------------------
 t/t3704-add-pathspec-file.sh      | 75 +--------------------------
 t/t7107-reset-pathspec-file.sh    | 84 +++----------------------------
 t/t7526-commit-pathspec-file.sh   | 75 +--------------------------
 5 files changed, 16 insertions(+), 368 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index 2dc8901bca..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git checkout --pathspec-from-file=list HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index 70e95ef3b6..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git restore --pathspec-from-file=list --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 2e0141fcce..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git add --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 52a44f033d..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	git rm fileA.t &&
-	echo fileA.t >list &&
-	git reset --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	git rm fileA.t fileB.t &&
@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+	git rm fileA.t fileB.t fileC.t fileD.t &&
+	printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
-	 D fileA.t
+	D  fileA.t
 	 D fileB.t
+	 D fileC.t
+	D  fileD.t
 	EOF
 	verify_expect
 '
 
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	git rm fileA.t &&
-	printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	git rm fileA.t &&
-	printf "\"file\\\\101.t\"" >list &&
-	# Note: "git reset" has not yet learned to fail on wrong pathspecs
-	git reset --pathspec-from-file=list --pathspec-file-nul &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	test_must_fail verify_expect
-'
-
 test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
 	restore_checkpoint &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index e7dc2ff8b1..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git commit --pathspec-from-file=list -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	# git    takes    \101 and spits     A
-	printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	# shell  takes \\\\101 and spits \\101
-	# printf takes   \\101 and spits  \101
-	printf "\"file\\\\101.t\"" >list &&
-	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-- 
gitgitgadget

[PATCH v3 0/3] t: rework tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

Please refer to commit messages for rationale.

This branch is a follow-up for [1] where part of branch was merged into `master` via [2].

Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.

Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.

[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/

Changes since V1
----------------
Small code formatting changes suggested in V1.

Alexandr Miloslavskiy (3):
  t: fix quotes tests for --pathspec-from-file
  t: directly test parse_pathspec_file()
  t: drop copy&pasted tests for --pathspec-from-file

 Makefile                            |   1 +
 t/helper/test-parse-pathspec-file.c |  33 +++++++++
 t/helper/test-tool.c                |   1 +
 t/helper/test-tool.h                |   1 +
 t/t0067-parse_pathspec_file.sh      | 108 ++++++++++++++++++++++++++++
 t/t2026-checkout-pathspec-file.sh   |  70 +-----------------
 t/t2072-restore-pathspec-file.sh    |  70 +-----------------
 t/t3704-add-pathspec-file.sh        |  70 +-----------------
 t/t7107-reset-pathspec-file.sh      |  79 +++-----------------
 t/t7526-commit-pathspec-file.sh     |  70 +-----------------
 10 files changed, 160 insertions(+), 343 deletions(-)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh


base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/503

Range-diff vs v2:

 1:  6193dc7396 ! 1:  88790669ce t: fix quotes tests for --pathspec-from-file
     @@ -7,7 +7,7 @@
          I had to escape it two times! Tests still worked due to `"` being
          preserved which in turn prevented pathspec from matching files.
      
     -    Fix this by properly escaping one more time.
     +    Fix this by using here-doc instead.
      
          Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
      
     @@ -19,10 +19,11 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	# git    takes    \101 and spits     A
     -+	printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
     ++	git checkout --pathspec-from-file=list HEAD^1 &&
       
       	cat >expect <<-\EOF &&
       	M  fileA.t
     @@ -31,9 +32,10 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" >list &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	printf "\"file\\\\101.t\"" >list &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
       	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
       '
       
     @@ -46,10 +48,11 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	# git    takes    \101 and spits     A
     -+	printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
     ++	git restore --pathspec-from-file=list --source=HEAD^1 &&
       
       	cat >expect <<-\EOF &&
       	 M fileA.t
     @@ -58,9 +61,10 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" >list &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	printf "\"file\\\\101.t\"" >list &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
       	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
       '
       
     @@ -73,10 +77,11 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	# git    takes    \101 and spits     A
     -+	printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
     ++	git add --pathspec-from-file=list &&
       
       	cat >expect <<-\EOF &&
       	A  fileA.t
     @@ -85,9 +90,10 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" >list &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	printf "\"file\\\\101.t\"" >list &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
       	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
       '
       
     @@ -99,12 +105,13 @@
       test_expect_success 'quotes' '
       	restore_checkpoint &&
       
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	# git    takes    \101 and spits     A
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
       	git rm fileA.t &&
      -	printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
     -+	printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
     ++	git reset --pathspec-from-file=list &&
       
       	cat >expect <<-\EOF &&
       	 D fileA.t
     @@ -112,11 +119,12 @@
       test_expect_success 'quotes not compatible with --pathspec-file-nul' '
       	restore_checkpoint &&
       
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     - 	git rm fileA.t &&
     +-	git rm fileA.t &&
      -	printf "\"file\\101.t\"" >list &&
     -+	printf "\"file\\\\101.t\"" >list &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
       	# Note: "git reset" has not yet learned to fail on wrong pathspecs
       	git reset --pathspec-from-file=list --pathspec-file-nul &&
       
     @@ -129,10 +137,11 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	# git    takes    \101 and spits     A
     -+	printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
     ++	git commit --pathspec-from-file=list -m "Commit" &&
       
       	cat >expect <<-\EOF &&
       	A	fileA.t
     @@ -141,9 +150,10 @@
       	restore_checkpoint &&
       
      -	printf "\"file\\101.t\"" >list &&
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	printf "\"file\\\\101.t\"" >list &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
       	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
       '
       
 2:  ab449ac15a ! 2:  68925c2712 t: directly test parse_pathspec_file()
     @@ -172,24 +172,28 @@
      +	fileA.t
      +	EOF
      +
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	# git    takes    \101 and spits     A
     -+	printf "\"file\\\\101.t\"" |
     -+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
     ++	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
      +
      +	test_cmp expect actual
      +'
      +
      +test_expect_success '--pathspec-file-nul takes quotes literally' '
     ++	# Note: there is an extra newline because --pathspec-file-nul takes
     ++	# input \n literally, too
      +	cat >expect <<-\EOF &&
      +	"file\101.t"
     ++
      +	EOF
      +
     -+	# shell  takes \\\\101 and spits \\101
     -+	# printf takes   \\101 and spits  \101
     -+	printf "\"file\\\\101.t\"" |
     -+	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
     ++	cat >list <<-\EOF &&
     ++	"file\101.t"
     ++	EOF
     ++
     ++	test-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&
      +
      +	test_cmp expect actual
      +'
 3:  88086cebce ! 3:  f71021b0dd t: drop copy&pasted tests for --pathspec-from-file
     @@ -88,10 +88,11 @@
      -test_expect_success 'quotes' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	# git    takes    \101 and spits     A
     --	printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
     +-	git checkout --pathspec-from-file=list HEAD^1 &&
      -
      -	cat >expect <<-\EOF &&
      -	M  fileA.t
     @@ -102,9 +103,10 @@
      -test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	printf "\"file\\\\101.t\"" >list &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
      -	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
      -'
      -
     @@ -188,10 +190,11 @@
      -test_expect_success 'quotes' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	# git    takes    \101 and spits     A
     --	printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
     +-	git restore --pathspec-from-file=list --source=HEAD^1 &&
      -
      -	cat >expect <<-\EOF &&
      -	 M fileA.t
     @@ -202,9 +205,10 @@
      -test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	printf "\"file\\\\101.t\"" >list &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
      -	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
      -'
      -
     @@ -288,10 +292,11 @@
      -test_expect_success 'quotes' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	# git    takes    \101 and spits     A
     --	printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
     +-	git add --pathspec-from-file=list &&
      -
      -	cat >expect <<-\EOF &&
      -	A  fileA.t
     @@ -302,9 +307,10 @@
      -test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	printf "\"file\\\\101.t\"" >list &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
      -	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
      -'
      -
     @@ -398,11 +404,12 @@
      -test_expect_success 'quotes' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	# git    takes    \101 and spits     A
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
      -	git rm fileA.t &&
     --	printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
     +-	git reset --pathspec-from-file=list &&
      -
      -	cat >expect <<-\EOF &&
      -	 D fileA.t
     @@ -413,10 +420,10 @@
      -test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	git rm fileA.t &&
     --	printf "\"file\\\\101.t\"" >list &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
      -	# Note: "git reset" has not yet learned to fail on wrong pathspecs
      -	git reset --pathspec-from-file=list --pathspec-file-nul &&
      -
     @@ -506,10 +513,11 @@
      -test_expect_success 'quotes' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	# git    takes    \101 and spits     A
     --	printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
     +-	git commit --pathspec-from-file=list -m "Commit" &&
      -
      -	cat >expect <<-\EOF &&
      -	A	fileA.t
     @@ -520,9 +528,10 @@
      -test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      -	restore_checkpoint &&
      -
     --	# shell  takes \\\\101 and spits \\101
     --	# printf takes   \\101 and spits  \101
     --	printf "\"file\\\\101.t\"" >list &&
     +-	cat >list <<-\EOF &&
     +-	"file\101.t"
     +-	EOF
     +-
      -	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
      -'
      -

-- 
gitgitgadget

[PATCH v3 1/3] t: fix quotes tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.

Fix this by using here-doc instead.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 11 +++++++++--
 t/t2072-restore-pathspec-file.sh  | 11 +++++++++--
 t/t3704-add-pathspec-file.sh      | 11 +++++++++--
 t/t7107-reset-pathspec-file.sh    | 12 +++++++++---
 t/t7526-commit-pathspec-file.sh   | 11 +++++++++--
 5 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..adad71f631 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git checkout --pathspec-from-file=list HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	M  fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
 '
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..b407f6b779 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git restore --pathspec-from-file=list --source=HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	 M fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
 '
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..61b6e51009 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git add --pathspec-from-file=list &&
 
 	cat >expect <<-\EOF &&
 	A  fileA.t
@@ -108,7 +112,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
 '
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..b0e84cdb42 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,12 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	git rm fileA.t &&
-	printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+	git reset --pathspec-from-file=list &&
 
 	cat >expect <<-\EOF &&
 	 D fileA.t
@@ -117,8 +121,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	git rm fileA.t &&
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	# Note: "git reset" has not yet learned to fail on wrong pathspecs
 	git reset --pathspec-from-file=list --pathspec-file-nul &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..4a7c11368d 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git commit --pathspec-from-file=list -m "Commit" &&
 
 	cat >expect <<-\EOF &&
 	A	fileA.t
@@ -111,7 +115,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
 '
 
-- 
gitgitgadget

[PATCH v3 2/3] t: directly test parse_pathspec_file()

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.

Introduce direct tests for `parse_pathspec_file()`.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 Makefile                            |   1 +
 t/helper/test-parse-pathspec-file.c |  33 +++++++++
 t/helper/test-tool.c                |   1 +
 t/helper/test-tool.h                |   1 +
 t/t0067-parse_pathspec_file.sh      | 108 ++++++++++++++++++++++++++++
 5 files changed, 144 insertions(+)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh

diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
 TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
 TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-pkt-line.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..02f4ccfd2a
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,33 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+	struct pathspec pathspec;
+	const char *pathspec_from_file = 0;
+	int pathspec_file_nul = 0, i;
+
+	static const char *const usage[] = {
+		"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+		NULL
+	};
+
+	struct option options[] = {
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+		OPT_END()
+	};
+
+	parse_options(argc, argv, 0, options, usage, 0);
+
+	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+			    pathspec_file_nul);
+
+	for (i = 0; i < pathspec.nr; i++)
+		printf("%s\n", pathspec.items[i].original);
+
+	clear_pathspec(&pathspec);
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
 	{ "oidmap", cmd__oidmap },
 	{ "online-cpus", cmd__online_cpus },
 	{ "parse-options", cmd__parse_options },
+	{ "parse-pathspec-file", cmd__parse_pathspec_file },
 	{ "path-utils", cmd__path_utils },
 	{ "pkt-line", cmd__pkt_line },
 	{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
 int cmd__oidmap(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__pkt_line(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..7bab49f361
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	echo fileA.t |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	echo fileA.t >list &&
+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\0fileB.t\0" |
+	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\nfileB.t\n" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\nfileB.t" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\r\nfileB.t\r\n" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+	# Note: there is an extra newline because --pathspec-file-nul takes
+	# input \n literally, too
+	cat >expect <<-\EOF &&
+	"file\101.t"
+
+	EOF
+
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	test-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&
+
+	test_cmp expect actual
+'
+
+test_done
-- 
gitgitgadget

[PATCH v3 3/3] t: drop copy&pasted tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).

I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 77 +---------------------------
 t/t2072-restore-pathspec-file.sh  | 77 +---------------------------
 t/t3704-add-pathspec-file.sh      | 77 +---------------------------
 t/t7107-reset-pathspec-file.sh    | 85 +++----------------------------
 t/t7526-commit-pathspec-file.sh   | 77 +---------------------------
 5 files changed, 16 insertions(+), 377 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index adad71f631..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git checkout --pathspec-from-file=list HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git checkout --pathspec-from-file=list HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index b407f6b779..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git restore --pathspec-from-file=list --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git restore --pathspec-from-file=list --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 61b6e51009..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git add --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,67 +46,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git add --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index b0e84cdb42..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	git rm fileA.t &&
-	echo fileA.t >list &&
-	git reset --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	git rm fileA.t fileB.t &&
@@ -63,77 +50,21 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+	git rm fileA.t fileB.t fileC.t fileD.t &&
+	printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
-	 D fileA.t
+	D  fileA.t
 	 D fileB.t
+	 D fileC.t
+	D  fileD.t
 	EOF
 	verify_expect
 '
 
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git rm fileA.t &&
-	git reset --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	# Note: "git reset" has not yet learned to fail on wrong pathspecs
-	git reset --pathspec-from-file=list --pathspec-file-nul &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	test_must_fail verify_expect
-'
-
 test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
 	restore_checkpoint &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4a7c11368d..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git commit --pathspec-from-file=list -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,67 +49,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git commit --pathspec-from-file=list -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-- 
gitgitgadget

[PATCH v4 0/3] t: rework tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

Please refer to commit messages for rationale.

This branch is a follow-up for [1] where part of branch was merged into `master` via [2].

Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.

Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.

[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/

Changes since V1
----------------
Small code formatting changes suggested in V1.

Changes since V2
----------------
Changed \\\\ escaping to use here-doc instead.

Changes since V3
----------------
Slightly improved commit message.

Alexandr Miloslavskiy (3):
  t: fix quotes tests for --pathspec-from-file
  t: directly test parse_pathspec_file()
  t: drop copy&pasted tests for --pathspec-from-file

 Makefile                            |   1 +
 t/helper/test-parse-pathspec-file.c |  33 +++++++++
 t/helper/test-tool.c                |   1 +
 t/helper/test-tool.h                |   1 +
 t/t0067-parse_pathspec_file.sh      | 108 ++++++++++++++++++++++++++++
 t/t2026-checkout-pathspec-file.sh   |  70 +-----------------
 t/t2072-restore-pathspec-file.sh    |  70 +-----------------
 t/t3704-add-pathspec-file.sh        |  70 +-----------------
 t/t7107-reset-pathspec-file.sh      |  79 +++-----------------
 t/t7526-commit-pathspec-file.sh     |  70 +-----------------
 10 files changed, 160 insertions(+), 343 deletions(-)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh


base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/503

Range-diff vs v3:

 1:  88790669ce = 1:  ce0c592bb4 t: fix quotes tests for --pathspec-from-file
 2:  68925c2712 = 2:  8748f3baf1 t: directly test parse_pathspec_file()
 3:  f71021b0dd ! 3:  d02a1eac0b t: drop copy&pasted tests for --pathspec-from-file
     @@ -3,9 +3,9 @@
          t: drop copy&pasted tests for --pathspec-from-file
      
          With direct tests for `parse_pathspec_file()` already in place, it is
     -    not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
     -    for every git command that uses it (I counted 13 commands that could use
     -    it eventually).
     +    not very reasonable to copy&paste 6 similar indirect tests for every git
     +    command that uses `parse_pathspec_file()`. I counted 13 potential git
     +    commands, which could eventually lead to 6*13=78 duplicate tests.
      
          I believe that indirect tests are redundant because I don't expect
          direct tests to ever disagree with indirect tests.

-- 
gitgitgadget

[PATCH v4 1/3] t: fix quotes tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.

Fix this by using here-doc instead.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 11 +++++++++--
 t/t2072-restore-pathspec-file.sh  | 11 +++++++++--
 t/t3704-add-pathspec-file.sh      | 11 +++++++++--
 t/t7107-reset-pathspec-file.sh    | 12 +++++++++---
 t/t7526-commit-pathspec-file.sh   | 11 +++++++++--
 5 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..adad71f631 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git checkout --pathspec-from-file=list HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	M  fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
 '
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..b407f6b779 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git restore --pathspec-from-file=list --source=HEAD^1 &&
 
 	cat >expect <<-\EOF &&
 	 M fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
 '
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..61b6e51009 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git add --pathspec-from-file=list &&
 
 	cat >expect <<-\EOF &&
 	A  fileA.t
@@ -108,7 +112,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
 '
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..b0e84cdb42 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,12 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	git rm fileA.t &&
-	printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+	git reset --pathspec-from-file=list &&
 
 	cat >expect <<-\EOF &&
 	 D fileA.t
@@ -117,8 +121,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	git rm fileA.t &&
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	# Note: "git reset" has not yet learned to fail on wrong pathspecs
 	git reset --pathspec-from-file=list --pathspec-file-nul &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..4a7c11368d 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,11 @@ test_expect_success 'CRLF delimiters' '
 test_expect_success 'quotes' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	git commit --pathspec-from-file=list -m "Commit" &&
 
 	cat >expect <<-\EOF &&
 	A	fileA.t
@@ -111,7 +115,10 @@ test_expect_success 'quotes' '
 test_expect_success 'quotes not compatible with --pathspec-file-nul' '
 	restore_checkpoint &&
 
-	printf "\"file\\101.t\"" >list &&
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
 	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
 '
 
-- 
gitgitgadget

[PATCH v4 2/3] t: directly test parse_pathspec_file()

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.

Introduce direct tests for `parse_pathspec_file()`.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 Makefile                            |   1 +
 t/helper/test-parse-pathspec-file.c |  33 +++++++++
 t/helper/test-tool.c                |   1 +
 t/helper/test-tool.h                |   1 +
 t/t0067-parse_pathspec_file.sh      | 108 ++++++++++++++++++++++++++++
 5 files changed, 144 insertions(+)
 create mode 100644 t/helper/test-parse-pathspec-file.c
 create mode 100755 t/t0067-parse_pathspec_file.sh

diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
 TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
 TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-pkt-line.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..02f4ccfd2a
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,33 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+	struct pathspec pathspec;
+	const char *pathspec_from_file = 0;
+	int pathspec_file_nul = 0, i;
+
+	static const char *const usage[] = {
+		"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+		NULL
+	};
+
+	struct option options[] = {
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+		OPT_END()
+	};
+
+	parse_options(argc, argv, 0, options, usage, 0);
+
+	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+			    pathspec_file_nul);
+
+	for (i = 0; i < pathspec.nr; i++)
+		printf("%s\n", pathspec.items[i].original);
+
+	clear_pathspec(&pathspec);
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
 	{ "oidmap", cmd__oidmap },
 	{ "online-cpus", cmd__online_cpus },
 	{ "parse-options", cmd__parse_options },
+	{ "parse-pathspec-file", cmd__parse_pathspec_file },
 	{ "path-utils", cmd__path_utils },
 	{ "pkt-line", cmd__pkt_line },
 	{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
 int cmd__oidmap(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__pkt_line(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..7bab49f361
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	echo fileA.t |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	echo fileA.t >list &&
+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\0fileB.t\0" |
+	test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\nfileB.t\n" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\nfileB.t" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	fileB.t
+	EOF
+
+	printf "fileA.t\r\nfileB.t\r\n" |
+	test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+	cat >expect <<-\EOF &&
+	fileA.t
+	EOF
+
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+	test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+	# Note: there is an extra newline because --pathspec-file-nul takes
+	# input \n literally, too
+	cat >expect <<-\EOF &&
+	"file\101.t"
+
+	EOF
+
+	cat >list <<-\EOF &&
+	"file\101.t"
+	EOF
+
+	test-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&
+
+	test_cmp expect actual
+'
+
+test_done
-- 
gitgitgadget

[PATCH v4 3/3] t: drop copy&pasted tests for --pathspec-from-file

[Alexandr Miloslavskiy via GitGitGadget]

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 similar indirect tests for every git
command that uses `parse_pathspec_file()`. I counted 13 potential git
commands, which could eventually lead to 6*13=78 duplicate tests.

I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 t/t2026-checkout-pathspec-file.sh | 77 +---------------------------
 t/t2072-restore-pathspec-file.sh  | 77 +---------------------------
 t/t3704-add-pathspec-file.sh      | 77 +---------------------------
 t/t7107-reset-pathspec-file.sh    | 85 +++----------------------------
 t/t7526-commit-pathspec-file.sh   | 77 +---------------------------
 5 files changed, 16 insertions(+), 377 deletions(-)

diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index adad71f631..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git checkout --pathspec-from-file=list HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	M  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git checkout --pathspec-from-file=list HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	M  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index b407f6b779..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git restore --pathspec-from-file=list --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	 M fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git restore --pathspec-from-file=list --source=HEAD^1 &&
-
-	cat >expect <<-\EOF &&
-	 M fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 61b6e51009..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git add --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,67 +46,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	A  fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git add --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	A  fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index b0e84cdb42..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	git rm fileA.t &&
-	echo fileA.t >list &&
-	git reset --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	git rm fileA.t fileB.t &&
@@ -63,77 +50,21 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	 D fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-	git rm fileA.t fileB.t &&
-	printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+	git rm fileA.t fileB.t fileC.t fileD.t &&
+	printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
 
 	cat >expect <<-\EOF &&
-	 D fileA.t
+	D  fileA.t
 	 D fileB.t
+	 D fileC.t
+	D  fileD.t
 	EOF
 	verify_expect
 '
 
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git rm fileA.t &&
-	git reset --pathspec-from-file=list &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	# Note: "git reset" has not yet learned to fail on wrong pathspecs
-	git reset --pathspec-from-file=list --pathspec-file-nul &&
-
-	cat >expect <<-\EOF &&
-	 D fileA.t
-	EOF
-	test_must_fail verify_expect
-'
-
 test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
 	restore_checkpoint &&
 
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4a7c11368d..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
 	test_cmp expect actual
 }
 
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
 	restore_checkpoint &&
 
 	echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
 	verify_expect
 '
 
-test_expect_success '--pathspec-from-file from file' '
-	restore_checkpoint &&
-
-	echo fileA.t >list &&
-	git commit --pathspec-from-file=list -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
 	restore_checkpoint &&
 
 	printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,67 +49,6 @@ test_expect_success 'NUL delimiters' '
 	verify_expect
 '
 
-test_expect_success 'LF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
-	restore_checkpoint &&
-
-	printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
-	restore_checkpoint &&
-
-	printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	A	fileB.t
-	EOF
-	verify_expect
-'
-
-test_expect_success 'quotes' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	git commit --pathspec-from-file=list -m "Commit" &&
-
-	cat >expect <<-\EOF &&
-	A	fileA.t
-	EOF
-	verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
-	restore_checkpoint &&
-
-	cat >list <<-\EOF &&
-	"file\101.t"
-	EOF
-
-	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
 test_expect_success 'only touches what was listed' '
 	restore_checkpoint &&
 
-- 
gitgitgadget

Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file

[Junio C Hamano]

"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
>
> Previously in [3] there were some concerns on whether removing
> copy&pasted tests is good. I still think that yes, it 's a good thing,
> mostly because of high volume of potential 13*6=78 duplicate tests.
>
> Still, I separated this change as last patch, so that the remaining
> part of the branch can be taken without it.

With the third step the series won't merge cleanly with other topic
you have in 'next' (t7107 gets somewhat heavy merge conflicts).

I'll queue the first two for now but let's clean them up post 2.25
release.

Thanks.

Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file

[Alexandr Miloslavskiy]

On 07.01.2020 22:13, Junio C Hamano wrote:
> With the third step the series won't merge cleanly with other topic
> you have in 'next' (t7107 gets somewhat heavy merge conflicts).
> 
> I'll queue the first two for now but let's clean them up post 2.25
> release.

OK, I will re-submit the remaining patch after 2.25.

I will implement the next --pathspec-from-file patches as if this third 
patch was accepted (that is, without copy&pasted tests).

Thanks for accepting this and other polishing branches, I was already 
quite pessimistic about them.

Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file

[Junio C Hamano]

Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:

> On 07.01.2020 22:13, Junio C Hamano wrote:
>> With the third step the series won't merge cleanly with other topic
>> you have in 'next' (t7107 gets somewhat heavy merge conflicts).
>>
>> I'll queue the first two for now but let's clean them up post 2.25
>> release.
>
> OK, I will re-submit the remaining patch after 2.25.
>
> I will implement the next --pathspec-from-file patches as if this
> third patch was accepted (that is, without copy&pasted tests).

I am not sure if that is a good idea.  I'd rather see the planned
new changes not to be taken hostage of the third step.

Besides, with the third step, your preference is not to test the
behaviour of end-user facing commands that would learn the option at
all and only test the underlying machinery with test-tool tests, no?
If you are not adding tests for the higher-level end-user facing
commands as part of these new series, would it make a difference if
the codebase has the third step applied (i.e. missing tests for the
end-user facing commands that have already learned the option) or
not (i.e. the commands that have already learned the option are
still tested end-to-end)?

Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file

[Alexandr Miloslavskiy]

On 08.01.2020 18:26, Junio C Hamano wrote:
>> I will implement the next --pathspec-from-file patches as if this
>> third patch was accepted (that is, without copy&pasted tests).
> 
> I am not sure if that is a good idea.  I'd rather see the planned
> new changes not to be taken hostage of the third step.

In my understanding, the new patches will not be taken hostage, they 
will simply adopt the new approach. Everything will work just fine 
whether or not third step is present.

> Besides, with the third step, your preference is not to test the
> behaviour of end-user facing commands that would learn the option at
> all and only test the underlying machinery with test-tool tests, no?

That's not exactly correct. Third step removes duplicate tests that give 
no real benefit. With test-tool tests in place and succceeding, these 
duplicate tests are super unlikely to fail.

I will still provide a few tests for every new command to make sure that 
said command works as intended. I will only skip indirectly testing 
global API again and again.

> If you are not adding tests for the higher-level end-user facing
> commands as part of these new series, would it make a difference if
> the codebase has the third step applied (i.e. missing tests for the
> end-user facing commands that have already learned the option) or
> not (i.e. the commands that have already learned the option are
> still tested end-to-end)?

I will be adding good tests and skip useless tests. For new commands, it 
doesn't really matter if "third step" patch is applied or not.

Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file

[Junio C Hamano]

Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:

> I will still provide a few tests for every new command to make sure
> that said command works as intended. I will only skip indirectly
> testing global API again and again.

Ah, OK.  Then leaving those removed by the third step there may get
in the way.  So let's assume that we'll have an updated third step
already applied and your new series are written on top of it.

> ... For new commands, it doesn't really matter if "third step"
> patch is applied or not.

OK, again.  Thanks for a clarification.