Fix verifying partial doubles errorsdan/verify-partial-doubles

11 files changed, 113 insertions, 65 deletions

diff --git a/lib/chef/provider/user.rb b/lib/chef/provider/user.rb
index f6ac72448e..d124076319 100644
--- a/lib/chef/provider/user.rb
+++ b/lib/chef/provider/user.rb
@@ -140,6 +140,10 @@ class Chef
         end
       end
 
+      def create_user
+        raise NotImplementedError
+      end
+
       def action_remove
         if @user_exists
           converge_by("remove user #{@new_resource.username}") do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index f3c7f84712..11a9fb6db3 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -183,6 +183,66 @@ RSpec.configure do |config|
   end
 end
 
+require 'chef/shell/shell_session'
+# Stubbed shell session subclass
+class TestableShellSession < Shell::ShellSession
+
+  def rebuild_node
+    nil
+  end
+
+  def rebuild_collection
+    nil
+  end
+
+  def loading
+    nil
+  end
+
+  def loading_complete
+    nil
+  end
+
+end
+
+# Double of irb's job manager used for unit tests
+class TestJobManager
+  attr_accessor :jobs
+
+  def select_shell_session(session_name)
+    nil
+  end
+
+  def switch(session_name)
+  end
+end
+
+# Extends an individual object with ShellExtensions for unit tests
+ObjectTestHarness = Proc.new do
+  extend Shell::Extensions::ObjectCoreExtensions
+
+  def conf=(new_conf)
+    @conf = new_conf
+  end
+
+  def conf
+    @conf
+  end
+
+  def jobs
+    nil
+  end
+
+  def irb(session_name)
+    nil
+  end
+
+  desc "rspecin'"
+  def rspec_method
+  end
+end
+
+
 require 'webrick/utils'
 
 #    Webrick uses a centralized/synchronized timeout manager. It works by
diff --git a/spec/support/shared/unit/provider/file.rb b/spec/support/shared/unit/provider/file.rb
index 86f32c9e89..fd532d88db 100644
--- a/spec/support/shared/unit/provider/file.rb
+++ b/spec/support/shared/unit/provider/file.rb
@@ -49,7 +49,7 @@ def setup_normal_file
     allow(File).to receive(:directory?).with(path).and_return(false)
     allow(File).to receive(:writable?).with(path).and_return(true)
     allow(file_symlink_class).to receive(:symlink?).with(path).and_return(false)
-    allow(File).to receive(:realpath?).with(path).and_return(normalized_path)
+    allow(File).to receive(:realpath).with(path).and_return(normalized_path)
   end
   allow(File).to receive(:directory?).with(enclosing_directory).and_return(true)
 end
@@ -57,7 +57,7 @@ end
 def setup_missing_file
   [ resource_path, normalized_path, windows_path].each do |path|
     allow(File).to receive(:file?).with(path).and_return(false)
-    allow(File).to receive(:realpath?).with(path).and_return(resource_path)
+    allow(File).to receive(:realpath).with(path).and_return(resource_path)
     allow(File).to receive(:exists?).with(path).and_return(false)
     allow(File).to receive(:exist?).with(path).and_return(false)
     allow(File).to receive(:directory?).with(path).and_return(false)
@@ -70,7 +70,7 @@ end
 def setup_symlink
   [ resource_path, normalized_path, windows_path].each do |path|
     allow(File).to receive(:file?).with(path).and_return(true)
-    allow(File).to receive(:realpath?).with(path).and_return(normalized_path)
+    allow(File).to receive(:realpath).with(path).and_return(normalized_path)
     allow(File).to receive(:exists?).with(path).and_return(true)
     allow(File).to receive(:exist?).with(path).and_return(true)
     allow(File).to receive(:directory?).with(path).and_return(false)
@@ -83,7 +83,7 @@ end
 def setup_unwritable_file
   [ resource_path, normalized_path, windows_path].each do |path|
     allow(File).to receive(:file?).with(path).and_return(false)
-    allow(File).to receive(:realpath?).with(path).and_raise(Errno::ENOENT)
+    allow(File).to receive(:realpath).with(path).and_raise(Errno::ENOENT)
     allow(File).to receive(:exists?).with(path).and_return(true)
     allow(File).to receive(:exist?).with(path).and_return(true)
     allow(File).to receive(:directory?).with(path).and_return(false)
@@ -96,7 +96,7 @@ end
 def setup_missing_enclosing_directory
   [ resource_path, normalized_path, windows_path].each do |path|
     allow(File).to receive(:file?).with(path).and_return(false)
-    allow(File).to receive(:realpath?).with(path).and_raise(Errno::ENOENT)
+    allow(File).to receive(:realpath).with(path).and_raise(Errno::ENOENT)
     allow(File).to receive(:exists?).with(path).and_return(false)
     allow(File).to receive(:exist?).with(path).and_return(false)
     allow(File).to receive(:directory?).with(path).and_return(false)
