Merge pull request #540 from cucumber/issue-478-restore-old-expand_pa…

…th-behavior

Restore old expand path behavior

.rubocop.yml

@@ -5,4 +5,9 @@ AllCops:
   DisplayCopNames: true
   TargetRubyVersion: 1.9
 
+# Use older RuboCop default
+Style/PercentLiteralDelimiters:
+  PreferredDelimiters:
+    '%w': ()
+
 inherit_from: .rubocop_todo.yml

.rubocop_todo.yml

@@ -299,14 +299,6 @@ Style/NumericPredicate:
 Style/PercentLiteralDelimiters:
   Exclude:
     - 'Rakefile'
-    - 'lib/aruba/cli.rb'
-    - 'lib/aruba/configuration.rb'
-    - 'lib/aruba/console/help.rb'
-    - 'lib/aruba/cucumber/command.rb'
-    - 'lib/aruba/tasks/docker_helpers.rb'
-    - 'spec/aruba/api_spec.rb'
-    - 'spec/aruba/aruba_path_spec.rb'
-    - 'spec/aruba/matchers/directory_spec.rb'
 
 # Offense count: 1
 # Cop supports --auto-correct.

features/04_aruba_api/core/expand_path.feature

@@ -1,8 +1,10 @@
 Feature: Expand paths with aruba
 
-  There are quite a few uses cases why you want to expand a path. `aruba` helps
+  There are quite a few uses cases why you want to expand a path. Aruba helps
   you with this by providing you the `expand_path`-method. This method expands
-  paths relative to the `aruba.current_directory`-directory.
+  paths relative to the `aruba.current_directory`-directory. Use of absolute
+  paths is discouraged, since the intent is to only access the isolated Aruba
+  working directory.
 
   Background:
     Given I use the fixture "cli-app"
@@ -14,7 +16,7 @@ Feature: Expand paths with aruba
 
     RSpec.describe 'Expand path', :type => :aruba do
       let(:path) { 'path/to/dir' }
-      it { expect(expand_path(path)).to eq File.join(aruba.root_directory, aruba.current_directory, path) }
+      it { expect(expand_path(path)).to eq File.join(aruba.config.home_directory, path) }
     end
     """
     When I run `rspec`
@@ -29,15 +31,56 @@ Feature: Expand paths with aruba
       let(:path) { 'path/to/dir' }
       let(:directory) { 'dir1' }
 
-      before(:each) { create_directory(directory) }
-      before(:each) { cd(directory) }
+      before do
+        create_directory directory
+        cd directory
+      end
 
-      it { expect(expand_path(path)).to eq File.join(aruba.root_directory, aruba.current_directory, path) }
+      it { expect(expand_path(path)).to eq File.join(aruba.config.home_directory, directory, path) }
     end
     """
     When I run `rspec`
     Then the specs should all pass
 
+  Scenario: Warn when using absolute path
+    Given a file named "spec/expand_path_spec.rb" with:
+    """ruby
+    require 'spec_helper'
+
+    RSpec.describe 'Expand path', :type => :aruba do
+      let(:path) { '/path/to/dir' }
+      it { expect(expand_path(path)).to eq path }
+    end
+    """
+    When I run `rspec`
+    Then the specs should all pass
+    And the output should contain:
+    """
+    Using absolute paths in Aruba is not recommended
+    """
+
+  Scenario: Silence warning about using absolute path
+
+    You can use config.allow_absolute_paths to silence the warning about the
+    use of absolute paths.
+
+    Given a file named "spec/expand_path_spec.rb" with:
+    """ruby
+    require 'spec_helper'
+
+    RSpec.describe 'Expand path', :type => :aruba do
+      let(:path) { '/path/to/dir' }
+      before { aruba.config.allow_absolute_paths = true }
+      it { expect(expand_path(path)).to eq path }
+    end
+    """
+    When I run `rspec`
+    Then the specs should all pass
+    And the output should not contain:
+    """
+    Using absolute paths in Aruba is not recommended
+    """
+
   Scenario: Raise an error if aruba's working directory does not exist
     Given a file named "spec/expand_path_spec.rb" with:
     """ruby
@@ -66,10 +109,15 @@ Feature: Expand paths with aruba
 
     RSpec.describe 'Expand path', :type => :aruba do
       let(:path) { '~/path/to/dir' }
+      let(:directory) { 'dir1' }
+
+      before do
+        create_directory(directory)
+        cd directory
+      end
 
-      it { expect(expand_path(path)).to eq File.join(aruba.config.root_directory, aruba.config.working_directory, 'path/to/dir') }
+      it { expect(expand_path(path)).to eq File.join(aruba.config.home_directory, 'path/to/dir') }
     end
     """
     When I run `rspec`
     Then the specs should all pass
