tests: modernize the test script t0010-racy-git.sh

From: aryangupta701 <[email protected]>

Modernize the formatting of the test script to align with current standards and improve its overall readability.

---
[GSOC] [PATCH] Modernize a test script

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1675%2Faryangupta701%2Ftest-modernize-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1675/aryangupta701/test-modernize-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1675

 t/t0010-racy-git.sh | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

index 837c8b7228b..04dc1cf3ff5 100755
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='racy GIT'
+test_description='racy git'
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

@@ -16,19 +16,18 @@ do
 	echo xyzzy >infocom
 
 	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part A" \
-	'test "" != "$files"'
+	test_expect_success 'Racy git trial #$trial part A' '
+		test "" != "$files"
+	'
 
 	sleep 1
 	echo xyzzy >cornerstone
 	git update-index --add cornerstone
 
 	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part B" \
-	'test "" != "$files"'
-
+	test_expect_success 'Racy git trial #$trial part B' '
+		test "" != "$files"
+	'
 done
 
 test_done

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0

gitgitgadget

The name in the "From:" header should match the name in the "Signed-off-by:" trailer.

The aim of this patch makes sense, but the implementation isn't quite correct.

From: Aryan Gupta <[email protected]>

Modernize the formatting of the test script to align with current standards and improve its overall readability.

---
[GSOC][PATCH] Modernize a test script

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1675%2Faryangupta701%2Ftest-modernize-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1675/aryangupta701/test-modernize-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1675

Range-diff vs v1:

1: 06377119ea3 ! 1: fa381d0b57a tests: modernize the test script t0010-racy-git.sh

     @@
       ## Metadata ##
     -Author: aryangupta701 <[email protected]>
     +Author: Aryan Gupta <[email protected]>
      
       ## Commit message ##
          tests: modernize the test script t0010-racy-git.sh
     @@ t/t0010-racy-git.sh: do
      -	test_expect_success \
      -	"Racy GIT trial #$trial part A" \
      -	'test "" != "$files"'
     -+	test_expect_success 'Racy git trial #$trial part A' '
     ++	test_expect_success "Racy git trial #$trial part A" '
      +		test "" != "$files"
      +	'
       
     @@ t/t0010-racy-git.sh: do
      -	"Racy GIT trial #$trial part B" \
      -	'test "" != "$files"'
      -
     -+	test_expect_success 'Racy git trial #$trial part B' '
     ++	test_expect_success "Racy git trial #$trial part B" '
      +		test "" != "$files"
      +	'
       done

 t/t0010-racy-git.sh | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

index 837c8b7228b..2ceac06c9c2 100755
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='racy GIT'
+test_description='racy git'
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh

@@ -16,19 +16,18 @@ do
 	echo xyzzy >infocom
 
 	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part A" \
-	'test "" != "$files"'
+	test_expect_success "Racy git trial #$trial part A" '
+		test "" != "$files"
+	'
 
 	sleep 1
 	echo xyzzy >cornerstone
 	git update-index --add cornerstone
 
 	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part B" \
-	'test "" != "$files"'
-
+	test_expect_success "Racy git trial #$trial part B" '
+		test "" != "$files"
+	'
 done
 
 test_done

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0

gitgitgadget

OK.

                test_expect_success \
                "Racy GIT trial #$trial part A" \
                'test "" != "$files"'

                echo xyzzy >cornerstone
                git update-index --add cornerstone

                files=$(git diff-files -p)

                test_expect_success \
                "Racy GIT trial #$trial part B" \
                'test "" != "$files"'

	test_expect_success "Racy GIT trial #$trial part A" '
		rm -f .git/index &&
		echo frotz >infocom &&
		git update-index --add infocom &&
		echo xyzzy >infocom &&

		files=$(git diff-files -p) &&
                test "" != "$files"
	'

	sleep 1

	test_expect_success "Racy GIT trial #$trial part B" '
		echo xyzzy >cornerstone &&
		git update-index --add cornerstone &&

		files=$(git diff-files -p) &&
		test "" != "$files"
	'

If taking it to this extent, then the modernized version of the last couple lines would be:

    git diff-files -p >out &&
    test_file_not_empty out

