+1 On Fri, Aug 13, 2010 at 4:13 PM, Paul Berry <[email protected]> wrote:
> This change is part of an ongoing effort to remove functionality from > TypeCollection that is not related to keeping track of a collection of > types. This reduces TypeCollection's linkage to the environment, > which is a step toward decoupling it from the type loading mechanism. > > Also, added a spec test to verify that TypeCollection.version is > correctly recomputed when types are re-imported. > > Signed-off-by: Paul Berry <[email protected]> > --- > lib/puppet/node/environment.rb | 22 ++++++++++- > lib/puppet/resource/type_collection.rb | 18 -------- > spec/integration/parser/compiler_spec.rb | 11 +++++ > spec/unit/application/apply_spec.rb | 11 ++--- > spec/unit/application/master_spec.rb | 9 ++-- > spec/unit/node/environment_spec.rb | 59 > ++++++++++++++++++++++++++-- > spec/unit/resource/type_collection_spec.rb | 52 ------------------------ > 7 files changed, 96 insertions(+), 86 deletions(-) > > diff --git a/lib/puppet/node/environment.rb > b/lib/puppet/node/environment.rb > index 3f67474..ad11a04 100644 > --- a/lib/puppet/node/environment.rb > +++ b/lib/puppet/node/environment.rb > @@ -81,7 +81,7 @@ class Puppet::Node::Environment > Thread.current[:known_resource_types] ||= synchronize { > if @known_resource_types.nil? or @known_resource_types.stale? > @known_resource_types = Puppet::Resource::TypeCollection.new(self) > - @known_resource_types.perform_initial_import > + perform_initial_import > end > @known_resource_types > } > @@ -143,5 +143,25 @@ class Puppet::Node::Environment > end > end > > + private > + > + def perform_initial_import > + return if Puppet.settings[:ignoreimport] > + parser = Puppet::Parser::Parser.new(self) > + if code = Puppet.settings.uninterpolated_value(:code, name.to_s) and > code != "" > + parser.string = code > + else > + file = Puppet.settings.value(:manifest, name.to_s) > + return unless File.exist?(file) > + parser.file = file > + end > + parser.parse > + rescue => detail > + msg = "Could not parse for environment #{self}: #{detail}" > + error = Puppet::Error.new(msg) > + error.set_backtrace(detail.backtrace) > + raise error > + end > + > @root = new(:'*root*') > end > diff --git a/lib/puppet/resource/type_collection.rb > b/lib/puppet/resource/type_collection.rb > index 3a327e2..4254b03 100644 > --- a/lib/puppet/resource/type_collection.rb > +++ b/lib/puppet/resource/type_collection.rb > @@ -110,24 +110,6 @@ class Puppet::Resource::TypeCollection > end > end > > - def perform_initial_import > - return if Puppet.settings[:ignoreimport] > - parser = Puppet::Parser::Parser.new(environment) > - if code = Puppet.settings.uninterpolated_value(:code, > environment.to_s) and code != "" > - parser.string = code > - else > - file = Puppet.settings.value(:manifest, environment.to_s) > - return unless File.exist?(file) > - parser.file = file > - end > - parser.parse > - rescue => detail > - msg = "Could not parse for environment #{environment}: #{detail}" > - error = Puppet::Error.new(msg) > - error.set_backtrace(detail.backtrace) > - raise error > - end > - > def stale? > @watched_files.values.detect { |file| file.changed? } > end > diff --git a/spec/integration/parser/compiler_spec.rb > b/spec/integration/parser/compiler_spec.rb > index 9158ad1..67e8e5e 100755 > --- a/spec/integration/parser/compiler_spec.rb > +++ b/spec/integration/parser/compiler_spec.rb > @@ -69,4 +69,15 @@ describe Puppet::Parser::Compiler do > notify_resource[:require].title.should == "Experiment::Baz" > end > end > + > + it "should recompute the version after input files are re-parsed" do > + Puppet[:code] = 'class foo { }' > + Time.stubs(:now).returns(1) > + node = Puppet::Node.new('mynode') > + Puppet::Parser::Compiler.compile(node).version.should == 1 > + Time.stubs(:now).returns(2) > + Puppet::Parser::Compiler.compile(node).version.should == 1 # no change > because files didn't change > + > > Puppet::Resource::TypeCollection.any_instance.stubs(:stale?).returns(true).then.returns(false) > # pretend change > + Puppet::Parser::Compiler.compile(node).version.should == 2 > + end > end > diff --git a/spec/unit/application/apply_spec.rb > b/spec/unit/application/apply_spec.rb > index 0b00d1a..a79ddba 100755 > --- a/spec/unit/application/apply_spec.rb > +++ b/spec/unit/application/apply_spec.rb > @@ -142,17 +142,16 @@ describe Puppet::Application::Apply do > > describe "the parseonly command" do > before :each do > - Puppet.stubs(:[]).with(:environment) > + @environment = Puppet::Node::Environment.new("env") > + Puppet.stubs(:[]).with(:environment).returns(@environment) > Puppet.stubs(:[]).with(:manifest).returns("site.pp") > Puppet.stubs(:err) > @apply.stubs(:exit) > @apply.options.stubs(:[]).with(:code).returns "some code" > - @collection = stub_everything > - Puppet::Resource::TypeCollection.stubs(:new).returns(@collection) > end > > - it "should use a Puppet Resource Type Collection to parse the file" > do > - @collection.expects(:perform_initial_import) > + it "should use the environment to parse the file" do > + @environment.stubs(:perform_initial_import) > @apply.parseonly > end > > @@ -162,7 +161,7 @@ describe Puppet::Application::Apply do > end > > it "should exit with exit code 1 if error" do > - > @collection.stubs(:perform_initial_import).raises(Puppet::ParseError) > + > @environment.stubs(:perform_initial_import).raises(Puppet::ParseError) > @apply.expects(:exit).with(1) > @apply.parseonly > end > diff --git a/spec/unit/application/master_spec.rb > b/spec/unit/application/master_spec.rb > index 0baa822..d230213 100644 > --- a/spec/unit/application/master_spec.rb > +++ b/spec/unit/application/master_spec.rb > @@ -257,16 +257,15 @@ describe Puppet::Application::Master do > > describe "the parseonly command" do > before :each do > - Puppet.stubs(:[]).with(:environment) > + @environment = Puppet::Node::Environment.new("env") > + Puppet.stubs(:[]).with(:environment).returns(@environment) > Puppet.stubs(:[]).with(:manifest).returns("site.pp") > Puppet.stubs(:err) > @master.stubs(:exit) > - @collection = stub_everything > - Puppet::Resource::TypeCollection.stubs(:new).returns(@collection) > end > > it "should use a Puppet Resource Type Collection to parse the file" > do > - @collection.expects(:perform_initial_import) > + @environment.expects(:perform_initial_import) > @master.parseonly > end > > @@ -276,7 +275,7 @@ describe Puppet::Application::Master do > end > > it "should exit with exit code 1 if error" do > - > @collection.stubs(:perform_initial_import).raises(Puppet::ParseError) > + > @environment.stubs(:perform_initial_import).raises(Puppet::ParseError) > @master.expects(:exit).with(1) > @master.parseonly > end > diff --git a/spec/unit/node/environment_spec.rb > b/spec/unit/node/environment_spec.rb > index 6edcce5..e495d30 100755 > --- a/spec/unit/node/environment_spec.rb > +++ b/spec/unit/node/environment_spec.rb > @@ -52,7 +52,7 @@ describe Puppet::Node::Environment do > before do > @env = Puppet::Node::Environment.new("dev") > @collection = Puppet::Resource::TypeCollection.new(@env) > - @collection.stubs(:perform_initial_import) > + @env.stubs(:perform_initial_import) > Thread.current[:known_resource_types] = nil > end > > @@ -66,9 +66,8 @@ describe Puppet::Node::Environment do > end > > it "should perform the initial import when creating a new collection" > do > - @collection.expects(:perform_initial_import) > - Puppet::Resource::TypeCollection.expects(:new).returns @collection > - > + @env = Puppet::Node::Environment.new("dev") > + @env.expects(:perform_initial_import) > @env.known_resource_types > end > > @@ -274,4 +273,56 @@ describe Puppet::Node::Environment do > @helper.environment.name.should == :foo > end > end > + > + describe "when performing initial import" do > + before do > + @parser = stub 'parser', :file= => nil, :string => nil, :parse => > nil > + Puppet::Parser::Parser.stubs(:new).returns @parser > + @env = Puppet::Node::Environment.new("env") > + end > + > + it "should create a new parser instance" do > + Puppet::Parser::Parser.expects(:new).returns @parser > + @env.instance_eval { perform_initial_import } > + end > + > + it "should set the parser's string to the 'code' setting and parse if > code is available" do > + Puppet.settings[:code] = "my code" > + @parser.expects(:string=).with "my code" > + @parser.expects(:parse) > + @env.instance_eval { perform_initial_import } > + end > + > + it "should set the parser's file to the 'manifest' setting and parse > if no code is available and the manifest is available" do > + File.stubs(:expand_path).with("/my/file").returns "/my/file" > + File.expects(:exist?).with("/my/file").returns true > + Puppet.settings[:manifest] = "/my/file" > + @parser.expects(:file=).with "/my/file" > + @parser.expects(:parse) > + @env.instance_eval { perform_initial_import } > + end > + > + it "should not attempt to load a manifest if none is present" do > + File.stubs(:expand_path).with("/my/file").returns "/my/file" > + File.expects(:exist?).with("/my/file").returns false > + Puppet.settings[:manifest] = "/my/file" > + @parser.expects(:file=).never > + @parser.expects(:parse).never > + @env.instance_eval { perform_initial_import } > + end > + > + it "should fail helpfully if there is an error importing" do > + File.stubs(:exist?).returns true > + @parser.expects(:parse).raises ArgumentError > + lambda { @env.instance_eval { perform_initial_import } }.should > raise_error(Puppet::Error) > + end > + > + it "should not do anything if the ignore_import settings is set" do > + Puppet.settings[:ignoreimport] = true > + @parser.expects(:string=).never > + @parser.expects(:file=).never > + @parser.expects(:parse).never > + @env.instance_eval { perform_initial_import } > + end > + end > end > diff --git a/spec/unit/resource/type_collection_spec.rb > b/spec/unit/resource/type_collection_spec.rb > index 4bad291..fc53e35 100644 > --- a/spec/unit/resource/type_collection_spec.rb > +++ b/spec/unit/resource/type_collection_spec.rb > @@ -416,58 +416,6 @@ describe Puppet::Resource::TypeCollection do > end > end > > - describe "when performing initial import" do > - before do > - @parser = stub 'parser', :file= => nil, :string => nil, :parse => > nil > - Puppet::Parser::Parser.stubs(:new).returns @parser > - @code = Puppet::Resource::TypeCollection.new("env") > - end > - > - it "should create a new parser instance" do > - Puppet::Parser::Parser.expects(:new).returns @parser > - @code.perform_initial_import > - end > - > - it "should set the parser's string to the 'code' setting and parse if > code is available" do > - Puppet.settings[:code] = "my code" > - @parser.expects(:string=).with "my code" > - @parser.expects(:parse) > - @code.perform_initial_import > - end > - > - it "should set the parser's file to the 'manifest' setting and parse > if no code is available and the manifest is available" do > - File.stubs(:expand_path).with("/my/file").returns "/my/file" > - File.expects(:exist?).with("/my/file").returns true > - Puppet.settings[:manifest] = "/my/file" > - @parser.expects(:file=).with "/my/file" > - @parser.expects(:parse) > - @code.perform_initial_import > - end > - > - it "should not attempt to load a manifest if none is present" do > - File.stubs(:expand_path).with("/my/file").returns "/my/file" > - File.expects(:exist?).with("/my/file").returns false > - Puppet.settings[:manifest] = "/my/file" > - @parser.expects(:file=).never > - @parser.expects(:parse).never > - @code.perform_initial_import > - end > - > - it "should fail helpfully if there is an error importing" do > - File.stubs(:exist?).returns true > - @parser.expects(:parse).raises ArgumentError > - lambda { @code.perform_initial_import }.should > raise_error(Puppet::Error) > - end > - > - it "should not do anything if the ignore_import settings is set" do > - Puppet.settings[:ignoreimport] = true > - @parser.expects(:string=).never > - @parser.expects(:file=).never > - @parser.expects(:parse).never > - @code.perform_initial_import > - end > - end > - > describe "when determining the configuration version" do > before do > @code = Puppet::Resource::TypeCollection.new("env") > -- > 1.7.2 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
