From c63e9c2394a30fe653908cd15967218d90fa34d6 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 2 May 2011 13:57:57 -0700 Subject: maint: reset more global state in testing faces... When we query for all faces we need to scan the entire Ruby load path, look for everything that looks like a face, and load it up. That is a fairly expensive operation, especially on a platform that has slow I/O. *cough* EC2 *cough* Because of that we only scan once, and assume that the list is static thereafter; this works OK out in the field, but sucks in testing where that global state gets in the way of the rest of our fiddling under the hood. This resets the '@loaded' member of the collection additionally, which is what should be done since we have reset the rest of the collection at the same time. We don't bother to reset it, as an extra scan during tests is not a problem. Reviewed-By: Nick Lewis --- spec/unit/interface/face_collection_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/unit') diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb index 890e06a9e..4358ef4b6 100755 --- a/spec/unit/interface/face_collection_spec.rb +++ b/spec/unit/interface/face_collection_spec.rb @@ -16,11 +16,12 @@ describe Puppet::Interface::FaceCollection do @original_faces = subject.instance_variable_get("@faces").dup @original_required = $".dup $".delete_if do |path| path =~ %r{/face/.*\.rb$} end - subject.instance_variable_get("@faces").clear + subject.instance_variable_get(:@faces).clear + subject.instance_variable_set(:@loaded, false) end after :each do - subject.instance_variable_set("@faces", @original_faces) + subject.instance_variable_set(:@faces, @original_faces) $".clear ; @original_required.each do |item| $" << item end end -- cgit From 86c6ec24f387fc70abc333fc4ac974b06b3ec80a Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 2 May 2011 14:31:54 -0700 Subject: maint: move the indirector face base out of puppet/face We used to shove the base class Puppet::Face::Indirector next to the actual faces; this made a bunch of things, including testing, confusing. Instead, move it away into the indirector where it lives with the rest of the indirector related things. Reviewed-By: Nick Lewis --- spec/unit/application/indirection_base_spec.rb | 4 +- spec/unit/face/indirector_spec.rb | 59 -------------------------- spec/unit/indirector/face_spec.rb | 59 ++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 61 deletions(-) delete mode 100755 spec/unit/face/indirector_spec.rb create mode 100755 spec/unit/indirector/face_spec.rb (limited to 'spec/unit') diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb index e0a9bebb4..910774c14 100755 --- a/spec/unit/application/indirection_base_spec.rb +++ b/spec/unit/application/indirection_base_spec.rb @@ -1,14 +1,14 @@ #!/usr/bin/env rspec require 'spec_helper' require 'puppet/application/indirection_base' -require 'puppet/face/indirector' +require 'puppet/indirector/face' ######################################################################## # Stub for testing; the names are critical, sadly. --daniel 2011-03-30 class Puppet::Application::TestIndirection < Puppet::Application::IndirectionBase end -face = Puppet::Face::Indirector.define(:testindirection, '0.0.1') do +face = Puppet::Indirector::Face.define(:testindirection, '0.0.1') do summary "fake summary" end # REVISIT: This horror is required because we don't allow anything to be diff --git a/spec/unit/face/indirector_spec.rb b/spec/unit/face/indirector_spec.rb deleted file mode 100755 index e7dd44f3d..000000000 --- a/spec/unit/face/indirector_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -#!/usr/bin/env rspec -require 'spec_helper' -require 'puppet/face/indirector' - -describe Puppet::Face::Indirector do - subject do - instance = Puppet::Face::Indirector.new(:test, '0.0.1') - indirection = stub('indirection', - :name => :stub_indirection, - :reset_terminus_class => nil) - instance.stubs(:indirection).returns indirection - instance - end - - it "should be able to return a list of indirections" do - Puppet::Face::Indirector.indirections.should be_include("catalog") - end - - it "should be able to return a list of terminuses for a given indirection" do - Puppet::Face::Indirector.terminus_classes(:catalog).should be_include("compiler") - end - - describe "as an instance" do - it "should be able to determine its indirection" do - # Loading actions here an get, um, complicated - Puppet::Face.stubs(:load_actions) - Puppet::Face::Indirector.new(:catalog, '0.0.1').indirection.should equal(Puppet::Resource::Catalog.indirection) - end - end - - [:find, :search, :save, :destroy].each do |method| - it "should define a '#{method}' action" do - Puppet::Face::Indirector.should be_action(method) - end - - it "should call the indirection method with options when the '#{method}' action is invoked" do - subject.indirection.expects(method).with(:test, {}) - subject.send(method, :test) - end - it "should forward passed options" do - subject.indirection.expects(method).with(:test, {'one'=>'1'}) - subject.send(method, :test, {'one'=>'1'}) - end - end - - it "should be able to override its indirection name" do - subject.set_indirection_name :foo - subject.indirection_name.should == :foo - end - - it "should be able to set its terminus class" do - subject.indirection.expects(:terminus_class=).with(:myterm) - subject.set_terminus(:myterm) - end - - it "should define a class-level 'info' action" do - Puppet::Face::Indirector.should be_action(:info) - end -end diff --git a/spec/unit/indirector/face_spec.rb b/spec/unit/indirector/face_spec.rb new file mode 100755 index 000000000..1530f7270 --- /dev/null +++ b/spec/unit/indirector/face_spec.rb @@ -0,0 +1,59 @@ +#!/usr/bin/env rspec +require 'spec_helper' +require 'puppet/indirector/face' + +describe Puppet::Indirector::Face do + subject do + instance = Puppet::Indirector::Face.new(:test, '0.0.1') + indirection = stub('indirection', + :name => :stub_indirection, + :reset_terminus_class => nil) + instance.stubs(:indirection).returns indirection + instance + end + + it "should be able to return a list of indirections" do + Puppet::Indirector::Face.indirections.should be_include("catalog") + end + + it "should be able to return a list of terminuses for a given indirection" do + Puppet::Indirector::Face.terminus_classes(:catalog).should be_include("compiler") + end + + describe "as an instance" do + it "should be able to determine its indirection" do + # Loading actions here an get, um, complicated + Puppet::Face.stubs(:load_actions) + Puppet::Indirector::Face.new(:catalog, '0.0.1').indirection.should equal(Puppet::Resource::Catalog.indirection) + end + end + + [:find, :search, :save, :destroy].each do |method| + it "should define a '#{method}' action" do + Puppet::Indirector::Face.should be_action(method) + end + + it "should call the indirection method with options when the '#{method}' action is invoked" do + subject.indirection.expects(method).with(:test, {}) + subject.send(method, :test) + end + it "should forward passed options" do + subject.indirection.expects(method).with(:test, {'one'=>'1'}) + subject.send(method, :test, {'one'=>'1'}) + end + end + + it "should be able to override its indirection name" do + subject.set_indirection_name :foo + subject.indirection_name.should == :foo + end + + it "should be able to set its terminus class" do + subject.indirection.expects(:terminus_class=).with(:myterm) + subject.set_terminus(:myterm) + end + + it "should define a class-level 'info' action" do + Puppet::Indirector::Face.should be_action(:info) + end +end -- cgit From 1b42725b5caab6f8e457e11fec2488fbe94e8e43 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 2 May 2011 15:14:25 -0700 Subject: (#7314) Faces fail horribly when one has a syntax error. When we hit a syntax error in any face, a whole bunch of unrelated face things would blow up in horrible ways. Stack traces for all... Now, instead, we catch that fault but specifically only in the face file and report it through our error logs, then quietly ignore the face. Reviewed-By: Nick Lewis --- spec/unit/interface/face_collection_spec.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'spec/unit') diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb index 4358ef4b6..4ad8787c5 100755 --- a/spec/unit/interface/face_collection_spec.rb +++ b/spec/unit/interface/face_collection_spec.rb @@ -22,7 +22,7 @@ describe Puppet::Interface::FaceCollection do after :each do subject.instance_variable_set(:@faces, @original_faces) - $".clear ; @original_required.each do |item| $" << item end + @original_required.each {|f| $".push f unless $".include? f } end describe "::prefix_match?" do @@ -160,4 +160,21 @@ describe Puppet::Interface::FaceCollection do end end end + + context "faulty faces" do + before :each do + $:.unshift "#{PuppetSpec::FIXTURE_DIR}/faulty_face" + end + + after :each do + $:.delete_if {|x| x == "#{PuppetSpec::FIXTURE_DIR}/faulty_face"} + end + + it "should not die if a face has a syntax error" do + subject.faces.should be_include :help + subject.faces.should_not be_include :syntax + @logs.should_not be_empty + @logs.first.message.should =~ /syntax error/ + end + end end -- cgit