From: AbdAlRahman Gad <abdobngad@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v4 4/8] [Newcomer] t7004: Do not prepare things outside test_expect_success
Date: Tue, 6 Aug 2024 11:41:29 +0300 [thread overview]
Message-ID: <fa477ba6-18d2-461f-86fb-4fff75464b9c@gmail.com> (raw)
In-Reply-To: <CAPig+cQehwONpjnjPVJEVvviistiSLODLNx-yFBusEPaihnf+w@mail.gmail.com>
On 8/6/24 06:29, Eric Sunshine wrote:
> On Mon, Aug 5, 2024 at 8:00 PM AbdAlRahman Gad <abdobngad@gmail.com> wrote:
>> Do not prepare expect and other things outside test_expect_success.
>> Also add '\' before EOF to avoid shell interpolation and '-' to allow
>> indentation of the body.
>>
>> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
>
> When you reroll, it might be a good idea to explain _why_ we don't
> want to run code outside of `test_expect_success`: in particular, if
> such code fails for some reason, we won't necessarily hear about it in
> a timely fashion (or perhaps at all). By placing all code inside
> `test_expect_success` it ensures that we know immediately if it fails.
Thanks! I'll do that in the next version.
>> test_expect_success GPG,RFC1991 \
>> 'reediting a signed tag body omits signature' '
>> + cat >fakeeditor <<-\EOF &&
>> + #!/bin/sh
>> + cp "$1" actual
>> + EOF
>> + chmod +x fakeeditor &&
>
> It's completely outside the scope of this particular patch, but
> another modernization which could be done in a separate patch (or a
> separate patch series -- it doesn't need to be part of this series)
> would be to take advantage of `write_script` which eliminates some of
> the boilerplate-work that the above code is doing. For instance:
>
> write_script fakeeditor <<-\EOF
> cp "$1" actual
> EOF
>
> Notice that `write_script` takes care of emitting the `#!/bin/sh` line
> and the `chmod +x`.
There is only two cases in this file. I'll add a patch to the end of v5.
next prev parent reply other threads:[~2024-08-06 8:41 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-05 23:59 [PATCH v4 0/8] t7004: Modernize the style AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 1/8] [Newcomer] t7004: Remove space after redirect operators AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 2/8] [Newcomer] t7004: Do not lose exit status to pipe AbdAlRahman Gad
2024-08-06 3:13 ` Eric Sunshine
2024-08-06 8:38 ` AbdAlRahman Gad
2024-08-06 18:05 ` Eric Sunshine
2024-08-05 23:59 ` [PATCH v4 3/8] [Newcomer] t7004: One command per line AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 4/8] [Newcomer] t7004: Do not prepare things outside test_expect_success AbdAlRahman Gad
2024-08-06 3:29 ` Eric Sunshine
2024-08-06 8:41 ` AbdAlRahman Gad [this message]
2024-08-05 23:59 ` [PATCH v4 5/8] [Newcomer] t7004: Use indented here-doc AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 6/8] [Newcomer] t7004: Description on the same line as test_expect_success AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 7/8] [Newcomer] t7004: test Description and test body seperated with backslash AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 8/8] [Newcomer] t7004: Use single quotes instead of double quotes AbdAlRahman Gad
2024-08-06 3:02 ` Eric Sunshine
2024-08-06 10:10 ` AbdAlRahman Gad
2024-08-06 17:21 ` Eric Sunshine
2024-08-06 16:35 ` Junio C Hamano
2024-08-06 17:11 ` Eric Sunshine
2024-08-06 0:45 ` [PATCH v4 0/8] t7004: Modernize the style Junio C Hamano
2024-08-06 8:44 ` 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=fa477ba6-18d2-461f-86fb-4fff75464b9c@gmail.com \
--to=abdobngad@gmail.com \
--cc=git@vger.kernel.org \
--cc=sunshine@sunshineco.com \
/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.