Yes. The modern style seems to prefer temporary files over variables; the reason probably is because it tends to be easier to remotely post-mortem?

Yes, that seems likely. Functions such as test_file_not_empty() also provide an immediate indication of what went wrong without having to spelunk the test detritus:

'foo' is not a non-empty file. [*]

whereas test "" != "$foo" provides no output at all, so it's not immediately clear which part of the test failed. Peff's verbose function was intended to mitigate that problem by making the expression verbose upon failure:

    verbose test "" != "$foo" &&

but never really caught on and was eventually retired by 8ddfce7144 (t: drop "verbose" helper function, 2023-05-08).

[*]: Admittedly, the double-negative in "'foo' is not a non-empty file." is more than a little confusing. It probably would have been better phrased as "'foo' should be empty but is not".

The double-negative confused me even when suggesting a replacement. What I meant was that a better phrasing would perhaps have been:

'foo' is empty but should not be

+1. Thanks for caring.

From: Aryan Gupta <[email protected]>

Modernize the formatting of the test script to align with current standards and improve its overall readability.

---
[GSOC][PATCH] Modernize a test script

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1675%2Faryangupta701%2Ftest-modernize-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1675/aryangupta701/test-modernize-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1675

Range-diff vs v2:

1: fa381d0b57a ! 1: 05ee9e8a458 tests: modernize the test script t0010-racy-git.sh

     @@ Commit message
          Signed-off-by: Aryan Gupta <[email protected]>
      
       ## t/t0010-racy-git.sh ##
     -@@
     - #!/bin/sh
     +@@ t/t0010-racy-git.sh: TEST_PASSES_SANITIZE_LEAK=true
       
     --test_description='racy GIT'
     -+test_description='racy git'
     - 
     - TEST_PASSES_SANITIZE_LEAK=true
     - . ./test-lib.sh
     -@@ t/t0010-racy-git.sh: do
     - 	echo xyzzy >infocom
     - 
     - 	files=$(git diff-files -p)
     + for trial in 0 1 2 3 4
     + do
     +-	rm -f .git/index
     +-	echo frotz >infocom
     +-	git update-index --add infocom
     +-	echo xyzzy >infocom
     +-
     +-	files=$(git diff-files -p)
      -	test_expect_success \
      -	"Racy GIT trial #$trial part A" \
      -	'test "" != "$files"'
     +-
      +	test_expect_success "Racy git trial #$trial part A" '
     -+		test "" != "$files"
     ++		rm -f .git/index &&
     ++		echo frotz >infocom &&
     ++		git update-index --add infocom &&
     ++		echo xyzzy >infocom &&
     ++
     ++		git diff-files -p >out &&
     ++		test_file_not_empty out
      +	'
     - 
       	sleep 1
     - 	echo xyzzy >cornerstone
     - 	git update-index --add cornerstone
     +-	echo xyzzy >cornerstone
     +-	git update-index --add cornerstone
       
     - 	files=$(git diff-files -p)
     +-	files=$(git diff-files -p)
      -	test_expect_success \
      -	"Racy GIT trial #$trial part B" \
      -	'test "" != "$files"'
     --
      +	test_expect_success "Racy git trial #$trial part B" '
     -+		test "" != "$files"
     ++		echo xyzzy >cornerstone &&
     ++		git update-index --add cornerstone &&
     + 
     ++		git diff-files -p >out &&
     ++		test_file_not_empty out
      +	'
       done

 t/t0010-racy-git.sh | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

index 837c8b7228b..84172a37390 100755
@@ -10,25 +10,24 @@ TEST_PASSES_SANITIZE_LEAK=true
 
 for trial in 0 1 2 3 4
 do
-	rm -f .git/index
-	echo frotz >infocom
-	git update-index --add infocom
-	echo xyzzy >infocom
-
-	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part A" \
-	'test "" != "$files"'
-
+	test_expect_success "Racy git trial #$trial part A" '
+		rm -f .git/index &&
+		echo frotz >infocom &&
+		git update-index --add infocom &&
+		echo xyzzy >infocom &&
+
+		git diff-files -p >out &&
+		test_file_not_empty out
+	'
 	sleep 1
