Merge pull request #86 from bashly-framework/refactor/install

Refactor Installer

Author: DannyBen

lib/completely/commands/install.rb

@@ -24,13 +24,6 @@ class Install < Base
       USAGE
 
       def run
-        if script_path == '-'
-          raise InstallError, "Nothing is piped on stdin" if $stdin.tty?
-
-          @script_path = tempfile.path
-          File.write script_path, $stdin.read
-        end
-
         if args['--dry']
           puts installer.install_command_string
           return
@@ -43,16 +36,12 @@ def run
         say 'You may need to restart your session to test it'
       end
 
-      def tempfile
-        @tempfile ||= Tempfile.new('stdin-completely-')
-      end
-
-    private
-
       def installer
         @installer ||= Installer.new(program: args['PROGRAM'], script_path: script_path)
       end
 
+    private
+
       def script_path
         @script_path ||= args['SCRIPT_PATH'] || 'completely.bash'
       end

lib/completely/installer.rb

@@ -3,6 +3,13 @@ class Installer
     attr_reader :program, :script_path
 
     def initialize(program:, script_path: nil)
+      if script_path == '-'
+        raise InstallError, 'Nothing is piped on stdin' if $stdin.tty?
+
+        script_path = tempfile.path
+        File.write script_path, $stdin.read
+      end
+
       @program = program
       @script_path = script_path
     end
@@ -60,6 +67,10 @@ def uninstall
 
   private
 
+    def tempfile
+      @tempfile ||= Tempfile.new('stdin-completely-')
+    end
+
     def target_exist?
       File.exist? target_path
     end
@@ -74,11 +85,15 @@ def root_user?
 
     def completions_path
       @completions_path ||= begin
+        result = nil
         target_directories.each do |target|
-          return target if Dir.exist? target
+          if Dir.exist? target
+            result = target
+            break
+          end
         end
 
-        nil
+        result
       end
     end
   end

spec/completely/commands/install_spec.rb

@@ -43,8 +43,6 @@
 
       expect { subject.execute %w[install completely-test -] }
         .to output_approval('cli/install/stdin-install')
-
-      expect(File.read subject.tempfile.path).to eq 'dummy data'
     end
   end
 
@@ -79,7 +77,6 @@
     end
   end
 
-
   context 'when the installer fails' do
     it 'raises an error' do
       allow(subject).to receive(:installer).and_return(mock_installer)

spec/completely/completions_spec.rb

@@ -6,7 +6,7 @@
 
   describe '::read' do
     it 'reads from io' do
-      io = double :io, read: "cli: [--help, --version]"
+      io = double :io, read: 'cli: [--help, --version]'
       expect(described_class.read(io).config.config).to eq({ 'cli' => %w[--help --version] })
     end
   end

spec/completely/config_spec.rb

@@ -3,7 +3,7 @@
 
   let(:path) { "spec/fixtures/#{file}.yaml" }
   let(:file) { 'nested' }
-  let(:config_string) { "cli: [--help, --version]" }
+  let(:config_string) { 'cli: [--help, --version]' }
   let(:config_hash) { { 'cli' => %w[--help --version] } }
 
   describe '::parse' do
@@ -13,7 +13,7 @@
 
     context 'when the string is not a valid YAML' do
       it 'raises ParseError' do
-        expect { described_class.parse("not: a: yaml") }.to raise_error(Completely::ParseError)
+        expect { described_class.parse('not: a: yaml') }.to raise_error(Completely::ParseError)
       end
     end
   end

spec/completely/installer_spec.rb

@@ -13,6 +13,20 @@
     %w[sudo rm -f] + targets
   end
 
+  describe '#initialize' do
+    context 'when script_path == "-"' do
+      let(:script_path) { '-' }
+
+      it 'reads the script from stdin and writes it to a temp file' do
+        allow($stdin).to receive_messages(tty?: false, read: 'dummy data')
+
+        subject
+
+        expect(File.read subject.script_path).to eq 'dummy data'
+      end
+    end
+  end
+
   describe '#target_directories' do
     it 'returns an array of potential completion directories' do
       expect(subject.target_directories).to be_an Array
@@ -25,16 +39,6 @@
       expect(subject.target_path)
         .to eq '/usr/share/bash-completion/completions/completely-test'
     end
-
-    # This method will not be called if there is no completions path
-    # The test is here to cover the nil fallback
-    context 'when no paths found' do
-      it 'returns nil as the base path' do
-        allow(subject).to receive(:target_directories).and_return([])
-
-        expect(subject.target_path).to eq '/completely-test'
-      end
-    end
   end
 
   describe '#install_command' do