From ef0307fbb5242599d11ae92050b2124ff793be41 Mon Sep 17 00:00:00 2001
From: Thibault Jouan <tj@a13.fr>
Date: Thu, 19 Dec 2013 21:17:03 +0000
Subject: [PATCH] Improve small details in specs:

* Fix coding standards;
* Simplify some expectations (eq instead of be matcher);
* Expect identity on block instead of calling;
* Change some before call as oneliners;
* Avoid shadowing variable names;
* Improve wording where needed.
---
 spec/producer/core/cli_spec.rb            | 14 +++++++-------
 spec/producer/core/condition/dsl_spec.rb  | 20 ++++++++------------
 spec/producer/core/condition_spec.rb      |  4 ++--
 spec/producer/core/env_spec.rb            |  2 +-
 spec/producer/core/interpreter_spec.rb    | 14 ++++++--------
 spec/producer/core/recipe/dsl_spec.rb     | 22 +++++++++++-----------
 spec/producer/core/recipe_spec.rb         |  4 ++--
 spec/producer/core/remote/fs_spec.rb      | 22 +++++++++-------------
 spec/producer/core/remote_spec.rb         | 12 ++++++------
 spec/producer/core/task/dsl_spec.rb       |  8 ++++----
 spec/producer/core/task_spec.rb           | 14 +++++++-------
 spec/producer/core/tests/has_env_spec.rb  |  2 +-
 spec/producer/core/tests/has_file_spec.rb |  2 +-
 spec/support/net_ssh_story_helpers.rb     |  2 +-
 spec/support/tests_helpers.rb             |  4 ++--
 15 files changed, 68 insertions(+), 78 deletions(-)

diff --git a/spec/producer/core/cli_spec.rb b/spec/producer/core/cli_spec.rb
index 9a04e57..a4c1d80 100644
--- a/spec/producer/core/cli_spec.rb
+++ b/spec/producer/core/cli_spec.rb
@@ -5,7 +5,7 @@ module Producer::Core
     include ExitHelpers
     include FixturesHelpers
 
-    let(:recipe_file) { fixture_path_for('recipes/empty.rb') }
+    let(:recipe_file) { fixture_path_for 'recipes/empty.rb' }
     let(:arguments)   { [recipe_file] }
     subject(:cli)     { CLI.new(arguments) }
 
@@ -23,13 +23,13 @@ module Producer::Core
 
     describe '#run!' do
       it 'checks the arguments' do
-        expect(cli).to receive(:check_arguments!)
+        expect(cli).to receive :check_arguments!
         cli.run!
       end
 
       it 'processes the tasks with the interpreter' do
         allow(cli.recipe).to receive(:tasks) { [:some_task] }
-        expect(cli.interpreter).to receive(:process).with([:some_task])
+        expect(cli.interpreter).to receive(:process).with [:some_task]
         cli.run!
       end
     end
@@ -61,12 +61,12 @@ module Producer::Core
 
     describe '#env' do
       it 'builds an environment with the current recipe' do
-        expect(Env).to receive(:new)
+        expect(Env).to receive :new
         cli.env
       end
 
       it 'returns the env' do
-        env = double('env')
+        env = double 'env'
         allow(Env).to receive(:new) { env }
         expect(cli.env).to be env
       end
@@ -88,12 +88,12 @@ module Producer::Core
 
     describe '#interpreter' do
       it 'builds a interpreter' do
-        expect(Interpreter).to receive(:new)
+        expect(Interpreter).to receive :new
         cli.interpreter
       end
 
       it 'returns the interpreter' do
-        interpreter = double('interpreter')
+        interpreter = double 'interpreter'
         allow(Interpreter).to receive(:new) { interpreter }
         expect(cli.interpreter).to be interpreter
       end
diff --git a/spec/producer/core/condition/dsl_spec.rb b/spec/producer/core/condition/dsl_spec.rb
index e9f486a..50bed76 100644
--- a/spec/producer/core/condition/dsl_spec.rb
+++ b/spec/producer/core/condition/dsl_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 module Producer::Core
   describe Condition::DSL do
     let(:block)   { proc { :some_condition_code } }
-    let(:env)     { double('env') }
+    let(:env)     { double 'env' }
     subject(:dsl) { Condition::DSL.new(env, &block) }
 
     %w[has_env has_file].each do |test|
@@ -22,7 +22,7 @@ module Producer::Core
       it 'evaluates the DSL sandbox code' do
         dsl = double('dsl').as_null_object
         allow(Condition::DSL).to receive(:new) { dsl }