-

lib/aruba/api/core.rb

@@ -117,8 +117,14 @@ def cd(dir, &block)
       #   # => <path>/test/fixtures/file
       #   expand_path('%/file')
       #
+      # @example Absolute directory
+      #
+      #   # => /foo/bar
+      #   expand_path('/foo/bar')
+      #
       # rubocop:disable Metrics/MethodLength
       # rubocop:disable Metrics/CyclomaticComplexity
+      # rubocop:disable Metrics/PerceivedComplexity
       def expand_path(file_name, dir_string = nil)
         # rubocop:disable Metrics/LineLength
         message = %(Filename "#{file_name}" needs to be a string. It cannot be nil or empty either.  Please use `expand_path('.')` if you want the current directory to be expanded.)
@@ -150,13 +156,21 @@ def expand_path(file_name, dir_string = nil)
           fail ArgumentError, %(Expanding "~/" to a relative path "#{path}" is not allowed) unless path.absolute?
 
           path.to_s
+        elsif absolute? file_name
+          unless aruba.config.allow_absolute_paths
+            aruba.logger.warn 'Using absolute paths in Aruba is not recommended.' \
+              ' Set config.allow_absolute_paths = true to silence this warning'
+          end
+          file_name
         else
           directory = File.join(aruba.root_directory, aruba.current_directory)
-          ArubaPath.new(File.join(*[directory, dir_string, file_name].compact)).expand_path.to_s
+          directory = File.expand_path(dir_string, directory) if dir_string
+          File.expand_path(file_name, directory)
         end
       end
       # rubocop:enable Metrics/MethodLength
-      # rubocop:enable  Metrics/CyclomaticComplexity
+      # rubocop:enable Metrics/CyclomaticComplexity
+      # rubocop:enable Metrics/PerceivedComplexity
 
       # Run block with environment
       #

lib/aruba/configuration.rb

@@ -50,6 +50,7 @@ class Configuration < BasicConfiguration
     option_accessor :console_history_file, contract: { String => String }, default: '~/.aruba_history'
 
     option_accessor :activate_announcer_on_command_failure, contract: { ArrayOf[Symbol] => ArrayOf[Symbol] }, default: []
+    option_accessor :allow_absolute_paths, contract: { Bool => Bool }, default: false
   end
 end
 

spec/aruba/api/core_spec.rb

