Re: [PATCH v5 0/8] [Newcomer] t7004: modernize the style
On 8/8/24 18:42, Junio C Hamano wrote:
Show 37 quoted lines
> AbdAlRahman Gad <abdobngad@gmail.com> writes:
>
>> - Remove whitespace after the redirect operators.
>>
>> - Move number of expect files prepared outside of
>> test_expect_success to be inside the tests that use it.
>>
>> - Split some lines that have two commands into two lines
>> one command each.
>>
>> - Turn some "<<\EOF" into "<<-\EOF" and indent their body.
>>
>> - Avoid using pipes in the output from "test-tool ref-store"
>> and write the output to a file.
>>
>> - Change test_expect_success that are seperated from its name
>> to be on the same line.
>>
>> - Avoid separating test Description and test body with backslash
>>
>> - Use single quotes instead of double quotes for test description and
>> body.
>>
>> - Use write_script which takes care of emitting the `#!/bin/sh` line
>> and the `chmod +x`.
>
> I gave another look and they all looked fine. Except the title of
> one step that said
>
> t7004: test Description and test body seperated with backslash
>
> was a "Huh? what does it even mean?".
>
> t7004: begin the test body on the same line as test_expect_success
>
> or something? I dunno.
>
Thanks! I'll send a v6 right away.
Show 23 quoted lines
>> There are still tests that could lose exit status to pipe. This needs
>> to be modernized too, I will fix them in another patch series.
>
> ;-)
>
> Another one that I noticed is that we have quite a lot of
>
> cat >expect <<-EOF &&
> v1.1.3
> v2.0
> v3.0
> EOF
>
> that can be shortend to
>
> test_write_lines >expect v1.1.3 v2.0 v3.0 &&
>
> To use without extra quoting, test_write_lines is more limited, but
> the majority of here-doc used for expect files in this test are
> enumeration of tag names that we can write without any extra frills,
> and test_write_lines may be a very good fit for these use cases.
>
> Thanks.
I'll work on them the next patch series.