Subject: Re: [PATCH v3 4/6] [Newcomer] t7004-tag: do not prepare except outside test_expect_success

From: Junio C Hamano <[email-masked]>
Date: Sun, 04 Aug 2024 09:46:55 -0700

There are other "prepare for next test" remaining even after
applying this patch, though.

> diff --git c/t/t7004-tag.sh w/t/t7004-tag.sh
> index cfe7653317..96aa908eaf 100755
> --- c/t/t7004-tag.sh
> +++ w/t/t7004-tag.sh
> ...
> -cat >expect <<EOF
> -mytag
> -EOF
>  test_expect_success 'Multiple -l or --list options are equivalent to one -l option' '
> +	cat >expect <<-\EOF &&
> +	mytag
> +	EOF
>  	git tag -l -l >actual &&
>  	test_cmp expect actual &&
>  	git tag --list --list >actual &&
>  	test_cmp expect actual &&
>  	git tag --list -l --list >actual &&
>  	test_cmp expect actual
>  '

Good.

> -cat >expect <<EOF
> -myhead
> -mytag
> -EOF
>  test_expect_success \
>  	'trying to delete tags without params should succeed and do nothing' '
> +	cat >expect <<-\EOF &&
> +	myhead
> +	mytag
> +	EOF
>  	git tag -l >actual &&
>  	test_cmp expect actual &&
>  	git tag -d &&
>  	git tag -l >actual &&
>  	test_cmp expect actual
>  '

Good too.

>  # creating annotated tags:
>  
>  get_tag_msg () {
>  	git cat-file tag "$1" | sed -e "/BEGIN PGP/q"
>  }
>  
>  # run test_tick before committing always gives the time in that timezone
>  get_tag_header () {
>  cat <<EOF
>  object $2
>  type $3
>  tag $1
>  tagger C O Mitter <[email-masked]> $4 -0700
>  
>  EOF
>  }
>
>  commit=$(git rev-parse HEAD)
>  time=$test_tick
>  
>  get_tag_header annotated-tag $commit commit $time >expect
>  echo "A message" >>expect
>  test_expect_success \
>  	'creating an annotated tag with -m message should succeed' '
>  	git tag -m "A message" annotated-tag &&
>  	get_tag_msg annotated-tag >actual &&
>  	test_cmp expect actual
>  '

The creation of $commit, holding the timestamp in $time, calling the
get_tag_header helper helper function, and addition of the message
to 'expect' file.  Moving all of them inside expect_success block is
perfectly in line with the theme of this step.

>  get_tag_header annotated-tag-edit $commit commit $time >expect
>  echo "An edited message" >>expect
>  test_expect_success 'set up editor' '
>  	write_script fakeeditor <<-\EOF
>  	sed -e "s/A message/An edited message/g" <"$1" >"$1-"
>  	mv "$1-" "$1"
>  	EOF
>  '
>  test_expect_success \
> ...

Ditto here.  In addition, there is a blank line lacking after the
above test piece and before the next one.

>  cat >msgfile <<EOF
>  Another message
>  in a file.
>  EOF
>  get_tag_header file-annotated-tag $commit commit $time >expect
>  cat msgfile >>expect
>  test_expect_success \
>  	'creating an annotated tag with -F messagefile should succeed' '
>  	git tag -F msgfile file-annotated-tag &&
>  	get_tag_msg file-annotated-tag >actual &&
>  	test_cmp expect actual
>  '

Ditto.

>  get_tag_header file-annotated-tag-edit $commit commit $time >expect
>  sed -e "s/Another message/Another edited message/g" msgfile >>expect
>  test_expect_success 'set up editor' '
>  	write_script fakeeditor <<-\EOF
>  	sed -e "s/Another message/Another edited message/g" <"$1" >"$1-"
>  	mv "$1-" "$1"
>  	EOF
>  '
>  test_expect_success \
>  	'creating an annotated tag with -F messagefile --edit should succeed' '
>  	GIT_EDITOR=./fakeeditor git tag -F msgfile --edit file-annotated-tag-edit &&
>  	get_tag_msg file-annotated-tag-edit >actual &&
>  	test_cmp expect actual
>  '
>  
>  cat >inputmsg <<EOF
>  A message from the
>  standard input
>  EOF
>  get_tag_header stdin-annotated-tag $commit commit $time >expect
>  cat inputmsg >>expect
>  test_expect_success 'creating an annotated tag with -F - should succeed' '
>  	git tag -F - stdin-annotated-tag <inputmsg &&
>  	get_tag_msg stdin-annotated-tag >actual &&
>  	test_cmp expect actual
>  '

Ditto.

