Re: [PATCH][Newcomer] t7004-tag: modernize the test script

AbdAlRahman Gad <abdobngad@xxxxxxxxx> · Sat, 3 Aug 2024 17:34:46 +0300

On 8/2/24 19:05, Junio C Hamano wrote:
AbdAlRahman Gad <abdobngad@xxxxxxxxx> writes:

Modernaize 't7004-tag.sh' by removing whitespace
after the redirect operators.

It is a good start.  There are other modernization opportunities in
this file, though.

  - Output from "test-tool ref-store" piped to "sed" means the exit
    status from an abnormal exit of "test-tool" is hidden.  They
    should be split into two commands.

  - Expected output file prepared outside test_expect_success that
    uses it.

  - Here-doc that does not interpolate leaving the EOF marker
    unquoted.

Thanks for the review. I've just sent a follow-up v2 patch fixing the 
things you mentioned. I also found other issues like:

some test_expect_success are seperated from its name like:

   test_expect_success \
   	'trying to delete tags without params should succeed and do nothing' '

but I preferred to send the patch first as it was getting very long and 
I also wanted to make sure that I am on the right path and not just 
fixing unrelated things.

Signed-off-by: AbdAlRahman Gad <abdobngad@xxxxxxxxx>
---
  t/t7004-tag.sh | 50 +++++++++++++++++++++++++-------------------------
  1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index fa6336edf9..3100a4c219 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -213,9 +213,9 @@ mytag
  EOF
  test_expect_success \
  	'trying to delete tags without params should succeed and do nothing' '
-	git tag -l > actual && test_cmp expect actual &&
+	git tag -l >actual && test_cmp expect actual &&
  	git tag -d &&
-	git tag -l > actual && test_cmp expect actual
+	git tag -l >actual && test_cmp expect actual
  '
  
  test_expect_success \
@@ -269,9 +269,9 @@ test_expect_success 'listing all tags should print them ordered' '
  	git tag a1 &&
  	git tag v1.0 &&
  	git tag t210 &&
-	git tag -l > actual &&
+	git tag -l >actual &&
  	test_cmp expect actual &&
-	git tag > actual &&
+	git tag >actual &&
  	test_cmp expect actual
  '
  
@@ -283,7 +283,7 @@ EOF
  test_expect_success \
  	'listing tags with substring as pattern must print those matching' '
  	rm *a* &&
-	git tag -l "*a*" > current &&
+	git tag -l "*a*" >current &&
  	test_cmp expect current
  '
  
@@ -293,7 +293,7 @@ v1.0.1
  EOF
  test_expect_success \
  	'listing tags with a suffix as pattern must print those matching' '
-	git tag -l "*.1" > actual &&
+	git tag -l "*.1" >actual &&
  	test_cmp expect actual
  '
  
@@ -303,7 +303,7 @@ t211
  EOF
  test_expect_success \
  	'listing tags with a prefix as pattern must print those matching' '
-	git tag -l "t21*" > actual &&
+	git tag -l "t21*" >actual &&
  	test_cmp expect actual
  '
  
@@ -312,7 +312,7 @@ a1
  EOF
  test_expect_success \
  	'listing tags using a name as pattern must print that one matching' '
-	git tag -l a1 > actual &&
+	git tag -l a1 >actual &&
  	test_cmp expect actual
  '
  
@@ -321,7 +321,7 @@ v1.0
  EOF
  test_expect_success \
  	'listing tags using a name as pattern must print that one matching' '
-	git tag -l v1.0 > actual &&
+	git tag -l v1.0 >actual &&
  	test_cmp expect actual
  '
  
@@ -331,13 +331,13 @@ v1.1.3
  EOF
  test_expect_success \
  	'listing tags with ? in the pattern should print those matching' '
-	git tag -l "v1.?.?" > actual &&
+	git tag -l "v1.?.?" >actual &&
  	test_cmp expect actual
  '
  
  test_expect_success \
  	'listing tags using v.* should print nothing because none have v.' '
-	git tag -l "v.*" > actual &&
+	git tag -l "v.*" >actual &&
  	test_must_be_empty actual
  '
  