-	echo xyzzy >cornerstone
-	git update-index --add cornerstone
 
-	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part B" \
-	'test "" != "$files"'
+	test_expect_success "Racy git trial #$trial part B" '
+		echo xyzzy >cornerstone &&
+		git update-index --add cornerstone &&
 
+		git diff-files -p >out &&
+		test_file_not_empty out
+	'
 done
 
 test_done

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0

gitgitgadget

Both the above proposed commit log message and the proposed changes to the code look good.

This is a tangent but as can be seen at

https://lore.kernel.org/git/[email protected]/raw

your message has these headers that are not syntactically correct and may even cause responses to bounce unless editted.

Cc: "Patrick Steinhardt [ ]" <[email protected]>,
"Michal =?UTF-8?Q?Such=C3=A1nek?= [ ]" <[email protected]>, "=?UTF-8?Q?Jean-No=C3=ABl?= AVILA [ ]" <[email protected]>, Kristoffer Haugsbakk <[[email protected]]>, Eric Sunshine <[email protected]>,
Aryan Gupta <[email protected]>,
Aryan Gupta <[email protected]>

I cannot tell if this is a bug in GGG, or a fault of a corrupt pull request thrown at GGG, but it is annoying that recipients have to edit the addresses in e-mail header. Can somebody take a look into it, please (or is it a known issue?).

Thanks.

From: Aryan Gupta <[email protected]>

Modernize the formatting of the test script to align with current standards and improve its overall readability.

---
[GSOC][PATCH] Modernize a test script

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1675%2Faryangupta701%2Ftest-modernize-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1675/aryangupta701/test-modernize-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1675

Range-diff vs v3:

1: 05ee9e8a458 = 1: 14c7137baea tests: modernize the test script t0010-racy-git.sh

 t/t0010-racy-git.sh | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

index 837c8b7228b..84172a37390 100755
@@ -10,25 +10,24 @@ TEST_PASSES_SANITIZE_LEAK=true
 
 for trial in 0 1 2 3 4
 do
-	rm -f .git/index
-	echo frotz >infocom
-	git update-index --add infocom
-	echo xyzzy >infocom
-
-	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part A" \
-	'test "" != "$files"'
-
+	test_expect_success "Racy git trial #$trial part A" '
+		rm -f .git/index &&
+		echo frotz >infocom &&
+		git update-index --add infocom &&
+		echo xyzzy >infocom &&
+
+		git diff-files -p >out &&
+		test_file_not_empty out
+	'
 	sleep 1
-	echo xyzzy >cornerstone
-	git update-index --add cornerstone
 
-	files=$(git diff-files -p)
-	test_expect_success \
-	"Racy GIT trial #$trial part B" \
-	'test "" != "$files"'
+	test_expect_success "Racy git trial #$trial part B" '
+		echo xyzzy >cornerstone &&
+		git update-index --add cornerstone &&
 
+		git diff-files -p >out &&
+		test_file_not_empty out
+	'
 done
 
 test_done

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0

gitgitgadget

This tells us that nothing changed in the patch since v3, so we can only wonder why you sent this v4.

Did you fix some headers? Please explain.

Otherwise, the patch looks good to me. Thanks.

Hey. Sorry for making a lot of mistakes in my emails.

I thought that there were some bugs in GGG due to which it sent some headers which were not syntactically correct. So I tried sending it again. And that was the whole purpose of this.

I was wondering about the same thing. I still see an unwanted "[ ]" around Kristoffer's e-mail address that will break responding to the message in your [PATCH v4] e-mail that can be seen at

https://lore.kernel.org/git/[email protected]/raw

so, the experiment revealed that it did send some headers were broken.

Thanks for a clarification.

This does not necessarily mean GGG is broken. The majority of the patches I see here from GGG are without these funny [square-bracket] around addresses at all, and this was the only patch (or it is possible that your other patches may have had the same issue; I do not remember) with that problem. It might be caused by what you feed GGG (e.g., the messages you give it in your pull request) that caused GGG to hiccup, perhaps?

Ohh yes. I think I got the problem. I have fixed it. Can I try sending the patch again now?