git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, sunshine@sunshineco.com,
	martin.agren@gmail.com, szeder.dev@gmail.com,
	Elijah Newren <newren@gmail.com>
Subject: [PATCHv4 0/5] Simple fixes to t7406
Date: Wed,  8 Aug 2018 09:31:02 -0700	[thread overview]
Message-ID: <20180808163107.12329-1-newren@gmail.com> (raw)
In-Reply-To: <20180807164905.3859-1-newren@gmail.com>

Changes since v3, all suggested/endorsed by Junio (range-diff at the end):
  - Moved the actual fix from being last patch in the series to the first
    (other patches in this series are just test code cleanups)
  - Anchored regexes to avoid matching another filename as a substring
  - Add test_path_exists() to test-lib-function.sh and use it (we had
    test_path_is_dir, test_path_is_file, and test_path_is_missing, but
    not simple test_path_exists)

Elijah Newren (5):
  t7406: fix call that was failing for the wrong reason
  t7406: simplify by using diff --name-only instead of diff --raw
  t7406: avoid having git commands upstream of a pipe
  t7406: prefer test_* helper functions to test -[feds]
  t7406: avoid using test_must_fail for commands other than git

 t/t7406-submodule-update.sh | 37 +++++++++++++++++++++++--------------
 t/test-lib-functions.sh     |  8 ++++++++
 2 files changed, 31 insertions(+), 14 deletions(-)

-:  ---------- > 1:  5f257af6c8 t7406: fix call that was failing for the wrong reason
1:  3c369bf73d ! 2:  9e5400a1ad t7406: simplify by using diff --name-only instead of diff --raw
    @@ -16,10 +16,10 @@
      	  compare_head
      	 ) &&
     -	 git diff --raw | grep "	submodule" &&
    -+	 git diff --name-only | grep submodule &&
    ++	 git diff --name-only | grep ^submodule$ &&
      	 git submodule update &&
     -	 git diff --raw | grep "	submodule" &&
    -+	 git diff --name-only | grep submodule &&
    ++	 git diff --name-only | grep ^submodule$ &&
      	 (cd submodule &&
      	  compare_head
      	 ) &&
    @@ -28,10 +28,12 @@
      	  compare_head
      	 ) &&
     -	 git diff --raw | grep "	submodule" &&
    -+	 git diff --name-only | grep submodule &&
    ++	 git diff --name-only | grep ^submodule$ &&
      	 git submodule update --checkout &&
    --	 test_must_fail git diff --raw \| grep "	submodule" &&
    -+	 test_must_fail git diff --name-only \| grep submodule &&
    +-	 git diff --raw >out &&
    +-	 ! grep "	submodule" out &&
    ++	 git diff --name-only >out &&
    ++	 ! grep ^submodule$ out &&
      	 (cd submodule &&
      	  test_must_fail compare_head
      	 ) &&