@@ -0,0 +1,179 @@
+require 'spec_helper'
+require 'aruba/api'
+require 'fileutils'
+
+describe Aruba::Api::Core do
+  include_context 'uses aruba API'
+
+  describe '#cd' do
+    before(:each) do
+      @directory_name = 'test_dir'
+      @directory_path = File.join(@aruba.aruba.current_directory, @directory_name)
+    end
+
+    context 'with a block given' do
+      it 'runs the passed block in the given directory' do
+        @aruba.create_directory @directory_name
+        full_path = File.expand_path(@directory_path)
+        @aruba.cd @directory_name do
+          expect(Dir.pwd).to eq full_path
+        end
+        expect(Dir.pwd).not_to eq full_path
+      end
+
+      it 'does not touch non-directory environment of the passed block' do
+        @aruba.create_directory @directory_name
+        @aruba.cd @directory_name do
+          expect(ENV['HOME']).not_to be_nil
+        end
+      end
+    end
+  end
+
+  describe '#expand_path' do
+    context 'when file_name is given' do
+      it { expect(@aruba.expand_path(@file_name)).to eq File.expand_path(@file_path) }
+    end
+
+    context 'when an absolute file_path is given' do
+      let(:logger) { @aruba.aruba.logger }
+
+      before do
+        allow(@aruba.aruba.logger).to receive :warn
+      end
+
+      it { expect(@aruba.expand_path(@file_path)).to eq @file_path }
+
+      it 'warns about it' do
+        @aruba.expand_path(@file_path)
+        expect(logger).to have_received(:warn).with(/Using absolute paths in Aruba is not recommended/)
+      end
+
+      it 'does not warn about it if told not to' do
+        @aruba.aruba.config.allow_absolute_paths = true
+        @aruba.expand_path(@file_path)
+        expect(logger).not_to have_received(:warn)
+      end
+    end
+
+    context 'when path contains "."' do
+      it { expect(@aruba.expand_path('.')).to eq File.expand_path(aruba.current_directory) }
+    end
+
+    context 'when path contains ".."' do
+      it { expect(@aruba.expand_path('path/..')).to eq File.expand_path(File.join(aruba.current_directory)) }
+    end
+
+    context 'when path is nil' do
+      it { expect { @aruba.expand_path(nil) }.to raise_error ArgumentError }
+    end
+
+    context 'when path is empty' do
+      it { expect { @aruba.expand_path('') }.to raise_error ArgumentError }
+    end
+
+    context 'when dir_path is given similar to File.expand_path ' do
+      it { expect(@aruba.expand_path(@file_name, 'path')).to eq File.expand_path(File.join(aruba.current_directory, 'path', @file_name)) }
+    end
+
+    context 'when file_name contains fixtures "%" string' do
+      let(:runtime) { instance_double('Aruba::Runtime') }
+      let(:config) { double('Aruba::Config') }
+      let(:environment) { instance_double('Aruba::Environment') }
+
+      let(:klass) do
+        Class.new do
+          include Aruba::Api
+
+          attr_reader :aruba
+
+          def initialize(aruba)
+            @aruba = aruba
+          end
+        end
+      end
+
+      before :each do
+        allow(config).to receive(:fixtures_path_prefix).and_return('%')
+        allow(config).to receive(:root_directory).and_return aruba.config.root_directory
+        allow(config).to receive(:working_directory).and_return aruba.config.working_directory
+      end
+
+      before :each do
+        allow(environment).to receive(:clear)
+        allow(environment).to receive(:update).and_return(environment)
+        allow(environment).to receive(:to_h).and_return('PATH' => aruba.current_directory.to_s)
+      end
+
+      before :each do
+        allow(runtime).to receive(:config).and_return config
+        allow(runtime).to receive(:environment).and_return environment
+        allow(runtime).to receive(:current_directory).and_return aruba.current_directory
+        allow(runtime).to receive(:root_directory).and_return aruba.root_directory
+        allow(runtime).to receive(:fixtures_directory).and_return File.join(aruba.root_directory, aruba.current_directory, 'spec', 'fixtures')
+      end
+
+      before :each do
+        @aruba = klass.new(runtime)
+        @aruba.touch 'spec/fixtures/file1'
+      end
+
+      it { expect(@aruba.expand_path('%/file1')).to eq File.expand_path(File.join(aruba.current_directory, 'spec', 'fixtures', 'file1')) }
+    end
+  end
+
+  describe '#with_environment' do
+    it 'modifies env for block' do
+      variable = 'THIS_IS_A_ENV_VAR'
+      ENV[variable] = '1'
+
+      @aruba.with_environment variable => '0' do
+        expect(ENV[variable]).to eq '0'
+      end
+
+      expect(ENV[variable]).to eq '1'
+    end
+
+    it 'works together with #set_environment_variable' do
+      variable = 'THIS_IS_A_ENV_VAR'
+      @aruba.set_environment_variable variable, '1'
+
+      @aruba.with_environment do
+        expect(ENV[variable]).to eq '1'
+        @aruba.set_environment_variable variable, '0'
+        @aruba.with_environment do
+          expect(ENV[variable]).to eq '0'
+        end
+        expect(ENV[variable]).to eq '1'
+      end
+    end
+
+    it 'works with a mix of ENV and #set_environment_variable' do
+      variable = 'THIS_IS_A_ENV_VAR'
+      @aruba.set_environment_variable variable, '1'
+      ENV[variable] = '2'
+      expect(ENV[variable]).to eq '2'
+
+      @aruba.with_environment do
+        expect(ENV[variable]).to eq '1'
+        @aruba.set_environment_variable variable, '0'
+        @aruba.with_environment do
+          expect(ENV[variable]).to eq '0'
+        end
+        expect(ENV[variable]).to eq '1'
+      end
+      expect(ENV[variable]).to eq '2'
+    end
+
+    it 'keeps values not set in argument' do
+      variable = 'THIS_IS_A_ENV_VAR'
+      ENV[variable] = '2'
+      expect(ENV[variable]).to eq '2'
+
+      @aruba.with_environment do
+        expect(ENV[variable]).to eq '2'
+      end
+      expect(ENV[variable]).to eq '2'
+    end
+  end
+end