Restore old expand path behavior #540
Conversation
mvz
force-pushed
the
issue-478-restore-old-expand_path-behavior
branch
from
3bc18cc to
130a583
Compare
There was a problem hiding this comment.
Thanks for making this monumental change!
Most of my feedback is concentrated on keeping the specs easy to read when they fail. Some of the context and it description texts have a hard time forming easy-to-read sentences when run in the --format documentation mode in RSpec.
(I guess this is mostly an effect of this being a huge change with some repeating bits, so it's nothing bad, it's just me being a proof-reading editor of some prose – which can easily be changed.)
spec/aruba/api/core_spec.rb
Outdated
| expect(Dir.pwd).not_to eq full_path | ||
| end | ||
|
|
||
| it 'does not touch non-directory environment the passed block' do |
There was a problem hiding this comment.
A missing word in this sentence?
spec/aruba/api/filesystem_spec.rb
Outdated
| it { expect(all_paths).to include expand_path(name) } | ||
| end | ||
|
|
||
| context 'when directory exist' do |
spec/aruba/api/filesystem_spec.rb
Outdated
| it { expect(all_paths).to include expand_path(name) } | ||
| end | ||
|
|
||
| context 'when nothing exist' do |
spec/aruba/api/filesystem_spec.rb
Outdated
| let(:name) { @file_name } | ||
| let(:path) { @file_path } | ||
|
|
||
| context 'when file exist' do |
spec/aruba/api/filesystem_spec.rb
Outdated
| it { expect(all_files).to include expand_path(name) } | ||
| end | ||
|
|
||
| context 'when directory exist' do |
spec/aruba/api/filesystem_spec.rb
Outdated
| it { expect(all_files).to eq [] } | ||
| end | ||
|
|
||
| context 'when nothing exist' do |
spec/aruba/api/filesystem_spec.rb
Outdated
| let(:name) { @file_name } | ||
| let(:path) { @file_path } | ||
|
|
||
| context 'when file exist' do |
There was a problem hiding this comment.
...and so on. I will not make further comments on this.
spec/aruba/api/filesystem_spec.rb
Outdated
| @aruba.touch(name, options) | ||
| end | ||
|
|
||
| context 'when does not exist' do |
There was a problem hiding this comment.
This clause has a “when” too much.
spec/aruba/api/filesystem_spec.rb
Outdated
| let(:name) { %w(directory1) } | ||
| let(:path) { Array(name).map { |p| File.join(@aruba.aruba.current_directory, p) } } | ||
|
|
||
| context 'when exist' do |
spec/aruba/api/filesystem_spec.rb
Outdated
| let(:path) { Array(name).map { |p| File.join(@aruba.aruba.current_directory, p) } } | ||
|
|
||
| context 'when exist' do | ||
| before(:each) { Array(path).each { |p| Aruba.platform.mkdir p } } |
There was a problem hiding this comment.
Pet peeve: use a name other than p since Kernel#p is a built-in debug function.
| @@ -13,6 +13,3 @@ | |||
| # Loading support files | |||
| Dir.glob(::File.expand_path('../support/*.rb', __FILE__)).each { |f| require_relative f } | |||
| Dir.glob(::File.expand_path('../support/**/*.rb', __FILE__)).each { |f| require_relative f } | |||
|
|
|||
| # Avoid writing "describe LocalPac::MyClass do [..]" but "describe MyClass do [..]" | |||
| include Aruba | |||
There was a problem hiding this comment.
Thank you for making this change.
|
Thanks, @olleolleolle. I fixed most of the odd wording. |
|
Great work! thanks for merging 👍 |
|
Will this be pushed out in a 0.14 point release (soon) or as part of 1.0? |
|
@matthutchinson This partiular PR will be part of 1.0, but #539 implements just the fix to |
Summary
Allow absolute paths in
#expand_path, but complain about it by default.Details
Makes
#expand_pathcorrectly handle absolute paths (like in #539), but in addition emit a warning when an absolute path is passed. Adds a configuration optionallow_absolute_pathsthat can be set totrueto silence this warning.Motivation and Context
Fixes #478.
How Has This Been Tested?
Specs were added for the case of handing absolute paths, for emission of a warning, and for silencing that warning.
Types of changes
Checklist: