fix buggy verification of primitive resource creation

Newly created primitive resources weren't being marked as updated in
real runs, because exists? was being run on an old object.  Furthermore,
the tests didn't catch this because #exists? on any instance of
Pacemaker::Resource::Primitive was being mocked to return true.  The fix
requires re-invoking Pacemaker::CIBObject.from_name after attempting to
create the new primitive, so that cib_object.exists? accurately reflects
whether the creation succeeded.  However, testing this required mocking
Mixlib::Shellout#stdout to first return "" and then secondly a
definition string for the created resource, and unfortunately this
exposed a nasty bug in rspec-mocks:

  https://github.com/rspec/rspec-mocks/issues/559

So we revamp the mocking of Mixlib::Shellout#stdout to use doubles
instead of #expect_any_instance_of.

libraries/chef/mixin/pacemaker/standard_cib_object.rb

@@ -45,6 +45,7 @@ class Chef
           action :nothing
         end.run_action(:run)
 
+        cib_object = Pacemaker::CIBObject.from_name(new_resource.name)
         if cib_object.exists?
           new_resource.updated_by_last_action(true)
           ::Chef::Log.info "Successfully configured #{cib_object}"

spec/helpers/cib_object.rb

@@ -15,11 +15,7 @@ shared_examples "a CIB object" do
   end
 
   it "should be instantiated via Pacemaker::CIBObject.from_name" do
-    Mixlib::ShellOut.any_instance.stub(:error!)
+    expect_definitions(fixture.definition_string)
-    expect_any_instance_of(Mixlib::ShellOut) \
-      .to receive(:stdout) \
-      .and_return(fixture.definition_string)
-
     obj = Pacemaker::CIBObject.from_name(fixture.name)
     expect_to_match_fixture(obj)
   end
@@ -30,10 +26,7 @@ shared_examples "a CIB object" do
   end
 
   it "should barf if the loaded definition's type is not colocation" do
-    Mixlib::ShellOut.any_instance.stub(:error!)
+    expect_definitions("clone foo blah blah")
-    expect_any_instance_of(Mixlib::ShellOut) \
-      .to receive(:stdout) \
-      .and_return("clone foo blah blah")
     expect { fixture.load_definition }.to \
       raise_error(Pacemaker::CIBObject::TypeMismatch,
                   "Expected #{object_type} type but loaded definition was type clone")
@@ -42,7 +35,7 @@ end
 
 shared_examples "action on non-existent resource" do |action, cmd, expected_error|
   it "should not attempt to #{action.to_s} a non-existent resource" do
-    expect_definition("")
+    expect_definitions("")
 
     if expected_error
       expect { provider.run_action action }.to \
@@ -62,18 +55,18 @@ module Chef::RSpec
       # "crm configure show" is executed by load_current_resource, and
       # again later on for the :create action, to see whether to create or
       # modify.
-      def expect_definition(definition)
+      def shellout_double(definition)
-        Mixlib::ShellOut.any_instance.stub(:run_command)
+        shellout = double(Mixlib::ShellOut)
-        Mixlib::ShellOut.any_instance.stub(:error!)
+        shellout.stub(:environment).and_return({})
-        expect_any_instance_of(Mixlib::ShellOut) \
+        shellout.stub(:run_command)
-          .to receive(:stdout) \
+        shellout.stub(:error!)
-          .and_return(definition)
+        expect(shellout).to receive(:stdout).and_return(definition)
+        shellout
       end
 
-      def expect_exists(exists)
+      def expect_definitions(*definitions)
-        expect_any_instance_of(cib_object_class) \
+        doubles = definitions.map { |d| shellout_double(d) }
-          .to receive(:exists?) \
+        Mixlib::ShellOut.stub(:new).and_return(*doubles)
-          .and_return(exists)
       end
     end
   end

spec/helpers/runnable_resource.rb

@@ -21,7 +21,7 @@ shared_examples "a runnable resource" do |fixture|
       :delete, "crm configure delete #{fixture.name}", nil
 
     it "should not delete a running resource" do
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
       expect_running(true)
 
       expected_error = "Cannot delete running #{fixture}"
@@ -34,7 +34,7 @@ shared_examples "a runnable resource" do |fixture|
     end
 
     it "should delete a non-running resource" do
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
       expect_running(false)
 
       provider.run_action :delete
@@ -52,7 +52,7 @@ shared_examples "a runnable resource" do |fixture|
       "Cannot start non-existent #{fixture}"
 
     it "should do nothing to a started resource" do
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
       expect_running(true)
 
       provider.run_action :start
@@ -64,7 +64,7 @@ shared_examples "a runnable resource" do |fixture|
 
     it "should start a stopped resource" do
       config = fixture.definition_string.sub("Started", "Stopped")
-      expect_definition(config)
+      expect_definitions(config)
       expect_running(false)
 
       provider.run_action :start
@@ -82,7 +82,7 @@ shared_examples "a runnable resource" do |fixture|
       "Cannot stop non-existent #{fixture}"
 
     it "should do nothing to a stopped resource" do
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
       expect_running(false)
 
       provider.run_action :stop
@@ -93,7 +93,7 @@ shared_examples "a runnable resource" do |fixture|
     end
 
     it "should stop a started resource" do
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
       expect_running(true)
 
       provider.run_action :stop

spec/libraries/pacemaker/resource/primitive_spec.rb

@@ -14,10 +14,6 @@ describe Pacemaker::Resource::Primitive do
     Chef::RSpec::Pacemaker::Config::KEYSTONE_PRIMITIVE_DEFINITION
   }
 
-  before(:each) do
-    Mixlib::ShellOut.any_instance.stub(:run_command)
-  end
-
   def object_type
     'primitive'
   end

spec/providers/colocation_spec.rb

@@ -35,7 +35,7 @@ describe "Chef::Provider::PacemakerColocation" do
     def test_modify(expected_cmds)
       yield
 
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
 
       provider.run_action :create
 

spec/providers/group_spec.rb

@@ -36,7 +36,7 @@ describe "Chef::Provider::PacemakerGroup" do
     def test_modify(expected_cmds)
       yield
 
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
 
       provider.run_action :create
 

spec/providers/primitive_spec.rb

@@ -38,7 +38,7 @@ describe "Chef::Provider::PacemakerPrimitive" do
     def test_modify(expected_cmds)
       yield
 
-      expect_definition(fixture.definition_string)
+      expect_definitions(fixture.definition_string)
 
       provider.run_action :create
 
@@ -99,10 +99,10 @@ describe "Chef::Provider::PacemakerPrimitive" do
     end
 
     it "should create a primitive if it doesn't already exist" do
-      expect_definition("")
+      # The first time, Mixlib::ShellOut needs to return an empty definition.
-      # Later, the :create action calls cib_object_class#exists? to check
+      # Then the resource gets created so the second time it needs to return
-      # that creation succeeded.
+      # the definition used for creation.
-      expect_exists(true)
+      expect_definitions("", fixture.definition_string)
 
       provider.run_action :create
 
@@ -113,7 +113,7 @@ describe "Chef::Provider::PacemakerPrimitive" do
     it "should barf if the primitive is already defined with the wrong agent" do
       existing_agent = "ocf:openstack:something-else"
       definition = fixture.definition_string.sub(fixture.agent, existing_agent)
-      expect_definition(definition)
+      expect_definitions(definition)
 
       expected_error = \
         "Existing #{fixture} has agent '#{existing_agent}' " \