-        expect(dsl).to receive(:evaluate)
+        expect(dsl).to receive :evaluate
         Condition::DSL.evaluate(env, &block)
       end
 
@@ -33,18 +33,16 @@ module Producer::Core
       end
 
       it 'returns the condition' do
-        condition = double('task')
+        condition = double 'task'
         allow(Condition).to receive(:new) { condition }
         expect(Condition::DSL.evaluate(env, &block)).to be condition
       end
     end
 
     describe '.define_test' do
-      let(:some_test_class) { double('SomeTest class') }
+      let(:some_test_class) { double 'SomeTest class' }
 
-      before do
-        Condition::DSL.define_test(:some_test, some_test_class)
-      end
+      before { Condition::DSL.define_test(:some_test, some_test_class) }
 
       it 'defines a new test keyword' do
         expect(dsl).to respond_to :some_test
@@ -82,12 +80,10 @@ module Producer::Core
       end
 
       context 'when a defined test keyword is called' do
-        let(:some_test_class) { double('SomeTest class') }
+        let(:some_test_class) { double 'SomeTest class' }
         let(:block)           { proc { some_test :some, :args } }
 
-        before do
-          Condition::DSL.define_test(:some_test, some_test_class)
-        end
+        before { Condition::DSL.define_test(:some_test, some_test_class) }
 
         it 'builds a new test with the env and given arguments' do
           expect(some_test_class).to receive(:new).with(env, :some, :args)
@@ -95,7 +91,7 @@ module Producer::Core
         end
 
         it 'registers the new test' do
-          some_test = double('SomeTest instance')
+          some_test = double 'SomeTest instance'
           allow(some_test_class).to receive(:new) { some_test }
           dsl.evaluate
           expect(dsl.tests).to include(some_test)
diff --git a/spec/producer/core/condition_spec.rb b/spec/producer/core/condition_spec.rb
index d97d378..912f2fd 100644
--- a/spec/producer/core/condition_spec.rb
+++ b/spec/producer/core/condition_spec.rb
@@ -8,7 +8,7 @@ module Producer::Core
     subject(:condition) { Condition.new(tests) }
 
     describe '.evaluate' do
-      let(:env)   { double('env') }
+      let(:env)   { double 'env' }
       let(:block) { proc { :some_condition_code } }
 
       it 'delegates to DSL.evaluate' do
@@ -20,7 +20,7 @@ module Producer::Core
       end
 
       it 'returns the evaluated condition' do
-        condition = double('condition')
+        condition = double 'condition'
         allow(Condition::DSL).to receive(:evaluate) { condition }
         expect(Condition.evaluate(env, &block)).to be condition
       end
diff --git a/spec/producer/core/env_spec.rb b/spec/producer/core/env_spec.rb
index 8410207..d4353f1 100644
--- a/spec/producer/core/env_spec.rb
+++ b/spec/producer/core/env_spec.rb
@@ -41,7 +41,7 @@ module Producer::Core
       end
 
       it 'returns the remote' do
-        remote = double('remote')
+        remote = double 'remote'
         allow(Remote).to receive(:new) { remote }
         expect(env.remote).to eq remote
       end
diff --git a/spec/producer/core/interpreter_spec.rb b/spec/producer/core/interpreter_spec.rb
index f6c78d4..79e5602 100644
--- a/spec/producer/core/interpreter_spec.rb
+++ b/spec/producer/core/interpreter_spec.rb
@@ -12,24 +12,22 @@ module Producer::Core
     end
 
     describe '#process_task' do
-      let(:action)  { double('action') }
+      let(:action)  { double 'action' }
       let(:task)    { double('task', actions: [action]).as_null_object }
 
       context 'when task condition is met' do
         it 'applies the actions' do
-          expect(action).to receive(:apply)
-          interpreter.process_task(task)
+          expect(action).to receive :apply
+          interpreter.process_task task
         end
       end
 
       context 'when task condition is not met' do
-        before do
-          allow(task).to receive(:condition_met?) { false }
-        end
+        before { allow(task).to receive(:condition_met?) { false } }
 
         it 'does not apply the actions' do
-          expect(action).not_to receive(:apply)
-          interpreter.process_task(task)
+          expect(action).not_to receive :apply
+          interpreter.process_task task
         end
       end
     end
