Merge branch 'remote-fs-di'

Move SFTP session building responsability from Remote::FS class to
Remote class, to decrease coupling in Remote::FS.

lib/producer/core/remote.rb

@@ -20,7 +20,7 @@ module Producer
       end
 
       def fs
-        @fs ||= Remote::FS.new(self)
+        @fs ||= Remote::FS.new(session.sftp.connect)
       end
 
       def execute(command)

lib/producer/core/remote/fs.rb

@@ -2,14 +2,10 @@ module Producer
   module Core
     class Remote
       class FS
-        attr_reader :remote
+        attr_reader :sftp
 
-        def initialize(remote)
+        def initialize(sftp)
-          @remote = remote
+          @sftp = sftp
-        end
-
-        def sftp
-          @sftp ||= @remote.session.sftp.connect
         end
 
         def dir?(path)

spec/producer/core/remote/fs_spec.rb

@@ -3,44 +3,21 @@ require 'spec_helper'
 module Producer::Core
   class Remote
     describe FS do
-      let(:remote)  { Remote.new('some_host.example') }
+      let(:sftp_file) { double 'sftp_file' }
-      subject(:fs)  { FS.new(remote) }
+      let(:sftp)      { double('sftp', file: sftp_file) }
+      subject(:fs)    { FS.new(sftp) }
 
       describe '#initialize' do
-        it 'assigns the remote given as argument' do
+        it 'assigns the sftp session' do
-          expect(fs.remote).to be remote
+          expect(fs.sftp).to be sftp
-        end
-      end
-
-      describe '#sftp', :ssh do
-        before { sftp_story }
-
-        it 'builds a new SFTP session' do
-          expect(remote.session.sftp).to receive :connect
-          fs.sftp
-        end
-
-        it 'returns the new SFTP session' do
-          session = double 'session'
-          allow(remote.session.sftp).to receive(:connect) { session }
-          expect(fs.sftp).to be session
-        end
-
-        it 'memoizes the FS' do
-          allow(remote.session.sftp).to receive(:connect) { Object.new }
-          expect(fs.sftp).to be fs.sftp
         end
       end
 
       describe '#dir?' do
-        let(:sftp)  { double('sftp').as_null_object }
         let(:path)  { 'some_directory_path' }
         let(:stat)  { double 'stat' }
 
-        before do
+        before { allow(sftp).to receive(:stat!).with(path) { stat } }
-          allow(fs).to receive(:sftp) { sftp }
-          allow(sftp).to receive(:stat!).with(path) { stat }
-        end
 
         context 'when path given as argument is a directory' do
           before { allow(stat).to receive(:directory?) { true } }
@@ -114,10 +91,7 @@ module Producer::Core
       end
 
       describe '#mkdir' do
-        let(:sftp)  { double 'sftp' }
+        let(:path) { 'some_directory_path' }
-        let(:path)  { 'some_directory_path' }
-
-        before { allow(fs).to receive(:sftp) { sftp } }
 
         it 'creates the directory' do
           expect(sftp).to receive(:mkdir!).with(path)
@@ -126,16 +100,12 @@ module Producer::Core
       end
 
       describe '#file_read' do
-        let(:sftp)    { double 'sftp' }
-        let(:file)    { double 'file' }
         let(:f)       { double 'f' }
         let(:path)    { 'some_file_path' }
         let(:content) { 'some_content' }
 
         before do
-          allow(fs).to receive(:sftp) { sftp }
+          allow(sftp_file).to receive(:open).and_yield(f)
-          allow(sftp).to receive(:file) { file }
-          allow(file).to receive(:open).and_yield(f)
           allow(f).to receive(:read) { content }
         end
 
@@ -147,7 +117,7 @@ module Producer::Core
           before do
             response = double 'response', code: '42', message: 'some message'
             ex = Net::SFTP::StatusException.new(response)
-            allow(file).to receive(:open).and_raise(ex)
+            allow(sftp_file).to receive(:open).and_raise(ex)
           end
 
           it 'returns nil' do
@@ -157,25 +127,18 @@ module Producer::Core
       end
 
       describe '#file_write' do
-        let(:sftp)    { double 'sftp' }
-        let(:file)    { double 'file' }
         let(:path)    { 'some_file_path' }
         let(:content) { 'some_content' }
 
-        before do
-          allow(fs).to receive(:sftp) { sftp }
-          allow(sftp).to receive(:file) { file }
-        end
-
         it 'opens the file' do
-          expect(file).to receive(:open).with(path, 'w')
+          expect(sftp_file).to receive(:open).with(path, 'w')
           fs.file_write path, content
         end
 
         it 'writes the content' do
-          expect(file).to receive(:open).with(any_args) do |&b|
+          expect(sftp_file).to receive(:open).with(any_args) do |&b|
-            expect(file).to receive(:write).with(content)
+            expect(sftp_file).to receive(:write).with(content)
-            b.call file
+            b.call sftp_file
           end
           fs.file_write path, content
         end

spec/producer/core/remote_spec.rb

@@ -68,20 +68,11 @@ module Producer::Core
     end
 
     describe '#fs' do
-      it 'builds a new FS' do
+      let(:sftp_session) { double 'sftp session' }
-        expect(Remote::FS).to receive :new
-        remote.fs
-      end
 
-      it 'returns the new FS instance' do
+      it 'returns an FS instance built with a new sftp session' do
-        fs = double 'fs'
+        remote.stub_chain(:session, :sftp, :connect) { sftp_session }
-        allow(Remote::FS).to receive(:new) { fs }
+        expect(remote.fs.sftp).to be sftp_session
-        expect(remote.fs).to be fs
-      end
-
-      it 'memoizes the FS' do
-        allow(Remote::FS).to receive(:new) { Object.new }
-        expect(remote.fs).to be remote.fs
       end
     end
 

spec/support/test_env_helpers.rb

@@ -26,6 +26,9 @@ module TestEnvHelpers
   end
 
   def build_remote
-    Producer::Core::Testing::MockRemote.new('some_host.test')
+    fs = RSpec::Mocks::Mock.new('remote fs', __declared_as: 'Double')
+    remote = Producer::Core::Testing::MockRemote.new('some_host.test')
+    remote.define_singleton_method(:fs) { fs }
+    remote
   end
 end