Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH] tests: modernize the test script t0010-racy-git.sh
@ 2024-02-29 12:23 Aryan Gupta via GitGitGadget
  2024-02-29 18:11 ` Eric Sunshine
  2024-02-29 21:57 ` [PATCH v2] " Aryan Gupta via GitGitGadget
  0 siblings, 2 replies; 18+ messages in thread
From: Aryan Gupta via GitGitGadget @ 2024-02-29 12:23 UTC (permalink / raw
  To: git
  Cc: Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ]

From: aryangupta701 <garyan447@gmail.com>

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

Signed-off-by: Aryan Gupta <garyan447@gmail.com>
---
    [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(-)

diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
index 837c8b7228b..04dc1cf3ff5 100755
--- a/t/t0010-racy-git.sh
+++ b/t/t0010-racy-git.sh
@@ -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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 12:23 [PATCH] tests: modernize the test script t0010-racy-git.sh Aryan Gupta via GitGitGadget
@ 2024-02-29 18:11 ` Eric Sunshine
  2024-02-29 21:57 ` [PATCH v2] " Aryan Gupta via GitGitGadget
  1 sibling, 0 replies; 18+ messages in thread
From: Eric Sunshine @ 2024-02-29 18:11 UTC (permalink / raw
  To: Aryan Gupta via GitGitGadget
  Cc: git, Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ], aryangupta701

On Thu, Feb 29, 2024 at 7:24 AM Aryan Gupta via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> From: aryangupta701 <garyan447@gmail.com>

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

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

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

> Signed-off-by: Aryan Gupta <garyan447@gmail.com>
> ---
> diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh> @@ -16,19 +16,18 @@ do
>         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"
> +       '

The variable `trial` takes on values 0-4, and that value is meant to
appear in the test title as the script runs:

    Racy GIT trial #0 part A
    Racy GIT trial #0 part B
    Racy GIT trial #1 part A
    Racy GIT trial #1 part B
    ...

However, by changing the title from a double-quote string to a
single-quote string, you inhibit interpolation of the `trial` variable
into the title, hence the test titles instead show up as:

    Racy GIT trial #$trial part A
    Racy GIT trial #$trial part B
    Racy GIT trial #$trial part A
    Racy GIT trial #$trial part B

which is undesirable.

So, the title should continue using double-quotes, and not be changed
to single-quotes.

The other change, which fixes the style of the test's body, appears correct.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 12:23 [PATCH] tests: modernize the test script t0010-racy-git.sh Aryan Gupta via GitGitGadget
  2024-02-29 18:11 ` Eric Sunshine
@ 2024-02-29 21:57 ` Aryan Gupta via GitGitGadget
  2024-02-29 22:19   ` Eric Sunshine
                     ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Aryan Gupta via GitGitGadget @ 2024-02-29 21:57 UTC (permalink / raw
  To: git
  Cc: Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ]

From: Aryan Gupta <garyan447@gmail.com>

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

Signed-off-by: Aryan Gupta <garyan447@gmail.com>
---
    [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 <garyan447@gmail.com>
     +Author: Aryan Gupta <garyan447@gmail.com>
      
       ## 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(-)

diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
index 837c8b7228b..2ceac06c9c2 100755
--- a/t/t0010-racy-git.sh
+++ b/t/t0010-racy-git.sh
@@ -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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 21:57 ` [PATCH v2] " Aryan Gupta via GitGitGadget
@ 2024-02-29 22:19   ` Eric Sunshine
  2024-02-29 23:14   ` Junio C Hamano
  2024-03-05 22:09   ` [PATCH v3] " Aryan Gupta via GitGitGadget
  2 siblings, 0 replies; 18+ messages in thread
From: Eric Sunshine @ 2024-02-29 22:19 UTC (permalink / raw
  To: Aryan Gupta via GitGitGadget
  Cc: git, Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ], Aryan Gupta

On Thu, Feb 29, 2024 at 4:57 PM Aryan Gupta via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> Modernize the formatting of the test script to align with current
> standards and improve its overall readability.
>
> Signed-off-by: Aryan Gupta <garyan447@gmail.com>
> ---
> diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
> @@ -16,19 +16,18 @@ do
>         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"
> +       '

This version (v2) addresses my review comments about v1. Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 21:57 ` [PATCH v2] " Aryan Gupta via GitGitGadget
  2024-02-29 22:19   ` Eric Sunshine
@ 2024-02-29 23:14   ` Junio C Hamano
  2024-02-29 23:22     ` Eric Sunshine
  2024-03-05 22:09   ` [PATCH v3] " Aryan Gupta via GitGitGadget
  2 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2024-02-29 23:14 UTC (permalink / raw
  To: Aryan Gupta via GitGitGadget
  Cc: git, Patrick Steinhardt, Michal Suchánek,
	Jean-Noël AVILA, Kristoffer Haugsbakk, Eric Sunshine,
	Aryan Gupta

"Aryan Gupta via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Aryan Gupta <garyan447@gmail.com>
>
> Modernize the formatting of the test script to align with current
> standards and improve its overall readability.

OK.

> diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
> index 837c8b7228b..2ceac06c9c2 100755
> --- a/t/t0010-racy-git.sh
> +++ b/t/t0010-racy-git.sh
> @@ -1,6 +1,6 @@
>  #!/bin/sh
>  
> -test_description='racy GIT'
> +test_description='racy git'

This does not look like updating formatting to the current standard,
or improving readability, though.

> -	test_expect_success \
> -	"Racy GIT trial #$trial part A" \
> -	'test "" != "$files"'
> +	test_expect_success "Racy git trial #$trial part A" '
> +		test "" != "$files"
> +	'
> ...
> -	test_expect_success \
> -	"Racy GIT trial #$trial part B" \
> -	'test "" != "$files"'
> -
> +	test_expect_success "Racy git trial #$trial part B" '
> +		test "" != "$files"
> +	'

These are not wrong per-se, but if we are updating, I wonder if we
want to get rid of statements outside the test_expect_success block,
not just the formatting of individual test_expect_success tests.

Looking at an iteration of the loop, the executable statements from
here ...

                rm -f .git/index
                echo frotz >infocom
                git update-index --add infocom
                echo xyzzy >infocom

                files=$(git diff-files -p)

... to here is what we would make part of one test, namely ...

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

... this one.  Then

                sleep 1

is a cricial delay to have before the next test, which we may want
to have outside to make it clear what is going on to readers.  But
the following parts ...

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

                files=$(git diff-files -p)

... up to here, we would make part of the next test ...

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

... in the modern style.

So, we may want to do it more like this, perhaps?

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

An added benefit of the more modern style to place as much as
possible to &&-chain in test_expect_success block is that we would
catch breakage in "git update-index" and other things used to set-up
the test as well.  With the original loop, breakages in things
outside the test_expect_success blocks will go unchecked.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 23:14   ` Junio C Hamano
@ 2024-02-29 23:22     ` Eric Sunshine
  2024-02-29 23:36       ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Sunshine @ 2024-02-29 23:22 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Aryan Gupta via GitGitGadget, git, Patrick Steinhardt,
	Michal Suchánek, Jean-Noël AVILA, Kristoffer Haugsbakk,
	Aryan Gupta

On Thu, Feb 29, 2024 at 6:14 PM Junio C Hamano <gitster@pobox.com> wrote:
> So, we may want to do it more like this, perhaps?
>
>         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"
>         '

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 23:22     ` Eric Sunshine
@ 2024-02-29 23:36       ` Junio C Hamano
  2024-02-29 23:52         ` Eric Sunshine
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2024-02-29 23:36 UTC (permalink / raw
  To: Eric Sunshine
  Cc: Aryan Gupta via GitGitGadget, git, Patrick Steinhardt,
	Michal Suchánek, Jean-Noël AVILA, Kristoffer Haugsbakk,
	Aryan Gupta

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Thu, Feb 29, 2024 at 6:14 PM Junio C Hamano <gitster@pobox.com> wrote:
>> So, we may want to do it more like this, perhaps?
>>
>>         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"
>>         '
>
> 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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 23:36       ` Junio C Hamano
@ 2024-02-29 23:52         ` Eric Sunshine
  2024-03-01  0:06           ` Eric Sunshine
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Sunshine @ 2024-02-29 23:52 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Aryan Gupta via GitGitGadget, git, Patrick Steinhardt,
	Michal Suchánek, Jean-Noël AVILA, Kristoffer Haugsbakk,
	Aryan Gupta

On Thu, Feb 29, 2024 at 6:36 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > 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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 23:52         ` Eric Sunshine
@ 2024-03-01  0:06           ` Eric Sunshine
  2024-03-01  2:03             ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Sunshine @ 2024-03-01  0:06 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Aryan Gupta via GitGitGadget, git, Patrick Steinhardt,
	Michal Suchánek, Jean-Noël AVILA, Kristoffer Haugsbakk,
	Aryan Gupta

On Thu, Feb 29, 2024 at 6:52 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> [*]: 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] tests: modernize the test script t0010-racy-git.sh
  2024-03-01  0:06           ` Eric Sunshine
@ 2024-03-01  2:03             ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2024-03-01  2:03 UTC (permalink / raw
  To: Eric Sunshine
  Cc: Aryan Gupta via GitGitGadget, git, Patrick Steinhardt,
	Michal Suchánek, Jean-Noël AVILA, Kristoffer Haugsbakk,
	Aryan Gupta

Eric Sunshine <sunshine@sunshineco.com> writes:

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3] tests: modernize the test script t0010-racy-git.sh
  2024-02-29 21:57 ` [PATCH v2] " Aryan Gupta via GitGitGadget
  2024-02-29 22:19   ` Eric Sunshine
  2024-02-29 23:14   ` Junio C Hamano
@ 2024-03-05 22:09   ` Aryan Gupta via GitGitGadget
  2024-03-05 23:02     ` Junio C Hamano
  2024-03-06  9:14     ` [PATCH v4] " Aryan Gupta via GitGitGadget
  2 siblings, 2 replies; 18+ messages in thread
From: Aryan Gupta via GitGitGadget @ 2024-03-05 22:09 UTC (permalink / raw
  To: git
  Cc: Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ]

From: Aryan Gupta <garyan447@gmail.com>

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

Signed-off-by: Aryan Gupta <garyan447@gmail.com>
---
    [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 <garyan447@gmail.com>
      
       ## 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(-)

diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
index 837c8b7228b..84172a37390 100755
--- a/t/t0010-racy-git.sh
+++ b/t/t0010-racy-git.sh
@@ -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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] tests: modernize the test script t0010-racy-git.sh
  2024-03-05 22:09   ` [PATCH v3] " Aryan Gupta via GitGitGadget
@ 2024-03-05 23:02     ` Junio C Hamano
  2024-03-06  9:14     ` [PATCH v4] " Aryan Gupta via GitGitGadget
  1 sibling, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2024-03-05 23:02 UTC (permalink / raw
  To: Aryan Gupta via GitGitGadget
  Cc: git, Patrick Steinhardt, Michal Suchánek,
	Jean-Noël AVILA, Kristoffer Haugsbakk, Eric Sunshine,
	Aryan Gupta, Johannes Schindelin

"Aryan Gupta via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Aryan Gupta <garyan447@gmail.com>
>
> Modernize the formatting of the test script to align with current
> standards and improve its overall readability.
>
> Signed-off-by: Aryan Gupta <garyan447@gmail.com>
> ---

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/pull.1675.v3.git.1709676557639.gitgitgadget@gmail.com/raw

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

    Cc: "Patrick Steinhardt [ ]" <ps@pks.im>,
        "Michal =?UTF-8?Q?Such=C3=A1nek?= [ ]" <msuchanek@suse.de>,
        "=?UTF-8?Q?Jean-No=C3=ABl?= AVILA [ ]" <jn.avila@free.fr>,
        Kristoffer Haugsbakk <[code@khaugsbakk.name]>,
        Eric Sunshine <sunshine@sunshineco.com>,
        Aryan Gupta <garyan447@gmail.com>,
        Aryan Gupta <garyan447@gmail.com>

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4] tests: modernize the test script t0010-racy-git.sh
  2024-03-05 22:09   ` [PATCH v3] " Aryan Gupta via GitGitGadget
  2024-03-05 23:02     ` Junio C Hamano
@ 2024-03-06  9:14     ` Aryan Gupta via GitGitGadget
  2024-03-07 13:28       ` Christian Couder
  1 sibling, 1 reply; 18+ messages in thread
From: Aryan Gupta via GitGitGadget @ 2024-03-06  9:14 UTC (permalink / raw
  To: git
  Cc: Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ]

From: Aryan Gupta <garyan447@gmail.com>

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

Signed-off-by: Aryan Gupta <garyan447@gmail.com>
---
    [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(-)

diff --git a/t/t0010-racy-git.sh b/t/t0010-racy-git.sh
index 837c8b7228b..84172a37390 100755
--- a/t/t0010-racy-git.sh
+++ b/t/t0010-racy-git.sh
@@ -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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] tests: modernize the test script t0010-racy-git.sh
  2024-03-06  9:14     ` [PATCH v4] " Aryan Gupta via GitGitGadget
@ 2024-03-07 13:28       ` Christian Couder
  2024-03-07 14:23         ` Aryan Gupta
  0 siblings, 1 reply; 18+ messages in thread
From: Christian Couder @ 2024-03-07 13:28 UTC (permalink / raw
  To: Aryan Gupta via GitGitGadget
  Cc: git, Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ], Eric Sunshine, Aryan Gupta

On Wed, Mar 6, 2024 at 10:46 AM Aryan Gupta via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Aryan Gupta <garyan447@gmail.com>
>
> Modernize the formatting of the test script to align with current
> standards and improve its overall readability.
>
> Signed-off-by: Aryan Gupta <garyan447@gmail.com>
> ---
>     [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

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.

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

Otherwise, the patch looks good to me. Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] tests: modernize the test script t0010-racy-git.sh
  2024-03-07 13:28       ` Christian Couder
@ 2024-03-07 14:23         ` Aryan Gupta
  2024-03-07 18:17           ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Aryan Gupta @ 2024-03-07 14:23 UTC (permalink / raw
  To: Christian Couder
  Cc: Aryan Gupta via GitGitGadget, git, Patrick Steinhardt [ ],
	Michal Suchánek [ ], Jean-Noël AVILA [ ], Eric Sunshine

On Thu, Mar 7, 2024 at 2:28 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Wed, Mar 6, 2024 at 10:46 AM Aryan Gupta via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Aryan Gupta <garyan447@gmail.com>
> >
> > Modernize the formatting of the test script to align with current
> > standards and improve its overall readability.
> >
> > Signed-off-by: Aryan Gupta <garyan447@gmail.com>
> > ---
> >     [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
>
> 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.
>
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.

> >  t/t0010-racy-git.sh | 31 +++++++++++++++----------------
> >  1 file changed, 15 insertions(+), 16 deletions(-)
>
> Otherwise, the patch looks good to me. Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] tests: modernize the test script t0010-racy-git.sh
  2024-03-07 14:23         ` Aryan Gupta
@ 2024-03-07 18:17           ` Junio C Hamano
  2024-03-07 18:30             ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2024-03-07 18:17 UTC (permalink / raw
  To: Aryan Gupta
  Cc: Christian Couder, Aryan Gupta via GitGitGadget, git,
	Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ], Eric Sunshine

Aryan Gupta <garyan447@gmail.com> writes:

> 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/pull.1675.v4.git.1709716446874.gitgitgadget@gmail.com/raw

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

Thanks for a clarification.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] tests: modernize the test script t0010-racy-git.sh
  2024-03-07 18:17           ` Junio C Hamano
@ 2024-03-07 18:30             ` Junio C Hamano
  2024-03-07 18:33               ` Aryan Gupta
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2024-03-07 18:30 UTC (permalink / raw
  To: Aryan Gupta
  Cc: Christian Couder, Aryan Gupta via GitGitGadget, git,
	Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ], Eric Sunshine

Junio C Hamano <gitster@pobox.com> writes:

> 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/pull.1675.v4.git.1709716446874.gitgitgadget@gmail.com/raw
>
> so, the experiment revealed that it did send some headers were
> broken.

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?



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4] tests: modernize the test script t0010-racy-git.sh
  2024-03-07 18:30             ` Junio C Hamano
@ 2024-03-07 18:33               ` Aryan Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Aryan Gupta @ 2024-03-07 18:33 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Christian Couder, Aryan Gupta via GitGitGadget, git,
	Patrick Steinhardt [ ], Michal Suchánek [ ],
	Jean-Noël AVILA [ ], Eric Sunshine

