On Mon, Aug 5, 2024 at 8:55 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
> ---
> t/t3206-range-diff.sh | 52 +++++++++++++++++++++----------------------
> diff --git 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?-*" &&
Do we care whether the action invoked by `test_when_finished` itself
succeeds or fails? In particular, should this be using `rm -f` rather
than `rm`?
test_when_finished "rm -f 000?-*" &&
> @@ -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 &&
Similarly, should this be using `|| :`?
test_when_finished "git notes remove topic unmodified || :" &&
