From 725c866d8abb461307566652f486cd8228f5989e Mon Sep 17 00:00:00 2001 From: Claire McQuin Date: Tue, 14 Oct 2014 11:29:51 -0700 Subject: Make action read-only, set at instance creation time. --- lib/chef/provider.rb | 33 ++++++++----- spec/unit/provider_spec.rb | 121 +++++++++++++++++++++++++-------------------- 2 files changed, 88 insertions(+), 66 deletions(-) diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb index bdfe826944..f6afa939f0 100644 --- a/lib/chef/provider.rb +++ b/lib/chef/provider.rb @@ -35,20 +35,24 @@ class Chef attr_reader :recipe_name attr_reader :cookbook_name - #-- - # TODO: this should be a reader, and the action should be passed in the - # constructor; however, many/most subclasses override the constructor so - # changing the arity would be a breaking change. Change this at the next - # break, e.g., Chef 11. - attr_accessor :action - - def initialize(new_resource, run_context) + attr_reader :action + + # @TODO Chef 13: Make action a required parameter. + # action is an optional parameter for Chef 12 so custom provider + # classes can be updated. + def initialize(new_resource, run_context, action=nil) @new_resource = new_resource - @action = action @current_resource = nil @run_context = run_context @converge_actions = nil + if action.nil? + Chef::Log.warn("Initializing a provider without an action parameter is deprecated" + + " in Chef 12 and will be removed in Chef 13. It is recommended that you update" + + " custom providers to reflect these changes.") + end + @action = action + @recipe_name = nil @cookbook_name = nil end @@ -93,11 +97,14 @@ class Chef run_context.events end + # @TODO Chef 13: Remove optional parameter action. def run_action(action=nil) - @action = action unless action.nil? - - # TODO: it would be preferable to get the action to be executed in the - # constructor... + unless action.nil? + Chef::Log.warn("Specifying an action to #run_action is deprecated in" + + " Chef 12 and will be removed in Chef 13. An action should be supplied" + + " to the provider constructor.") + @action = action + end # user-defined LWRPs may include unsafe load_current_resource methods that cannot be run in whyrun mode if !whyrun_mode? || whyrun_supported? diff --git a/spec/unit/provider_spec.rb b/spec/unit/provider_spec.rb index 9b89fc1888..8fc6195403 100644 --- a/spec/unit/provider_spec.rb +++ b/spec/unit/provider_spec.rb @@ -50,109 +50,124 @@ class ConvergeActionDemonstrator < Chef::Provider end describe Chef::Provider do - before(:each) do - @cookbook_collection = Chef::CookbookCollection.new([]) - @node = Chef::Node.new - @node.name "latte" - @events = Chef::EventDispatch::Dispatcher.new - @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events) - @resource = Chef::Resource.new("funk", @run_context) - @resource.cookbook_name = "a_delicious_pie" - @provider = Chef::Provider.new(@resource, @run_context) + let(:cookbook_collection) { Chef::CookbookCollection.new([]) } + + let(:node) do + n = Chef::Node.new + n.name "latte" + n + end + + let(:events) { Chef::EventDispatch::Dispatcher.new } + + let(:run_context) { Chef::RunContext.new(node, cookbook_collection, events) } + + let(:resource) do + res = Chef::Resource.new("funk", run_context) + res.cookbook_name = "a_delicious_pie" + res end + let(:provider) { Chef::Provider.new(resource, run_context, :nothing) } + it "should mixin shell_out" do - expect(@provider.respond_to?(:shell_out)).to be true + expect(provider.respond_to?(:shell_out)).to be true end it "should mixin shell_out!" do - expect(@provider.respond_to?(:shell_out!)).to be true + expect(provider.respond_to?(:shell_out!)).to be true end it "should mixin shell_out_with_systems_locale" do - expect(@provider.respond_to?(:shell_out_with_systems_locale)).to be true + expect(provider.respond_to?(:shell_out_with_systems_locale)).to be true end it "should store the resource passed to new as new_resource" do - @provider.new_resource.should eql(@resource) + provider.new_resource.should eql(resource) end it "should store the node passed to new as node" do - @provider.node.should eql(@node) + provider.node.should eql(node) end it "should have nil for current_resource by default" do - @provider.current_resource.should eql(nil) + provider.current_resource.should eql(nil) end it "should not support whyrun by default" do - @provider.send(:whyrun_supported?).should eql(false) + provider.send(:whyrun_supported?).should eql(false) end it "should return true for action_nothing" do - @provider.action_nothing.should eql(true) + provider.action_nothing.should eql(true) end it "evals embedded recipes with a pristine resource collection" do - @provider.run_context.instance_variable_set(:@resource_collection, "doesn't matter what this is") + provider.run_context.instance_variable_set(:@resource_collection, "doesn't matter what this is") temporary_collection = nil - snitch = Proc.new {temporary_collection = @run_context.resource_collection} - @provider.send(:recipe_eval, &snitch) - temporary_collection.should be_an_instance_of(Chef::ResourceCollection) - @provider.run_context.instance_variable_get(:@resource_collection).should == "doesn't matter what this is" - end + snitch = Proc.new {temporary_collection = run_context.resource_collection} + provider.send(:recipe_eval, &snitch) + expect(temporary_collection).to be_an_instance_of(Chef::ResourceCollection) + expect(provider.run_context.instance_variable_get(:@resource_collection)).to eq "doesn't matter what this is" + end + + describe "when creating the temporary run context" do + let(:run_context) do + rc = Chef::RunContext.new(node, cookbook_collection, events) + # we actually want to test that RunContext#load is never called, but we + # can't stub all instances of an object with rspec's mocks. :/ + allow(Chef::RunContext).to receive(:new).and_raise("not supposed to happen again") + rc + end + let(:snitch) { Proc.new { temporary_collection = run_context.resource_collection } } - it "does not re-load recipes when creating the temporary run context" do - # we actually want to test that RunContext#load is never called, but we - # can't stub all instances of an object with rspec's mocks. :/ - Chef::RunContext.stub(:new).and_raise("not supposed to happen") - snitch = Proc.new {temporary_collection = @run_context.resource_collection} - @provider.send(:recipe_eval, &snitch) + it "does not re-load recipes" do + provider.send(:recipe_eval, &snitch) + end end context "when no converge actions are queued" do before do - @provider.stub(:whyrun_supported?).and_return(true) - @provider.stub(:load_current_resource) + allow(provider).to receive(:whyrun_supported?).and_return(true) + allow(provider).to receive(:load_current_resource) end it "does not mark the new resource as updated" do - @resource.should_not be_updated - @resource.should_not be_updated_by_last_action + expect(resource).to_not be_updated + expect(resource).to_not be_updated_by_last_action end end context "when converge actions have been added to the queue" do describe "and provider supports whyrun mode" do - before do - @provider = ConvergeActionDemonstrator.new(@resource, @run_context) - end + let(:provider) { ConvergeActionDemonstrator.new(resource, run_context, :foo) } it "should tell us that it does support whyrun" do - @provider.should be_whyrun_supported + expect(provider).to be_whyrun_supported end it "queues up converge actions" do - @provider.action_foo - @provider.send(:converge_actions).should have(1).actions + provider.action_foo + expect(provider.send(:converge_actions)).to have(1).actions end it "executes pending converge actions to converge the system" do - @provider.run_action(:foo) - @provider.instance_variable_get(:@system_state_altered).should be_true + provider.run_action + expect(provider.instance_variable_get(:@system_state_altered)).to be_true end it "marks the resource as updated" do - @provider.run_action(:foo) - @resource.should be_updated - @resource.should be_updated_by_last_action + provider.run_action + expect(resource).to be_updated + expect(resource).to be_updated_by_last_action end end describe "and provider does not support whyrun mode" do + let(:provider) { NoWhyrunDemonstrator.new(resource, run_context, :foo) } + before do Chef::Config[:why_run] = true - @provider = NoWhyrunDemonstrator.new(@resource, @run_context) end after do @@ -160,20 +175,20 @@ describe Chef::Provider do end it "should tell us that it doesn't support whyrun" do - @provider.should_not be_whyrun_supported + expect(provider).to_not be_whyrun_supported end it "should automatically generate a converge_by block on the provider's behalf" do - @provider.run_action(:foo) - @provider.send(:converge_actions).should have(0).actions - @provider.system_state_altered.should be_false + provider.run_action + expect(provider.send(:converge_actions)).to have(0).actions + expect(provider.system_state_altered).to be_false end it "should automatically execute the generated converge_by block" do - @provider.run_action(:foo) - @provider.system_state_altered.should be_false - @resource.should_not be_updated - @resource.should_not be_updated_by_last_action + provider.run_action + expect(provider.system_state_altered).to be_false + expect(resource).to_not be_updated + expect(resource).to_not be_updated_by_last_action end end end -- cgit v1.2.1