On Thu, Mar 7, 2024 at 7:30 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > 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/pull.1675.v4.git.1709716446874.gitgitgadget@gmail.com/raw
> >
> > so, the experiment revealed that it did send some headers were
> > broken.
>
> 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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-03-07 18:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-29 12:23 [PATCH] tests: modernize the test script t0010-racy-git.sh Aryan Gupta via GitGitGadget
2024-02-29 18:11 ` Eric Sunshine
2024-02-29 21:57 ` [PATCH v2] " Aryan Gupta via GitGitGadget
2024-02-29 22:19   ` Eric Sunshine
2024-02-29 23:14   ` Junio C Hamano
2024-02-29 23:22     ` Eric Sunshine
2024-02-29 23:36       ` Junio C Hamano
2024-02-29 23:52         ` Eric Sunshine
2024-03-01  0:06           ` Eric Sunshine
2024-03-01  2:03             ` Junio C Hamano
2024-03-05 22:09   ` [PATCH v3] " Aryan Gupta via GitGitGadget
2024-03-05 23:02     ` Junio C Hamano
2024-03-06  9:14     ` [PATCH v4] " Aryan Gupta via GitGitGadget
2024-03-07 13:28       ` Christian Couder
2024-03-07 14:23         ` Aryan Gupta
2024-03-07 18:17           ` Junio C Hamano
2024-03-07 18:30             ` Junio C Hamano
2024-03-07 18:33               ` Aryan Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).