From: Junio C Hamano <gitster@pobox.com>
To: Jerry Zhang <jerry@skydio.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH V2] patch-id: fix scan_hunk_header on diffs with 1 line of before/after
Date: Mon, 31 Jan 2022 15:05:25 -0800 [thread overview]
Message-ID: <xmqqmtjbh5fu.fsf@gitster.g> (raw)
In-Reply-To: <20220129024757.24763-1-jerry@skydio.com> (Jerry Zhang's message of "Fri, 28 Jan 2022 18:47:57 -0800")
Jerry Zhang <jerry@skydio.com> writes:
> diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh
> index 80f4a65b28..34a386aee2 100755
> --- a/t/t4204-patch-id.sh
> +++ b/t/t4204-patch-id.sh
> @@ -200,6 +200,36 @@ EOF
> test_expect_success 'patch-id handles no-nl-at-eof markers' '
> cat nonl | calc_patch_id nonl &&
> cat withnl | calc_patch_id withnl &&
> test_cmp patch-id_nonl patch-id_withnl
> '
This is not a new issue, but this old test script needs updating.
And worse yet, your new test copies the existing badness and make it
twice as bad.
We frown upon doing anything outside test_expect_* blocks these
days, so the commands to prepare nonl and withnl files before thie
precontext of this patch needs to be folded into the test body.
Also, sending a single file using "cat" into pipeline is an
antipattern. Just redirect into the downstream command instead,
i.e.
test_expect_success 'patch-id handles no-nl-at-eof markers' '
cat >nonl <<-\EOF &&
diff --git i/a w/a
index e69de29..2e65efe 100644
--- i/a
+++ w/a
@@ -0,0 +1 @@
+a
\ No newline at end of file
diff --git i/b w/b
index e69de29..6178079 100644
--- i/b
+++ w/b
@@ -0,0 +1 @@
+b
EOF
cat >withnl <<-\EOF &&
... likewise ...
EOF
calc_patch_id nonl <nonl &&
calc_patch_id withnl <withnl &&
test_cmp patch-i_ nonl patch-id-withnl
'
or something like that.
I think the "handles no-nl-at-eof markers" test at the end is the
first and only serious offender, so instead of leaving it after the
series, we probalby would want to have a preliminary clean-up patch
before this patch, and fix the test this patch adds.
Thanks.
prev parent reply other threads:[~2022-01-31 23:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-25 22:16 [PATCH] patch-id: fix scan_hunk_header on diffs with -U1 Jerry Zhang
2022-01-26 5:13 ` Junio C Hamano
2022-01-29 2:47 ` [PATCH V2] patch-id: fix scan_hunk_header on diffs with 1 line of before/after Jerry Zhang
2022-01-31 18:55 ` Junio C Hamano
2022-01-31 22:54 ` Junio C Hamano
2022-01-31 23:05 ` Junio C Hamano [this message]
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=xmqqmtjbh5fu.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jerry@skydio.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).