Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: >> + test_when_finished "git tag -d branch-and-tag-name" && >> + git tag branch-and-tag-name && > > If git-tag crashes before actually creating the new tag, then "git tag > -d", passed to test_when_finished(), will error out too, which is > probably undesirable since "cleanup code" isn't expected to error out. Ah, I somehow thought that clean-up actions set up via when_finished are allowed to fail without affecting the outcome, but apparently I was mistaken. This however can be argued both ways---if you create a tag first and try to set up the clean-up action, during which you may get in trouble and end up leaving the tag behind. So rather than swapping the two lines, explicitly preparing for the case the clean-up action fails, i.e. the first alternative below, would be a good fix. Also it is a good question if the tag need to be even cleaned up. > You could fix it this way: > > test_when_finished "git tag -d branch-and-tag-name || :" && > git tag branch-and-tag-name && > > or, even better, just swap the two lines: > > git tag branch-and-tag-name && > test_when_finished "git tag -d branch-and-tag-name" && > However, do you even need to clean up the tag? Are there tests > following this one which expect a certain set of tags and fail if this > new one is present? If not, a simpler approach might be just to leave > the tag alone (and the branch too if that doesn't need to be cleaned > up). > >> + git branch --show-current >actual && >> + test_cmp expect actual >> +' A bigger question we may want to ask ourselves is if we want to detect failures from these clean-up actions in the first place. There are many hits from "git grep 'when_finished .*|| :' t/", which may be a sign that the when_finished mechanism was misdesigned and we should simply ignore the exit status from the clean-up actions instead. I haven't gone through the list of when_finished clean-up actions that do not end with "|| :"; I suspect some of them are simply being sloppy and would want to have "|| :", but what I want to find out out of such an audit is if there is a legitimate case where it helps to catch failures in the clean-up actions. If there is none, then ...
