{"thread":{"id":"52540","subject":"[PATCH 0/3] t: rework tests for --pathspec-from-file","startedAt":"2019-12-30T17:42:31Z","lastAt":"2020-01-08T18:50:14Z","messageCount":26,"participants":["Alexandr Miloslavskiy via GitGitGadget","Junio C Hamano","Alexandr Miloslavskiy","Eric Sunshine","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"389078","messageId":"pull.503.git.1577727747.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":null,"subject":"[PATCH 0/3] t: rework tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T17:42:24Z","receivedAt":"2019-12-30T17:42:31Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Please refer to commit messages for rationale.\n\nThis branch is a follow-up for [1] where part of branch was merged into `master` via [2].\n\nPreviously in [3] there were some concerns on whether removing\ncopy&pasted tests is good. I still think that yes, it 's a good thing,\nmostly because of high volume of potential 13*6=78 duplicate tests.\n\nStill, I separated this change as last patch, so that the remaining\npart of the branch can be taken without it.\n\n[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/\n[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/\n[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/\n\nAlexandr Miloslavskiy (3):\n t: fix quotes tests for --pathspec-from-file\n t: directly test parse_pathspec_file()\n t: drop copy&pasted tests for --pathspec-from-file\n\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 34 +++++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++++++++\n t/t2026-checkout-pathspec-file.sh | 70 +----------------------\n t/t2072-restore-pathspec-file.sh | 70 +----------------------\n t/t3704-add-pathspec-file.sh | 70 +----------------------\n t/t7107-reset-pathspec-file.sh | 79 +++----------------------\n t/t7526-commit-pathspec-file.sh | 70 +----------------------\n 10 files changed, 142 insertions(+), 343 deletions(-)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/503\n-- \ngitgitgadget\n"},{"id":"389079","messageId":"ab9519298db009ab7f508c16b1c4ebe655b7cd80.1577727747.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.git.1577727747.gitgitgadget@gmail.com","subject":"[PATCH 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T17:42:25Z","receivedAt":"2019-12-30T17:42:32Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWhile working on the next patch, I also noticed that quotes testing via\n`\"\\\"file\\\\101.t\\\"\"` was somewhat incorrect: I escaped `\\` one time while\nI had to escape it two times! Tests still worked due to `\"` being\npreserved which in turn prevented pathspec from matching files.\n\nFix this by properly escaping one more time.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 9 +++++++--\n t/t2072-restore-pathspec-file.sh | 9 +++++++--\n t/t3704-add-pathspec-file.sh | 9 +++++++--\n t/t7107-reset-pathspec-file.sh | 9 +++++++--\n t/t7526-commit-pathspec-file.sh | 9 +++++++--\n 5 files changed, 35 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex f62fd27440..2dc8901bca 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \tM fileA.t\n@@ -120,7 +123,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n '\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex db58e83735..70e95ef3b6 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \t M fileA.t\n@@ -120,7 +123,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n '\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 3cfdb669b7..2e0141fcce 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n \tA fileA.t\n@@ -108,7 +111,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n '\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 6b1a731fff..52a44f033d 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n \tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n \t D fileA.t\n@@ -117,8 +120,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n \tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n \tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex 4b58901ed6..e7dc2ff8b1 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n \n \tcat >expect <<-\\EOF &&\n \tA\tfileA.t\n@@ -111,7 +114,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"389080","messageId":"27383a5b084b5e68152b08eb96fb4ddaf6d87f82.1577727747.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.git.1577727747.gitgitgadget@gmail.com","subject":"[PATCH 2/3] t: directly test parse_pathspec_file()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T17:42:26Z","receivedAt":"2019-12-30T17:42:34Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nPreviously, `parse_pathspec_file()` was tested indirectly by invoking\ngit commands with properly crafted inputs. As demonstrated by the\nprevious bugfix, testing complicated black boxes indirectly can lead to\ntests that silently test the wrong thing.\n\nIntroduce direct tests for `parse_pathspec_file()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 34 +++++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++++++++\n 5 files changed, 126 insertions(+)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\ndiff --git a/Makefile b/Makefile\nindex 09f98b777c..0061f96e8a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o\n TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\ndiff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c\nnew file mode 100644\nindex 0000000000..e7f525feb9\n--- /dev/null\n+++ b/t/helper/test-parse-pathspec-file.c\n@@ -0,0 +1,34 @@\n+#include \"test-tool.h\"\n+#include \"parse-options.h\"\n+#include \"pathspec.h\"\n+#include \"gettext.h\"\n+\n+int cmd__parse_pathspec_file(int argc, const char **argv)\n+{\n+\tstruct pathspec pathspec;\n+\tconst char *pathspec_from_file = 0;\n+\tint pathspec_file_nul = 0, i;\n+\n+\tstatic const char *const usage[] = {\n+\t\t\"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]\",\n+\t\tNULL\n+\t};\n+\n+\tstruct option options[] = {\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n+\t\tOPT_END()\n+\t};\n+\n+\tparse_options(argc, argv, 0, options, usage, 0);\n+\n+\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n+\t\t\t pathspec_file_nul);\n+\n+\tfor (i = 0; i < pathspec.nr; i++) {\n+\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n+\t}\n+\n+\tclear_pathspec(&pathspec);\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex f20989d449..c9a232d238 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {\n \t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n+\t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 8ed2af71d1..c8549fd87f 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);\n int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n+int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\ndiff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh\nnew file mode 100755\nindex 0000000000..df7b319713\n--- /dev/null\n+++ b/t/t0067-parse_pathspec_file.sh\n@@ -0,0 +1,89 @@\n+#!/bin/sh\n+\n+test_description='Test parse_pathspec_file()'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'one item from stdin' '\n+\techo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'one item from file' '\n+\techo fileA.t >list &&\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\tprintf \"fileA.t\\0fileB.t\\0\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\tprintf \"fileA.t\\nfileB.t\\n\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\tprintf \"fileA.t\\nfileB.t\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'quotes' '\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--pathspec-file-nul takes quotes literally' '\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"389081","messageId":"daef256db3d740c88f3c363409de606f15dc4841.1577727747.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.git.1577727747.gitgitgadget@gmail.com","subject":"[PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T17:42:27Z","receivedAt":"2019-12-30T17:42:36Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWith direct tests for `parse_pathspec_file()` already in place, it is\nnot very reasonable to copy&paste 6 tests for `parse_pathspec_file()`\nfor every git command that uses it (I counted 13 commands that could use\nit eventually).\n\nI believe that indirect tests are redundant because I don't expect\ndirect tests to ever disagree with indirect tests.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 75 +--------------------------\n t/t2072-restore-pathspec-file.sh | 75 +--------------------------\n t/t3704-add-pathspec-file.sh | 75 +--------------------------\n t/t7107-reset-pathspec-file.sh | 84 +++----------------------------\n t/t7526-commit-pathspec-file.sh | 75 +--------------------------\n 5 files changed, 16 insertions(+), 368 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex 2dc8901bca..559b4528d7 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&\n@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex 70e95ef3b6..9b3125d582 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&\n@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 2e0141fcce..9009f8a9ac 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -23,7 +23,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git add --pathspec-from-file=- &&\n@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit add --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git add --pathspec-from-file=- --pathspec-file-nul &&\n@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 52a44f033d..5b845f4f7c 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -25,7 +25,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t &&\n@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\techo fileA.t >list &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t fileB.t &&\n@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n+test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n-\t D fileA.t\n+\tD fileA.t\n \t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n \tEOF\n \tverify_expect\n '\n \n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n-\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\ttest_must_fail verify_expect\n-'\n-\n test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex e7dc2ff8b1..8d6c652690 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -26,7 +26,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-- \ngitgitgadget\n"},{"id":"389090","messageId":"xmqq8smthcib.fsf@gitster-ct.c.googlers.com","threadId":"52540","inReplyTo":"27383a5b084b5e68152b08eb96fb4ddaf6d87f82.1577727747.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] t: directly test parse_pathspec_file()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-30T18:52:28Z","receivedAt":"2019-12-30T18:52:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c\n> new file mode 100644\n> index 0000000000..e7f525feb9\n> --- /dev/null\n> +++ b/t/helper/test-parse-pathspec-file.c\n> @@ -0,0 +1,34 @@\n> +#include \"test-tool.h\"\n> +#include \"parse-options.h\"\n> +#include \"pathspec.h\"\n> +#include \"gettext.h\"\n> + ...\n> +\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n> +\t\t\t pathspec_file_nul);\n> +\n> +\tfor (i = 0; i < pathspec.nr; i++) {\n> +\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n> +\t}\n\nNo need for {} around a single statement block.\n\n> diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh\n> new file mode 100755\n> index 0000000000..df7b319713\n> --- /dev/null\n> +++ b/t/t0067-parse_pathspec_file.sh\n> @@ -0,0 +1,89 @@\n> +#!/bin/sh\n> +\n> +test_description='Test parse_pathspec_file()'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'one item from stdin' '\n> +\techo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\tfileA.t\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nThe use of the blank lines are somewhat inconsistent here.\n\n> + ...\n> +test_expect_success 'NUL delimiters' '\n> +\tprintf \"fileA.t\\0fileB.t\\0\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n\nFold line immediately after the pipe (same for the earlier and later ones).\n\n> +\tcat >expect <<-\\EOF &&\n> +\tfileA.t\n> +\tfileB.t\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nIf you want to have a gap between the steps, i.e. \"capturing the\nactual output\", \"creating the ideal output\", and \"seeing how they\ndiffer\", using blank like this is OK:\n\n\tprintf \"fileA.t\\0fileB.t\\0\" |\n\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n\n\tcat >expect <<-\\EOF &&\n\tfileA.t\n\tfileB.t\n\tEOF\n\n\ttest_cmp expect actual\n\nI thought we typically prepare the ideal output sample before\ncapturing the actual output, so if we follow that convention, the\nabove becomes\n\n\tcat >expect <<-\\EOF &&\n\tfileA.t\n\tfileB.t\n\tEOF\n\n\tprintf \"fileA.t\\0fileB.t\\0\" |\n\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n\n\ttest_cmp expect actual\n\n\n> +test_expect_success 'quotes' '\n> +\t# shell takes \\\\\\\\101 and spits \\\\101\n> +\t# printf takes \\\\101 and spits \\101\n> +\t# git takes \\101 and spits A\n> +\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\tfileA.t\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success '--pathspec-file-nul takes quotes literally' '\n> +\t# shell takes \\\\\\\\101 and spits \\\\101\n> +\t# printf takes \\\\101 and spits \\101\n> +\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\t\"file\\101.t\"\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nTesting low level machinery like this is of course a good idea, in\naddition to the end-to-end tests that make sure that the machinery\nis called correctly from the higher layer.\n\nThanks.\n"},{"id":"389091","messageId":"pull.503.v2.git.1577733329.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.git.1577727747.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] t: rework tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T19:15:25Z","receivedAt":"2019-12-30T19:15:34Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Please refer to commit messages for rationale.\n\nThis branch is a follow-up for [1] where part of branch was merged into `master` via [2].\n\nPreviously in [3] there were some concerns on whether removing\ncopy&pasted tests is good. I still think that yes, it 's a good thing,\nmostly because of high volume of potential 13*6=78 duplicate tests.\n\nStill, I separated this change as last patch, so that the remaining\npart of the branch can be taken without it.\n\n[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/\n[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/\n[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/\n\nChanges since V1\n----------------\nSmall code formatting changes suggested in V1.\n\nAlexandr Miloslavskiy (3):\n t: fix quotes tests for --pathspec-from-file\n t: directly test parse_pathspec_file()\n t: drop copy&pasted tests for --pathspec-from-file\n\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 33 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 104 ++++++++++++++++++++++++++++\n t/t2026-checkout-pathspec-file.sh | 70 +------------------\n t/t2072-restore-pathspec-file.sh | 70 +------------------\n t/t3704-add-pathspec-file.sh | 70 +------------------\n t/t7107-reset-pathspec-file.sh | 79 +++------------------\n t/t7526-commit-pathspec-file.sh | 70 +------------------\n 10 files changed, 156 insertions(+), 343 deletions(-)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/503\n\nRange-diff vs v1:\n\n 1: ab9519298d = 1: 6193dc7396 t: fix quotes tests for --pathspec-from-file\n 2: 27383a5b08 ! 2: ab449ac15a t: directly test parse_pathspec_file()\n @@ -55,9 +55,8 @@\n +\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n +\t\t\t pathspec_file_nul);\n +\n -+\tfor (i = 0; i < pathspec.nr; i++) {\n ++\tfor (i = 0; i < pathspec.nr; i++)\n +\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n -+\t}\n +\n +\tclear_pathspec(&pathspec);\n +\treturn 0;\n @@ -99,84 +98,99 @@\n +. ./test-lib.sh\n +\n +test_expect_success 'one item from stdin' '\n -+\techo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\tfileA.t\n +\tEOF\n ++\n ++\techo fileA.t |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'one item from file' '\n -+\techo fileA.t >list &&\n -+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\tfileA.t\n +\tEOF\n ++\n ++\techo fileA.t >list &&\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'NUL delimiters' '\n -+\tprintf \"fileA.t\\0fileB.t\\0\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\tfileA.t\n +\tfileB.t\n +\tEOF\n ++\n ++\tprintf \"fileA.t\\0fileB.t\\0\" |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'LF delimiters' '\n -+\tprintf \"fileA.t\\nfileB.t\\n\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\tfileA.t\n +\tfileB.t\n +\tEOF\n ++\n ++\tprintf \"fileA.t\\nfileB.t\\n\" |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'no trailing delimiter' '\n -+\tprintf \"fileA.t\\nfileB.t\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\tfileA.t\n +\tfileB.t\n +\tEOF\n ++\n ++\tprintf \"fileA.t\\nfileB.t\" |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'CRLF delimiters' '\n -+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\tfileA.t\n +\tfileB.t\n +\tEOF\n ++\n ++\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'quotes' '\n ++\tcat >expect <<-\\EOF &&\n ++\tfileA.t\n ++\tEOF\n ++\n +\t# shell takes \\\\\\\\101 and spits \\\\101\n +\t# printf takes \\\\101 and spits \\101\n +\t# git takes \\101 and spits A\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n ++\tprintf \"\\\"file\\\\\\\\101.t\\\"\" |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n +\n -+\tcat >expect <<-\\EOF &&\n -+\tfileA.t\n -+\tEOF\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success '--pathspec-file-nul takes quotes literally' '\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n -+\n +\tcat >expect <<-\\EOF &&\n +\t\"file\\101.t\"\n +\tEOF\n ++\n ++\t# shell takes \\\\\\\\101 and spits \\\\101\n ++\t# printf takes \\\\101 and spits \\101\n ++\tprintf \"\\\"file\\\\\\\\101.t\\\"\" |\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n ++\n +\ttest_cmp expect actual\n +'\n +\n 3: daef256db3 = 3: 88086cebce t: drop copy&pasted tests for --pathspec-from-file\n\n-- \ngitgitgadget\n"},{"id":"389092","messageId":"ab449ac15a4fe792253430c7731a0707de51edeb.1577733329.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v2.git.1577733329.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] t: directly test parse_pathspec_file()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T19:15:27Z","receivedAt":"2019-12-30T19:15:36Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nPreviously, `parse_pathspec_file()` was tested indirectly by invoking\ngit commands with properly crafted inputs. As demonstrated by the\nprevious bugfix, testing complicated black boxes indirectly can lead to\ntests that silently test the wrong thing.\n\nIntroduce direct tests for `parse_pathspec_file()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 33 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 104 ++++++++++++++++++++++++++++\n 5 files changed, 140 insertions(+)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\ndiff --git a/Makefile b/Makefile\nindex 09f98b777c..0061f96e8a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o\n TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\ndiff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c\nnew file mode 100644\nindex 0000000000..02f4ccfd2a\n--- /dev/null\n+++ b/t/helper/test-parse-pathspec-file.c\n@@ -0,0 +1,33 @@\n+#include \"test-tool.h\"\n+#include \"parse-options.h\"\n+#include \"pathspec.h\"\n+#include \"gettext.h\"\n+\n+int cmd__parse_pathspec_file(int argc, const char **argv)\n+{\n+\tstruct pathspec pathspec;\n+\tconst char *pathspec_from_file = 0;\n+\tint pathspec_file_nul = 0, i;\n+\n+\tstatic const char *const usage[] = {\n+\t\t\"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]\",\n+\t\tNULL\n+\t};\n+\n+\tstruct option options[] = {\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n+\t\tOPT_END()\n+\t};\n+\n+\tparse_options(argc, argv, 0, options, usage, 0);\n+\n+\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n+\t\t\t pathspec_file_nul);\n+\n+\tfor (i = 0; i < pathspec.nr; i++)\n+\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n+\n+\tclear_pathspec(&pathspec);\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex f20989d449..c9a232d238 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {\n \t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n+\t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 8ed2af71d1..c8549fd87f 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);\n int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n+int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\ndiff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh\nnew file mode 100755\nindex 0000000000..77b44f6702\n--- /dev/null\n+++ b/t/t0067-parse_pathspec_file.sh\n@@ -0,0 +1,104 @@\n+#!/bin/sh\n+\n+test_description='Test parse_pathspec_file()'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'one item from stdin' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\techo fileA.t |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'one item from file' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\techo fileA.t >list &&\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\0fileB.t\\0\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\nfileB.t\\n\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\nfileB.t\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'quotes' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--pathspec-file-nul takes quotes literally' '\n+\tcat >expect <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"389093","messageId":"6193dc7396b9cc6cb78f382c1b1679d6bb455fe4.1577733329.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v2.git.1577733329.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T19:15:26Z","receivedAt":"2019-12-30T19:15:37Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWhile working on the next patch, I also noticed that quotes testing via\n`\"\\\"file\\\\101.t\\\"\"` was somewhat incorrect: I escaped `\\` one time while\nI had to escape it two times! Tests still worked due to `\"` being\npreserved which in turn prevented pathspec from matching files.\n\nFix this by properly escaping one more time.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 9 +++++++--\n t/t2072-restore-pathspec-file.sh | 9 +++++++--\n t/t3704-add-pathspec-file.sh | 9 +++++++--\n t/t7107-reset-pathspec-file.sh | 9 +++++++--\n t/t7526-commit-pathspec-file.sh | 9 +++++++--\n 5 files changed, 35 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex f62fd27440..2dc8901bca 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \tM fileA.t\n@@ -120,7 +123,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n '\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex db58e83735..70e95ef3b6 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \t M fileA.t\n@@ -120,7 +123,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n '\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 3cfdb669b7..2e0141fcce 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n \tA fileA.t\n@@ -108,7 +111,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n '\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 6b1a731fff..52a44f033d 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n \tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n \t D fileA.t\n@@ -117,8 +120,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n \tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n \tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex 4b58901ed6..e7dc2ff8b1 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n \n \tcat >expect <<-\\EOF &&\n \tA\tfileA.t\n@@ -111,7 +114,9 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n \ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"389094","messageId":"88086cebce73bcbc3eaf437bd86af5545d40e73b.1577733329.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v2.git.1577733329.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] t: drop copy&pasted tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-30T19:15:28Z","receivedAt":"2019-12-30T19:15:39Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWith direct tests for `parse_pathspec_file()` already in place, it is\nnot very reasonable to copy&paste 6 tests for `parse_pathspec_file()`\nfor every git command that uses it (I counted 13 commands that could use\nit eventually).\n\nI believe that indirect tests are redundant because I don't expect\ndirect tests to ever disagree with indirect tests.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 75 +--------------------------\n t/t2072-restore-pathspec-file.sh | 75 +--------------------------\n t/t3704-add-pathspec-file.sh | 75 +--------------------------\n t/t7107-reset-pathspec-file.sh | 84 +++----------------------------\n t/t7526-commit-pathspec-file.sh | 75 +--------------------------\n 5 files changed, 16 insertions(+), 368 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex 2dc8901bca..559b4528d7 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&\n@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex 70e95ef3b6..9b3125d582 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&\n@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 2e0141fcce..9009f8a9ac 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -23,7 +23,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git add --pathspec-from-file=- &&\n@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit add --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git add --pathspec-from-file=- --pathspec-file-nul &&\n@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 52a44f033d..5b845f4f7c 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -25,7 +25,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t &&\n@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\techo fileA.t >list &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t fileB.t &&\n@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n+test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n-\t D fileA.t\n+\tD fileA.t\n \t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n \tEOF\n \tverify_expect\n '\n \n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n-\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\ttest_must_fail verify_expect\n-'\n-\n test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex e7dc2ff8b1..8d6c652690 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -26,7 +26,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\t# git takes \\101 and spits A\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\t# shell takes \\\\\\\\101 and spits \\\\101\n-\t# printf takes \\\\101 and spits \\101\n-\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-- \ngitgitgadget\n"},{"id":"389095","messageId":"97224460-c432-960a-0324-ea4f64f7c082@syntevo.com","threadId":"52540","inReplyTo":"xmqq8smthcib.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] t: directly test parse_pathspec_file()","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-12-30T19:16:03Z","receivedAt":"2019-12-30T19:16:08Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Thanks for having a look! I think I have fixed all mentioned issues in V2.\n"},{"id":"389108","messageId":"CAPig+cSSqAxuHYg9DxuJzC7m2HAt8F2YPNxT0x5+SksCGic4MA@mail.gmail.com","threadId":"52540","inReplyTo":"6193dc7396b9cc6cb78f382c1b1679d6bb455fe4.1577733329.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-12-30T21:55:58Z","receivedAt":"2019-12-30T21:56:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Dec 30, 2019 at 2:15 PM Alexandr Miloslavskiy via GitGitGadget\n wrote:\n> While working on the next patch, I also noticed that quotes testing via\n> `\"\\\"file\\\\101.t\\\"\"` was somewhat incorrect: I escaped `\\` one time while\n> I had to escape it two times! Tests still worked due to `\"` being\n> preserved which in turn prevented pathspec from matching files.\n>\n> Fix this by properly escaping one more time.\n>\n> Signed-off-by: Alexandr Miloslavskiy \n> ---\n> diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\n> @@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '\n> - printf \"\\\"file\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n> + # shell takes \\\\\\\\101 and spits \\\\101\n> + # printf takes \\\\101 and spits \\101\n> + # git takes \\101 and spits A\n> + printf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n\nSo, you want git-checkout to receive the following, quotes, backslash,\nand no newline, on its standard input?\n\n \"file\\101.t\"\n\nIf so, another way to achieve the same without taxing the brain of the\nreader or the next person who works on this code would be:\n\n tr -d \"\\012\" | git checkout --pathspec-from-file=- HEAD^1 <<-\\EOF &&\n \"file\\101.t\"\n EOF\n\nAlthough it's three lines long, the body of the here-doc is the\nliteral text you want sent to the Git command, so no counting\nbackslashes, and no need for a lengthy in-code comment.\n\nBut is the \"no newline\" bit indeed intentional? If not, then a simple\necho would be even easier (though with a bit more escaping):\n\n echo \"\\\"file\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n"},{"id":"389121","messageId":"20191231002607.GC13606@google.com","threadId":"52540","inReplyTo":"CAPig+cSSqAxuHYg9DxuJzC7m2HAt8F2YPNxT0x5+SksCGic4MA@mail.gmail.com","subject":"Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-12-31T00:26:07Z","receivedAt":"2019-12-31T00:26:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Sunshine wrote:\n\n> But is the \"no newline\" bit indeed intentional? If not, then a simple\n> echo would be even easier (though with a bit more escaping):\n>\n> echo \"\\\"file\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n\nFor portability, that would be\n\n\tprintf \"%s\\n\" \"\\\"file\\101.t\\\"\" | ...\n\nbecause some implementations of echo interpret escapes by default.\n\nThanks,\nJonathan\n"},{"id":"389124","messageId":"pull.503.v3.git.1577786032.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v2.git.1577733329.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] t: rework tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T09:53:49Z","receivedAt":"2019-12-31T09:53:58Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Please refer to commit messages for rationale.\n\nThis branch is a follow-up for [1] where part of branch was merged into `master` via [2].\n\nPreviously in [3] there were some concerns on whether removing\ncopy&pasted tests is good. I still think that yes, it 's a good thing,\nmostly because of high volume of potential 13*6=78 duplicate tests.\n\nStill, I separated this change as last patch, so that the remaining\npart of the branch can be taken without it.\n\n[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/\n[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/\n[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/\n\nChanges since V1\n----------------\nSmall code formatting changes suggested in V1.\n\nAlexandr Miloslavskiy (3):\n t: fix quotes tests for --pathspec-from-file\n t: directly test parse_pathspec_file()\n t: drop copy&pasted tests for --pathspec-from-file\n\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 33 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++\n t/t2026-checkout-pathspec-file.sh | 70 +-----------------\n t/t2072-restore-pathspec-file.sh | 70 +-----------------\n t/t3704-add-pathspec-file.sh | 70 +-----------------\n t/t7107-reset-pathspec-file.sh | 79 +++-----------------\n t/t7526-commit-pathspec-file.sh | 70 +-----------------\n 10 files changed, 160 insertions(+), 343 deletions(-)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/503\n\nRange-diff vs v2:\n\n 1: 6193dc7396 ! 1: 88790669ce t: fix quotes tests for --pathspec-from-file\n @@ -7,7 +7,7 @@\n I had to escape it two times! Tests still worked due to `\"` being\n preserved which in turn prevented pathspec from matching files.\n \n - Fix this by properly escaping one more time.\n + Fix this by using here-doc instead.\n \n Signed-off-by: Alexandr Miloslavskiy \n \n @@ -19,10 +19,11 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\t# git takes \\101 and spits A\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n ++\tgit checkout --pathspec-from-file=list HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \tM fileA.t\n @@ -31,9 +32,10 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n \ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n '\n \n @@ -46,10 +48,11 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\t# git takes \\101 and spits A\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n ++\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \t M fileA.t\n @@ -58,9 +61,10 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n \ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n '\n \n @@ -73,10 +77,11 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\t# git takes \\101 and spits A\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n ++\tgit add --pathspec-from-file=list &&\n \n \tcat >expect <<-\\EOF &&\n \tA fileA.t\n @@ -85,9 +90,10 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n \ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n '\n \n @@ -99,12 +105,13 @@\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\t# git takes \\101 and spits A\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n \tgit rm fileA.t &&\n -\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n ++\tgit reset --pathspec-from-file=list &&\n \n \tcat >expect <<-\\EOF &&\n \t D fileA.t\n @@ -112,11 +119,12 @@\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n - \tgit rm fileA.t &&\n +-\tgit rm fileA.t &&\n -\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n \t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n \tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n \n @@ -129,10 +137,11 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\t# git takes \\101 and spits A\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n ++\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n \n \tcat >expect <<-\\EOF &&\n \tA\tfileA.t\n @@ -141,9 +150,10 @@\n \trestore_checkpoint &&\n \n -\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n \ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n '\n \n 2: ab449ac15a ! 2: 68925c2712 t: directly test parse_pathspec_file()\n @@ -172,24 +172,28 @@\n +\tfileA.t\n +\tEOF\n +\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\t# git takes \\101 and spits A\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" |\n -+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n +\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success '--pathspec-file-nul takes quotes literally' '\n ++\t# Note: there is an extra newline because --pathspec-file-nul takes\n ++\t# input \\n literally, too\n +\tcat >expect <<-\\EOF &&\n +\t\"file\\101.t\"\n ++\n +\tEOF\n +\n -+\t# shell takes \\\\\\\\101 and spits \\\\101\n -+\t# printf takes \\\\101 and spits \\101\n -+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" |\n -+\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n ++\tcat >list <<-\\EOF &&\n ++\t\"file\\101.t\"\n ++\tEOF\n ++\n ++\ttest-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&\n +\n +\ttest_cmp expect actual\n +'\n 3: 88086cebce ! 3: f71021b0dd t: drop copy&pasted tests for --pathspec-from-file\n @@ -88,10 +88,11 @@\n -test_expect_success 'quotes' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\t# git takes \\101 and spits A\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n +-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n -\n -\tcat >expect <<-\\EOF &&\n -\tM fileA.t\n @@ -102,9 +103,10 @@\n -test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n -\ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n -'\n -\n @@ -188,10 +190,11 @@\n -test_expect_success 'quotes' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\t# git takes \\101 and spits A\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n +-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n -\n -\tcat >expect <<-\\EOF &&\n -\t M fileA.t\n @@ -202,9 +205,10 @@\n -test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n -\ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n -'\n -\n @@ -288,10 +292,11 @@\n -test_expect_success 'quotes' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\t# git takes \\101 and spits A\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n +-\tgit add --pathspec-from-file=list &&\n -\n -\tcat >expect <<-\\EOF &&\n -\tA fileA.t\n @@ -302,9 +307,10 @@\n -test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n -\ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n -'\n -\n @@ -398,11 +404,12 @@\n -test_expect_success 'quotes' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\t# git takes \\101 and spits A\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n -\tgit rm fileA.t &&\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n +-\tgit reset --pathspec-from-file=list &&\n -\n -\tcat >expect <<-\\EOF &&\n -\t D fileA.t\n @@ -413,10 +420,10 @@\n -test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\tgit rm fileA.t &&\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n -\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n -\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n -\n @@ -506,10 +513,11 @@\n -test_expect_success 'quotes' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\t# git takes \\101 and spits A\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n +-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n -\n -\tcat >expect <<-\\EOF &&\n -\tA\tfileA.t\n @@ -520,9 +528,10 @@\n -test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n -\trestore_checkpoint &&\n -\n --\t# shell takes \\\\\\\\101 and spits \\\\101\n --\t# printf takes \\\\101 and spits \\101\n --\tprintf \"\\\"file\\\\\\\\101.t\\\"\" >list &&\n +-\tcat >list <<-\\EOF &&\n +-\t\"file\\101.t\"\n +-\tEOF\n +-\n -\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n -'\n -\n\n-- \ngitgitgadget\n"},{"id":"389125","messageId":"68925c2712ab103af088417a6a80c782a90f3d51.1577786032.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v3.git.1577786032.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] t: directly test parse_pathspec_file()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T09:53:51Z","receivedAt":"2019-12-31T09:53:58Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nPreviously, `parse_pathspec_file()` was tested indirectly by invoking\ngit commands with properly crafted inputs. As demonstrated by the\nprevious bugfix, testing complicated black boxes indirectly can lead to\ntests that silently test the wrong thing.\n\nIntroduce direct tests for `parse_pathspec_file()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 33 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++\n 5 files changed, 144 insertions(+)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\ndiff --git a/Makefile b/Makefile\nindex 09f98b777c..0061f96e8a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o\n TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\ndiff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c\nnew file mode 100644\nindex 0000000000..02f4ccfd2a\n--- /dev/null\n+++ b/t/helper/test-parse-pathspec-file.c\n@@ -0,0 +1,33 @@\n+#include \"test-tool.h\"\n+#include \"parse-options.h\"\n+#include \"pathspec.h\"\n+#include \"gettext.h\"\n+\n+int cmd__parse_pathspec_file(int argc, const char **argv)\n+{\n+\tstruct pathspec pathspec;\n+\tconst char *pathspec_from_file = 0;\n+\tint pathspec_file_nul = 0, i;\n+\n+\tstatic const char *const usage[] = {\n+\t\t\"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]\",\n+\t\tNULL\n+\t};\n+\n+\tstruct option options[] = {\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n+\t\tOPT_END()\n+\t};\n+\n+\tparse_options(argc, argv, 0, options, usage, 0);\n+\n+\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n+\t\t\t pathspec_file_nul);\n+\n+\tfor (i = 0; i < pathspec.nr; i++)\n+\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n+\n+\tclear_pathspec(&pathspec);\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex f20989d449..c9a232d238 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {\n \t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n+\t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 8ed2af71d1..c8549fd87f 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);\n int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n+int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\ndiff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh\nnew file mode 100755\nindex 0000000000..7bab49f361\n--- /dev/null\n+++ b/t/t0067-parse_pathspec_file.sh\n@@ -0,0 +1,108 @@\n+#!/bin/sh\n+\n+test_description='Test parse_pathspec_file()'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'one item from stdin' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\techo fileA.t |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'one item from file' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\techo fileA.t >list &&\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\0fileB.t\\0\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\nfileB.t\\n\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\nfileB.t\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'quotes' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--pathspec-file-nul takes quotes literally' '\n+\t# Note: there is an extra newline because --pathspec-file-nul takes\n+\t# input \\n literally, too\n+\tcat >expect <<-\\EOF &&\n+\t\"file\\101.t\"\n+\n+\tEOF\n+\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"389126","messageId":"88790669ce44d591bad6b6f8c2985e6c8009f74e.1577786032.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v3.git.1577786032.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T09:53:50Z","receivedAt":"2019-12-31T09:53:59Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWhile working on the next patch, I also noticed that quotes testing via\n`\"\\\"file\\\\101.t\\\"\"` was somewhat incorrect: I escaped `\\` one time while\nI had to escape it two times! Tests still worked due to `\"` being\npreserved which in turn prevented pathspec from matching files.\n\nFix this by using here-doc instead.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 11 +++++++++--\n t/t2072-restore-pathspec-file.sh | 11 +++++++++--\n t/t3704-add-pathspec-file.sh | 11 +++++++++--\n t/t7107-reset-pathspec-file.sh | 12 +++++++++---\n t/t7526-commit-pathspec-file.sh | 11 +++++++++--\n 5 files changed, 45 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex f62fd27440..adad71f631 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit checkout --pathspec-from-file=list HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \tM fileA.t\n@@ -120,7 +124,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n '\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex db58e83735..b407f6b779 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \t M fileA.t\n@@ -120,7 +124,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n '\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 3cfdb669b7..61b6e51009 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -97,7 +97,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit add --pathspec-from-file=list &&\n \n \tcat >expect <<-\\EOF &&\n \tA fileA.t\n@@ -108,7 +112,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n '\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 6b1a731fff..b0e84cdb42 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -105,8 +105,12 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\tgit reset --pathspec-from-file=list &&\n \n \tcat >expect <<-\\EOF &&\n \t D fileA.t\n@@ -117,8 +121,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n \tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex 4b58901ed6..4a7c11368d 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -100,7 +100,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n \n \tcat >expect <<-\\EOF &&\n \tA\tfileA.t\n@@ -111,7 +115,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"389127","messageId":"f71021b0dd65593eac2932991e437d739037bedb.1577786032.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v3.git.1577786032.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] t: drop copy&pasted tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T09:53:52Z","receivedAt":"2019-12-31T09:54:01Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWith direct tests for `parse_pathspec_file()` already in place, it is\nnot very reasonable to copy&paste 6 tests for `parse_pathspec_file()`\nfor every git command that uses it (I counted 13 commands that could use\nit eventually).\n\nI believe that indirect tests are redundant because I don't expect\ndirect tests to ever disagree with indirect tests.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 77 +---------------------------\n t/t2072-restore-pathspec-file.sh | 77 +---------------------------\n t/t3704-add-pathspec-file.sh | 77 +---------------------------\n t/t7107-reset-pathspec-file.sh | 85 +++----------------------------\n t/t7526-commit-pathspec-file.sh | 77 +---------------------------\n 5 files changed, 16 insertions(+), 377 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex adad71f631..559b4528d7 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&\n@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex b407f6b779..9b3125d582 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&\n@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 61b6e51009..9009f8a9ac 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -23,7 +23,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git add --pathspec-from-file=- &&\n@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit add --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git add --pathspec-from-file=- --pathspec-file-nul &&\n@@ -58,67 +46,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit add --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex b0e84cdb42..5b845f4f7c 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -25,7 +25,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t &&\n@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\techo fileA.t >list &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t fileB.t &&\n@@ -63,77 +50,21 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n+test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n-\t D fileA.t\n+\tD fileA.t\n \t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n \tEOF\n \tverify_expect\n '\n \n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit rm fileA.t &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n-\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\ttest_must_fail verify_expect\n-'\n-\n test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex 4a7c11368d..8d6c652690 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -26,7 +26,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n@@ -61,67 +49,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-- \ngitgitgadget\n"},{"id":"389128","messageId":"17de8859-10c2-b3e0-df5d-bf6d2681a528@syntevo.com","threadId":"52540","inReplyTo":"20191231002607.GC13606@google.com","subject":"Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-12-31T10:01:47Z","receivedAt":"2019-12-31T10:01:54Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Luckily, trailing \\n didn't matter much, and I could also send input via \ncommandline argument instead of stdin, so here-doc is really the most \nreadable solution here.\n\nFixed it in V3, thanks for your suggestion!\n\nOn 30.12.2019 22:55, Eric Sunshine wrote:\n > So, you want git-checkout to receive the following, quotes, backslash,\n > and no newline, on its standard input?\n >\n > \"file\\101.t\"\n >\n > If so, another way to achieve the same without taxing the brain of the\n > reader or the next person who works on this code would be:\n >\n > tr -d \"\\012\" | git checkout --pathspec-from-file=- HEAD^1 <<-\\EOF &&\n > \"file\\101.t\"\n > EOF\n >\n > Although it's three lines long, the body of the here-doc is the\n > literal text you want sent to the Git command, so no counting\n > backslashes, and no need for a lengthy in-code comment.\n >\n > But is the \"no newline\" bit indeed intentional? If not, then a simple\n > echo would be even easier (though with a bit more escaping):\n >\n > echo \"\\\"file\\101.t\\\"\" | git checkout --pathspec-from-file=- \nHEAD^1 &&\n >\n\nOn 31.12.2019 1:26, Jonathan Nieder wrote:\n>> But is the \"no newline\" bit indeed intentional? If not, then a simple\n>> echo would be even easier (though with a bit more escaping):\n>>\n>> echo \"\\\"file\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n> \n> For portability, that would be\n> \n> \tprintf \"%s\\n\" \"\\\"file\\101.t\\\"\" | ...\n> \n> because some implementations of echo interpret escapes by default.\n"},{"id":"389129","messageId":"pull.503.v4.git.1577787313.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v3.git.1577786032.gitgitgadget@gmail.com","subject":"[PATCH v4 0/3] t: rework tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T10:15:10Z","receivedAt":"2019-12-31T10:15:18Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Please refer to commit messages for rationale.\n\nThis branch is a follow-up for [1] where part of branch was merged into `master` via [2].\n\nPreviously in [3] there were some concerns on whether removing\ncopy&pasted tests is good. I still think that yes, it 's a good thing,\nmostly because of high volume of potential 13*6=78 duplicate tests.\n\nStill, I separated this change as last patch, so that the remaining\npart of the branch can be taken without it.\n\n[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/\n[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/\n[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/\n\nChanges since V1\n----------------\nSmall code formatting changes suggested in V1.\n\nChanges since V2\n----------------\nChanged \\\\\\\\ escaping to use here-doc instead.\n\nChanges since V3\n----------------\nSlightly improved commit message.\n\nAlexandr Miloslavskiy (3):\n t: fix quotes tests for --pathspec-from-file\n t: directly test parse_pathspec_file()\n t: drop copy&pasted tests for --pathspec-from-file\n\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 33 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++\n t/t2026-checkout-pathspec-file.sh | 70 +-----------------\n t/t2072-restore-pathspec-file.sh | 70 +-----------------\n t/t3704-add-pathspec-file.sh | 70 +-----------------\n t/t7107-reset-pathspec-file.sh | 79 +++-----------------\n t/t7526-commit-pathspec-file.sh | 70 +-----------------\n 10 files changed, 160 insertions(+), 343 deletions(-)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\n\nbase-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/503\n\nRange-diff vs v3:\n\n 1: 88790669ce = 1: ce0c592bb4 t: fix quotes tests for --pathspec-from-file\n 2: 68925c2712 = 2: 8748f3baf1 t: directly test parse_pathspec_file()\n 3: f71021b0dd ! 3: d02a1eac0b t: drop copy&pasted tests for --pathspec-from-file\n @@ -3,9 +3,9 @@\n t: drop copy&pasted tests for --pathspec-from-file\n \n With direct tests for `parse_pathspec_file()` already in place, it is\n - not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`\n - for every git command that uses it (I counted 13 commands that could use\n - it eventually).\n + not very reasonable to copy&paste 6 similar indirect tests for every git\n + command that uses `parse_pathspec_file()`. I counted 13 potential git\n + commands, which could eventually lead to 6*13=78 duplicate tests.\n \n I believe that indirect tests are redundant because I don't expect\n direct tests to ever disagree with indirect tests.\n\n-- \ngitgitgadget\n"},{"id":"389130","messageId":"ce0c592bb436112727d3636ce9a77494cd0ba8e3.1577787313.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v4.git.1577787313.gitgitgadget@gmail.com","subject":"[PATCH v4 1/3] t: fix quotes tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T10:15:11Z","receivedAt":"2019-12-31T10:15:19Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWhile working on the next patch, I also noticed that quotes testing via\n`\"\\\"file\\\\101.t\\\"\"` was somewhat incorrect: I escaped `\\` one time while\nI had to escape it two times! Tests still worked due to `\"` being\npreserved which in turn prevented pathspec from matching files.\n\nFix this by using here-doc instead.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 11 +++++++++--\n t/t2072-restore-pathspec-file.sh | 11 +++++++++--\n t/t3704-add-pathspec-file.sh | 11 +++++++++--\n t/t7107-reset-pathspec-file.sh | 12 +++++++++---\n t/t7526-commit-pathspec-file.sh | 11 +++++++++--\n 5 files changed, 45 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex f62fd27440..adad71f631 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git checkout --pathspec-from-file=- HEAD^1 &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit checkout --pathspec-from-file=list HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \tM fileA.t\n@@ -120,7 +124,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n '\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex db58e83735..b407f6b779 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n \n \tcat >expect <<-\\EOF &&\n \t M fileA.t\n@@ -120,7 +124,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n '\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 3cfdb669b7..61b6e51009 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -97,7 +97,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git add --pathspec-from-file=- &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit add --pathspec-from-file=list &&\n \n \tcat >expect <<-\\EOF &&\n \tA fileA.t\n@@ -108,7 +112,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n '\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 6b1a731fff..b0e84cdb42 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -105,8 +105,12 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\tgit reset --pathspec-from-file=list &&\n \n \tcat >expect <<-\\EOF &&\n \t D fileA.t\n@@ -117,8 +121,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n \tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex 4b58901ed6..4a7c11368d 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -100,7 +100,11 @@ test_expect_success 'CRLF delimiters' '\n test_expect_success 'quotes' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n \n \tcat >expect <<-\\EOF &&\n \tA\tfileA.t\n@@ -111,7 +115,10 @@ test_expect_success 'quotes' '\n test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n \trestore_checkpoint &&\n \n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n \ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"389131","messageId":"8748f3baf1d92f2e642c529ba3856714f847a468.1577787313.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v4.git.1577787313.gitgitgadget@gmail.com","subject":"[PATCH v4 2/3] t: directly test parse_pathspec_file()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T10:15:12Z","receivedAt":"2019-12-31T10:15:20Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nPreviously, `parse_pathspec_file()` was tested indirectly by invoking\ngit commands with properly crafted inputs. As demonstrated by the\nprevious bugfix, testing complicated black boxes indirectly can lead to\ntests that silently test the wrong thing.\n\nIntroduce direct tests for `parse_pathspec_file()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 33 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++\n 5 files changed, 144 insertions(+)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\ndiff --git a/Makefile b/Makefile\nindex 09f98b777c..0061f96e8a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o\n TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\ndiff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c\nnew file mode 100644\nindex 0000000000..02f4ccfd2a\n--- /dev/null\n+++ b/t/helper/test-parse-pathspec-file.c\n@@ -0,0 +1,33 @@\n+#include \"test-tool.h\"\n+#include \"parse-options.h\"\n+#include \"pathspec.h\"\n+#include \"gettext.h\"\n+\n+int cmd__parse_pathspec_file(int argc, const char **argv)\n+{\n+\tstruct pathspec pathspec;\n+\tconst char *pathspec_from_file = 0;\n+\tint pathspec_file_nul = 0, i;\n+\n+\tstatic const char *const usage[] = {\n+\t\t\"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]\",\n+\t\tNULL\n+\t};\n+\n+\tstruct option options[] = {\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n+\t\tOPT_END()\n+\t};\n+\n+\tparse_options(argc, argv, 0, options, usage, 0);\n+\n+\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n+\t\t\t pathspec_file_nul);\n+\n+\tfor (i = 0; i < pathspec.nr; i++)\n+\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n+\n+\tclear_pathspec(&pathspec);\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex f20989d449..c9a232d238 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {\n \t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n+\t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 8ed2af71d1..c8549fd87f 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);\n int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n+int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\ndiff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh\nnew file mode 100755\nindex 0000000000..7bab49f361\n--- /dev/null\n+++ b/t/t0067-parse_pathspec_file.sh\n@@ -0,0 +1,108 @@\n+#!/bin/sh\n+\n+test_description='Test parse_pathspec_file()'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'one item from stdin' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\techo fileA.t |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'one item from file' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\techo fileA.t >list &&\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\0fileB.t\\0\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\nfileB.t\\n\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\nfileB.t\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" |\n+\ttest-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'quotes' '\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--pathspec-file-nul takes quotes literally' '\n+\t# Note: there is an extra newline because --pathspec-file-nul takes\n+\t# input \\n literally, too\n+\tcat >expect <<-\\EOF &&\n+\t\"file\\101.t\"\n+\n+\tEOF\n+\n+\tcat >list <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"389132","messageId":"d02a1eac0b8dcf21593e8b596983b1731b6450b1.1577787313.git.gitgitgadget@gmail.com","threadId":"52540","inReplyTo":"pull.503.v4.git.1577787313.gitgitgadget@gmail.com","subject":"[PATCH v4 3/3] t: drop copy&pasted tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-31T10:15:13Z","receivedAt":"2019-12-31T10:15:22Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nWith direct tests for `parse_pathspec_file()` already in place, it is\nnot very reasonable to copy&paste 6 similar indirect tests for every git\ncommand that uses `parse_pathspec_file()`. I counted 13 potential git\ncommands, which could eventually lead to 6*13=78 duplicate tests.\n\nI believe that indirect tests are redundant because I don't expect\ndirect tests to ever disagree with indirect tests.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2026-checkout-pathspec-file.sh | 77 +---------------------------\n t/t2072-restore-pathspec-file.sh | 77 +---------------------------\n t/t3704-add-pathspec-file.sh | 77 +---------------------------\n t/t7107-reset-pathspec-file.sh | 85 +++----------------------------\n t/t7526-commit-pathspec-file.sh | 77 +---------------------------\n 5 files changed, 16 insertions(+), 377 deletions(-)\n\ndiff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh\nindex adad71f631..559b4528d7 100755\n--- a/t/t2026-checkout-pathspec-file.sh\n+++ b/t/t2026-checkout-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&\n@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git checkout --pathspec-from-file=- HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tM fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit checkout --pathspec-from-file=list HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tM fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh\nindex b407f6b779..9b3125d582 100755\n--- a/t/t2072-restore-pathspec-file.sh\n+++ b/t/t2072-restore-pathspec-file.sh\n@@ -35,7 +35,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&\n@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&\n@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git restore --pathspec-from-file=- --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\t M fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit restore --pathspec-from-file=list --source=HEAD^1 &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t M fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh\nindex 61b6e51009..9009f8a9ac 100755\n--- a/t/t3704-add-pathspec-file.sh\n+++ b/t/t3704-add-pathspec-file.sh\n@@ -23,7 +23,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git add --pathspec-from-file=- &&\n@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit add --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git add --pathspec-from-file=- --pathspec-file-nul &&\n@@ -58,67 +46,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git add --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tA fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit add --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git add --pathspec-from-file=list --pathspec-file-nul\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex b0e84cdb42..5b845f4f7c 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -25,7 +25,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t &&\n@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\techo fileA.t >list &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t fileB.t &&\n@@ -63,77 +50,21 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n+test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n-\t D fileA.t\n+\tD fileA.t\n \t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n \tEOF\n \tverify_expect\n '\n \n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit rm fileA.t &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n-\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\ttest_must_fail verify_expect\n-'\n-\n test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n \trestore_checkpoint &&\n \ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex 4a7c11368d..8d6c652690 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -26,7 +26,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n+test_expect_success '--pathspec-file-nul' '\n \trestore_checkpoint &&\n \n \tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n@@ -61,67 +49,6 @@ test_expect_success 'NUL delimiters' '\n \tverify_expect\n '\n \n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tcat >list <<-\\EOF &&\n-\t\"file\\101.t\"\n-\tEOF\n-\n-\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-- \ngitgitgadget\n"},{"id":"389426","messageId":"xmqqh8173r8e.fsf@gitster-ct.c.googlers.com","threadId":"52540","inReplyTo":"pull.503.v4.git.1577787313.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-07T21:13:05Z","receivedAt":"2020-01-07T21:13:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> This branch is a follow-up for [1] where part of branch was merged into `master` via [2].\n>\n> Previously in [3] there were some concerns on whether removing\n> copy&pasted tests is good. I still think that yes, it 's a good thing,\n> mostly because of high volume of potential 13*6=78 duplicate tests.\n>\n> Still, I separated this change as last patch, so that the remaining\n> part of the branch can be taken without it.\n\nWith the third step the series won't merge cleanly with other topic\nyou have in 'next' (t7107 gets somewhat heavy merge conflicts).\n\nI'll queue the first two for now but let's clean them up post 2.25\nrelease.\n\nThanks.\n"},{"id":"389480","messageId":"12861b02-386c-3ae8-cd2f-ffe07c6aabc7@syntevo.com","threadId":"52540","inReplyTo":"xmqqh8173r8e.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-01-08T15:32:43Z","receivedAt":"2020-01-08T15:32:50Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 07.01.2020 22:13, Junio C Hamano wrote:\n> With the third step the series won't merge cleanly with other topic\n> you have in 'next' (t7107 gets somewhat heavy merge conflicts).\n> \n> I'll queue the first two for now but let's clean them up post 2.25\n> release.\n\nOK, I will re-submit the remaining patch after 2.25.\n\nI will implement the next --pathspec-from-file patches as if this third \npatch was accepted (that is, without copy&pasted tests).\n\nThanks for accepting this and other polishing branches, I was already \nquite pessimistic about them.\n"},{"id":"389487","messageId":"xmqqimll3lmn.fsf@gitster-ct.c.googlers.com","threadId":"52540","inReplyTo":"12861b02-386c-3ae8-cd2f-ffe07c6aabc7@syntevo.com","subject":"Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-08T17:26:24Z","receivedAt":"2020-01-08T17:26:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandr Miloslavskiy writes:\n\n> On 07.01.2020 22:13, Junio C Hamano wrote:\n>> With the third step the series won't merge cleanly with other topic\n>> you have in 'next' (t7107 gets somewhat heavy merge conflicts).\n>>\n>> I'll queue the first two for now but let's clean them up post 2.25\n>> release.\n>\n> OK, I will re-submit the remaining patch after 2.25.\n>\n> I will implement the next --pathspec-from-file patches as if this\n> third patch was accepted (that is, without copy&pasted tests).\n\nI am not sure if that is a good idea. I'd rather see the planned\nnew changes not to be taken hostage of the third step.\n\nBesides, with the third step, your preference is not to test the\nbehaviour of end-user facing commands that would learn the option at\nall and only test the underlying machinery with test-tool tests, no?\nIf you are not adding tests for the higher-level end-user facing\ncommands as part of these new series, would it make a difference if\nthe codebase has the third step applied (i.e. missing tests for the\nend-user facing commands that have already learned the option) or\nnot (i.e. the commands that have already learned the option are\nstill tested end-to-end)?\n"},{"id":"389490","messageId":"0b54d95c-a5e0-c156-b972-a1e171678785@syntevo.com","threadId":"52540","inReplyTo":"xmqqimll3lmn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-01-08T17:42:13Z","receivedAt":"2020-01-08T17:42:20Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 08.01.2020 18:26, Junio C Hamano wrote:\n>> I will implement the next --pathspec-from-file patches as if this\n>> third patch was accepted (that is, without copy&pasted tests).\n> \n> I am not sure if that is a good idea. I'd rather see the planned\n> new changes not to be taken hostage of the third step.\n\nIn my understanding, the new patches will not be taken hostage, they \nwill simply adopt the new approach. Everything will work just fine \nwhether or not third step is present.\n\n> Besides, with the third step, your preference is not to test the\n> behaviour of end-user facing commands that would learn the option at\n> all and only test the underlying machinery with test-tool tests, no?\n\nThat's not exactly correct. Third step removes duplicate tests that give \nno real benefit. With test-tool tests in place and succceeding, these \nduplicate tests are super unlikely to fail.\n\nI will still provide a few tests for every new command to make sure that \nsaid command works as intended. I will only skip indirectly testing \nglobal API again and again.\n\n> If you are not adding tests for the higher-level end-user facing\n> commands as part of these new series, would it make a difference if\n> the codebase has the third step applied (i.e. missing tests for the\n> end-user facing commands that have already learned the option) or\n> not (i.e. the commands that have already learned the option are\n> still tested end-to-end)?\n\nI will be adding good tests and skip useless tests. For new commands, it \ndoesn't really matter if \"third step\" patch is applied or not.\n"},{"id":"389493","messageId":"xmqq1rs93hr2.fsf@gitster-ct.c.googlers.com","threadId":"52540","inReplyTo":"0b54d95c-a5e0-c156-b972-a1e171678785@syntevo.com","subject":"Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-08T18:50:09Z","receivedAt":"2020-01-08T18:50:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandr Miloslavskiy writes:\n\n> I will still provide a few tests for every new command to make sure\n> that said command works as intended. I will only skip indirectly\n> testing global API again and again.\n\nAh, OK. Then leaving those removed by the third step there may get\nin the way. So let's assume that we'll have an updated third step\nalready applied and your new series are written on top of it.\n\n> ... For new commands, it doesn't really matter if \"third step\"\n> patch is applied or not.\n\nOK, again. Thanks for a clarification.\n\n"}]}