Move the directory maker into RSpec::Support #74
Conversation
| Dir.mkdir(stack) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
👍 This looks much cleaner ( ﾟヮﾟ) Any reason for the multi-line unless? Just personal taste?
There was a problem hiding this comment.
@cupakromer will prevent churn in future if anything else needs to happen in that conditional. Makes diffs easier to read IMO.
|
@myronmarston @cupakromer can I get another review pass on this? |
| describe ".mkdir_p" do | ||
| include_context "isolated directory" | ||
|
|
||
| let(:dirname) { File.join(%w{tmp a recursive structure}) } |
There was a problem hiding this comment.
I perfer [ rather than { for these sorts of arrays since [ is the normal array character. With rubocop we can configure and enforce this as well.
|
One more suggestion: since we want the behavior of this to mirror that of |
| expect { | ||
| DirectoryMaker.mkdir_p(dirname) | ||
| }.not_to raise_error | ||
| end |
There was a problem hiding this comment.
I'd prefer this to be a positive expectations:
Dir.mkdir("tmp")
DirectoryMaker.mkdir_p(dirname)
expect(Dir.exist?(dirname)).to be trueThis verifies we actually create the remaining directories. Additionally, if an error is raised the spec will still fail.
|
Would be good to squash this down, too. |
| it "makes directories recursively" do | ||
| DirectoryMaker.mkdir_p(File.expand_path(dirname)) | ||
| expect(File.exist?(dirname)).to be true | ||
| end |
I like this idea 👍 |
|
@myronmarston @cupakromer it appears that 1.8.7 has no |
|
Let's use |
|
Just to throw it out there, there is def dir_exist?(dirname)
File.exist?(dirname) && File.directory?(dirname)
end |
|
I'd be happy with that solution |
|
@myronmarston squash and merge when green? |
|
(｡◕‿◕｡) LGTM |
|
A review comment got suppressed about using |
|
I'm gonna merge this when travis goes green. |
|
(after squashing) |
Added so that we don't have to depend on FileUtils and its mkdir_p implementation anywhere in any of the other RSpec gems.
Move the directory maker into RSpec::Support
Required before rspec/rspec-core#1565 can be merged.