Bug #6884

test_fileutils.rb might end up rm -rf the whole current directory

Added by Benoit Daloze about 3 years ago. Updated about 3 years ago.

[ruby-core:<unknown>]
Status:Closed
Priority:Normal
Assignee:-
ruby -v:ruby 2.0.0dev (2012-08-15 trunk 36709) [x86_64-darwin10.8.0] Backport:

Description

Hi,
I was preparing r36709 and launching make test-all and I interrupted it (Ctrl+C) as I forgot something.
The next thing I saw is that my repository was deleted, as well as almost half the files.
I think I don't need to tell you how that is fun. Hopefully I had a recent backup.

So, the culprit seems the fileutils tests coupled with #teardown being in an ensure clause.
You can see this behavior in https://gist.github.com/ab7d5ea2ae68eda6398b (if you press Ctrl+C within 2 seconds or not), and in
test/fileutils/test_fileutils.rb lines 97-111. If #setup is interrupted, then #teardown is still run, because it is called an ensure block (in lib/minitest/unit.rb line 1074).
What probably happened in my case is #setup did not yet Dir.chdir'd, and thus Dir.pwd in #teardown was the original current directory, and it happily rm -rf'd that at line 110.

The fix for fileutils tests is obvious, Dir.pwd should not be used there.
I'll fix them unless objection.

But I'm worried about #teardown always being run (due to the ensure clause).
Is that safe in the general case? It means the code must handle any part of #setup which was ran, which is increasingly complex.

Sorry for the story-telling style, but I think it might better represent the danger in this issue.

Associated revisions

Revision 36756
Added by Benoit Daloze about 3 years ago

make FileUtils tests safe when interrupting in setup

  • test/fileutils/test_fileutils.rb (TestFileUtils#teardown): do not assume cwd is TMPROOT and never remove current directory. [Bug #6884]

Revision 36756
Added by Benoit Daloze about 3 years ago

make FileUtils tests safe when interrupting in setup

  • test/fileutils/test_fileutils.rb (TestFileUtils#teardown): do not assume cwd is TMPROOT and never remove current directory. [Bug #6884]

History

#1 Updated by Benoit Daloze about 3 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r36756.
Benoit, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


make FileUtils tests safe when interrupting in setup

  • test/fileutils/test_fileutils.rb (TestFileUtils#teardown): do not assume cwd is TMPROOT and never remove current directory. [Bug #6884]

Also available in: Atom PDF