diff --git a/spec/producer/core/recipe/dsl_spec.rb b/spec/producer/core/recipe/dsl_spec.rb
index 03e0568..656540e 100644
--- a/spec/producer/core/recipe/dsl_spec.rb
+++ b/spec/producer/core/recipe/dsl_spec.rb
@@ -4,14 +4,14 @@ module Producer::Core
   describe Recipe::DSL do
     include FixturesHelpers
 
-    let(:code)    { proc { } }
+    let(:code)    { proc { :some_recipe_code } }
     let(:env)     { double('env').as_null_object }
     subject(:dsl) { Recipe::DSL.new(&code) }
 
     describe '.evaluate' do
       let(:code) { 'nil' }
 
-      it 'builds a new DSL sandbox with given code' do
+      it 'builds a new DSL sandbox with given code as string' do
         expect(Recipe::DSL).to receive(:new).with(code).and_call_original
         Recipe::DSL.evaluate(code, env)
       end
@@ -24,7 +24,7 @@ module Producer::Core
       end
 
       it 'builds a recipe with evaluated tasks' do
-        dsl = Recipe::DSL.new('task(:some_task) { }')
+        dsl = Recipe::DSL.new { task(:some_task) { } }
         allow(Recipe::DSL).to receive(:new) { dsl }
         expect(Recipe).to receive(:new).with(dsl.tasks)
         Recipe::DSL.evaluate(code, env)
@@ -62,7 +62,7 @@ module Producer::Core
       let(:code) { proc { task(:some_task) { } } }
 
       it 'returns registered tasks' do
-        dsl.evaluate(env)
+        dsl.evaluate env
         expect(dsl.tasks[0].name).to eq :some_task
       end
     end
@@ -70,11 +70,11 @@ module Producer::Core
     describe '#evaluate' do
       it 'evaluates its code' do
         dsl = Recipe::DSL.new { throw :recipe_code }
-        expect { dsl.evaluate(env) }.to throw_symbol :recipe_code
+        expect { dsl.evaluate env }.to throw_symbol :recipe_code
       end
 
       it 'returns itself' do
-        expect(dsl.evaluate(env)).to eq dsl
+        expect(dsl.evaluate env).to eq dsl
       end
     end
 
@@ -85,8 +85,8 @@ module Producer::Core
         let(:code) { proc { env.some_message } }
 
         it 'returns the current environment' do
-          expect(env).to receive(:some_message)
-          dsl.evaluate(env)
+          expect(env).to receive :some_message
+          dsl.evaluate env
         end
       end
 
@@ -96,7 +96,7 @@ module Producer::Core
         subject(:dsl)   { Recipe::DSL.new(code) }
 
         it 'sources the recipe given as argument' do
-          expect { dsl.evaluate(env) }.to throw_symbol :recipe_code
+          expect { dsl.evaluate env }.to throw_symbol :recipe_code
         end
       end
 
@@ -114,8 +114,8 @@ module Producer::Core
 
         it 'builds a new evaluated task' do
           expect(Task)
-            .to receive(:evaluate).with(:some_task, env) do |&block|
-              expect(block.call).to eq :some_value
+            .to receive(:evaluate).with(:some_task, env) do |&b|
+              expect(b.call).to eq :some_value
             end
           dsl
         end
diff --git a/spec/producer/core/recipe_spec.rb b/spec/producer/core/recipe_spec.rb
index ab25003..05a1760 100644
--- a/spec/producer/core/recipe_spec.rb
+++ b/spec/producer/core/recipe_spec.rb
@@ -7,7 +7,7 @@ module Producer::Core
     subject(:recipe) { Recipe.new }
 
     describe '.evaluate_from_file' do
-      let(:env)       { double('env') }
+      let(:env)       { double 'env' }
       let(:filepath)  { fixture_path_for 'recipes/empty.rb' }
 
       it 'delegates to DSL.evaluate with the recipe file content' do
@@ -17,7 +17,7 @@ module Producer::Core
       end
 
       it 'returns the evaluated recipe' do
-        recipe = double('recipe')
+        recipe = double 'recipe'
         allow(Recipe::DSL).to receive(:evaluate) { recipe }
         expect(Recipe.evaluate_from_file(filepath, env)).to be recipe
       end
diff --git a/spec/producer/core/remote/fs_spec.rb b/spec/producer/core/remote/fs_spec.rb
index 1f09007..ecc7d43 100644
--- a/spec/producer/core/remote/fs_spec.rb
+++ b/spec/producer/core/remote/fs_spec.rb
@@ -15,12 +15,12 @@ module Producer::Core
       before { sftp_story }
 
       it 'builds a new SFTP session' do
