Re: [PATCH v5 0/8] [Newcomer] t7004: modernize the style

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2024, 15:42 UTC
Message-ID: <xmqq5xsbkolo.fsf@gitster.g>
In-Reply-To: <20240807130259.28381-1-abdobngad@gmail.com>

AbdAlRahman Gad <abdobngad@gmail.com> writes:

Show 23 quoted lines

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

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