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

Mike Hommey [off-list ref] 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.

 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".

On Fri, Nov 16, 2007 at 10:26:58AM -0800, Junio C Hamano wrote:

Mike Hommey [off-list ref] writes:

quoted

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.

quoted

 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

This should probably be tested in another test block.

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".

Which is roughly what my patch does, except it doesn't check for
ordering.

Mike