[PATCH] t3206: test_when_finished before dirtying operations, not after

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH] t3206: test_when_finished before dirtying operations, not after
Date: Mon, 05 Aug 2024 17:55:10 -0700	[thread overview]
Message-ID: <xmqqttfyzd01.fsf@gitster.g> (raw)

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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * During a recent review of a topic, I noticed this file was
   littered with issues, and marked it as leftoverbits to clean it
   up after the dust settled.  The dust had settled, so it is a good
   time to clean up.

 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..d3bcd74236 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 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' '
-	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_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_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_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_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 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 &&
-	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 &&
+	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 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 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 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 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 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-* &&
-- 
2.46.0-235-g968ce1ce0e

next             reply	other threads:[~2024-08-06  0:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-06  0:55 Junio C Hamano [this message]
2024-08-06  2:21 ` [PATCH] t3206: test_when_finished before dirtying operations, not after Eric Sunshine
2024-08-06 16:52   ` Junio C Hamano
2024-08-06 17:04 ` [PATCH v2] " Junio C Hamano

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:a767c3520 dfblob:d3bcd7423 )
 OR (
bs:"[PATCH] t3206: test_when_finished before dirtying operations, not after" )
	(help)

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=xmqqttfyzd01.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /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.