diff --git a/spec/unit/provider/remote_file/ftp_spec.rb b/spec/unit/provider/remote_file/ftp_spec.rb
index dbbddd8e84..10c8e5a4ee 100644
--- a/spec/unit/provider/remote_file/ftp_spec.rb
+++ b/spec/unit/provider/remote_file/ftp_spec.rb
@@ -53,7 +53,8 @@ describe Chef::Provider::RemoteFile::FTP do
 
   let(:tempfile) do
     t = StringIO.new
-    allow(t).to receive(:path).and_return(tempfile_path)
+    example = self
+    t.define_singleton_method(:path) { example.tempfile_path }
     t
   end
 
@@ -137,7 +138,6 @@ describe Chef::Provider::RemoteFile::FTP do
       let(:uri) { URI.parse("ftp://opscode.com:8021/seattle.txt") }
 
       it "should connect on an alternate port when one is provided" do
-        uri = URI.parse("ftp://opscode.com:8021/seattle.txt")
         expect(ftp).to receive(:connect).with("opscode.com", 8021)
         fetcher.fetch
       end
diff --git a/spec/unit/provider/remote_file_spec.rb b/spec/unit/provider/remote_file_spec.rb
index de4a897847..d59d635a7c 100644
--- a/spec/unit/provider/remote_file_spec.rb
+++ b/spec/unit/provider/remote_file_spec.rb
@@ -32,7 +32,7 @@ describe Chef::Provider::RemoteFile do
   end
 
   let(:content) do
-    content = double('Chef::Provider::File::Content::RemoteFile')
+    double('Chef::Provider::File::Content::RemoteFile')
   end
 
   let(:node) { double('Chef::Node') }
@@ -48,7 +48,6 @@ describe Chef::Provider::RemoteFile do
   subject(:provider) do
     provider = described_class.new(resource, run_context)
     allow(provider).to receive(:content).and_return(content)
-    allow(provider).to receive(:update_new_resource_checksum).and_return(nil) # Otherwise it doesn't behave like a File provider
     provider
   end
 
diff --git a/spec/unit/provider/service/debian_service_spec.rb b/spec/unit/provider/service/debian_service_spec.rb
index a4667e8ce8..c6d8ede23a 100644
--- a/spec/unit/provider/service/debian_service_spec.rb
+++ b/spec/unit/provider/service/debian_service_spec.rb
@@ -46,8 +46,6 @@ describe Chef::Provider::Service::Debian do
 
     context "when update-rc.d shows init linked to rc*.d/" do
       before do
-        allow(@provider).to receive(:assert_update_rcd_available)
-
         result = <<-UPDATE_RC_D_SUCCESS
   Removing any system startup links for /etc/init.d/chef ...
     /etc/rc0.d/K20chef
@@ -79,7 +77,6 @@ describe Chef::Provider::Service::Debian do
 
     context "when update-rc.d shows init isn't linked to rc*.d/" do
       before do
-        allow(@provider).to receive(:assert_update_rcd_available)
         @status = double("Status", :exitstatus => 0)
         @stdout = StringIO.new(
           " Removing any system startup links for /etc/init.d/chef ...")
@@ -196,8 +193,6 @@ insserv: dryrun, not creating .depend.boot, .depend.start, and .depend.stop
       context "on #{model}" do
         context "when update-rc.d shows init linked to rc*.d/" do
           before do
-            allow(@provider).to receive(:assert_update_rcd_available)
-
             @stdout = StringIO.new(expected_results["linked"]["stdout"])
             @stderr = StringIO.new(expected_results["linked"]["stderr"])
             @status = double("Status", :exitstatus => 0, :stdout => @stdout)
@@ -223,7 +218,6 @@ insserv: dryrun, not creating .depend.boot, .depend.start, and .depend.stop
 
         context "when update-rc.d shows init isn't linked to rc*.d/" do
           before do
-            allow(@provider).to receive(:assert_update_rcd_available)
             @stdout = StringIO.new(expected_results["not linked"]["stdout"])
             @stderr = StringIO.new(expected_results["not linked"]["stderr"])
             @status = double("Status", :exitstatus => 0, :stdout => @stdout)
diff --git a/spec/unit/provider/service/openbsd_service_spec.rb b/spec/unit/provider/service/openbsd_service_spec.rb
index 1b5206470e..0e5d0d3ba9 100644
--- a/spec/unit/provider/service/openbsd_service_spec.rb
+++ b/spec/unit/provider/service/openbsd_service_spec.rb
@@ -134,8 +134,6 @@ describe Chef::Provider::Service::Openbsd do
       stub_etc_rcd_script
       provider.current_resource = current_resource
       current_resource.service_name(new_resource.service_name)
-
-      allow(provider).to receive(:service_enable_variable_name).and_return("#{new_resource.service_name}_enable")
     end
 
     context "when the service is builtin" do
@@ -248,7 +246,6 @@ describe Chef::Provider::Service::Openbsd do
       stub_etc_rcd_script
       expect(provider).to receive(:determine_current_status!)
       current_resource.running(false)
-      allow(provider).to receive(:service_enable_variable_name).and_return "#{new_resource.service_name}_enable"
       expect(::File).to receive(:open).with("/etc/rc.d/#{new_resource.service_name}")
      end
 