2:  ba50d6b0f3 ! 3:  4e8cdf60f4 t7406: avoid having git commands upstream of a pipe
    @@ -26,13 +26,13 @@
      	  git checkout master &&
      	  compare_head
      	 ) &&
    --	 git diff --name-only | grep submodule &&
    +-	 git diff --name-only | grep ^submodule$ &&
     +	 git diff --name-only >out &&
    -+	 grep submodule out &&
    ++	 grep ^submodule$ out &&
      	 git submodule update &&
    --	 git diff --name-only | grep submodule &&
    +-	 git diff --name-only | grep ^submodule$ &&
     +	 git diff --name-only >out &&
    -+	 grep submodule out &&
    ++	 grep ^submodule$ out &&
      	 (cd submodule &&
      	  compare_head
      	 ) &&
    @@ -40,12 +40,12 @@
      	  git checkout master &&
      	  compare_head
      	 ) &&
    --	 git diff --name-only | grep submodule &&
    +-	 git diff --name-only | grep ^submodule$ &&
     +	 git diff --name-only >out &&
    -+	 grep submodule out &&
    ++	 grep ^submodule$ out &&
      	 git submodule update --checkout &&
    - 	 test_must_fail git diff --name-only \| grep submodule &&
    - 	 (cd submodule &&
    +	 git diff --name-only >out &&
    +	 ! grep ^submodule$ out &&
     @@
      	 H=$(git rev-parse --short HEAD) &&
      	 git commit -am "pre move" &&
3:  42f7b7f225 ! 4:  f171cbcc9a t7406: prefer test_* helper functions to test -[feds]
    @@ -6,13 +6,12 @@
         test failures, so use the test_* helper functions from
         test-lib-functions.sh.
     
    -    Note: The use of 'test_path_is_file submodule/.git' may look odd, but
    -    it is a file which is populated with a
    +    Also, add test_path_exists() to test-lib-function.sh while at it, so
    +    that we don't need to worry whether submodule/.git is a file or a
    +    directory.  It currently is a file with contents of the form
            gitdir: ../.git/modules/submodule
    -    directive.  If, in the future, handling of the submodule is changed and
    -    submodule/.git becomes a directory we can change this to
    -    test_path_is_dir (or perhaps write a test_path_exists helper function
    -    that doesn't care whether the path is a file or a directory).
    +    but it could be changed in the future to be a directory; this test
    +    only really cares that it exists.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
     
    @@ -34,8 +33,27 @@
      	 git submodule update --init &&
     -	 test -e submodule/.git &&
     -	 test_must_fail test -e none/.git
    -+	 test_path_is_file submodule/.git &&
    ++	 test_path_exists submodule/.git &&
     +	 test_path_is_missing none/.git
      	)
      '
      
    +
    +diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
    +--- a/t/test-lib-functions.sh
    ++++ b/t/test-lib-functions.sh
    +@@
    +	fi
    + }
    +
    ++test_path_exists () {
    ++	if ! test -e "$1"
    ++	then
    ++		echo "Path $1 doesn't exist. $2"
    ++		false
    ++	fi
    ++}
    ++
    + # Check if the directory exists and is empty as expected, barf otherwise.
    + test_dir_is_empty () {
    +	test_path_is_dir "$1" &&
4:  54cf6531ec ! 5:  a44c566321 t7406: avoid using test_must_fail for commands other than git
    @@ -8,8 +8,8 @@
     --- a/t/t7406-submodule-update.sh
     +++ b/t/t7406-submodule-update.sh
     @@
    - 	 git submodule update --checkout &&
    - 	 test_must_fail git diff --name-only \| grep submodule &&
    +	 git diff --name-only >out &&
    +	 ! grep ^submodule$ out &&
      	 (cd submodule &&
     -	  test_must_fail compare_head
     +	  ! compare_head
5:  3019f2d01c < -:  ---------- t7406: fix call that was failing for the wrong reason

-- 
2.18.0.556.g1604670984

  parent reply	other threads:[~2018-08-08 16:31 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03 23:14 [PATCH] t7406: Make a test_must_fail call fail for the right reason Elijah Newren
2018-08-03 23:40 ` Eric Sunshine
2018-08-03 23:42   ` Eric Sunshine
2018-08-03 23:41 ` Junio C Hamano
2018-08-06 15:25 ` [PATCH 0/2] Simple fixes to t7406 Elijah Newren
2018-08-06 15:25   ` [PATCH 1/3] t7406: simplify by using diff --name-only instead of diff --raw Elijah Newren
2018-08-06 15:25   ` [PATCH 2/3] t7406: avoid having git commands upstream of a pipe Elijah Newren
2018-08-06 15:48     ` SZEDER Gábor
2018-08-06 16:09       ` Elijah Newren
2018-08-06 15:25   ` [PATCH 3/3] t7406: make a test_must_fail call fail for the right reason Elijah Newren
2018-08-07  9:07     ` SZEDER Gábor
2018-08-07 13:46       ` Elijah Newren
2018-08-07  7:50   ` [PATCH 0/2] Simple fixes to t7406 Martin Ågren
2018-08-07 13:40     ` Elijah Newren
2018-08-07 16:49   ` [PATCHv3 0/5] " Elijah Newren
2018-08-07 16:49     ` [PATCHv3 1/5] t7406: simplify by using diff --name-only instead of diff --raw Elijah Newren
2018-08-07 17:29       ` Junio C Hamano
2018-08-07 17:40         ` Elijah Newren
2018-08-07 18:18           ` Junio C Hamano
2018-08-07 16:49     ` [PATCHv3 2/5] t7406: avoid having git commands upstream of a pipe Elijah Newren
2018-08-07 16:49     ` [PATCHv3 3/5] t7406: prefer test_* helper functions to test -[feds] Elijah Newren
2018-08-07 17:34       ` Junio C Hamano
2018-08-07 16:49     ` [PATCHv3 4/5] t7406: avoid using test_must_fail for commands other than git Elijah Newren
2018-08-07 16:49     ` [PATCHv3 5/5] t7406: fix call that was failing for the wrong reason Elijah Newren
2018-08-07 17:37       ` Junio C Hamano
2018-08-08 16:31     ` Elijah Newren [this message]
2018-08-08 16:31       ` [PATCHv4 1/5] " Elijah Newren
2018-08-08 16:31       ` [PATCHv4 2/5] t7406: simplify by using diff --name-only instead of diff --raw Elijah Newren
2018-08-08 16:31       ` [PATCHv4 3/5] t7406: avoid having git commands upstream of a pipe Elijah Newren
2018-08-08 16:31       ` [PATCHv4 4/5] t7406: prefer test_* helper functions to test -[feds] Elijah Newren
2018-08-08 16:31       ` [PATCHv4 5/5] t7406: avoid using test_must_fail for commands other than git Elijah Newren
2018-08-13 20:28     ` [PATCHv3 0/5] Simple fixes to t7406 SZEDER Gábor
2018-08-18 20:52       ` Elijah Newren

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=20180808163107.12329-1-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.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).