{"thread":{"id":"61889","subject":"[PATCH][Newcomer] t7004-tag: modernize the test script","startedAt":"2024-08-02T06:47:48Z","lastAt":"2024-08-03T16:29:47Z","messageCount":4,"participants":["AbdAlRahman Gad","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"499935","messageId":"20240802064719.513498-1-abdobngad@gmail.com","threadId":"61889","inReplyTo":null,"subject":"[PATCH][Newcomer] t7004-tag: modernize the test script","fromName":"AbdAlRahman Gad","fromEmail":"abdobngad@gmail.com","sentAt":"2024-08-02T06:47:19Z","receivedAt":"2024-08-02T06:47:48Z","isPatch":true,"sender":{"key":"abdobngad@gmail.com","avatar":"https://avatars.githubusercontent.com/u/89566409?v=4"},"body":"Modernaize 't7004-tag.sh' by removing whitespace\nafter the redirect operators.\n\nSigned-off-by: AbdAlRahman Gad <abdobngad@gmail.com>\n---\n t/t7004-tag.sh | 50 +++++++++++++++++++++++++-------------------------\n 1 file changed, 25 insertions(+), 25 deletions(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex fa6336edf9..3100a4c219 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -213,9 +213,9 @@ mytag\n EOF\n test_expect_success \\\n \t'trying to delete tags without params should succeed and do nothing' '\n-\tgit tag -l > actual && test_cmp expect actual &&\n+\tgit tag -l >actual && test_cmp expect actual &&\n \tgit tag -d &&\n-\tgit tag -l > actual && test_cmp expect actual\n+\tgit tag -l >actual && test_cmp expect actual\n '\n \n test_expect_success \\\n@@ -269,9 +269,9 @@ test_expect_success 'listing all tags should print them ordered' '\n \tgit tag a1 &&\n \tgit tag v1.0 &&\n \tgit tag t210 &&\n-\tgit tag -l > actual &&\n+\tgit tag -l >actual &&\n \ttest_cmp expect actual &&\n-\tgit tag > actual &&\n+\tgit tag >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -283,7 +283,7 @@ EOF\n test_expect_success \\\n \t'listing tags with substring as pattern must print those matching' '\n \trm *a* &&\n-\tgit tag -l \"*a*\" > current &&\n+\tgit tag -l \"*a*\" >current &&\n \ttest_cmp expect current\n '\n \n@@ -293,7 +293,7 @@ v1.0.1\n EOF\n test_expect_success \\\n \t'listing tags with a suffix as pattern must print those matching' '\n-\tgit tag -l \"*.1\" > actual &&\n+\tgit tag -l \"*.1\" >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -303,7 +303,7 @@ t211\n EOF\n test_expect_success \\\n \t'listing tags with a prefix as pattern must print those matching' '\n-\tgit tag -l \"t21*\" > actual &&\n+\tgit tag -l \"t21*\" >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -312,7 +312,7 @@ a1\n EOF\n test_expect_success \\\n \t'listing tags using a name as pattern must print that one matching' '\n-\tgit tag -l a1 > actual &&\n+\tgit tag -l a1 >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -321,7 +321,7 @@ v1.0\n EOF\n test_expect_success \\\n \t'listing tags using a name as pattern must print that one matching' '\n-\tgit tag -l v1.0 > actual &&\n+\tgit tag -l v1.0 >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -331,13 +331,13 @@ v1.1.3\n EOF\n test_expect_success \\\n \t'listing tags with ? in the pattern should print those matching' '\n-\tgit tag -l \"v1.?.?\" > actual &&\n+\tgit tag -l \"v1.?.?\" >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success \\\n \t'listing tags using v.* should print nothing because none have v.' '\n-\tgit tag -l \"v.*\" > actual &&\n+\tgit tag -l \"v.*\" >actual &&\n \ttest_must_be_empty actual\n '\n \n@@ -349,7 +349,7 @@ v1.1.3\n EOF\n test_expect_success \\\n \t'listing tags using v* should print only those having v' '\n-\tgit tag -l \"v*\" > actual &&\n+\tgit tag -l \"v*\" >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -1171,7 +1171,7 @@ test_expect_success GPG \\\n \tgit tag -v emptyfile-signed-tag\n '\n \n-printf '\\n\\n  \\n\\t\\nLeading blank lines\\n' > sigblanksfile\n+printf '\\n\\n  \\n\\t\\nLeading blank lines\\n' >sigblanksfile\n printf '\\n\\t \\t  \\nRepeated blank lines\\n' >>sigblanksfile\n printf '\\n\\n\\nTrailing spaces      \\t  \\n' >>sigblanksfile\n printf '\\nTrailing blank lines\\n\\n\\t \\n\\n' >>sigblanksfile\n@@ -1569,7 +1569,7 @@ test_expect_success \\\n \n test_expect_success \\\n \t'message in editor has initial comment' '\n-\t! (GIT_EDITOR=cat git tag -a initial-comment > actual)\n+\t! (GIT_EDITOR=cat git tag -a initial-comment >actual)\n '\n \n test_expect_success 'message in editor has initial comment: first line' '\n@@ -1587,7 +1587,7 @@ test_expect_success \\\n '\n \n get_tag_header reuse $commit commit $time >expect\n-echo \"An annotation to be reused\" >> expect\n+echo \"An annotation to be reused\" >>expect\n test_expect_success \\\n \t'overwriting an annotated tag should use its previous body' '\n \tgit tag -a -m \"An annotation to be reused\" reuse &&\n@@ -1639,7 +1639,7 @@ hash3=$(git rev-parse HEAD)\n \n # simple linear checks of --continue\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v0.2.1\n v1.0\n v1.0.1\n@@ -1679,7 +1679,7 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n \ttest_must_be_empty actual\n \"\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v2.0\n EOF\n \n@@ -1688,7 +1688,7 @@ test_expect_success 'checking that second commit only has one tag' \"\n \ttest_cmp expected actual\n \"\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v0.2.1\n v1.0\n v1.0.1\n@@ -1705,7 +1705,7 @@ test_expect_success 'checking that third commit has no tags' \"\n \ttest_must_be_empty actual\n \"\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v0.2.1\n v1.0\n v1.0.1\n@@ -1723,14 +1723,14 @@ test_expect_success 'conversely --no-contains on the third commit lists all tags\n test_expect_success 'creating simple branch' '\n \tgit branch stable v2.0 &&\n         git checkout stable &&\n-\techo foo-3.0 > foo &&\n+\techo foo-3.0 >foo &&\n \tgit commit foo -m fourth &&\n \tgit tag v3.0\n '\n \n hash4=$(git rev-parse HEAD)\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v3.0\n EOF\n \n@@ -1739,7 +1739,7 @@ test_expect_success 'checking that branch head only has one tag' \"\n \ttest_cmp expected actual\n \"\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v0.2.1\n v1.0\n v1.0.1\n@@ -1757,7 +1757,7 @@ test_expect_success 'merging original branch into this branch' '\n         git tag v4.0\n '\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v4.0\n EOF\n \n@@ -1766,7 +1766,7 @@ test_expect_success 'checking that original branch head has one tag now' \"\n \ttest_cmp expected actual\n \"\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v0.2.1\n v1.0\n v1.0.1\n@@ -1780,7 +1780,7 @@ test_expect_success 'checking that original branch head with --no-contains lists\n \ttest_cmp expected actual\n \"\n \n-cat > expected <<EOF\n+cat >expected <<EOF\n v0.2.1\n v1.0\n v1.0.1\n-- \n2.43.0\n\n"},{"id":"499956","messageId":"xmqqttg2ewqm.fsf@gitster.g","threadId":"61889","inReplyTo":"20240802064719.513498-1-abdobngad@gmail.com","subject":"Re: [PATCH][Newcomer] t7004-tag: modernize the test script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-02T16:05:21Z","receivedAt":"2024-08-02T16:05:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"AbdAlRahman Gad <abdobngad@gmail.com> writes:\n\n> Modernaize 't7004-tag.sh' by removing whitespace\n> after the redirect operators.\n\nIt is a good start.  There are other modernization opportunities in\nthis file, though.\n\n - Output from \"test-tool ref-store\" piped to \"sed\" means the exit\n   status from an abnormal exit of \"test-tool\" is hidden.  They\n   should be split into two commands.\n\n - Expected output file prepared outside test_expect_success that\n   uses it.\n\n - Here-doc that does not interpolate leaving the EOF marker\n   unquoted.\n\n> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>\n> ---\n>  t/t7004-tag.sh | 50 +++++++++++++++++++++++++-------------------------\n>  1 file changed, 25 insertions(+), 25 deletions(-)\n>\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index fa6336edf9..3100a4c219 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -213,9 +213,9 @@ mytag\n>  EOF\n>  test_expect_success \\\n>  \t'trying to delete tags without params should succeed and do nothing' '\n> -\tgit tag -l > actual && test_cmp expect actual &&\n> +\tgit tag -l >actual && test_cmp expect actual &&\n>  \tgit tag -d &&\n> -\tgit tag -l > actual && test_cmp expect actual\n> +\tgit tag -l >actual && test_cmp expect actual\n>  '\n>  \n>  test_expect_success \\\n> @@ -269,9 +269,9 @@ test_expect_success 'listing all tags should print them ordered' '\n>  \tgit tag a1 &&\n>  \tgit tag v1.0 &&\n>  \tgit tag t210 &&\n> -\tgit tag -l > actual &&\n> +\tgit tag -l >actual &&\n>  \ttest_cmp expect actual &&\n> -\tgit tag > actual &&\n> +\tgit tag >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -283,7 +283,7 @@ EOF\n>  test_expect_success \\\n>  \t'listing tags with substring as pattern must print those matching' '\n>  \trm *a* &&\n> -\tgit tag -l \"*a*\" > current &&\n> +\tgit tag -l \"*a*\" >current &&\n>  \ttest_cmp expect current\n>  '\n>  \n> @@ -293,7 +293,7 @@ v1.0.1\n>  EOF\n>  test_expect_success \\\n>  \t'listing tags with a suffix as pattern must print those matching' '\n> -\tgit tag -l \"*.1\" > actual &&\n> +\tgit tag -l \"*.1\" >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -303,7 +303,7 @@ t211\n>  EOF\n>  test_expect_success \\\n>  \t'listing tags with a prefix as pattern must print those matching' '\n> -\tgit tag -l \"t21*\" > actual &&\n> +\tgit tag -l \"t21*\" >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -312,7 +312,7 @@ a1\n>  EOF\n>  test_expect_success \\\n>  \t'listing tags using a name as pattern must print that one matching' '\n> -\tgit tag -l a1 > actual &&\n> +\tgit tag -l a1 >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -321,7 +321,7 @@ v1.0\n>  EOF\n>  test_expect_success \\\n>  \t'listing tags using a name as pattern must print that one matching' '\n> -\tgit tag -l v1.0 > actual &&\n> +\tgit tag -l v1.0 >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -331,13 +331,13 @@ v1.1.3\n>  EOF\n>  test_expect_success \\\n>  \t'listing tags with ? in the pattern should print those matching' '\n> -\tgit tag -l \"v1.?.?\" > actual &&\n> +\tgit tag -l \"v1.?.?\" >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n>  test_expect_success \\\n>  \t'listing tags using v.* should print nothing because none have v.' '\n> -\tgit tag -l \"v.*\" > actual &&\n> +\tgit tag -l \"v.*\" >actual &&\n>  \ttest_must_be_empty actual\n>  '\n>  \n> @@ -349,7 +349,7 @@ v1.1.3\n>  EOF\n>  test_expect_success \\\n>  \t'listing tags using v* should print only those having v' '\n> -\tgit tag -l \"v*\" > actual &&\n> +\tgit tag -l \"v*\" >actual &&\n>  \ttest_cmp expect actual\n>  '\n>  \n> @@ -1171,7 +1171,7 @@ test_expect_success GPG \\\n>  \tgit tag -v emptyfile-signed-tag\n>  '\n>  \n> -printf '\\n\\n  \\n\\t\\nLeading blank lines\\n' > sigblanksfile\n> +printf '\\n\\n  \\n\\t\\nLeading blank lines\\n' >sigblanksfile\n>  printf '\\n\\t \\t  \\nRepeated blank lines\\n' >>sigblanksfile\n>  printf '\\n\\n\\nTrailing spaces      \\t  \\n' >>sigblanksfile\n>  printf '\\nTrailing blank lines\\n\\n\\t \\n\\n' >>sigblanksfile\n> @@ -1569,7 +1569,7 @@ test_expect_success \\\n>  \n>  test_expect_success \\\n>  \t'message in editor has initial comment' '\n> -\t! (GIT_EDITOR=cat git tag -a initial-comment > actual)\n> +\t! (GIT_EDITOR=cat git tag -a initial-comment >actual)\n>  '\n>  \n>  test_expect_success 'message in editor has initial comment: first line' '\n> @@ -1587,7 +1587,7 @@ test_expect_success \\\n>  '\n>  \n>  get_tag_header reuse $commit commit $time >expect\n> -echo \"An annotation to be reused\" >> expect\n> +echo \"An annotation to be reused\" >>expect\n>  test_expect_success \\\n>  \t'overwriting an annotated tag should use its previous body' '\n>  \tgit tag -a -m \"An annotation to be reused\" reuse &&\n> @@ -1639,7 +1639,7 @@ hash3=$(git rev-parse HEAD)\n>  \n>  # simple linear checks of --continue\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v0.2.1\n>  v1.0\n>  v1.0.1\n> @@ -1679,7 +1679,7 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n>  \ttest_must_be_empty actual\n>  \"\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v2.0\n>  EOF\n>  \n> @@ -1688,7 +1688,7 @@ test_expect_success 'checking that second commit only has one tag' \"\n>  \ttest_cmp expected actual\n>  \"\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v0.2.1\n>  v1.0\n>  v1.0.1\n> @@ -1705,7 +1705,7 @@ test_expect_success 'checking that third commit has no tags' \"\n>  \ttest_must_be_empty actual\n>  \"\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v0.2.1\n>  v1.0\n>  v1.0.1\n> @@ -1723,14 +1723,14 @@ test_expect_success 'conversely --no-contains on the third commit lists all tags\n>  test_expect_success 'creating simple branch' '\n>  \tgit branch stable v2.0 &&\n>          git checkout stable &&\n> -\techo foo-3.0 > foo &&\n> +\techo foo-3.0 >foo &&\n>  \tgit commit foo -m fourth &&\n>  \tgit tag v3.0\n>  '\n>  \n>  hash4=$(git rev-parse HEAD)\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v3.0\n>  EOF\n>  \n> @@ -1739,7 +1739,7 @@ test_expect_success 'checking that branch head only has one tag' \"\n>  \ttest_cmp expected actual\n>  \"\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v0.2.1\n>  v1.0\n>  v1.0.1\n> @@ -1757,7 +1757,7 @@ test_expect_success 'merging original branch into this branch' '\n>          git tag v4.0\n>  '\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v4.0\n>  EOF\n>  \n> @@ -1766,7 +1766,7 @@ test_expect_success 'checking that original branch head has one tag now' \"\n>  \ttest_cmp expected actual\n>  \"\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v0.2.1\n>  v1.0\n>  v1.0.1\n> @@ -1780,7 +1780,7 @@ test_expect_success 'checking that original branch head with --no-contains lists\n>  \ttest_cmp expected actual\n>  \"\n>  \n> -cat > expected <<EOF\n> +cat >expected <<EOF\n>  v0.2.1\n>  v1.0\n>  v1.0.1\n"},{"id":"500005","messageId":"dcded6a3-e284-4d52-b36f-dacc056bbc5b@gmail.com","threadId":"61889","inReplyTo":"xmqqttg2ewqm.fsf@gitster.g","subject":"Re: [PATCH][Newcomer] t7004-tag: modernize the test script","fromName":"AbdAlRahman Gad","fromEmail":"abdobngad@gmail.com","sentAt":"2024-08-03T14:34:46Z","receivedAt":"2024-08-03T14:34:50Z","isPatch":true,"sender":{"key":"abdobngad@gmail.com","avatar":"https://avatars.githubusercontent.com/u/89566409?v=4"},"body":"\n\nOn 8/2/24 19:05, Junio C Hamano wrote:\n> AbdAlRahman Gad <abdobngad@gmail.com> writes:\n> \n>> Modernaize 't7004-tag.sh' by removing whitespace\n>> after the redirect operators.\n> \n> It is a good start.  There are other modernization opportunities in\n> this file, though.\n> \n>   - Output from \"test-tool ref-store\" piped to \"sed\" means the exit\n>     status from an abnormal exit of \"test-tool\" is hidden.  They\n>     should be split into two commands.\n> \n>   - Expected output file prepared outside test_expect_success that\n>     uses it.\n> \n>   - Here-doc that does not interpolate leaving the EOF marker\n>     unquoted.\n\nThanks for the review. I've just sent a follow-up v2 patch fixing the \nthings you mentioned. I also found other issues like:\n\nsome test_expect_success are seperated from its name like:\n\n    test_expect_success \\\n    \t'trying to delete tags without params should succeed and do nothing' '\n\nbut I preferred to send the patch first as it was getting very long and \nI also wanted to make sure that I am on the right path and not just \nfixing unrelated things.\n\n>> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>\n>> ---\n>>   t/t7004-tag.sh | 50 +++++++++++++++++++++++++-------------------------\n>>   1 file changed, 25 insertions(+), 25 deletions(-)\n>>\n>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n>> index fa6336edf9..3100a4c219 100755\n>> --- a/t/t7004-tag.sh\n>> +++ b/t/t7004-tag.sh\n>> @@ -213,9 +213,9 @@ mytag\n>>   EOF\n>>   test_expect_success \\\n>>   \t'trying to delete tags without params should succeed and do nothing' '\n>> -\tgit tag -l > actual && test_cmp expect actual &&\n>> +\tgit tag -l >actual && test_cmp expect actual &&\n>>   \tgit tag -d &&\n>> -\tgit tag -l > actual && test_cmp expect actual\n>> +\tgit tag -l >actual && test_cmp expect actual\n>>   '\n>>   \n>>   test_expect_success \\\n>> @@ -269,9 +269,9 @@ test_expect_success 'listing all tags should print them ordered' '\n>>   \tgit tag a1 &&\n>>   \tgit tag v1.0 &&\n>>   \tgit tag t210 &&\n>> -\tgit tag -l > actual &&\n>> +\tgit tag -l >actual &&\n>>   \ttest_cmp expect actual &&\n>> -\tgit tag > actual &&\n>> +\tgit tag >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> @@ -283,7 +283,7 @@ EOF\n>>   test_expect_success \\\n>>   \t'listing tags with substring as pattern must print those matching' '\n>>   \trm *a* &&\n>> -\tgit tag -l \"*a*\" > current &&\n>> +\tgit tag -l \"*a*\" >current &&\n>>   \ttest_cmp expect current\n>>   '\n>>   \n>> @@ -293,7 +293,7 @@ v1.0.1\n>>   EOF\n>>   test_expect_success \\\n>>   \t'listing tags with a suffix as pattern must print those matching' '\n>> -\tgit tag -l \"*.1\" > actual &&\n>> +\tgit tag -l \"*.1\" >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> @@ -303,7 +303,7 @@ t211\n>>   EOF\n>>   test_expect_success \\\n>>   \t'listing tags with a prefix as pattern must print those matching' '\n>> -\tgit tag -l \"t21*\" > actual &&\n>> +\tgit tag -l \"t21*\" >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> @@ -312,7 +312,7 @@ a1\n>>   EOF\n>>   test_expect_success \\\n>>   \t'listing tags using a name as pattern must print that one matching' '\n>> -\tgit tag -l a1 > actual &&\n>> +\tgit tag -l a1 >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> @@ -321,7 +321,7 @@ v1.0\n>>   EOF\n>>   test_expect_success \\\n>>   \t'listing tags using a name as pattern must print that one matching' '\n>> -\tgit tag -l v1.0 > actual &&\n>> +\tgit tag -l v1.0 >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> @@ -331,13 +331,13 @@ v1.1.3\n>>   EOF\n>>   test_expect_success \\\n>>   \t'listing tags with ? in the pattern should print those matching' '\n>> -\tgit tag -l \"v1.?.?\" > actual &&\n>> +\tgit tag -l \"v1.?.?\" >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>>   test_expect_success \\\n>>   \t'listing tags using v.* should print nothing because none have v.' '\n>> -\tgit tag -l \"v.*\" > actual &&\n>> +\tgit tag -l \"v.*\" >actual &&\n>>   \ttest_must_be_empty actual\n>>   '\n>>   \n>> @@ -349,7 +349,7 @@ v1.1.3\n>>   EOF\n>>   test_expect_success \\\n>>   \t'listing tags using v* should print only those having v' '\n>> -\tgit tag -l \"v*\" > actual &&\n>> +\tgit tag -l \"v*\" >actual &&\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> @@ -1171,7 +1171,7 @@ test_expect_success GPG \\\n>>   \tgit tag -v emptyfile-signed-tag\n>>   '\n>>   \n>> -printf '\\n\\n  \\n\\t\\nLeading blank lines\\n' > sigblanksfile\n>> +printf '\\n\\n  \\n\\t\\nLeading blank lines\\n' >sigblanksfile\n>>   printf '\\n\\t \\t  \\nRepeated blank lines\\n' >>sigblanksfile\n>>   printf '\\n\\n\\nTrailing spaces      \\t  \\n' >>sigblanksfile\n>>   printf '\\nTrailing blank lines\\n\\n\\t \\n\\n' >>sigblanksfile\n>> @@ -1569,7 +1569,7 @@ test_expect_success \\\n>>   \n>>   test_expect_success \\\n>>   \t'message in editor has initial comment' '\n>> -\t! (GIT_EDITOR=cat git tag -a initial-comment > actual)\n>> +\t! (GIT_EDITOR=cat git tag -a initial-comment >actual)\n>>   '\n>>   \n>>   test_expect_success 'message in editor has initial comment: first line' '\n>> @@ -1587,7 +1587,7 @@ test_expect_success \\\n>>   '\n>>   \n>>   get_tag_header reuse $commit commit $time >expect\n>> -echo \"An annotation to be reused\" >> expect\n>> +echo \"An annotation to be reused\" >>expect\n>>   test_expect_success \\\n>>   \t'overwriting an annotated tag should use its previous body' '\n>>   \tgit tag -a -m \"An annotation to be reused\" reuse &&\n>> @@ -1639,7 +1639,7 @@ hash3=$(git rev-parse HEAD)\n>>   \n>>   # simple linear checks of --continue\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v0.2.1\n>>   v1.0\n>>   v1.0.1\n>> @@ -1679,7 +1679,7 @@ test_expect_success 'checking that first commit is in all tags (relative)' \"\n>>   \ttest_must_be_empty actual\n>>   \"\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v2.0\n>>   EOF\n>>   \n>> @@ -1688,7 +1688,7 @@ test_expect_success 'checking that second commit only has one tag' \"\n>>   \ttest_cmp expected actual\n>>   \"\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v0.2.1\n>>   v1.0\n>>   v1.0.1\n>> @@ -1705,7 +1705,7 @@ test_expect_success 'checking that third commit has no tags' \"\n>>   \ttest_must_be_empty actual\n>>   \"\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v0.2.1\n>>   v1.0\n>>   v1.0.1\n>> @@ -1723,14 +1723,14 @@ test_expect_success 'conversely --no-contains on the third commit lists all tags\n>>   test_expect_success 'creating simple branch' '\n>>   \tgit branch stable v2.0 &&\n>>           git checkout stable &&\n>> -\techo foo-3.0 > foo &&\n>> +\techo foo-3.0 >foo &&\n>>   \tgit commit foo -m fourth &&\n>>   \tgit tag v3.0\n>>   '\n>>   \n>>   hash4=$(git rev-parse HEAD)\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v3.0\n>>   EOF\n>>   \n>> @@ -1739,7 +1739,7 @@ test_expect_success 'checking that branch head only has one tag' \"\n>>   \ttest_cmp expected actual\n>>   \"\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v0.2.1\n>>   v1.0\n>>   v1.0.1\n>> @@ -1757,7 +1757,7 @@ test_expect_success 'merging original branch into this branch' '\n>>           git tag v4.0\n>>   '\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v4.0\n>>   EOF\n>>   \n>> @@ -1766,7 +1766,7 @@ test_expect_success 'checking that original branch head has one tag now' \"\n>>   \ttest_cmp expected actual\n>>   \"\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v0.2.1\n>>   v1.0\n>>   v1.0.1\n>> @@ -1780,7 +1780,7 @@ test_expect_success 'checking that original branch head with --no-contains lists\n>>   \ttest_cmp expected actual\n>>   \"\n>>   \n>> -cat > expected <<EOF\n>> +cat >expected <<EOF\n>>   v0.2.1\n>>   v1.0\n>>   v1.0.1\n\n"},{"id":"500007","messageId":"xmqqmslt7eob.fsf@gitster.g","threadId":"61889","inReplyTo":"dcded6a3-e284-4d52-b36f-dacc056bbc5b@gmail.com","subject":"Re: [PATCH][Newcomer] t7004-tag: modernize the test script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-03T16:29:40Z","receivedAt":"2024-08-03T16:29:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"AbdAlRahman Gad <abdobngad@gmail.com> writes:\n\n>> It is a good start.  There are other modernization opportunities in\n>> this file, though.\n>>   - Output from \"test-tool ref-store\" piped to \"sed\" means the exit\n>>     status from an abnormal exit of \"test-tool\" is hidden.  They\n>>     should be split into two commands.\n>>   - Expected output file prepared outside test_expect_success that\n>>     uses it.\n>>   - Here-doc that does not interpolate leaving the EOF marker\n>>     unquoted.\n>\n> Thanks for the review. I've just sent a follow-up v2 patch fixing the\n> things you mentioned. I also found other issues like:\n>\n> some test_expect_success are seperated from its name like:\n>\n>    test_expect_success \\\n>    \t'trying to delete tags without params should succeed and do nothing' '\n>\n> but I preferred to send the patch first as it was getting very long\n> and I also wanted to make sure that I am on the right path and not\n> just fixing unrelated things.\n\nTo deal with \"is this getting too long?\" you can split this into a\nseries of multiple patches, e.g.\n\n    [PATCH 0/n] t7004: modernize the style\n      cover letter that gives an overview of the series\n\n    [PATCH 1/n] t7004: description on the same line as test_expect_success\n      the one you pointed out above\n\n    [PATCH 2/n] t7004: redirection operator\n      the patch you sent earlier\n\n    [PATCH 3/n] t7004: do not lose exit status to pipe\n      split \"test-tool ... | sed\" pipeline into two commands\n      to avoid losing exit status from test-tool\n\n    [PATCH 4/n] t7004: one command per line\n      fix lines like these:\n\tgit tag -l >actual && test_cmp expect actual &&\n      to\n\tgit tag -l >actual &&\n\ttest_cmp expect actual &&\n\n    [PATCH 5/n] t7004: here-doc modernization\n      use <<-EOF or <<-\\EOF to indent here-doc\n      use \\EOF not EOF when not interpolating\n\n    [PATCH 6/n] t7004: do not do things outside test_expect_success\n      do not prepare expect and other things outside test_expect_success\n\nwould make a thorough series, while keeping each step still\nreasonably short, I suspect.\n"}]}