>  # blank and empty messages:
>  
>  get_tag_header empty-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with an empty -m message should succeed' '
>  	git tag -m "" empty-annotated-tag &&
>  	get_tag_msg empty-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  >emptyfile
>  get_tag_header emptyfile-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with an empty -F messagefile should succeed' '
>  	git tag -F emptyfile emptyfile-annotated-tag &&
>  	get_tag_msg emptyfile-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  printf '\n\n  \n\t\nLeading blank lines\n' >blanksfile
>  printf '\n\t \t  \nRepeated blank lines\n' >>blanksfile
>  printf '\n\n\nTrailing spaces      \t  \n' >>blanksfile
>  printf '\nTrailing blank lines\n\n\t \n\n' >>blanksfile
>  get_tag_header blanks-annotated-tag $commit commit $time >expect
>  cat >>expect <<EOF
>  Leading blank lines
>  
>  Repeated blank lines
>  
>  Trailing spaces
>  
>  Trailing blank lines
>  EOF
>  test_expect_success \
>  	'extra blanks in the message for an annotated tag should be removed' '
>  	git tag -F blanksfile blanks-annotated-tag &&
>  	get_tag_msg blanks-annotated-tag >actual &&
>  	test_cmp expect actual
>  '

Ditto.

>  get_tag_header blank-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with blank -m message with spaces should succeed' '
>  	git tag -m "     " blank-annotated-tag &&
>  	get_tag_msg blank-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  echo '     ' >blankfile
>  echo ''      >>blankfile
>  echo '  '    >>blankfile
>  get_tag_header blankfile-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with blank -F messagefile with spaces should succeed' '
>  	git tag -F blankfile blankfile-annotated-tag &&
>  	get_tag_msg blankfile-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  printf '      ' >blanknonlfile
>  get_tag_header blanknonlfile-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with -F file of spaces and no newline should succeed' '
>  	git tag -F blanknonlfile blanknonlfile-annotated-tag &&
>  	get_tag_msg blanknonlfile-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  # messages with commented lines:
>  
>  cat >commentsfile <<EOF
>  # A comment
>  
>  ############
>  The message.
>  ############
>  One line.
>  
>  
>  # commented lines
>  # commented lines
>  
>  Another line.
>  # comments
>  
>  Last line.
>  EOF
>  get_tag_header comments-annotated-tag $commit commit $time >expect
>  cat >>expect <<EOF
>  The message.
>  One line.
>  
>  Another line.
>  
>  Last line.
>  EOF
>  test_expect_success \
>  	'creating a tag using a -F messagefile with #comments should succeed' '
>  	git tag -F commentsfile comments-annotated-tag &&
>  	get_tag_msg comments-annotated-tag >actual &&
>  	test_cmp expect actual
>  '

Ditto.

>  get_tag_header comment-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with a #comment in the -m message should succeed' '
>  	git tag -m "#comment" comment-annotated-tag &&
>  	get_tag_msg comment-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  echo '#comment' >commentfile
>  echo ''         >>commentfile
>  echo '####'     >>commentfile
>  get_tag_header commentfile-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with #comments in the -F messagefile should succeed' '
>  	git tag -F commentfile commentfile-annotated-tag &&
>  	get_tag_msg commentfile-annotated-tag >actual &&
>  	test_cmp expect actual
>  '
>  
>  printf '#comment' >commentnonlfile
>  get_tag_header commentnonlfile-annotated-tag $commit commit $time >expect
>  test_expect_success \
>  	'creating a tag with a file of #comment and no newline should succeed' '
>  	git tag -F commentnonlfile commentnonlfile-annotated-tag &&
>  	get_tag_msg commentnonlfile-annotated-tag >actual &&
>  	test_cmp expect actual
>  '

Ditto.

>  echo 'tag line one' >annotagmsg
>  echo 'tag line two' >>annotagmsg
>  echo 'tag line three' >>annotagmsg
>  test_expect_success \
>  	'listing many message lines of a non-signed tag should succeed' '
>  	git tag -F annotagmsg tag-lines &&
>  ...

Ditto.

>  # creating and verifying signed tags:
>  
>  get_tag_header signed-tag $commit commit $time >expect
>  echo 'A signed tag message' >>expect
>  echo '-----BEGIN PGP SIGNATURE-----' >>expect
>  test_expect_success GPG 'creating a signed tag with -m message should succeed' '
>  	git tag -s -m "A signed tag message" signed-tag &&
>  	get_tag_msg signed-tag >actual &&
>  	test_cmp expect actual
>  '

Ditto.

>  get_tag_header u-signed-tag $commit commit $time >expect
>  echo 'Another message' >>expect
>  echo '-----BEGIN PGP SIGNATURE-----' >>expect
>  test_expect_success GPG 'sign with a given key id' '
>  
>  	git tag -u [email-masked] -m "Another message" u-signed-tag &&
>  	get_tag_msg u-signed-tag >actual &&
>  	test_cmp expect actual
>  
>  '

Ditto.

If it is too much to include all of the above in the same patch I
think it would be OK to stop at "move prepatation of the expect file
using cat and here-doc into test_expect_success block", as long as
we make it clear in the log message that we are drawing the line
there in the patch, and leaving other similar issues untouched to
keep the step manageably short.  Adding a new patch to clean up the
"other similar issues" is entirely optional, but doing so would make
the series even more thorough.

Thanks.