From: "lilinchao@oschina.cn" <lilinchao@oschina.cn>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git <git@vger.kernel.org>
Subject: Re: Re: [PATCH] ls-files: update test style
Date: Fri, 24 Jun 2022 12:57:59 +0800 [thread overview]
Message-ID: <2022062412565939884621@oschina.cn> (raw)
In-Reply-To: 220623.86wnd7k5un.gmgdl@evledraar.gmail.com
>
>On Thu, Jun 23 2022, Li Linchao via GitGitGadget wrote:
>
>> From: Li Linchao <lilinchao@oschina.cn>
>>
>> Update test style in t/t30[*].sh for uniformity, that's to
>> keep test title the same line with helper function itself.
>
>We have a few of these sorts of old style tests, and it's good to update
>them.
Yes. Currently there are at least 400+ old style tests :). It not easy for me
to fix them all at once with some magic regex expressions, so I'm not going
to update them all in one patch. But I think, first of all, we can explicitly
document which test style we prefer first.
>
>> Write test code like this:
>> diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
>> index 48cec4e5f88..76361b92336 100755
>> --- a/t/t3001-ls-files-others-exclude.sh
>> +++ b/t/t3001-ls-files-others-exclude.sh
>> @@ -67,26 +67,26 @@ echo '!*.2
>>
>> allignores='.gitignore one/.gitignore one/two/.gitignore'
>>
>> -test_expect_success \
>> - 'git ls-files --others with various exclude options.' \
>> - 'git ls-files --others \
>> +test_expect_success 'git ls-files --others with various exclude options.' '
>> + git ls-files --others \
>> --exclude=\*.6 \
>> --exclude-per-directory=.gitignore \
>> --exclude-from=.git/ignore \
>> >output &&
>
>This though really stops too short, here we end up with:
>
> <TAB>git-ls-files --others \
> <7 spaces>--exclude [...]
>
OK.
>> - test_cmp expect output'
>> + test_cmp expect output
>
>And you've space-indented this test_cmp, presumably the below has the
>same issues (I didn't check in detail)
>
>Instead the argument lists should be <TAB><TAB> indented, and the rest
>should be TAB indented.
OK.
>
>> +'
>>
>> # Test \r\n (MSDOS-like systems)
>> printf '*.1\r\n/*.3\r\n!*.6\r\n' >.gitignore
>>
>> -test_expect_success \
>> - 'git ls-files --others with \r\n line endings.' \
>> - 'git ls-files --others \
>> +test_expect_success 'git ls-files --others with \r\n line endings.' '
>> + git ls-files --others \
>> --exclude=\*.6 \
>> --exclude-per-directory=.gitignore \
>> --exclude-from=.git/ignore \
>> >output &&
>> - test_cmp expect output'
>> + test_cmp expect output
>> +'
>
>Aside from the above I think it's also worth incorporating all the
>"printf", "echo", "cat" etc. that we do into the "test_expect_success"
>themselves, and if they're needed by more than one test perhaps make
>them a "setup" helper function (which would test_when_finished "rm -f
>.gitignore" clean up after itself).
Yes, make sense.
>
>That's obviously bigger than some whitespace changes, so we could punt
>on it for now, but as we're looking at this anyway we could convert
>fully to a more modern style in a follow-up commit...
>
>> -test_expect_success \
>> - 'git ls-files with path restriction with --.' \
>> - 'git ls-files --others -- path0 >output &&
>> +test_expect_success 'git ls-files with path restriction with --.' '
>> + git ls-files --others -- path0 >output &&
>> test_cmp output - <<EOF
>> path0
>> EOF
>> '
>
>On the topic of leaving things on the table: here we could use "<<-EOF"
>(or actually better "<<-\EOF") instead, and indent the here-doc, as we
>usually do.
OK, will do.
next prev parent reply other threads:[~2022-06-24 5:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-23 8:46 [PATCH] ls-files: update test style Li Linchao via GitGitGadget
2022-06-23 10:50 ` Ævar Arnfjörð Bjarmason
2022-06-24 4:57 ` lilinchao [this message]
2022-06-23 17:09 ` Junio C Hamano
2022-06-24 5:05 ` lilinchao
2022-06-28 9:14 ` [PATCH v2] " Li Linchao via GitGitGadget
2022-06-28 9:51 ` [PATCH v3] " Li Linchao via GitGitGadget
2022-06-28 20:12 ` Junio C Hamano
2022-06-29 7:12 ` lilinchao
2022-06-30 15:54 ` Junio C Hamano
2022-06-30 5:59 ` [PATCH v4] " Li Linchao via GitGitGadget
2022-07-01 11:03 ` [PATCH v5] " Li Linchao via GitGitGadget
2022-07-01 21:46 ` Junio C Hamano
2022-07-03 15:49 ` [PATCH v6] " Li Linchao via GitGitGadget
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2022062412565939884621@oschina.cn \
--to=lilinchao@oschina.cn \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).