| Message ID | 20b95372-12cf-49bd-b1b7-dc069e7c86dd@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | format-patch: assume --cover-letter for diff in multi-patch series | expand |
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? 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), 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.
On Fri, Jun 07, 2024 at 10:14:10AM -0700, Junio C Hamano wrote: > 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> writes: >> 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? That's following the "grep for them" advice ;-) >> 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 :-) It does protect us from careless test writers. At least, when we see the care has been taken to make sure the "clean-up" tasks covers both cases where the main test did or failed to create the cruft to be removed, that assures us that the test writers were thinking it through. But of course, those who blindly cut and paste the "|| :" pattern would fool such protection measure X-<. ;-)
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 && git format-patch -1 --stdout >out && ! grep "notes config message" out && @@ -848,10 +848,10 @@ 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 note1 add -m "this is note 1" HEAD && - test_when_finished git notes --ref note1 remove HEAD && git notes --ref note2 add -m "this is note 2" HEAD && - test_when_finished git notes --ref note2 remove HEAD && git format-patch -1 --stdout >out && ! grep "this is note 1" out && @@ -892,10 +892,10 @@ test_expect_success 'format-patch with multiple notes refs' ' 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 note1 add -m "this is note 1" HEAD && - test_when_finished git notes --ref note1 remove HEAD && git notes --ref note2 add -m "this is note 2" HEAD && - test_when_finished git notes --ref note2 remove HEAD && git config format.notes note1 && git format-patch -1 --stdout >out &&
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(-)