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

[PATCH v2 01/18] t7107, t7526: directly test parse_pathspec_file()

From
AGAlexandr Miloslavskiy via GitGitGadget <gitgitgadget@gmail.com>
Date
Dec 16, 2019, 15:47 UTC
Message-ID
<8d5fb9f40b8fc766ef022f910529e6308d9c2d80.1576511287.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.490.v2.git.1576511287.gitgitgadget@gmail.com>
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
In my previous patches, `parse_pathspec_file()` was tested indirectly by
invoking `git reset` and `git commit` with properly crafted inputs. This
has some disadvantages:
1) A number of tests are copy&pasted for every command where
   `--pathspec-from-file` is supported. With just two commands, it
   wasn't too bad, but I'm going to extend support to many more
   commands, which would make a handful of low-value tests.
2) Tests are located in suboptimal test packages
3) Tests are indirect

Fix this by testing `parse_pathspec_file()` directly via a new test helper.

While working on it, 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 `"` which prevented pathspec from matching files.

Fix this by properly escaping one more time.
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 +++++++++++++++++++++++++++
 t/t7107-reset-pathspec-file.sh      | 94 +++--------------------------
 t/t7526-commit-pathspec-file.sh     | 70 +--------------------
 7 files changed, 136 insertions(+), 154 deletions(-)
 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 b7d7374dac..fd7bcaac9c 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
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..f36fce27b9 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,71 +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 &&
-
-	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 &&
-
-	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 &&
 
@@ -137,19 +74,4 @@ test_expect_success '--pathspec-from-file is not compatible with --soft or --har
 	test_must_fail git reset --hard --pathspec-from-file=list
 '
 
-test_expect_success 'only touches what was listed' '
-	restore_checkpoint &&
-
-	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 fileB.t
-	 D fileC.t
-	D  fileD.t
-	EOF
-	verify_expect
-'
-
 test_done
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index a06b683534..4e592f7472 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,60 +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 &&
-
-	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 &&
-
-	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
Previous: Alexandr Miloslavskiy via GitGitGadgetNext: Junio C Hamano
Message 32 of 67 in “Extend --pathspec-from-file to git add, checkout”
  1. 00/16 Extend --pathspec-from-file to git add, checkoutAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  2. 02/16 commit: forbid --pathspec-from-file --allAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  3. Phillip WoodDec 16, 2019
  4. Alexandr MiloslavskiyDec 16, 2019
  5. 01/16 t7107, t7526: directly test parse_pathspec_file()Alexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  6. 06/16 doc: checkout: fix broken text referenceAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  7. 03/16 cmd_add: prepare for next patchAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  8. 11/16 parse_branchname_arg(): easier to understand variablesAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  9. 08/16 doc: restore: synchronize <pathspec> descriptionAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  10. 10/16 checkout: die() on ambiguous tracking branchesAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  11. 14/16 parse_branchname_arg(): refactor the decision makingAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  12. 15/16 t2024: cover more casesAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  13. 13/16 parse_branchname_arg(): update code commentsAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  14. 09/16 parse_branchname_arg(): extract part as new functionAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  15. 16/16 checkout, restore: support the --pathspec-from-file optionAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  16. 12/16 parse_branchname_arg(): introduce expect_commit_onlyAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  17. 05/16 doc: checkout: remove duplicate synopsisAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  18. 04/16 add: support the --pathspec-from-file optionAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  19. 07/16 doc: checkout: synchronize <pathspec> descriptionAlexandr Miloslavskiy via GitGitGadget, Dec 12, 2019
  20. 00/18 Extend --pathspec-from-file to git add, checkoutAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  21. 02/18 t7526: add tests for error conditionsAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  22. Junio C HamanoDec 18, 2019
  23. Alexandr MiloslavskiyDec 19, 2019
  24. 04/18 commit: forbid --pathspec-from-file --allAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  25. Phillip WoodDec 17, 2019
  26. Junio C HamanoDec 18, 2019
  27. Junio C HamanoDec 18, 2019
  28. Junio C HamanoDec 18, 2019
  29. Alexandr MiloslavskiyDec 19, 2019
  30. 05/18 cmd_add: prepare for next patchAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  31. 06/18 add: support the --pathspec-from-file optionAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  32. 01/18 t7107, t7526: directly test parse_pathspec_file()Alexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  33. Junio C HamanoDec 18, 2019
  34. Junio C HamanoDec 19, 2019
  35. Alexandr MiloslavskiyDec 19, 2019
  36. 09/18 doc: checkout: synchronize <pathspec> descriptionAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  37. 08/18 doc: checkout: fix broken text referenceAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  38. 13/18 parse_branchname_arg(): easier to understand variablesAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  39. 14/18 parse_branchname_arg(): introduce expect_commit_onlyAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  40. 17/18 t2024: cover more casesAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  41. 15/18 parse_branchname_arg(): update code commentsAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  42. 18/18 checkout, restore: support the --pathspec-from-file optionAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  43. 12/18 checkout: die() on ambiguous tracking branchesAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  44. 10/18 doc: restore: synchronize <pathspec> descriptionAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  45. 11/18 parse_branchname_arg(): extract part as new functionAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  46. 07/18 doc: checkout: remove duplicate synopsisAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  47. 16/18 parse_branchname_arg(): refactor the decision makingAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  48. 03/18 t7107: add tests for error conditionsAlexandr Miloslavskiy via GitGitGadget, Dec 16, 2019
  49. 00/18 Extend --pathspec-from-file to git add, checkoutAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  50. 02/18 t7526: add tests for error conditionsAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  51. 01/18 t7107, t7526: directly test parse_pathspec_file()Alexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  52. 03/18 t7107: add tests for error conditionsAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  53. 04/18 commit: forbid --pathspec-from-file --allAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  54. 05/18 cmd_add: prepare for next patchAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  55. 07/18 doc: checkout: remove duplicate synopsisAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  56. 08/18 doc: checkout: fix broken text referenceAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  57. 09/18 doc: checkout: synchronize <pathspec> descriptionAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  58. 06/18 add: support the --pathspec-from-file optionAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  59. 11/18 parse_branchname_arg(): extract part as new functionAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  60. 10/18 doc: restore: synchronize <pathspec> descriptionAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  61. 13/18 parse_branchname_arg(): easier to understand variablesAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  62. 17/18 t2024: cover more casesAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  63. 16/18 parse_branchname_arg(): refactor the decision makingAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  64. 14/18 parse_branchname_arg(): simplify argument eatingAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  65. 12/18 checkout: die() on ambiguous tracking branchesAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  66. 18/18 checkout, restore: support the --pathspec-from-file optionAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019
  67. 15/18 parse_branchname_arg(): update code commentsAlexandr Miloslavskiy via GitGitGadget, Dec 19, 2019

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

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