Wait, oops. Version is failing a "rake unit" test ... it looks like we were relying on the TypeCollection object getting replaced when the .pp files were altered. This patch prevents the version number from being incremented.
On Thu, Aug 12, 2010 at 3:05 PM, Jesse A Wolfe <[email protected]> wrote: > +1 (as a partial pairing partner) > > > On Thu, Aug 12, 2010 at 2:10 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. >> >> Signed-off-by: Paul Berry <[email protected]> >> --- >> lib/puppet/node/environment.rb | 36 +++++++++++- >> lib/puppet/parser/compiler.rb | 2 +- >> lib/puppet/parser/parser_support.rb | 2 +- >> lib/puppet/resource/type_collection.rb | 32 ---------- >> spec/unit/application/apply_spec.rb | 11 ++-- >> spec/unit/application/master_spec.rb | 9 +-- >> spec/unit/node/environment_spec.rb | 87 >> ++++++++++++++++++++++++++- >> spec/unit/parser/compiler_spec.rb | 4 +- >> spec/unit/parser/parser_spec.rb | 2 +- >> spec/unit/resource/type_collection_spec.rb | 81 >> -------------------------- >> 10 files changed, 132 insertions(+), 134 deletions(-) >> >> diff --git a/lib/puppet/node/environment.rb >> b/lib/puppet/node/environment.rb >> index 3f67474..e43dca7 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,39 @@ class Puppet::Node::Environment >> end >> end >> >> + def version >> + return @version if defined?(@version) >> + >> + if self[:config_version] == "" >> + @version = Time.now.to_i >> + return @version >> + end >> + >> + @version = Puppet::Util.execute([self[:config_version]]).strip >> + >> + rescue Puppet::ExecutionFailure => e >> + raise Puppet::ParseError, "Unable to set config_version: >> #{e.message}" >> + 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/parser/compiler.rb b/lib/puppet/parser/compiler.rb >> index 61bb13c..21b6d84 100644 >> --- a/lib/puppet/parser/compiler.rb >> +++ b/lib/puppet/parser/compiler.rb >> @@ -426,7 +426,7 @@ class Puppet::Parser::Compiler >> >> # For maintaining the relationship between scopes and their resources. >> @catalog = Puppet::Resource::Catalog.new(@node.name) >> - @catalog.version = known_resource_types.version >> + @catalog.version = environment.version >> >> # Create our initial scope and a resource that will evaluate main. >> @topscope = Puppet::Parser::Scope.new(:compiler => self) >> diff --git a/lib/puppet/parser/parser_support.rb >> b/lib/puppet/parser/parser_support.rb >> index 97d985c..f3f98e8 100644 >> --- a/lib/puppet/parser/parser_support.rb >> +++ b/lib/puppet/parser/parser_support.rb >> @@ -223,7 +223,7 @@ class Puppet::Parser::Parser >> end >> >> def version >> - known_resource_types.version >> + environment.version >> end >> >> # Add a new file to be checked when we're checking to see if we should >> be >> diff --git a/lib/puppet/resource/type_collection.rb >> b/lib/puppet/resource/type_collection.rb >> index 3a327e2..1a013bc 100644 >> --- a/lib/puppet/resource/type_collection.rb >> +++ b/lib/puppet/resource/type_collection.rb >> @@ -110,42 +110,10 @@ 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 >> >> - def version >> - return @version if defined?(@version) >> - >> - if environment[:config_version] == "" >> - @version = Time.now.to_i >> - return @version >> - end >> - >> - @version = Puppet::Util.execute([environment[:config_version]]).strip >> - >> - rescue Puppet::ExecutionFailure => e >> - raise Puppet::ParseError, "Unable to set config_version: >> #{e.message}" >> - end >> - >> def watch_file(file) >> @watched_files[file] = Puppet::Util::LoadedFile.new(file) >> 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..1bc0221 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,84 @@ 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 >> + >> + describe "when determining the configuration version" do >> + before do >> + @env = Puppet::Node::Environment.new("env") >> + end >> + >> + it "should default to the current time" do >> + time = Time.now >> + >> + Time.stubs(:now).returns time >> + @env.version.should == time.to_i >> + end >> + >> + it "should use the output of the environment's config_version setting >> if one is provided" do >> + @env.stubs(:[]).with(:config_version).returns("/my/foo") >> + >> + Puppet::Util.expects(:execute).with(["/my/foo"]).returns "output\n" >> + @env.version.should == "output" >> + end >> + >> + it "should raise a puppet parser error if executing config_version >> fails" do >> + @env.stubs(:[]).with(:config_version).returns("test") >> + >> Puppet::Util.expects(:execute).raises(Puppet::ExecutionFailure.new("msg")) >> + >> + lambda { @env.version }.should raise_error(Puppet::ParseError) >> + end >> + >> + end >> end >> diff --git a/spec/unit/parser/compiler_spec.rb >> b/spec/unit/parser/compiler_spec.rb >> index e8c06dd..8befccc 100755 >> --- a/spec/unit/parser/compiler_spec.rb >> +++ b/spec/unit/parser/compiler_spec.rb >> @@ -96,8 +96,8 @@ describe Puppet::Parser::Compiler do >> @compiler.ast_nodes?.should be_true >> end >> >> - it "should copy the known_resource_types version to the catalog" do >> - @compiler.catalog.version.should == @known_resource_types.version >> + it "should copy the environment version to the catalog" do >> + @compiler.catalog.version.should == @compiler.environment.version >> end >> >> it "should copy any node classes into the class list" do >> diff --git a/spec/unit/parser/parser_spec.rb >> b/spec/unit/parser/parser_spec.rb >> index 0657ab3..7d0428f 100755 >> --- a/spec/unit/parser/parser_spec.rb >> +++ b/spec/unit/parser/parser_spec.rb >> @@ -334,7 +334,7 @@ describe Puppet::Parser do >> >> describe "when determining the configuration version" do >> it "should determine it from the resource type collection" do >> - @parser.known_resource_types.expects(:version).returns "foo" >> + @parser.environment.expects(:version).returns "foo" >> @parser.version.should == "foo" >> end >> end >> diff --git a/spec/unit/resource/type_collection_spec.rb >> b/spec/unit/resource/type_collection_spec.rb >> index 4bad291..9115d90 100644 >> --- a/spec/unit/resource/type_collection_spec.rb >> +++ b/spec/unit/resource/type_collection_spec.rb >> @@ -415,85 +415,4 @@ describe Puppet::Resource::TypeCollection do >> @loader.should_not be_stale >> 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") >> - end >> - >> - it "should default to the current time" do >> - time = Time.now >> - >> - Time.stubs(:now).returns time >> - @code.version.should == time.to_i >> - end >> - >> - it "should use the output of the environment's config_version setting >> if one is provided" do >> - >> @code.environment.stubs(:[]).with(:config_version).returns("/my/foo") >> - >> - Puppet::Util.expects(:execute).with(["/my/foo"]).returns "output\n" >> - @code.version.should == "output" >> - end >> - >> - it "should raise a puppet parser error if executing config_version >> fails" do >> - @code.environment.stubs(:[]).with(:config_version).returns("test") >> - >> Puppet::Util.expects(:execute).raises(Puppet::ExecutionFailure.new("msg")) >> - >> - lambda { @code.version }.should raise_error(Puppet::ParseError) >> - end >> - >> - end >> - >> end >> -- >> 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.
