* [PATCH 0/3] t: rework tests for --pathspec-from-file
@ 2019-12-30 17:42 Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
` (3 more replies)
0 siblings, 4 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano
Please refer to commit messages for rationale.
This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.
Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.
[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/
Alexandr Miloslavskiy (3):
t: fix quotes tests for --pathspec-from-file
t: directly test parse_pathspec_file()
t: drop copy&pasted tests for --pathspec-from-file
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 34 +++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++++++++
t/t2026-checkout-pathspec-file.sh | 70 +----------------------
t/t2072-restore-pathspec-file.sh | 70 +----------------------
t/t3704-add-pathspec-file.sh | 70 +----------------------
t/t7107-reset-pathspec-file.sh | 79 +++----------------------
t/t7526-commit-pathspec-file.sh | 70 +----------------------
10 files changed, 142 insertions(+), 343 deletions(-)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/503
--
gitgitgadget
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-30 17:42 [PATCH 0/3] t: rework tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 17:42 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.
Fix this by properly escaping one more time.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 9 +++++++--
t/t2072-restore-pathspec-file.sh | 9 +++++++--
t/t3704-add-pathspec-file.sh | 9 +++++++--
t/t7107-reset-pathspec-file.sh | 9 +++++++--
t/t7526-commit-pathspec-file.sh | 9 +++++++--
5 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..2dc8901bca 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
'
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..70e95ef3b6 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
'
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..2e0141fcce 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
cat >expect <<-\EOF &&
A fileA.t
@@ -108,7 +111,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
'
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..52a44f033d 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
git rm fileA.t &&
- printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+ printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
D fileA.t
@@ -117,8 +120,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
git rm fileA.t &&
- printf "\"file\\101.t\"" >list &&
+ printf "\"file\\\\101.t\"" >list &&
# Note: "git reset" has not yet learned to fail on wrong pathspecs
git reset --pathspec-from-file=list --pathspec-file-nul &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..e7dc2ff8b1 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
cat >expect <<-\EOF &&
A fileA.t
@@ -111,7 +114,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/3] t: directly test parse_pathspec_file()
2019-12-30 17:42 [PATCH 0/3] t: rework tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 17:42 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-30 18:52 ` Junio C Hamano
2019-12-30 17:42 ` [PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
3 siblings, 1 reply; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.
Introduce direct tests for `parse_pathspec_file()`.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 34 +++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++++++++
5 files changed, 126 insertions(+)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-oidmap.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..e7f525feb9
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,34 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+ struct pathspec pathspec;
+ const char *pathspec_from_file = 0;
+ int pathspec_file_nul = 0, i;
+
+ static const char *const usage[] = {
+ "test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+ NULL
+ };
+
+ struct option options[] = {
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_END()
+ };
+
+ parse_options(argc, argv, 0, options, usage, 0);
+
+ parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+ pathspec_file_nul);
+
+ for (i = 0; i < pathspec.nr; i++) {
+ printf("%s\n", pathspec.items[i].original);
+ }
+
+ clear_pathspec(&pathspec);
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "oidmap", cmd__oidmap },
{ "online-cpus", cmd__online_cpus },
{ "parse-options", cmd__parse_options },
+ { "parse-pathspec-file", cmd__parse_pathspec_file },
{ "path-utils", cmd__path_utils },
{ "pkt-line", cmd__pkt_line },
{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
int cmd__oidmap(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
int cmd__path_utils(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..df7b319713
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,89 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+ echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+ echo fileA.t >list &&
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+ printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+ printf "fileA.t\nfileB.t\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+ printf "fileA.t\nfileB.t" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+ printf "fileA.t\r\nfileB.t\r\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ cat >expect <<-\EOF &&
+ "file\101.t"
+ EOF
+ test_cmp expect actual
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file
2019-12-30 17:42 [PATCH 0/3] t: rework tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 17:42 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).
I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 75 +--------------------------
t/t2072-restore-pathspec-file.sh | 75 +--------------------------
t/t3704-add-pathspec-file.sh | 75 +--------------------------
t/t7107-reset-pathspec-file.sh | 84 +++----------------------------
t/t7526-commit-pathspec-file.sh | 75 +--------------------------
5 files changed, 16 insertions(+), 368 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index 2dc8901bca..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index 70e95ef3b6..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 2e0141fcce..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 52a44f033d..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- git rm fileA.t &&
- echo fileA.t >list &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
git rm fileA.t fileB.t &&
@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
restore_checkpoint &&
- git rm fileA.t fileB.t &&
- printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+ git rm fileA.t fileB.t fileC.t fileD.t &&
+ printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
- D fileA.t
+ D fileA.t
D fileB.t
+ D fileC.t
+ D fileD.t
EOF
verify_expect
'
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- git rm fileA.t &&
- printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- git rm fileA.t &&
- printf "\"file\\\\101.t\"" >list &&
- # Note: "git reset" has not yet learned to fail on wrong pathspecs
- git reset --pathspec-from-file=list --pathspec-file-nul &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- test_must_fail verify_expect
-'
-
test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
restore_checkpoint &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index e7dc2ff8b1..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] t: directly test parse_pathspec_file()
2019-12-30 17:42 ` [PATCH 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 18:52 ` Junio C Hamano
2019-12-30 19:16 ` Alexandr Miloslavskiy
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2019-12-30 18:52 UTC (permalink / raw)
To: Alexandr Miloslavskiy via GitGitGadget; +Cc: git, Alexandr Miloslavskiy
"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
> new file mode 100644
> index 0000000000..e7f525feb9
> --- /dev/null
> +++ b/t/helper/test-parse-pathspec-file.c
> @@ -0,0 +1,34 @@
> +#include "test-tool.h"
> +#include "parse-options.h"
> +#include "pathspec.h"
> +#include "gettext.h"
> + ...
> + parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
> + pathspec_file_nul);
> +
> + for (i = 0; i < pathspec.nr; i++) {
> + printf("%s\n", pathspec.items[i].original);
> + }
No need for {} around a single statement block.
> diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
> new file mode 100755
> index 0000000000..df7b319713
> --- /dev/null
> +++ b/t/t0067-parse_pathspec_file.sh
> @@ -0,0 +1,89 @@
> +#!/bin/sh
> +
> +test_description='Test parse_pathspec_file()'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'one item from stdin' '
> + echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
> +
> + cat >expect <<-\EOF &&
> + fileA.t
> + EOF
> + test_cmp expect actual
> +'
The use of the blank lines are somewhat inconsistent here.
> + ...
> +test_expect_success 'NUL delimiters' '
> + printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
Fold line immediately after the pipe (same for the earlier and later ones).
> + cat >expect <<-\EOF &&
> + fileA.t
> + fileB.t
> + EOF
> + test_cmp expect actual
> +'
If you want to have a gap between the steps, i.e. "capturing the
actual output", "creating the ideal output", and "seeing how they
differ", using blank like this is OK:
printf "fileA.t\0fileB.t\0" |
test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
cat >expect <<-\EOF &&
fileA.t
fileB.t
EOF
test_cmp expect actual
I thought we typically prepare the ideal output sample before
capturing the actual output, so if we follow that convention, the
above becomes
cat >expect <<-\EOF &&
fileA.t
fileB.t
EOF
printf "fileA.t\0fileB.t\0" |
test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
test_cmp expect actual
> +test_expect_success 'quotes' '
> + # shell takes \\\\101 and spits \\101
> + # printf takes \\101 and spits \101
> + # git takes \101 and spits A
> + printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
> +
> + cat >expect <<-\EOF &&
> + fileA.t
> + EOF
> + test_cmp expect actual
> +'
> +
> +test_expect_success '--pathspec-file-nul takes quotes literally' '
> + # shell takes \\\\101 and spits \\101
> + # printf takes \\101 and spits \101
> + printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
> +
> + cat >expect <<-\EOF &&
> + "file\101.t"
> + EOF
> + test_cmp expect actual
> +'
Testing low level machinery like this is of course a good idea, in
addition to the end-to-end tests that make sure that the machinery
is called correctly from the higher layer.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 0/3] t: rework tests for --pathspec-from-file
2019-12-30 17:42 [PATCH 0/3] t: rework tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
` (2 preceding siblings ...)
2019-12-30 17:42 ` [PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 19:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
` (3 more replies)
3 siblings, 4 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 19:15 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano
Please refer to commit messages for rationale.
This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.
Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.
[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/
Changes since V1
----------------
Small code formatting changes suggested in V1.
Alexandr Miloslavskiy (3):
t: fix quotes tests for --pathspec-from-file
t: directly test parse_pathspec_file()
t: drop copy&pasted tests for --pathspec-from-file
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 33 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 104 ++++++++++++++++++++++++++++
t/t2026-checkout-pathspec-file.sh | 70 +------------------
t/t2072-restore-pathspec-file.sh | 70 +------------------
t/t3704-add-pathspec-file.sh | 70 +------------------
t/t7107-reset-pathspec-file.sh | 79 +++------------------
t/t7526-commit-pathspec-file.sh | 70 +------------------
10 files changed, 156 insertions(+), 343 deletions(-)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/503
Range-diff vs v1:
1: ab9519298d = 1: 6193dc7396 t: fix quotes tests for --pathspec-from-file
2: 27383a5b08 ! 2: ab449ac15a t: directly test parse_pathspec_file()
@@ -55,9 +55,8 @@
+ parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+ pathspec_file_nul);
+
-+ for (i = 0; i < pathspec.nr; i++) {
++ for (i = 0; i < pathspec.nr; i++)
+ printf("%s\n", pathspec.items[i].original);
-+ }
+
+ clear_pathspec(&pathspec);
+ return 0;
@@ -99,84 +98,99 @@
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
-+ echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
-+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
++
++ echo fileA.t |
++ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
++
+ test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
-+ echo fileA.t >list &&
-+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
-+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
++
++ echo fileA.t >list &&
++ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
++
+ test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
-+ printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
-+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
++
++ printf "fileA.t\0fileB.t\0" |
++ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
++
+ test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
-+ printf "fileA.t\nfileB.t\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
-+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
++
++ printf "fileA.t\nfileB.t\n" |
++ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
++
+ test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
-+ printf "fileA.t\nfileB.t" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
-+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
++
++ printf "fileA.t\nfileB.t" |
++ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
++
+ test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
-+ printf "fileA.t\r\nfileB.t\r\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
-+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
++
++ printf "fileA.t\r\nfileB.t\r\n" |
++ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
++
+ test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
++ cat >expect <<-\EOF &&
++ fileA.t
++ EOF
++
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
-+ printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
++ printf "\"file\\\\101.t\"" |
++ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
-+ cat >expect <<-\EOF &&
-+ fileA.t
-+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
-+
+ cat >expect <<-\EOF &&
+ "file\101.t"
+ EOF
++
++ # shell takes \\\\101 and spits \\101
++ # printf takes \\101 and spits \101
++ printf "\"file\\\\101.t\"" |
++ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
++
+ test_cmp expect actual
+'
+
3: daef256db3 = 3: 88086cebce t: drop copy&pasted tests for --pathspec-from-file
--
gitgitgadget
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 19:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-30 21:55 ` Eric Sunshine
2019-12-30 19:15 ` [PATCH v2 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
` (2 subsequent siblings)
3 siblings, 1 reply; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 19:15 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.
Fix this by properly escaping one more time.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 9 +++++++--
t/t2072-restore-pathspec-file.sh | 9 +++++++--
t/t3704-add-pathspec-file.sh | 9 +++++++--
t/t7107-reset-pathspec-file.sh | 9 +++++++--
t/t7526-commit-pathspec-file.sh | 9 +++++++--
5 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..2dc8901bca 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
'
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..70e95ef3b6 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
'
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..2e0141fcce 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
cat >expect <<-\EOF &&
A fileA.t
@@ -108,7 +111,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
'
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..52a44f033d 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
git rm fileA.t &&
- printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+ printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
D fileA.t
@@ -117,8 +120,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
git rm fileA.t &&
- printf "\"file\\101.t\"" >list &&
+ printf "\"file\\\\101.t\"" >list &&
# Note: "git reset" has not yet learned to fail on wrong pathspecs
git reset --pathspec-from-file=list --pathspec-file-nul &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..e7dc2ff8b1 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
cat >expect <<-\EOF &&
A fileA.t
@@ -111,7 +114,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/3] t: directly test parse_pathspec_file()
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 19:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 19:15 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.
Introduce direct tests for `parse_pathspec_file()`.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 33 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 104 ++++++++++++++++++++++++++++
5 files changed, 140 insertions(+)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-oidmap.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..02f4ccfd2a
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,33 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+ struct pathspec pathspec;
+ const char *pathspec_from_file = 0;
+ int pathspec_file_nul = 0, i;
+
+ static const char *const usage[] = {
+ "test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+ NULL
+ };
+
+ struct option options[] = {
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_END()
+ };
+
+ parse_options(argc, argv, 0, options, usage, 0);
+
+ parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+ pathspec_file_nul);
+
+ for (i = 0; i < pathspec.nr; i++)
+ printf("%s\n", pathspec.items[i].original);
+
+ clear_pathspec(&pathspec);
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "oidmap", cmd__oidmap },
{ "online-cpus", cmd__online_cpus },
{ "parse-options", cmd__parse_options },
+ { "parse-pathspec-file", cmd__parse_pathspec_file },
{ "path-utils", cmd__path_utils },
{ "pkt-line", cmd__pkt_line },
{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
int cmd__oidmap(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
int cmd__path_utils(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..77b44f6702
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ echo fileA.t |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ echo fileA.t >list &&
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\0fileB.t\0" |
+ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\nfileB.t\n" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\nfileB.t" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\r\nfileB.t\r\n" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+ cat >expect <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" |
+ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 3/3] t: drop copy&pasted tests for --pathspec-from-file
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 19:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 19:15 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).
I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 75 +--------------------------
t/t2072-restore-pathspec-file.sh | 75 +--------------------------
t/t3704-add-pathspec-file.sh | 75 +--------------------------
t/t7107-reset-pathspec-file.sh | 84 +++----------------------------
t/t7526-commit-pathspec-file.sh | 75 +--------------------------
5 files changed, 16 insertions(+), 368 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index 2dc8901bca..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index 70e95ef3b6..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 2e0141fcce..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 52a44f033d..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- git rm fileA.t &&
- echo fileA.t >list &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
git rm fileA.t fileB.t &&
@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
restore_checkpoint &&
- git rm fileA.t fileB.t &&
- printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+ git rm fileA.t fileB.t fileC.t fileD.t &&
+ printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
- D fileA.t
+ D fileA.t
D fileB.t
+ D fileC.t
+ D fileD.t
EOF
verify_expect
'
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- git rm fileA.t &&
- printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- git rm fileA.t &&
- printf "\"file\\\\101.t\"" >list &&
- # Note: "git reset" has not yet learned to fail on wrong pathspecs
- git reset --pathspec-from-file=list --pathspec-file-nul &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- test_must_fail verify_expect
-'
-
test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
restore_checkpoint &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index e7dc2ff8b1..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] t: directly test parse_pathspec_file()
2019-12-30 18:52 ` Junio C Hamano
@ 2019-12-30 19:16 ` Alexandr Miloslavskiy
0 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy @ 2019-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano, Alexandr Miloslavskiy via GitGitGadget; +Cc: git
Thanks for having a look! I think I have fixed all mentioned issues in V2.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-30 19:15 ` [PATCH v2 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-30 21:55 ` Eric Sunshine
2019-12-31 0:26 ` Jonathan Nieder
0 siblings, 1 reply; 26+ messages in thread
From: Eric Sunshine @ 2019-12-30 21:55 UTC (permalink / raw)
To: Alexandr Miloslavskiy via GitGitGadget
Cc: Git List, Alexandr Miloslavskiy, Junio C Hamano
On Mon, Dec 30, 2019 at 2:15 PM Alexandr Miloslavskiy via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> While working on the next patch, I also noticed that quotes testing via
> `"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
> I had to escape it two times! Tests still worked due to `"` being
> preserved which in turn prevented pathspec from matching files.
>
> Fix this by properly escaping one more time.
>
> Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
> ---
> diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
> @@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
> - printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
> + # shell takes \\\\101 and spits \\101
> + # printf takes \\101 and spits \101
> + # git takes \101 and spits A
> + printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
So, you want git-checkout to receive the following, quotes, backslash,
and no newline, on its standard input?
"file\101.t"
If so, another way to achieve the same without taxing the brain of the
reader or the next person who works on this code would be:
tr -d "\012" | git checkout --pathspec-from-file=- HEAD^1 <<-\EOF &&
"file\101.t"
EOF
Although it's three lines long, the body of the here-doc is the
literal text you want sent to the Git command, so no counting
backslashes, and no need for a lengthy in-code comment.
But is the "no newline" bit indeed intentional? If not, then a simple
echo would be even easier (though with a bit more escaping):
echo "\"file\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-30 21:55 ` Eric Sunshine
@ 2019-12-31 0:26 ` Jonathan Nieder
2019-12-31 10:01 ` Alexandr Miloslavskiy
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Nieder @ 2019-12-31 0:26 UTC (permalink / raw)
To: Eric Sunshine
Cc: Alexandr Miloslavskiy via GitGitGadget, Git List,
Alexandr Miloslavskiy, Junio C Hamano
Eric Sunshine wrote:
> But is the "no newline" bit indeed intentional? If not, then a simple
> echo would be even easier (though with a bit more escaping):
>
> echo "\"file\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
For portability, that would be
printf "%s\n" "\"file\101.t\"" | ...
because some implementations of echo interpret escapes by default.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 0/3] t: rework tests for --pathspec-from-file
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
` (2 preceding siblings ...)
2019-12-30 19:15 ` [PATCH v2 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 9:53 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
` (3 more replies)
3 siblings, 4 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 9:53 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano
Please refer to commit messages for rationale.
This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.
Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.
[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/
Changes since V1
----------------
Small code formatting changes suggested in V1.
Alexandr Miloslavskiy (3):
t: fix quotes tests for --pathspec-from-file
t: directly test parse_pathspec_file()
t: drop copy&pasted tests for --pathspec-from-file
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 33 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++
t/t2026-checkout-pathspec-file.sh | 70 +-----------------
t/t2072-restore-pathspec-file.sh | 70 +-----------------
t/t3704-add-pathspec-file.sh | 70 +-----------------
t/t7107-reset-pathspec-file.sh | 79 +++-----------------
t/t7526-commit-pathspec-file.sh | 70 +-----------------
10 files changed, 160 insertions(+), 343 deletions(-)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/503
Range-diff vs v2:
1: 6193dc7396 ! 1: 88790669ce t: fix quotes tests for --pathspec-from-file
@@ -7,7 +7,7 @@
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.
- Fix this by properly escaping one more time.
+ Fix this by using here-doc instead.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
@@ -19,10 +19,11 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ # git takes \101 and spits A
-+ printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
++ git checkout --pathspec-from-file=list HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -31,9 +32,10 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ printf "\"file\\\\101.t\"" >list &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
'
@@ -46,10 +48,11 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ # git takes \101 and spits A
-+ printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
++ git restore --pathspec-from-file=list --source=HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -58,9 +61,10 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ printf "\"file\\\\101.t\"" >list &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
'
@@ -73,10 +77,11 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ # git takes \101 and spits A
-+ printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
++ git add --pathspec-from-file=list &&
cat >expect <<-\EOF &&
A fileA.t
@@ -85,9 +90,10 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ printf "\"file\\\\101.t\"" >list &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
'
@@ -99,12 +105,13 @@
test_expect_success 'quotes' '
restore_checkpoint &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ # git takes \101 and spits A
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
git rm fileA.t &&
- printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
-+ printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
++ git reset --pathspec-from-file=list &&
cat >expect <<-\EOF &&
D fileA.t
@@ -112,11 +119,12 @@
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
- git rm fileA.t &&
+- git rm fileA.t &&
- printf "\"file\\101.t\"" >list &&
-+ printf "\"file\\\\101.t\"" >list &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
# Note: "git reset" has not yet learned to fail on wrong pathspecs
git reset --pathspec-from-file=list --pathspec-file-nul &&
@@ -129,10 +137,11 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ # git takes \101 and spits A
-+ printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
++ git commit --pathspec-from-file=list -m "Commit" &&
cat >expect <<-\EOF &&
A fileA.t
@@ -141,9 +150,10 @@
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ printf "\"file\\\\101.t\"" >list &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
'
2: ab449ac15a ! 2: 68925c2712 t: directly test parse_pathspec_file()
@@ -172,24 +172,28 @@
+ fileA.t
+ EOF
+
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ # git takes \101 and spits A
-+ printf "\"file\\\\101.t\"" |
-+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
++ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
++ # Note: there is an extra newline because --pathspec-file-nul takes
++ # input \n literally, too
+ cat >expect <<-\EOF &&
+ "file\101.t"
++
+ EOF
+
-+ # shell takes \\\\101 and spits \\101
-+ # printf takes \\101 and spits \101
-+ printf "\"file\\\\101.t\"" |
-+ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
++ cat >list <<-\EOF &&
++ "file\101.t"
++ EOF
++
++ test-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
3: 88086cebce ! 3: f71021b0dd t: drop copy&pasted tests for --pathspec-from-file
@@ -88,10 +88,11 @@
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- # git takes \101 and spits A
-- printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
+- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
@@ -102,9 +103,10 @@
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- printf "\"file\\\\101.t\"" >list &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
- test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
@@ -188,10 +190,11 @@
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- # git takes \101 and spits A
-- printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
+- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
@@ -202,9 +205,10 @@
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- printf "\"file\\\\101.t\"" >list &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
- test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
@@ -288,10 +292,11 @@
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- # git takes \101 and spits A
-- printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
+- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
@@ -302,9 +307,10 @@
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- printf "\"file\\\\101.t\"" >list &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
- test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
@@ -398,11 +404,12 @@
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- # git takes \101 and spits A
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
- git rm fileA.t &&
-- printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
+- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
@@ -413,10 +420,10 @@
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- git rm fileA.t &&
-- printf "\"file\\\\101.t\"" >list &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
- # Note: "git reset" has not yet learned to fail on wrong pathspecs
- git reset --pathspec-from-file=list --pathspec-file-nul &&
-
@@ -506,10 +513,11 @@
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- # git takes \101 and spits A
-- printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
+- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
@@ -520,9 +528,10 @@
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
-- # shell takes \\\\101 and spits \\101
-- # printf takes \\101 and spits \101
-- printf "\"file\\\\101.t\"" >list &&
+- cat >list <<-\EOF &&
+- "file\101.t"
+- EOF
+-
- test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
--
gitgitgadget
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 9:53 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 9:53 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.
Fix this by using here-doc instead.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 11 +++++++++--
t/t2072-restore-pathspec-file.sh | 11 +++++++++--
t/t3704-add-pathspec-file.sh | 11 +++++++++--
t/t7107-reset-pathspec-file.sh | 12 +++++++++---
t/t7526-commit-pathspec-file.sh | 11 +++++++++--
5 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..adad71f631 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git checkout --pathspec-from-file=list HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
'
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..b407f6b779 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git restore --pathspec-from-file=list --source=HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
'
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..61b6e51009 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git add --pathspec-from-file=list &&
cat >expect <<-\EOF &&
A fileA.t
@@ -108,7 +112,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
'
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..b0e84cdb42 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,12 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
git rm fileA.t &&
- printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+ git reset --pathspec-from-file=list &&
cat >expect <<-\EOF &&
D fileA.t
@@ -117,8 +121,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- git rm fileA.t &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
# Note: "git reset" has not yet learned to fail on wrong pathspecs
git reset --pathspec-from-file=list --pathspec-file-nul &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..4a7c11368d 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git commit --pathspec-from-file=list -m "Commit" &&
cat >expect <<-\EOF &&
A fileA.t
@@ -111,7 +115,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 2/3] t: directly test parse_pathspec_file()
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 9:53 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 9:53 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.
Introduce direct tests for `parse_pathspec_file()`.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 33 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++
5 files changed, 144 insertions(+)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-oidmap.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..02f4ccfd2a
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,33 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+ struct pathspec pathspec;
+ const char *pathspec_from_file = 0;
+ int pathspec_file_nul = 0, i;
+
+ static const char *const usage[] = {
+ "test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+ NULL
+ };
+
+ struct option options[] = {
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_END()
+ };
+
+ parse_options(argc, argv, 0, options, usage, 0);
+
+ parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+ pathspec_file_nul);
+
+ for (i = 0; i < pathspec.nr; i++)
+ printf("%s\n", pathspec.items[i].original);
+
+ clear_pathspec(&pathspec);
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "oidmap", cmd__oidmap },
{ "online-cpus", cmd__online_cpus },
{ "parse-options", cmd__parse_options },
+ { "parse-pathspec-file", cmd__parse_pathspec_file },
{ "path-utils", cmd__path_utils },
{ "pkt-line", cmd__pkt_line },
{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
int cmd__oidmap(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
int cmd__path_utils(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..7bab49f361
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ echo fileA.t |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ echo fileA.t >list &&
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\0fileB.t\0" |
+ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\nfileB.t\n" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\nfileB.t" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\r\nfileB.t\r\n" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+ # Note: there is an extra newline because --pathspec-file-nul takes
+ # input \n literally, too
+ cat >expect <<-\EOF &&
+ "file\101.t"
+
+ EOF
+
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ test-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v3 3/3] t: drop copy&pasted tests for --pathspec-from-file
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 9:53 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 9:53 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).
I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 77 +---------------------------
t/t2072-restore-pathspec-file.sh | 77 +---------------------------
t/t3704-add-pathspec-file.sh | 77 +---------------------------
t/t7107-reset-pathspec-file.sh | 85 +++----------------------------
t/t7526-commit-pathspec-file.sh | 77 +---------------------------
5 files changed, 16 insertions(+), 377 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index adad71f631..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index b407f6b779..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 61b6e51009..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,67 +46,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index b0e84cdb42..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- git rm fileA.t &&
- echo fileA.t >list &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
git rm fileA.t fileB.t &&
@@ -63,77 +50,21 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
restore_checkpoint &&
- git rm fileA.t fileB.t &&
- printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+ git rm fileA.t fileB.t fileC.t fileD.t &&
+ printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
- D fileA.t
+ D fileA.t
D fileB.t
+ D fileC.t
+ D fileD.t
EOF
verify_expect
'
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git rm fileA.t &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- # Note: "git reset" has not yet learned to fail on wrong pathspecs
- git reset --pathspec-from-file=list --pathspec-file-nul &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- test_must_fail verify_expect
-'
-
test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
restore_checkpoint &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4a7c11368d..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,67 +49,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-31 0:26 ` Jonathan Nieder
@ 2019-12-31 10:01 ` Alexandr Miloslavskiy
0 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy @ 2019-12-31 10:01 UTC (permalink / raw)
To: Jonathan Nieder, Eric Sunshine
Cc: Alexandr Miloslavskiy via GitGitGadget, Git List, Junio C Hamano
Luckily, trailing \n didn't matter much, and I could also send input via
commandline argument instead of stdin, so here-doc is really the most
readable solution here.
Fixed it in V3, thanks for your suggestion!
On 30.12.2019 22:55, Eric Sunshine wrote:
> So, you want git-checkout to receive the following, quotes, backslash,
> and no newline, on its standard input?
>
> "file\101.t"
>
> If so, another way to achieve the same without taxing the brain of the
> reader or the next person who works on this code would be:
>
> tr -d "\012" | git checkout --pathspec-from-file=- HEAD^1 <<-\EOF &&
> "file\101.t"
> EOF
>
> Although it's three lines long, the body of the here-doc is the
> literal text you want sent to the Git command, so no counting
> backslashes, and no need for a lengthy in-code comment.
>
> But is the "no newline" bit indeed intentional? If not, then a simple
> echo would be even easier (though with a bit more escaping):
>
> echo "\"file\101.t\"" | git checkout --pathspec-from-file=-
HEAD^1 &&
>
On 31.12.2019 1:26, Jonathan Nieder wrote:
>> But is the "no newline" bit indeed intentional? If not, then a simple
>> echo would be even easier (though with a bit more escaping):
>>
>> echo "\"file\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
>
> For portability, that would be
>
> printf "%s\n" "\"file\101.t\"" | ...
>
> because some implementations of echo interpret escapes by default.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 0/3] t: rework tests for --pathspec-from-file
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
` (2 preceding siblings ...)
2019-12-31 9:53 ` [PATCH v3 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 10:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
` (3 more replies)
3 siblings, 4 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 10:15 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Eric Sunshine, Alexandr Miloslavskiy,
Junio C Hamano
Please refer to commit messages for rationale.
This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.
Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.
[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/
Changes since V1
----------------
Small code formatting changes suggested in V1.
Changes since V2
----------------
Changed \\\\ escaping to use here-doc instead.
Changes since V3
----------------
Slightly improved commit message.
Alexandr Miloslavskiy (3):
t: fix quotes tests for --pathspec-from-file
t: directly test parse_pathspec_file()
t: drop copy&pasted tests for --pathspec-from-file
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 33 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++
t/t2026-checkout-pathspec-file.sh | 70 +-----------------
t/t2072-restore-pathspec-file.sh | 70 +-----------------
t/t3704-add-pathspec-file.sh | 70 +-----------------
t/t7107-reset-pathspec-file.sh | 79 +++-----------------
t/t7526-commit-pathspec-file.sh | 70 +-----------------
10 files changed, 160 insertions(+), 343 deletions(-)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/503
Range-diff vs v3:
1: 88790669ce = 1: ce0c592bb4 t: fix quotes tests for --pathspec-from-file
2: 68925c2712 = 2: 8748f3baf1 t: directly test parse_pathspec_file()
3: f71021b0dd ! 3: d02a1eac0b t: drop copy&pasted tests for --pathspec-from-file
@@ -3,9 +3,9 @@
t: drop copy&pasted tests for --pathspec-from-file
With direct tests for `parse_pathspec_file()` already in place, it is
- not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
- for every git command that uses it (I counted 13 commands that could use
- it eventually).
+ not very reasonable to copy&paste 6 similar indirect tests for every git
+ command that uses `parse_pathspec_file()`. I counted 13 potential git
+ commands, which could eventually lead to 6*13=78 duplicate tests.
I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.
--
gitgitgadget
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 1/3] t: fix quotes tests for --pathspec-from-file
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 10:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
` (2 subsequent siblings)
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 10:15 UTC (permalink / raw)
To: git
Cc: Jonathan Nieder, Eric Sunshine, Alexandr Miloslavskiy,
Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.
Fix this by using here-doc instead.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 11 +++++++++--
t/t2072-restore-pathspec-file.sh | 11 +++++++++--
t/t3704-add-pathspec-file.sh | 11 +++++++++--
t/t7107-reset-pathspec-file.sh | 12 +++++++++---
t/t7526-commit-pathspec-file.sh | 11 +++++++++--
5 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..adad71f631 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git checkout --pathspec-from-file=list HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
'
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..b407f6b779 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git restore --pathspec-from-file=list --source=HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +124,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
'
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..61b6e51009 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git add --pathspec-from-file=list &&
cat >expect <<-\EOF &&
A fileA.t
@@ -108,7 +112,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
'
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..b0e84cdb42 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,12 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
git rm fileA.t &&
- printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+ git reset --pathspec-from-file=list &&
cat >expect <<-\EOF &&
D fileA.t
@@ -117,8 +121,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- git rm fileA.t &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
# Note: "git reset" has not yet learned to fail on wrong pathspecs
git reset --pathspec-from-file=list --pathspec-file-nul &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..4a7c11368d 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ git commit --pathspec-from-file=list -m "Commit" &&
cat >expect <<-\EOF &&
A fileA.t
@@ -111,7 +115,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 2/3] t: directly test parse_pathspec_file()
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 10:15 ` Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2020-01-07 21:13 ` [PATCH v4 0/3] t: rework " Junio C Hamano
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 10:15 UTC (permalink / raw)
To: git
Cc: Jonathan Nieder, Eric Sunshine, Alexandr Miloslavskiy,
Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.
Introduce direct tests for `parse_pathspec_file()`.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 33 +++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 108 ++++++++++++++++++++++++++++
5 files changed, 144 insertions(+)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-oidmap.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..02f4ccfd2a
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,33 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+ struct pathspec pathspec;
+ const char *pathspec_from_file = 0;
+ int pathspec_file_nul = 0, i;
+
+ static const char *const usage[] = {
+ "test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+ NULL
+ };
+
+ struct option options[] = {
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_END()
+ };
+
+ parse_options(argc, argv, 0, options, usage, 0);
+
+ parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+ pathspec_file_nul);
+
+ for (i = 0; i < pathspec.nr; i++)
+ printf("%s\n", pathspec.items[i].original);
+
+ clear_pathspec(&pathspec);
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "oidmap", cmd__oidmap },
{ "online-cpus", cmd__online_cpus },
{ "parse-options", cmd__parse_options },
+ { "parse-pathspec-file", cmd__parse_pathspec_file },
{ "path-utils", cmd__path_utils },
{ "pkt-line", cmd__pkt_line },
{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
int cmd__oidmap(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
int cmd__path_utils(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..7bab49f361
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ echo fileA.t |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ echo fileA.t >list &&
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\0fileB.t\0" |
+ test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\nfileB.t\n" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\nfileB.t" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+
+ printf "fileA.t\r\nfileB.t\r\n" |
+ test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+ # Note: there is an extra newline because --pathspec-file-nul takes
+ # input \n literally, too
+ cat >expect <<-\EOF &&
+ "file\101.t"
+
+ EOF
+
+ cat >list <<-\EOF &&
+ "file\101.t"
+ EOF
+
+ test-tool parse-pathspec-file --pathspec-from-file=list --pathspec-file-nul >actual &&
+
+ test_cmp expect actual
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v4 3/3] t: drop copy&pasted tests for --pathspec-from-file
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
@ 2019-12-31 10:15 ` Alexandr Miloslavskiy via GitGitGadget
2020-01-07 21:13 ` [PATCH v4 0/3] t: rework " Junio C Hamano
3 siblings, 0 replies; 26+ messages in thread
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-31 10:15 UTC (permalink / raw)
To: git
Cc: Jonathan Nieder, Eric Sunshine, Alexandr Miloslavskiy,
Junio C Hamano, Alexandr Miloslavskiy
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 similar indirect tests for every git
command that uses `parse_pathspec_file()`. I counted 13 potential git
commands, which could eventually lead to 6*13=78 duplicate tests.
I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 77 +---------------------------
t/t2072-restore-pathspec-file.sh | 77 +---------------------------
t/t3704-add-pathspec-file.sh | 77 +---------------------------
t/t7107-reset-pathspec-file.sh | 85 +++----------------------------
t/t7526-commit-pathspec-file.sh | 77 +---------------------------
5 files changed, 16 insertions(+), 377 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index adad71f631..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index b407f6b779..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,67 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 61b6e51009..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,67 +46,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index b0e84cdb42..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- git rm fileA.t &&
- echo fileA.t >list &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
git rm fileA.t fileB.t &&
@@ -63,77 +50,21 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
restore_checkpoint &&
- git rm fileA.t fileB.t &&
- printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+ git rm fileA.t fileB.t fileC.t fileD.t &&
+ printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
- D fileA.t
+ D fileA.t
D fileB.t
+ D fileC.t
+ D fileD.t
EOF
verify_expect
'
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git rm fileA.t &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- # Note: "git reset" has not yet learned to fail on wrong pathspecs
- git reset --pathspec-from-file=list --pathspec-file-nul &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- test_must_fail verify_expect
-'
-
test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
restore_checkpoint &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4a7c11368d..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,67 +49,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- cat >list <<-\EOF &&
- "file\101.t"
- EOF
-
- test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
` (2 preceding siblings ...)
2019-12-31 10:15 ` [PATCH v4 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
@ 2020-01-07 21:13 ` Junio C Hamano
2020-01-08 15:32 ` Alexandr Miloslavskiy
3 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2020-01-07 21:13 UTC (permalink / raw)
To: Alexandr Miloslavskiy via GitGitGadget
Cc: git, Jonathan Nieder, Eric Sunshine, Alexandr Miloslavskiy
"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
>
> Previously in [3] there were some concerns on whether removing
> copy&pasted tests is good. I still think that yes, it 's a good thing,
> mostly because of high volume of potential 13*6=78 duplicate tests.
>
> Still, I separated this change as last patch, so that the remaining
> part of the branch can be taken without it.
With the third step the series won't merge cleanly with other topic
you have in 'next' (t7107 gets somewhat heavy merge conflicts).
I'll queue the first two for now but let's clean them up post 2.25
release.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file
2020-01-07 21:13 ` [PATCH v4 0/3] t: rework " Junio C Hamano
@ 2020-01-08 15:32 ` Alexandr Miloslavskiy
2020-01-08 17:26 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Alexandr Miloslavskiy @ 2020-01-08 15:32 UTC (permalink / raw)
To: Junio C Hamano, Alexandr Miloslavskiy via GitGitGadget
Cc: git, Jonathan Nieder, Eric Sunshine
On 07.01.2020 22:13, Junio C Hamano wrote:
> With the third step the series won't merge cleanly with other topic
> you have in 'next' (t7107 gets somewhat heavy merge conflicts).
>
> I'll queue the first two for now but let's clean them up post 2.25
> release.
OK, I will re-submit the remaining patch after 2.25.
I will implement the next --pathspec-from-file patches as if this third
patch was accepted (that is, without copy&pasted tests).
Thanks for accepting this and other polishing branches, I was already
quite pessimistic about them.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file
2020-01-08 15:32 ` Alexandr Miloslavskiy
@ 2020-01-08 17:26 ` Junio C Hamano
2020-01-08 17:42 ` Alexandr Miloslavskiy
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2020-01-08 17:26 UTC (permalink / raw)
To: Alexandr Miloslavskiy
Cc: Alexandr Miloslavskiy via GitGitGadget, git, Jonathan Nieder,
Eric Sunshine
Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:
> On 07.01.2020 22:13, Junio C Hamano wrote:
>> With the third step the series won't merge cleanly with other topic
>> you have in 'next' (t7107 gets somewhat heavy merge conflicts).
>>
>> I'll queue the first two for now but let's clean them up post 2.25
>> release.
>
> OK, I will re-submit the remaining patch after 2.25.
>
> I will implement the next --pathspec-from-file patches as if this
> third patch was accepted (that is, without copy&pasted tests).
I am not sure if that is a good idea. I'd rather see the planned
new changes not to be taken hostage of the third step.
Besides, with the third step, your preference is not to test the
behaviour of end-user facing commands that would learn the option at
all and only test the underlying machinery with test-tool tests, no?
If you are not adding tests for the higher-level end-user facing
commands as part of these new series, would it make a difference if
the codebase has the third step applied (i.e. missing tests for the
end-user facing commands that have already learned the option) or
not (i.e. the commands that have already learned the option are
still tested end-to-end)?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file
2020-01-08 17:26 ` Junio C Hamano
@ 2020-01-08 17:42 ` Alexandr Miloslavskiy
2020-01-08 18:50 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Alexandr Miloslavskiy @ 2020-01-08 17:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Alexandr Miloslavskiy via GitGitGadget, git, Jonathan Nieder,
Eric Sunshine
On 08.01.2020 18:26, Junio C Hamano wrote:
>> I will implement the next --pathspec-from-file patches as if this
>> third patch was accepted (that is, without copy&pasted tests).
>
> I am not sure if that is a good idea. I'd rather see the planned
> new changes not to be taken hostage of the third step.
In my understanding, the new patches will not be taken hostage, they
will simply adopt the new approach. Everything will work just fine
whether or not third step is present.
> Besides, with the third step, your preference is not to test the
> behaviour of end-user facing commands that would learn the option at
> all and only test the underlying machinery with test-tool tests, no?
That's not exactly correct. Third step removes duplicate tests that give
no real benefit. With test-tool tests in place and succceeding, these
duplicate tests are super unlikely to fail.
I will still provide a few tests for every new command to make sure that
said command works as intended. I will only skip indirectly testing
global API again and again.
> If you are not adding tests for the higher-level end-user facing
> commands as part of these new series, would it make a difference if
> the codebase has the third step applied (i.e. missing tests for the
> end-user facing commands that have already learned the option) or
> not (i.e. the commands that have already learned the option are
> still tested end-to-end)?
I will be adding good tests and skip useless tests. For new commands, it
doesn't really matter if "third step" patch is applied or not.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 0/3] t: rework tests for --pathspec-from-file
2020-01-08 17:42 ` Alexandr Miloslavskiy
@ 2020-01-08 18:50 ` Junio C Hamano
0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2020-01-08 18:50 UTC (permalink / raw)
To: Alexandr Miloslavskiy
Cc: Alexandr Miloslavskiy via GitGitGadget, git, Jonathan Nieder,
Eric Sunshine
Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:
> I will still provide a few tests for every new command to make sure
> that said command works as intended. I will only skip indirectly
> testing global API again and again.
Ah, OK. Then leaving those removed by the third step there may get
in the way. So let's assume that we'll have an updated third step
already applied and your new series are written on top of it.
> ... For new commands, it doesn't really matter if "third step"
> patch is applied or not.
OK, again. Thanks for a clarification.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-01-08 18:50 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-30 17:42 [PATCH 0/3] t: rework tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 17:42 ` [PATCH 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-30 18:52 ` Junio C Hamano
2019-12-30 19:16 ` Alexandr Miloslavskiy
2019-12-30 17:42 ` [PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-30 21:55 ` Eric Sunshine
2019-12-31 0:26 ` Jonathan Nieder
2019-12-31 10:01 ` Alexandr Miloslavskiy
2019-12-30 19:15 ` [PATCH v2 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-30 19:15 ` [PATCH v2 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-31 9:53 ` [PATCH v3 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 0/3] t: rework " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 1/3] t: fix quotes " Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 2/3] t: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-31 10:15 ` [PATCH v4 3/3] t: drop copy&pasted tests for --pathspec-from-file Alexandr Miloslavskiy via GitGitGadget
2020-01-07 21:13 ` [PATCH v4 0/3] t: rework " Junio C Hamano
2020-01-08 15:32 ` Alexandr Miloslavskiy
2020-01-08 17:26 ` Junio C Hamano
2020-01-08 17:42 ` Alexandr Miloslavskiy
2020-01-08 18:50 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).