diff --git a/spec/unit/provider/service/upstart_service_spec.rb b/spec/unit/provider/service/upstart_service_spec.rb
index ca7ce8f930..1638d6205d 100644
--- a/spec/unit/provider/service/upstart_service_spec.rb
+++ b/spec/unit/provider/service/upstart_service_spec.rb
@@ -212,7 +212,7 @@ describe Chef::Provider::Service::Upstart do
     end
 
     it "should enable the service if it is not enabled" do
-      @file = Object.new
+      @file = instance_double("Chef::Util::FileEdit")
       allow(Chef::Util::FileEdit).to receive(:new).and_return(@file)
       allow(@current_resource).to receive(:enabled).and_return(false)
       expect(@file).to receive(:search_file_replace)
@@ -221,7 +221,7 @@ describe Chef::Provider::Service::Upstart do
     end
 
     it "should disable the service if it is enabled" do
-      @file = Object.new
+      @file = instance_double("Chef::Util::FileEdit")
       allow(Chef::Util::FileEdit).to receive(:new).and_return(@file)
       allow(@current_resource).to receive(:enabled).and_return(true)
       expect(@file).to receive(:search_file_replace)
diff --git a/spec/unit/provider/service/windows_spec.rb b/spec/unit/provider/service/windows_spec.rb
index 784a2232b2..71464336cf 100644
--- a/spec/unit/provider/service/windows_spec.rb
+++ b/spec/unit/provider/service/windows_spec.rb
@@ -20,6 +20,44 @@
 require 'spec_helper'
 require 'mixlib/shellout'
 
+# A class that implements the same API as Win32::Service. All methods raise
+# errors unless stubbed or redefined.
+#
+# This class is used because we cannot load win32-specific libraries on
+# non-windows platforms, so we replace the constant with a subclass of this for
+# unit tests. We need to provide the same API in order to enable the
+# `verify_partial_doubles` RSpec option.
+class Win32ServiceDouble
+  AUTO_START = 0x00000002
+  DEMAND_START = 0x00000003
+  DISABLED = 0x00000004
+
+  def self.status(arg)
+    raise "NOT IMPLEMENTED: the status method must be stubbed"
+  end
+
+  def self.config_info(arg)
+    raise "NOT IMPLEMENTED: the config_info method must be stubbed"
+  end
+
+  def self.exists?(arg)
+    raise "NOT IMPLEMENTED: the exists? method must be stubbed"
+  end
+
+  def self.configure(arg)
+    raise "NOT IMPLEMENTED: the exists? method must be stubbed"
+  end
+
+  def self.start(arg)
+    raise "NOT IMPLEMENTED: the start method must be stubbed"
+  end
+
+  def self.stop(arg)
+    raise "NOT IMPLEMENTED: the stop method must be stubbed"
+  end
+
+end
+
 describe Chef::Provider::Service::Windows, "load_current_resource" do
   before(:each) do
     @node = Chef::Node.new
@@ -30,10 +68,7 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
     @provider.current_resource = Chef::Resource::WindowsService.new("current-chef")
     Object.send(:remove_const, 'Win32') if defined?(Win32)
     Win32 = Module.new
-    Win32::Service = Class.new
-    Win32::Service::AUTO_START = 0x00000002
-    Win32::Service::DEMAND_START = 0x00000003
-    Win32::Service::DISABLED = 0x00000004
+    Win32::Service = Class.new(Win32ServiceDouble)
     allow(Win32::Service).to receive(:status).with(@new_resource.service_name).and_return(
       double("StatusStruct", :current_state => "running"))
     allow(Win32::Service).to receive(:config_info).with(@new_resource.service_name).and_return(
diff --git a/spec/unit/shell/shell_session_spec.rb b/spec/unit/shell/shell_session_spec.rb
index d72e3fa1bb..b6ff5a2e0f 100644
--- a/spec/unit/shell/shell_session_spec.rb
+++ b/spec/unit/shell/shell_session_spec.rb
@@ -18,27 +18,6 @@
 require 'spec_helper'
 require "ostruct"
 
-
-class TestableShellSession < Shell::ShellSession
-
-  def rebuild_node
-    nil
-  end
-
-  def rebuild_collection
-    nil
-  end
-
-  def loading
-    nil
-  end
-
-  def loading_complete
-    nil
-  end
-
-end
-
 describe Shell::ShellSession do
 
   it "is a singleton object" do
diff --git a/spec/unit/shell_spec.rb b/spec/unit/shell_spec.rb
index 0e028f4359..b892ca273e 100644
--- a/spec/unit/shell_spec.rb
+++ b/spec/unit/shell_spec.rb
@@ -18,26 +18,6 @@
 require 'spec_helper'
 require "ostruct"
 
-ObjectTestHarness = Proc.new do
-  extend Shell::Extensions::ObjectCoreExtensions
-
-  def conf=(new_conf)
-    @conf = new_conf
-  end
-
-  def conf
-    @conf
-  end
-
-  desc "rspecin'"
-  def rspec_method
-  end
-end
-
-class TestJobManager
-  attr_accessor :jobs
-end
-
 describe Shell do
 
   before do