-        expect(remote.session.sftp).to receive(:connect)
+        expect(remote.session.sftp).to receive :connect
         fs.sftp
       end
 
       it 'returns the new SFTP session' do
-        session = double('session')
+        session = double 'session'
         allow(remote.session.sftp).to receive(:connect) { session }
         expect(fs.sftp).to be session
       end
@@ -35,7 +35,7 @@ module Producer::Core
     # part of net-ssh story helpers, which are more close to integration tests.
     describe '#has_file?', :ssh do
       let(:file_path) { "some_file_path" }
-      let(:stat)      { double('stat') }
+      let(:stat)      { double 'stat' }
 
       before do
         sftp_story
@@ -44,35 +44,31 @@ module Producer::Core
 
       context 'when path given as argument exists' do
         context 'when path is a file' do
-          before do
-            allow(stat).to receive(:file?) { true }
-          end
+          before { allow(stat).to receive(:file?) { true } }
 
           it 'returns true' do
-            expect(fs.has_file?(file_path)).to be true
+            expect(fs.has_file? file_path).to be true
           end
         end
 
         context 'when path is not a file' do
-          before do
-            allow(stat).to receive(:file?) { false }
-          end
+          before { allow(stat).to receive(:file?) { false } }
 
           it 'returns false' do
-            expect(fs.has_file?(file_path)).to be false
+            expect(fs.has_file? file_path).to be false
           end
         end
       end
 
       context 'when querying the path raises a Net::SFTP::StatusException' do
         before do
-          response = double('response', code: '42', message: 'some message')
+          response = double 'response', code: '42', message: 'some message'
           ex = Net::SFTP::StatusException.new(response)
           allow(stat).to receive(:file?).and_raise(ex)
         end
 
         it 'returns false' do
-          expect(fs.has_file?(file_path)).to be false
+          expect(fs.has_file? file_path).to be false
         end
       end
     end
diff --git a/spec/producer/core/remote_spec.rb b/spec/producer/core/remote_spec.rb
index 5dd8f95..10c129a 100644
--- a/spec/producer/core/remote_spec.rb
+++ b/spec/producer/core/remote_spec.rb
@@ -7,7 +7,7 @@ module Producer::Core
 
     describe '#hostname' do
       it 'returns the assignated hostname' do
-        expect(remote.hostname).to be hostname
+        expect(remote.hostname).to eq hostname
       end
     end
 
@@ -69,12 +69,12 @@ module Producer::Core
 
     describe '#fs' do
       it 'builds a new FS' do
-        expect(Remote::FS).to receive(:new)
+        expect(Remote::FS).to receive :new
         remote.fs
       end
 
       it 'returns the new FS instance' do
-        fs = double('fs')
+        fs = double 'fs'
         allow(Remote::FS).to receive(:new) { fs }
         expect(remote.fs).to be fs
       end
@@ -103,7 +103,7 @@ module Producer::Core
           ch.sends_exec command
           ch.gets_data arguments
         end
-        expect(remote.execute(command)).to eq arguments
+        expect(remote.execute command).to eq arguments
       end
 
       it 'raises an exception when the exit status code is not 0' do
@@ -112,7 +112,7 @@ module Producer::Core
           ch.gets_data arguments
           ch.gets_exit_status 1
         end
-        expect { remote.execute(command) }
+        expect { remote.execute command }
           .to raise_error(RemoteCommandExecutionError)
       end
     end
@@ -134,7 +134,7 @@ module Producer::Core
       end
 
       it 'returns the environment' do
-        environment = double('environment')
+        environment = double 'environment'
         allow(Remote::Environment).to receive(:new) { environment }
         expect(remote.environment).to be environment
       end
diff --git a/spec/producer/core/task/dsl_spec.rb b/spec/producer/core/task/dsl_spec.rb
index 8e27e95..db589c6 100644
--- a/spec/producer/core/task/dsl_spec.rb
+++ b/spec/producer/core/task/dsl_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 module Producer::Core
   describe Task::DSL do
     let(:block)   { proc { } }
-    let(:env)     { double('env') }
+    let(:env)     { double 'env' }
     subject(:dsl) { Task::DSL.new(&block) }
 
     %w[echo sh].each do |action|
@@ -42,7 +42,7 @@ module Producer::Core
       end
 
       it 'returns the task' do
-        task = double('task')
+        task = double 'task'
         allow(Task).to receive(:new) { task }
         expect(Task::DSL.evaluate(name, env, &block)).to be task
       end