@@ -349,7 +349,7 @@ v1.1.3
  EOF
  test_expect_success \
  	'listing tags using v* should print only those having v' '
-	git tag -l "v*" > actual &&
+	git tag -l "v*" >actual &&
  	test_cmp expect actual
  '
  
@@ -1171,7 +1171,7 @@ test_expect_success GPG \
  	git tag -v emptyfile-signed-tag
  '
  
-printf '\n\n  \n\t\nLeading blank lines\n' > sigblanksfile
+printf '\n\n  \n\t\nLeading blank lines\n' >sigblanksfile
  printf '\n\t \t  \nRepeated blank lines\n' >>sigblanksfile
  printf '\n\n\nTrailing spaces      \t  \n' >>sigblanksfile
  printf '\nTrailing blank lines\n\n\t \n\n' >>sigblanksfile
@@ -1569,7 +1569,7 @@ test_expect_success \
  
  test_expect_success \
  	'message in editor has initial comment' '
-	! (GIT_EDITOR=cat git tag -a initial-comment > actual)
+	! (GIT_EDITOR=cat git tag -a initial-comment >actual)
  '
  
  test_expect_success 'message in editor has initial comment: first line' '
@@ -1587,7 +1587,7 @@ test_expect_success \
  '
  
  get_tag_header reuse $commit commit $time >expect
-echo "An annotation to be reused" >> expect
+echo "An annotation to be reused" >>expect
  test_expect_success \
  	'overwriting an annotated tag should use its previous body' '
  	git tag -a -m "An annotation to be reused" reuse &&
@@ -1639,7 +1639,7 @@ hash3=$(git rev-parse HEAD)
  
  # simple linear checks of --continue
  
-cat > expected <<EOF
+cat >expected <<EOF
  v0.2.1
  v1.0
  v1.0.1
@@ -1679,7 +1679,7 @@ test_expect_success 'checking that first commit is in all tags (relative)' "
  	test_must_be_empty actual
  "
  
-cat > expected <<EOF
+cat >expected <<EOF
  v2.0
  EOF
  
@@ -1688,7 +1688,7 @@ test_expect_success 'checking that second commit only has one tag' "
  	test_cmp expected actual
  "
  
-cat > expected <<EOF
+cat >expected <<EOF
  v0.2.1
  v1.0
  v1.0.1
@@ -1705,7 +1705,7 @@ test_expect_success 'checking that third commit has no tags' "
  	test_must_be_empty actual
  "
  
-cat > expected <<EOF
+cat >expected <<EOF
  v0.2.1
  v1.0
  v1.0.1
@@ -1723,14 +1723,14 @@ test_expect_success 'conversely --no-contains on the third commit lists all tags
  test_expect_success 'creating simple branch' '
  	git branch stable v2.0 &&
          git checkout stable &&
-	echo foo-3.0 > foo &&
+	echo foo-3.0 >foo &&
  	git commit foo -m fourth &&
  	git tag v3.0
  '
  
  hash4=$(git rev-parse HEAD)
  
-cat > expected <<EOF
+cat >expected <<EOF
  v3.0
  EOF
  
@@ -1739,7 +1739,7 @@ test_expect_success 'checking that branch head only has one tag' "
  	test_cmp expected actual
  "
  
-cat > expected <<EOF
+cat >expected <<EOF
  v0.2.1
  v1.0
  v1.0.1
@@ -1757,7 +1757,7 @@ test_expect_success 'merging original branch into this branch' '
          git tag v4.0
  '
  
-cat > expected <<EOF
+cat >expected <<EOF
  v4.0
  EOF
  
@@ -1766,7 +1766,7 @@ test_expect_success 'checking that original branch head has one tag now' "
  	test_cmp expected actual
  "
  
-cat > expected <<EOF
+cat >expected <<EOF
  v0.2.1
  v1.0
  v1.0.1
@@ -1780,7 +1780,7 @@ test_expect_success 'checking that original branch head with --no-contains lists
  	test_cmp expected actual
  "
  
-cat > expected <<EOF
+cat >expected <<EOF
  v0.2.1
  v1.0
  v1.0.1








