Re: [PATCH] Fix t7004 which fails with retarded sed

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2007, 18:26 UTC
Message-ID: <7vpryaxczx.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1195233971-12288-1-git-send-email-mh@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> Brown paper bag fix to avoid test failure with retarded sed. The test
> by itself didn't catch what it was supposed to, anyways.

I'd rather not to say "retarded sed". It is not fair to blame the implementation when the script we feed to it is not portable.

Show 7 quoted lines

>  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
> +	grep -e "^$" actual > /dev/null 2>&1 &&
> +	grep -e "^#" actual > /dev/null 2>&1 &&
> +	! grep -e "^[^#]" actual > /dev/null 2>&1

The test simulates a case where

	$ git tag -a initial-comment

is run and the user exits the editor without editing the file to add any text to annotate the tag. The behaviour we want from such an invocation is (1) the user sees a sensible template in the editor, and (2) the command to error out, saying "Hey, you did not leave any message for us to use".

So the earlier part of the test

	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&

is already bogus with respect to the latter point. It will happily continue if "git tag" erroneously returns success, and the above does not catch it.

	if GIT_EDITOR=cat git tag -a initial-comment >actual
        then
        	echo >&2 oops we should have errored out
                false
	else
        	: happy -- anything else we want to check?
	fi

The "actual" file contains what the user saw in the editor and returned to the "git tag" command.

The template we give to the user begins with a blank line, followed by a brief instruction "write a tag message" as comments to be stripped. The test is trying to make sure that is what was given to the user. As you already discussed in the thread, the exact wording may change in the future and we would not want to adjust the test every time.

I think the important points about this template are:

 * 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 "#".  I do not think we
   would want to check the wording of this instruction.

 * And it has nothing else, as the expected behaviour is "Hey
   you did not leave any message".