git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 0/3] t: rework tests for --pathspec-from-file
Date: Tue, 31 Dec 2019 09:53:49 +0000	[thread overview]
Message-ID: <pull.503.v3.git.1577786032.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.503.v2.git.1577733329.gitgitgadget@gmail.com>

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

  parent reply	other threads:[~2019-12-31  9:53 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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   ` Alexandr Miloslavskiy via GitGitGadget [this message]
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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=pull.503.v3.git.1577786032.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

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

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