@@ -76,7 +76,7 @@ module Producer::Core
       let(:block) { proc { throw :task_code } }
 
       it 'evaluates its code' do
-        expect { dsl.evaluate(env) }
+        expect { dsl.evaluate env }
           .to throw_symbol :task_code
       end
 
@@ -86,7 +86,7 @@ module Producer::Core
 
         before do
           Task::DSL.define_action(:some_action, some_action_class)
-          dsl.evaluate(env)
+          dsl.evaluate env
         end
 
         it 'registers the action' do
diff --git a/spec/producer/core/task_spec.rb b/spec/producer/core/task_spec.rb
index 707f3e1..b31888b 100644
--- a/spec/producer/core/task_spec.rb
+++ b/spec/producer/core/task_spec.rb
@@ -3,24 +3,24 @@ require 'spec_helper'
 module Producer::Core
   describe Task do
     let(:name)      { :some_task }
-    let(:action)    { double('action') }
-    let(:condition) { double('condition') }
+    let(:action)    { double 'action' }
+    let(:condition) { double 'condition' }
     subject(:task)  { Task.new(name, [action], condition) }
 
     describe '.evaluate' do
-      let(:env)   { double('env') }
-      let(:block) { proc { :some_value } }
+      let(:env)   { double 'env' }
+      let(:block) { proc { :some_task_code } }
 
       it 'delegates to DSL.evaluate' do
         expect(Task::DSL)
           .to receive(:evaluate).with(name, env) do |&b|
-            expect(b.call).to eq :some_value
+            expect(b).to be block
           end
         Task.evaluate(name, env, &block)
       end
 
       it 'returns the evaluated task' do
-        task = double('task')
+        task = double 'task'
         allow(Task::DSL).to receive(:evaluate) { task }
         expect(Task.evaluate(name, env, &block)).to be task
       end
@@ -40,7 +40,7 @@ module Producer::Core
       end
 
       context 'when only the name is given as argument' do
-        subject(:task)  { Task.new(name) }
+        subject(:task) { Task.new(name) }
 
         it 'assigns no action' do
           expect(task.actions).to be_empty
diff --git a/spec/producer/core/tests/has_env_spec.rb b/spec/producer/core/tests/has_env_spec.rb
index e79ee01..dec0329 100644
--- a/spec/producer/core/tests/has_env_spec.rb
+++ b/spec/producer/core/tests/has_env_spec.rb
@@ -11,7 +11,7 @@ module Producer::Core
     end
 
     describe '#verify' do
-      let(:environment) { double('environment') }
+      let(:environment) { double 'environment' }
 
       before do
         allow(env.remote).to receive(:environment) { environment }
diff --git a/spec/producer/core/tests/has_file_spec.rb b/spec/producer/core/tests/has_file_spec.rb
index 9347854..7135776 100644
--- a/spec/producer/core/tests/has_file_spec.rb
+++ b/spec/producer/core/tests/has_file_spec.rb
@@ -19,7 +19,7 @@ module Producer::Core
       end
 
       it 'returns the file existence' do
-        existence = double('existence')
+        existence = double 'existence'
         allow(env.remote.fs).to receive(:has_file?) { existence }
         expect(has_file.verify).to be existence
       end
diff --git a/spec/support/net_ssh_story_helpers.rb b/spec/support/net_ssh_story_helpers.rb
index bad0402..5d2931f 100644
--- a/spec/support/net_ssh_story_helpers.rb
+++ b/spec/support/net_ssh_story_helpers.rb
@@ -36,7 +36,7 @@ module NetSSHStoryHelpers
   def sftp_story
     story do |session|
       ch = session.opens_channel
-      ch.sends_subsystem('sftp')
+      ch.sends_subsystem 'sftp'
       ch.sends_packet(
         Net::SFTP::Constants::PacketTypes::FXP_INIT, :long,
         Net::SFTP::Session::HIGHEST_PROTOCOL_VERSION_SUPPORTED
diff --git a/spec/support/tests_helpers.rb b/spec/support/tests_helpers.rb
index 7e982d1..d5cc9d8 100644
--- a/spec/support/tests_helpers.rb
+++ b/spec/support/tests_helpers.rb
@@ -1,9 +1,9 @@
 module TestsHelpers
   def test_ok
-    double('test', pass?: true)
+    double 'test', pass?: true
   end
 
   def test_ko
-    double('test', pass?: false)
+    double 'test', pass?: false
   end
 end