From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v2] t3206: test_when_finished before dirtying operations, not after
Date: Tue, 06 Aug 2024 10:04:13 -0700 [thread overview]
Message-ID: <xmqqwmkttwfm.fsf@gitster.g> (raw)
In-Reply-To: <xmqqttfyzd01.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 05 Aug 2024 17:55:10 -0700")
Many existing tests in this script perform operation(s) and then use
test_when_finished to define how to undo the effect of the
operation(s).
This is backwards. When your operation(s) fail before you manage to
successfully call test_when_finished (remember, that these commands
must be all &&-chained, so a failure of an earlier operation mean
your test_when_finished may not be executed at all). You must
establish how to clean up your mess with test_when_finished before
you create the mess to be cleaned up.
Also make sure that the body of test_when_finished deals with case
where the cruft it wants to remove failed to be created, by using
"rm -f" (instead of "rm") to remove potential cruft files, and
having "|| :" after "git notes remove" to remove potential cruft
notes---both of these by default fail when asked to remove something
that does not exist, instead of being silently idempotent no-ops.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* a range-diff appears at the end. The last paragraph of the
proposed log message is new, and the differences in patch text
are about making sure test_when_finished commands are prepared
for their body failing.
t/t3206-range-diff.sh | 52 +++++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index a767c3520e..d2ca43d6aa 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -533,9 +533,9 @@ test_expect_success 'dual-coloring' '
for prev in topic main..topic
do
test_expect_success "format-patch --range-diff=$prev" '
+ test_when_finished "rm -f 000?-*" &&
git format-patch --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
test_line_count = 5 actual &&
test_grep "^Range-diff:$" 0000-* &&
grep "= 1: .* s/5/A" 0000-* &&
@@ -560,32 +560,32 @@ test_expect_success "explicit --no-cover-letter defeats implied --cover-letter"
'
test_expect_success 'format-patch --range-diff as commentary' '
+ test_when_finished "rm -f 0001-*" &&
git format-patch --range-diff=HEAD~1 HEAD~1 >actual &&
- test_when_finished "rm 0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff:$" 0001-* &&
grep "> 1: .* new message" 0001-*
'
test_expect_success 'format-patch --range-diff reroll-count with a non-integer' '
+ test_when_finished "rm -f v2.9-0001-*" &&
git format-patch --range-diff=HEAD~1 -v2.9 HEAD~1 >actual &&
- test_when_finished "rm v2.9-0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff:$" v2.9-0001-* &&
grep "> 1: .* new message" v2.9-0001-*
'
test_expect_success 'format-patch --range-diff reroll-count with a integer' '
+ test_when_finished "rm -f v2-0001-*" &&
git format-patch --range-diff=HEAD~1 -v2 HEAD~1 >actual &&
- test_when_finished "rm v2-0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff ..* v1:$" v2-0001-* &&
grep "> 1: .* new message" v2-0001-*
'
test_expect_success 'format-patch --range-diff with v0' '
+ test_when_finished "rm -f v0-0001-*" &&
git format-patch --range-diff=HEAD~1 -v0 HEAD~1 >actual &&
- test_when_finished "rm v0-0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff:$" v0-0001-* &&
grep "> 1: .* new message" v0-0001-*
@@ -606,9 +606,9 @@ test_expect_success 'basic with modified format.pretty without "commit "' '
'
test_expect_success 'range-diff compares notes by default' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
git range-diff --no-color main..topic main..unmodified \
>actual &&
sed s/Z/\ /g >expect <<-EOF &&
@@ -630,9 +630,9 @@ test_expect_success 'range-diff compares notes by default' '
'
test_expect_success 'range-diff with --no-notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
git range-diff --no-color --no-notes main..topic main..unmodified \
>actual &&
cat >expect <<-EOF &&
@@ -645,12 +645,12 @@ test_expect_success 'range-diff with --no-notes' '
'
test_expect_success 'range-diff with multiple --notes' '
+ test_when_finished "git notes --ref=note1 remove topic unmodified || :" &&
git notes --ref=note1 add -m "topic note1" topic &&
git notes --ref=note1 add -m "unmodified note1" unmodified &&
- test_when_finished git notes --ref=note1 remove topic unmodified &&
+ test_when_finished "git notes --ref=note2 remove topic unmodified || :" &&
git notes --ref=note2 add -m "topic note2" topic &&
git notes --ref=note2 add -m "unmodified note2" unmodified &&
- test_when_finished git notes --ref=note2 remove topic unmodified &&
git range-diff --no-color --notes=note1 --notes=note2 main..topic main..unmodified \
>actual &&
sed s/Z/\ /g >expect <<-EOF &&
@@ -678,12 +678,12 @@ test_expect_success 'range-diff with multiple --notes' '
# `range-diff` should act like `log` with regards to notes
test_expect_success 'range-diff with --notes=custom does not show default notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
+ test_when_finished "git notes --ref=custom remove topic unmodified || :" &&
git notes --ref=custom add -m "topic note" topic &&
git notes --ref=custom add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
- test_when_finished git notes --ref=custom remove topic unmodified &&
git range-diff --notes=custom main..topic main..unmodified \
>actual &&
! grep "## Notes ##" actual &&
@@ -691,12 +691,12 @@ test_expect_success 'range-diff with --notes=custom does not show default notes'
'
test_expect_success 'format-patch --range-diff does not compare notes by default' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
+ test_when_finished "rm -f 000?-*" &&
git format-patch --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
test_line_count = 5 actual &&
test_grep "^Range-diff:$" 0000-* &&
grep "= 1: .* s/5/A" 0000-* &&
@@ -708,26 +708,26 @@ test_expect_success 'format-patch --range-diff does not compare notes by default
'
test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
- git notes --ref=custom add -m "topic note (custom)" topic &&
git notes add -m "unmodified note" unmodified &&
+ test_when_finished "git notes --ref=custom remove topic unmodified || :" &&
+ git notes --ref=custom add -m "topic note (custom)" topic &&
git notes --ref=custom add -m "unmodified note (custom)" unmodified &&
- test_when_finished git notes remove topic unmodified &&
- test_when_finished git notes --ref=custom remove topic unmodified &&
+ test_when_finished "rm -f 000?-*" &&
git format-patch --notes=custom --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
grep "## Notes (custom) ##" 0000-* &&
! grep "## Notes ##" 0000-*
'
test_expect_success 'format-patch --range-diff with --no-notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
+ test_when_finished "rm -f 000?-*" &&
git format-patch --no-notes --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
test_line_count = 5 actual &&
test_grep "^Range-diff:$" 0000-* &&
grep "= 1: .* s/5/A" 0000-* &&
@@ -739,12 +739,12 @@ test_expect_success 'format-patch --range-diff with --no-notes' '
'
test_expect_success 'format-patch --range-diff with --notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
+ test_when_finished "rm -f 000?-*" &&
git format-patch --notes --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
test_line_count = 5 actual &&
test_grep "^Range-diff:$" 0000-* &&
grep "= 1: .* s/5/A" 0000-* &&
@@ -767,13 +767,13 @@ test_expect_success 'format-patch --range-diff with --notes' '
'
test_expect_success 'format-patch --range-diff with format.notes config' '
+ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
test_config format.notes true &&
+ test_when_finished "rm -f 000?-*" &&
git format-patch --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
test_line_count = 5 actual &&
test_grep "^Range-diff:$" 0000-* &&
grep "= 1: .* s/5/A" 0000-* &&
@@ -796,15 +796,15 @@ test_expect_success 'format-patch --range-diff with format.notes config' '
'
test_expect_success 'format-patch --range-diff with multiple notes' '
+ test_when_finished "git notes --ref=note1 remove topic unmodified || :" &&
git notes --ref=note1 add -m "topic note1" topic &&
git notes --ref=note1 add -m "unmodified note1" unmodified &&
- test_when_finished git notes --ref=note1 remove topic unmodified &&
+ test_when_finished "git notes --ref=note2 remove topic unmodified || :" &&
git notes --ref=note2 add -m "topic note2" topic &&
git notes --ref=note2 add -m "unmodified note2" unmodified &&
- test_when_finished git notes --ref=note2 remove topic unmodified &&
+ test_when_finished "rm -f 000?-*" &&
git format-patch --notes=note1 --notes=note2 --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
test_line_count = 5 actual &&
test_grep "^Range-diff:$" 0000-* &&
grep "= 1: .* s/5/A" 0000-* &&
Range-diff:
1: 591c4d512f ! 1: 3a3c1a1a44 t3206: test_when_finished before dirtying operations, not after
@@ Commit message
establish how to clean up your mess with test_when_finished before
you create the mess to be cleaned up.
+ Also make sure that the body of test_when_finished deals with case
+ where the cruft it wants to remove failed to be created, by using
+ "rm -f" (instead of "rm") to remove potential cruft files, and
+ having "|| :" after "git notes remove" to remove potential cruft
+ notes---both of these by default fail when asked to remove something
+ that does not exist, instead of being silently idempotent no-ops.
+
+ Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
## t/t3206-range-diff.sh ##
@@ t/t3206-range-diff.sh: test_expect_success 'dual-coloring' '
for prev in topic main..topic
do
test_expect_success "format-patch --range-diff=$prev" '
-+ test_when_finished "rm 000?-*" &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
@@ t/t3206-range-diff.sh: test_expect_success "explicit --no-cover-letter defeats i
'
test_expect_success 'format-patch --range-diff as commentary' '
-- git format-patch --range-diff=HEAD~1 HEAD~1 >actual &&
- test_when_finished "rm 0001-*" &&
-+ git format-patch --range-diff=HEAD~1 HEAD~1 >actual &&
++ test_when_finished "rm -f 0001-*" &&
+ git format-patch --range-diff=HEAD~1 HEAD~1 >actual &&
+- test_when_finished "rm 0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff:$" 0001-* &&
grep "> 1: .* new message" 0001-*
'
test_expect_success 'format-patch --range-diff reroll-count with a non-integer' '
-- git format-patch --range-diff=HEAD~1 -v2.9 HEAD~1 >actual &&
- test_when_finished "rm v2.9-0001-*" &&
-+ git format-patch --range-diff=HEAD~1 -v2.9 HEAD~1 >actual &&
++ test_when_finished "rm -f v2.9-0001-*" &&
+ git format-patch --range-diff=HEAD~1 -v2.9 HEAD~1 >actual &&
+- test_when_finished "rm v2.9-0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff:$" v2.9-0001-* &&
grep "> 1: .* new message" v2.9-0001-*
'
test_expect_success 'format-patch --range-diff reroll-count with a integer' '
-- git format-patch --range-diff=HEAD~1 -v2 HEAD~1 >actual &&
- test_when_finished "rm v2-0001-*" &&
-+ git format-patch --range-diff=HEAD~1 -v2 HEAD~1 >actual &&
++ test_when_finished "rm -f v2-0001-*" &&
+ git format-patch --range-diff=HEAD~1 -v2 HEAD~1 >actual &&
+- test_when_finished "rm v2-0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff ..* v1:$" v2-0001-* &&
grep "> 1: .* new message" v2-0001-*
'
test_expect_success 'format-patch --range-diff with v0' '
-- git format-patch --range-diff=HEAD~1 -v0 HEAD~1 >actual &&
- test_when_finished "rm v0-0001-*" &&
-+ git format-patch --range-diff=HEAD~1 -v0 HEAD~1 >actual &&
++ test_when_finished "rm -f v0-0001-*" &&
+ git format-patch --range-diff=HEAD~1 -v0 HEAD~1 >actual &&
+- test_when_finished "rm v0-0001-*" &&
test_line_count = 1 actual &&
test_grep "^Range-diff:$" v0-0001-* &&
grep "> 1: .* new message" v0-0001-*
@@ t/t3206-range-diff.sh: test_expect_success 'basic with modified format.pretty wi
'
test_expect_success 'range-diff compares notes by default' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
@@ t/t3206-range-diff.sh: test_expect_success 'range-diff compares notes by default
'
test_expect_success 'range-diff with --no-notes' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
@@ t/t3206-range-diff.sh: test_expect_success 'range-diff with --no-notes' '
'
test_expect_success 'range-diff with multiple --notes' '
-+ test_when_finished git notes --ref=note1 remove topic unmodified &&
++ test_when_finished "git notes --ref=note1 remove topic unmodified || :" &&
git notes --ref=note1 add -m "topic note1" topic &&
git notes --ref=note1 add -m "unmodified note1" unmodified &&
- test_when_finished git notes --ref=note1 remove topic unmodified &&
-+ test_when_finished git notes --ref=note2 remove topic unmodified &&
++ test_when_finished "git notes --ref=note2 remove topic unmodified || :" &&
git notes --ref=note2 add -m "topic note2" topic &&
git notes --ref=note2 add -m "unmodified note2" unmodified &&
- test_when_finished git notes --ref=note2 remove topic unmodified &&
@@ t/t3206-range-diff.sh: test_expect_success 'range-diff with multiple --notes' '
# `range-diff` should act like `log` with regards to notes
test_expect_success 'range-diff with --notes=custom does not show default notes' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
-+ test_when_finished git notes --ref=custom remove topic unmodified &&
++ test_when_finished "git notes --ref=custom remove topic unmodified || :" &&
git notes --ref=custom add -m "topic note" topic &&
git notes --ref=custom add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
@@ t/t3206-range-diff.sh: test_expect_success 'range-diff with --notes=custom does
'
test_expect_success 'format-patch --range-diff does not compare notes by default' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
-+ test_when_finished "rm 000?-*" &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
@@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff does not c
'
test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
- git notes --ref=custom add -m "topic note (custom)" topic &&
git notes add -m "unmodified note" unmodified &&
-- git notes --ref=custom add -m "unmodified note (custom)" unmodified &&
-- test_when_finished git notes remove topic unmodified &&
- test_when_finished git notes --ref=custom remove topic unmodified &&
++ test_when_finished "git notes --ref=custom remove topic unmodified || :" &&
+ git notes --ref=custom add -m "topic note (custom)" topic &&
-+ git notes --ref=custom add -m "unmodified note (custom)" unmodified &&
-+ test_when_finished "rm 000?-*" &&
+ git notes --ref=custom add -m "unmodified note (custom)" unmodified &&
+- test_when_finished git notes remove topic unmodified &&
+- test_when_finished git notes --ref=custom remove topic unmodified &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --notes=custom --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
@@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff does not c
'
test_expect_success 'format-patch --range-diff with --no-notes' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
-+ test_when_finished "rm 000?-*" &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --no-notes --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
@@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with --no-
'
test_expect_success 'format-patch --range-diff with --notes' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
-+ test_when_finished "rm 000?-*" &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --notes --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
@@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with --not
'
test_expect_success 'format-patch --range-diff with format.notes config' '
-+ test_when_finished git notes remove topic unmodified &&
++ test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
git notes add -m "unmodified note" unmodified &&
- test_when_finished git notes remove topic unmodified &&
test_config format.notes true &&
-+ test_when_finished "rm 000?-*" &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
@@ t/t3206-range-diff.sh: test_expect_success 'format-patch --range-diff with forma
'
test_expect_success 'format-patch --range-diff with multiple notes' '
-+ test_when_finished git notes --ref=note1 remove topic unmodified &&
++ test_when_finished "git notes --ref=note1 remove topic unmodified || :" &&
git notes --ref=note1 add -m "topic note1" topic &&
git notes --ref=note1 add -m "unmodified note1" unmodified &&
- test_when_finished git notes --ref=note1 remove topic unmodified &&
-+ test_when_finished git notes --ref=note2 remove topic unmodified &&
++ test_when_finished "git notes --ref=note2 remove topic unmodified || :" &&
git notes --ref=note2 add -m "topic note2" topic &&
git notes --ref=note2 add -m "unmodified note2" unmodified &&
- test_when_finished git notes --ref=note2 remove topic unmodified &&
-+ test_when_finished "rm 000?-*" &&
++ test_when_finished "rm -f 000?-*" &&
git format-patch --notes=note1 --notes=note2 --cover-letter --range-diff=$prev \
main..unmodified >actual &&
- test_when_finished "rm 000?-*" &&
--
2.46.0-235-g968ce1ce0e
prev parent reply other threads:[~2024-08-06 17:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-06 0:55 [PATCH] t3206: test_when_finished before dirtying operations, not after Junio C Hamano
2024-08-06 2:21 ` Eric Sunshine
2024-08-06 16:52 ` Junio C Hamano
2024-08-06 17:04 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwmkttwfm.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sunshine@sunshineco.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.