Re: [PATCH v4 1/2] t4014: cleanups in a few tests
On Fri, Jun 07, 2024 at 10:14:10AM -0700, Junio C Hamano wrote:
Show 25 quoted lines
> Rubén Justo <rjusto@gmail.com> writes:
>
> > Arrange things we are going to create to be removed at end, and then
> > start creating them. That way, we will clean them up even if we fail
> > after creating some but before the end of the command.
> >
> > Signed-off-by: Rubén Justo <rjusto@gmail.com>
> > ---
> > t/t4014-format-patch.sh | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> > index e37a1411ee..5fb5250df4 100755
> > --- a/t/t4014-format-patch.sh
> > +++ b/t/t4014-format-patch.sh
> > @@ -820,8 +820,8 @@ test_expect_success 'format-patch --notes --signoff' '
> > '
> >
> > test_expect_success 'format-patch notes output control' '
> > + test_when_finished "git notes remove HEAD" &&
> > git notes add -m "notes config message" HEAD &&
> > - test_when_finished git notes remove HEAD &&
>
> If "notes add" fails to create a note for HEAD, test_when_finished
> would notice that it cannot remove a note from HEAD, wouldn't it?
Yep. Something like this, no?
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index de9e8455b3..1088c435e0 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -820,7 +820,7 @@ test_expect_success 'format-patch --notes --signoff' '
'
test_expect_success 'format-patch notes output control' '
- test_when_finished "git notes remove HEAD" &&
+ test_when_finished "git notes remove HEAD || :" &&
git notes add -m "notes config message" HEAD &&
git format-patch -1 --stdout >out &&
@@ -849,7 +849,7 @@ test_expect_success 'format-patch notes output control' '
test_expect_success 'format-patch with multiple notes refs' '
test_when_finished "git notes --ref note1 remove HEAD;
- git notes --ref note2 remove HEAD" &&
+ git notes --ref note2 remove HEAD || :" &&
git notes --ref note1 add -m "this is note 1" HEAD &&
git notes --ref note2 add -m "this is note 2" HEAD &&
@@ -893,7 +893,7 @@ test_expect_success 'format-patch with multiple notes refs in config' '
test_when_finished "test_unconfig format.notes" &&
test_when_finished "git notes --ref note1 remove HEAD;
- git notes --ref note2 remove HEAD" &&
+ git notes --ref note2 remove HEAD || :" &&
git notes --ref note1 add -m "this is note 1" HEAD &&
git notes --ref note2 add -m "this is note 2" HEAD &&
> If you do
>
> ! grep "notes config message" out &&
> git format-patch -1 --stdout --no-notes --notes >out &&
> - grep "notes config message" out
> + grep "notes config message" out &&
> + git notes remove HEAD
> '
>
> at the end of this passing test to remove the note from HEAD (so
> that when-finished handler has nothing to remove), and run "sh
> t4014-format-patch.sh -i -v", this test piece 4014.70 fails with
>
> ...
> notes config message
> Removing note for object HEAD
> Object HEAD has no note
> not ok 70 - format-patch notes output control
>
> A failure in the when-finished handler is noticed (which we might
> argue is a misfeature)
Dropping it doesn't seem like something to be strongly opposed to :-)
> , and that is why it is a good idea to write
>
> test_when_finished 'rm -f cruft-that-may-be-created' &&
> do what might create cruft-that-may-be-created
>
> with "-f".
>
> A standard trick can be found in the output of
>
> $ git grep 'finished.*|| *:' t/
>
> Thanks.
Thank you.
Rubén Justo <rjusto@gmail.com>