All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: AbdAlRahman Gad <abdobngad@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 7/8] t7004: use single quotes instead of double quotes
Date: Thu, 08 Aug 2024 08:36:00 -0700	[thread overview]
Message-ID: <xmqqed6zkowv.fsf@gitster.g> (raw)
In-Reply-To: <20240807130259.28381-8-abdobngad@gmail.com> (AbdAlRahman Gad's message of "Wed, 7 Aug 2024 15:58:43 +0300")

AbdAlRahman Gad <abdobngad@gmail.com> writes:

> Some test bodies and test description are surrounded with double
> quotes instead of single quotes, violating our coding style.
>
> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
> ---
>  t/t7004-tag.sh | 70 +++++++++++++++++++++++++-------------------------
>  1 file changed, 35 insertions(+), 35 deletions(-)

The conversion in this step can cause unintended breakage and needs
to be carefully done, so I checked with "git show -W" (I probably
should have done -U999 instead of just -W).

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 2b15ede1f3..046a5bd9bc 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1583,7 +1583,7 @@ test_expect_success 'creating third commit without tag' '
>  
>  # simple linear checks of --continue
>  
> -test_expect_success 'checking that first commit is in all tags (hash)' "
> +test_expect_success 'checking that first commit is in all tags (hash)' '
>  	hash3=$(git rev-parse HEAD) &&

The original used to resolve "HEAD" while formulating the command
line of test_expect_success.  Now we resolve "HEAD" as the first
thing the body given to test_expect_success is run.  As HEAD does
not move between these two points in time, hash3 would get the same
value either way.


>  	cat >expected <<-\EOF &&
>  	v0.2.1
> @@ -1594,10 +1594,10 @@ test_expect_success 'checking that first commit is in all tags (hash)' "
>  	EOF
>  	git tag -l --contains $hash1 v* >actual &&
>  	test_cmp expected actual
> -"
> +'

The argument to the "--contains" option was interpolated while the
command line of test_expect_success was formulated.  If the body of
this test_expect_success modified the value of $hash1, this conversion
would have changed what the tested command did, but because nobody
assigns to $hash1 after it gets assigned (and this is true for other
$hash[0-9] variables), this conversion is OK.

>  for option in --contains --with --no-contains --without --merged --no-merged --points-at
>  do
> -	test_expect_success "mixing incompatible modes with $option is forbidden" "
> +	test_expect_success "mixing incompatible modes with $option is forbidden" '
>  		test_must_fail git tag -d $option HEAD &&
>  		test_must_fail git tag -d $option HEAD some-tag &&
>  		test_must_fail git tag -v $option HEAD
> -	"
> -	test_expect_success "Doing 'git tag --list-like $option <commit> <pattern> is permitted" "
> +	'
> +	test_expect_success "Doing 'git tag --list-like $option <commit> <pattern> is permitted" '
>  		git tag -n $option HEAD HEAD &&
>  		git tag $option HEAD HEAD &&
>  		git tag $option
> -	"
> +	'

Likewise.

The value of $option of course changes for each iteration of the
loop, and the original interpolated it into the tested scripts while
test_expect_success command lines were formulated in the original.
Now the variable $option is used just like any other variable while
the body is run, and the tested scripts behave identically either
way.

Looks good.  Thanks.

  reply	other threads:[~2024-08-08 15:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-07 12:58 [PATCH v5 0/8] [Newcomer] t7004: modernize the style AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 1/8] t7004: remove space after redirect operators AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 2/8] t7004: one command per line AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 3/8] t7004: do not prepare things outside test_expect_success AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 4/8] t7004: use indented here-doc AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 5/8] t7004: description on the same line as test_expect_success AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 6/8] t7004: test description and test body seperated with backslash AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 7/8] t7004: use single quotes instead of double quotes AbdAlRahman Gad
2024-08-08 15:36   ` Junio C Hamano [this message]
2024-08-07 12:58 ` [PATCH v5 8/8] t7004: make use of write_script AbdAlRahman Gad
2024-08-08 15:42 ` [PATCH v5 0/8] [Newcomer] t7004: modernize the style Junio C Hamano
2024-08-08 16:18   ` AbdAlRahman Gad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqed6zkowv.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=abdobngad@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.