From: Benoit Sigoure Date: Fri, 16 Nov 2007 21:04:57 GMT Subject: Re: [PATCH] Fix and improve t7004 Message-ID: In-Reply-To: <1195244917-25659-1-git-send-email-mh@glandium.org> On Nov 16, 2007, at 9:28 PM, Mike Hommey wrote: > 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 > --- > 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