Torsten Bögershausen <tboegi@xxxxxx> writes: > [] >>> >>> - cd "$(dirname "$remove_trash")" && >>> - rm -rf "$(basename "$remove_trash")" || >>> - error "Tests passed but test cleanup failed; aborting" >>> + cd "$(dirname "$TRASH_DIRECTORY")" && >>> + rm -fr "$TRASH_DIRECTORY" || >>> + error "Tests passed but test cleanup failed; aborting" >>> + fi >> >> Yeah, that looks good to me. > > Does it ? > Checking the error code of "rm -f" doesn't work as expected from the script: > rm -rf DoesNotExist ; echo $? > 0 > > I think it should be > >>> + cd "$(dirname "$TRASH_DIRECTORY")" && >>> + rm -r "$TRASH_DIRECTORY" || >>> + error "Tests passed but test cleanup failed; aborting" What Peff told you in his response is all correct, but besides, you can see the patch and realize that the original has been using "rm -rf" for ages. This change is about not using $remove_trash variable, as it felt brittle and error prone than detecting $debug case and removing $TRASH_DIRECTORY. So in that sense, I shouldn't have even rolled the removal of basename into it. Having said that, because people care about the number of subprocess invocations, I am tempted to suggest doing cd "$TRASH_DIRECTORY/.." && rm -fr "$TRASH_DIRECTORY" || error ... which should reduce another process ;-)