Re: [PATCH] Fix and improve t7004

From: Benoit Sigoure <tsuna@lrde.epita.fr>
Date: Nov 16, 2007, 21:04 UTC
Message-ID: <EEE710FA-0408-489F-8128-B4C1F06D34FF@lrde.epita.fr>
In-Reply-To: <1195244917-25659-1-git-send-email-mh@glandium.org>

On Nov 16, 2007, at 9:28 PM, Mike Hommey wrote:

Show 41 quoted lines

> Brown paper bag fix to avoid using non portable sed syntax. The
> test by itself didn't catch what it was supposed to, anyways.
>
> The new test first checks whether the user exited the editor
> without editing the file, then whether what the user was
> presented in the editor was any useful to her, which we define
> as the following:
>  * It begins with a single blank line, where the invoked editor
>    would typically place the editing curser at so that the user
>    can immediately start typing;
>
>  * It has some instruction but that comes after that initial
>    blank line, all lines prefixed with "#".
>
>  * And it has nothing else, as the expected behaviour is "Hey
>    you did not leave any message".
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  t/t7004-tag.sh |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 096fe33..42b1f97 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1004,10 +1004,17 @@ test_expect_failure \
>  	'verify signed tag fails when public key is not present' \
>  	'git-tag -v signed-tag'
>
> +test_expect_failure \
> +	'git-tag -a fails if tag annotation is empty' '
> +	GIT_EDITOR=cat git tag -a initial-comment > /dev/null 2>&1
> +'
> +
>  test_expect_success \
>  	'message in editor has initial comment' '
>  	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
> -	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
> +	( read empty ;
> +	  [ "$empty" ] && exit 1 ;

What is this meant to do?  Did you mean [ -n "$empty" ] ?

> +	  ! grep -ve "^#" > /dev/null 2>&1 ) < actual

The double negation is harder to read.  May I suggest something along  
these lines (which seems more readable to me):
   while read line; do
     case $line in #(
       '#'*) ;; # Accept comments (
       *) exit 1;;
     esac
   done

Advantages:
   The purpose of the test is more obvious
   The test is more easily extendable
   The test saves a fork ;o

>  '
>
>  get_tag_header reuse $commit commit $time >expect

Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory