* [PATCH] t9001: use a more distinct fake BugID
@ 2024-09-15 11:31 Jeff King
2024-09-16 7:27 ` Patrick Steinhardt
0 siblings, 1 reply; 2+ messages in thread
From: Jeff King @ 2024-09-15 11:31 UTC (permalink / raw)
To: git
In the test "cc list is sanitized", we feed a commit with a variety of
trailers to send-email, and then check its output to see how it handled
them. For most of them, we are grepping for a specific mention of the
header, but there's a "BugID" header which we expect to be ignored. We
confirm this by grepping for "12345", the fake BugID, and making sure it
is not present.
But we can be fooled by false positives! I just tracked down a flaky
test failure here that was caused by matching this unrelated line in the
output:
<20240914090449.612345-1-author@example.com>
which will change from run to run based on the time, pid, etc.
Ideally we'd tighten the regex to make this more specifically, but since
the point is that it _shouldn't_ be mentioned, it's hard to say what the
right match would be (e.g., would there be a leading space?).
Instead, let's just choose a match that is much less likely to appear.
The actual content of the header isn't important, since it's supposed to
be ignored.
Signed-off-by: Jeff King <peff@peff.net>
---
I guess this is fairly unlikely, as re-running the test with --stress
didn't reproduce after a few hundred attempts. Back of the envelope, I
guess any 5-digit sequence has a 1-in-10^5 chance of matching our
target. There are 2 in chances in a 6-digit pid. Some in the date, but
as there's no December 34th or hour 34, you're limited to a few specific
times like 01:23:45 (or at 11am), 12:34:5x, or perhaps 23:45 on the 1st,
11th, or 21st of the month.
So I think I just got really unlucky, but after spending several minutes
debugging, I wouldn't want to wish it on anybody else. ;)
t/t9001-send-email.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index df5336bb7e..e2430f7bfa 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1324,7 +1324,7 @@ test_expect_success $PREREQ 'cc list is sanitized' '
Reviewed-by: Füñný Nâmé <odd_?=mail@example.com>
Reported-by: bugger on Jira
Reported-by: Douglas Reporter <doug@example.com> [from Jira profile]
- BugID: 12345
+ BugID: 12345should-not-appear
Co-developed-by: "C. O. Developer" <codev@example.com>
Signed-off-by: A. U. Thor <thor.au@example.com>
EOF
@@ -1337,7 +1337,7 @@ test_expect_success $PREREQ 'cc list is sanitized' '
" <odd_?=mail@example.com>" actual-show-all-headers &&
test_grep "^(body) Ignoring Reported-by .* bugger on Jira" actual-show-all-headers &&
test_grep "^(body) Adding cc: Douglas Reporter <doug@example.com>" actual-show-all-headers &&
- test_grep ! "12345" actual-show-all-headers &&
+ test_grep ! "12345should-not-appear" actual-show-all-headers &&
test_grep "^(body) Adding cc: \"C. O. Developer\" <codev@example.com>" actual-show-all-headers &&
test_grep "^(body) Adding cc: \"A. U. Thor\" <thor.au@example.com>" actual-show-all-headers
'
--
2.46.1.893.gc4b01a7614
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] t9001: use a more distinct fake BugID
2024-09-15 11:31 [PATCH] t9001: use a more distinct fake BugID Jeff King
@ 2024-09-16 7:27 ` Patrick Steinhardt
0 siblings, 0 replies; 2+ messages in thread
From: Patrick Steinhardt @ 2024-09-16 7:27 UTC (permalink / raw)
To: Jeff King; +Cc: git
On Sun, Sep 15, 2024 at 07:31:15AM -0400, Jeff King wrote:
> In the test "cc list is sanitized", we feed a commit with a variety of
> trailers to send-email, and then check its output to see how it handled
> them. For most of them, we are grepping for a specific mention of the
> header, but there's a "BugID" header which we expect to be ignored. We
> confirm this by grepping for "12345", the fake BugID, and making sure it
> is not present.
>
> But we can be fooled by false positives! I just tracked down a flaky
> test failure here that was caused by matching this unrelated line in the
> output:
>
> <20240914090449.612345-1-author@example.com>
>
> which will change from run to run based on the time, pid, etc.
>
> Ideally we'd tighten the regex to make this more specifically, but since
> the point is that it _shouldn't_ be mentioned, it's hard to say what the
> right match would be (e.g., would there be a leading space?).
>
> Instead, let's just choose a match that is much less likely to appear.
> The actual content of the header isn't important, since it's supposed to
> be ignored.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I guess this is fairly unlikely, as re-running the test with --stress
> didn't reproduce after a few hundred attempts. Back of the envelope, I
> guess any 5-digit sequence has a 1-in-10^5 chance of matching our
> target. There are 2 in chances in a 6-digit pid. Some in the date, but
> as there's no December 34th or hour 34, you're limited to a few specific
> times like 01:23:45 (or at 11am), 12:34:5x, or perhaps 23:45 on the 1st,
> 11th, or 21st of the month.
>
> So I think I just got really unlucky, but after spending several minutes
> debugging, I wouldn't want to wish it on anybody else. ;)
Heh, unlucky indeed. In any case, the change makes sense to me, thanks!
Patrick
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-09-16 7:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-15 11:31 [PATCH] t9001: use a more distinct fake BugID Jeff King
2024-09-16 7:27 ` Patrick Steinhardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox