From f45d0b5a20d4d73b22776e650f8a7b7cde5493a1 Mon Sep 17 00:00:00 2001 From: Thibault Jouan Date: Thu, 23 Jan 2014 22:21:10 +0000 Subject: [PATCH 1/2] Modify Remote::FS interface: * Accept SFTP session in FS constructor; * Modify Remote to build the session required by FS; * Change TestEnvHelpers#build_remote in order to provided a test double as the remote FS. --- lib/producer/core/remote.rb | 2 +- lib/producer/core/remote/fs.rb | 10 ++----- spec/producer/core/remote/fs_spec.rb | 43 ++++------------------------ spec/producer/core/remote_spec.rb | 17 +++-------- spec/support/test_env_helpers.rb | 5 +++- 5 files changed, 18 insertions(+), 59 deletions(-) diff --git a/lib/producer/core/remote.rb b/lib/producer/core/remote.rb index 34ff8de..06fd525 100644 --- a/lib/producer/core/remote.rb +++ b/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) diff --git a/lib/producer/core/remote/fs.rb b/lib/producer/core/remote/fs.rb index 8915ed0..6760ba5 100644 --- a/lib/producer/core/remote/fs.rb +++ b/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) - @remote = remote - end - - def sftp - @sftp ||= @remote.session.sftp.connect + def initialize(sftp) + @sftp = sftp end def dir?(path) diff --git a/spec/producer/core/remote/fs_spec.rb b/spec/producer/core/remote/fs_spec.rb index 9056b9f..3f6f61f 100644 --- a/spec/producer/core/remote/fs_spec.rb +++ b/spec/producer/core/remote/fs_spec.rb @@ -3,44 +3,20 @@ require 'spec_helper' module Producer::Core class Remote describe FS do - let(:remote) { Remote.new('some_host.example') } - subject(:fs) { FS.new(remote) } + let(:sftp) { double 'sftp' } + subject(:fs) { FS.new(sftp) } describe '#initialize' do - it 'assigns the remote given as argument' do - expect(fs.remote).to be remote - 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 + it 'assigns the sftp session' do + expect(fs.sftp).to be sftp end end describe '#dir?' do - let(:sftp) { double('sftp').as_null_object } let(:path) { 'some_directory_path' } let(:stat) { double 'stat' } - before do - allow(fs).to receive(:sftp) { sftp } - allow(sftp).to receive(:stat!).with(path) { stat } - end + before { allow(sftp).to receive(:stat!).with(path) { stat } } context 'when path given as argument is a directory' do before { allow(stat).to receive(:directory?) { true } } @@ -114,10 +90,7 @@ module Producer::Core end describe '#mkdir' do - let(:sftp) { double 'sftp' } - let(:path) { 'some_directory_path' } - - before { allow(fs).to receive(:sftp) { sftp } } + let(:path) { 'some_directory_path' } it 'creates the directory' do expect(sftp).to receive(:mkdir!).with(path) @@ -126,14 +99,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).to receive(:file) { file } allow(file).to receive(:open).and_yield(f) allow(f).to receive(:read) { content } @@ -157,13 +128,11 @@ 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 diff --git a/spec/producer/core/remote_spec.rb b/spec/producer/core/remote_spec.rb index ebc8a40..17a6ee7 100644 --- a/spec/producer/core/remote_spec.rb +++ b/spec/producer/core/remote_spec.rb @@ -68,20 +68,11 @@ module Producer::Core end describe '#fs' do - it 'builds a new FS' do - expect(Remote::FS).to receive :new - remote.fs - end + let(:sftp_session) { double 'sftp session' } - it 'returns the new FS instance' do - fs = double 'fs' - allow(Remote::FS).to receive(:new) { fs } - 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 + it 'returns an FS instance built with a new sftp session' do + remote.stub_chain(:session, :sftp, :connect) { sftp_session } + expect(remote.fs.sftp).to be sftp_session end end diff --git a/spec/support/test_env_helpers.rb b/spec/support/test_env_helpers.rb index 5797a6c..2fa33db 100644 --- a/spec/support/test_env_helpers.rb +++ b/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 From 64a6ed678a1644eb6530ded101e9f389e2244905 Mon Sep 17 00:00:00 2001 From: Thibault Jouan Date: Fri, 14 Feb 2014 01:28:41 +0000 Subject: [PATCH 2/2] Refactor Remote::FS specs --- spec/producer/core/remote/fs_spec.rb | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/spec/producer/core/remote/fs_spec.rb b/spec/producer/core/remote/fs_spec.rb index 3f6f61f..2642a7c 100644 --- a/spec/producer/core/remote/fs_spec.rb +++ b/spec/producer/core/remote/fs_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' module Producer::Core class Remote describe FS do - let(:sftp) { double 'sftp' } - subject(:fs) { FS.new(sftp) } + let(:sftp_file) { double 'sftp_file' } + let(:sftp) { double('sftp', file: sftp_file) } + subject(:fs) { FS.new(sftp) } describe '#initialize' do it 'assigns the sftp session' do @@ -99,14 +100,12 @@ module Producer::Core end describe '#file_read' do - let(:file) { double 'file' } let(:f) { double 'f' } let(:path) { 'some_file_path' } let(:content) { 'some_content' } before do - allow(sftp).to receive(:file) { file } - allow(file).to receive(:open).and_yield(f) + allow(sftp_file).to receive(:open).and_yield(f) allow(f).to receive(:read) { content } end @@ -118,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 @@ -128,23 +127,18 @@ module Producer::Core end describe '#file_write' do - let(:file) { double 'file' } let(:path) { 'some_file_path' } let(:content) { 'some_content' } - before do - 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(file).to receive(:write).with(content) - b.call file + expect(sftp_file).to receive(:open).with(any_args) do |&b| + expect(sftp_file).to receive(:write).with(content) + b.call sftp_file end fs.file_write path, content end