From 7e5ca648112a2703ba827f96f36fe2a5f7d1c751 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 7 Jul 2011 00:03:51 -0700 Subject: Making Fact json handling more resilient We were failing if any values were nil, and values were often nil, at least in testing. We now only include non-nil values, and we handle nil values just fine. Signed-off-by: Luke Kanies Reviewed-by: Nick Lewis --- spec/unit/node/facts_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'spec/unit/node') diff --git a/spec/unit/node/facts_spec.rb b/spec/unit/node/facts_spec.rb index efaa76e12..6d2b0a7ac 100755 --- a/spec/unit/node/facts_spec.rb +++ b/spec/unit/node/facts_spec.rb @@ -128,6 +128,20 @@ describe Puppet::Node::Facts, "when indirecting" do result['timestamp'].should == facts.timestamp.to_s result['expiration'].should == facts.expiration.to_s end + + it "should not include nil values" do + facts = Puppet::Node::Facts.new("foo", {'a' => 1, 'b' => 2, 'c' => 3}) + pson = PSON.parse(facts.to_pson) + pson.should_not be_include("expiration") + end + + it "should be able to handle nil values" do + pson = %Q({"name": "foo", "values": {"a": "1", "b": "2", "c": "3"}}) + format = Puppet::Network::FormatHandler.format('pson') + facts = format.intern(Puppet::Node::Facts,pson) + facts.name.should == 'foo' + facts.expiration.should be_nil + end end end end -- cgit From 462a95e3d077b1915a919399b846068816c84583 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Mon, 18 Jul 2011 23:05:35 -0700 Subject: Fix tests with "relative" paths on Windows Absolute paths on Unix, e.g. /foo/bar, are not absolute on Windows, which breaks many test cases. This commit adds a method to PuppetSpec::Files.make_absolute that makes the path absolute in test cases. On Unix (Puppet.features.posix?) it is a no-op. On Windows, (Puppet.features.microsoft_windows?) the drive from the current working directory is prepended. Reviewed-by: Jacob Helwig --- spec/unit/node/environment_spec.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'spec/unit/node') diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index d1badfa3a..144e82e0c 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -144,13 +144,18 @@ describe Puppet::Node::Environment do end describe "when validating modulepath or manifestdir directories" do + before :each do + @path_one = make_absolute('/one') + @path_two = make_absolute('/two') + end + it "should not return non-directories" do env = Puppet::Node::Environment.new("testing") - FileTest.expects(:directory?).with("/one").returns true - FileTest.expects(:directory?).with("/two").returns false + FileTest.expects(:directory?).with(@path_one).returns true + FileTest.expects(:directory?).with(@path_two).returns false - env.validate_dirs(%w{/one /two}).should == %w{/one} + env.validate_dirs([@path_one, @path_two]).should == [@path_one] end it "should use the current working directory to fully-qualify unqualified paths" do @@ -158,7 +163,7 @@ describe Puppet::Node::Environment do env = Puppet::Node::Environment.new("testing") two = File.join(Dir.getwd, "two") - env.validate_dirs(%w{/one two}).should == ["/one", two] + env.validate_dirs([@path_one, 'two']).should == [@path_one, two] end end -- cgit From d198fedf65e472b384666fc9ae3bef487852068a Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 21 Jul 2011 11:53:06 -0700 Subject: Rework Puppet::Util::Cacher to only expire using TTLs We have removed every usage of cached_attr in which the attribute needs to be manually expired. Thus, the only meaningful behavior provided by Puppet::Util::Cacher is expiration based on TTLs. This commit reworks the cacher to only support that behavior. Rather than accepting an options hash, of which :ttl is the only available option, cached_attr now requires a second argument, which is the TTL. TTLs are now used to compute expirations, which are stored and used for expiring values. Previously, we stored a timestamp and used it and the TTL to determine whether the attribute was expired. This had the potentially undesirable side effect that the lifetime of a cached attribute could be extended after its insertion by modifying the TTL setting for the cache. Now, the lifetime of an attribute is determined when it is set, and is thereafter immutable, aside from deliberately re-setting the expiration for that particular attribute. Reviewed-By: Jacob Helwig --- spec/unit/node/environment_spec.rb | 34 ++++++++-------------------------- spec/unit/node/facts_spec.rb | 4 ---- 2 files changed, 8 insertions(+), 30 deletions(-) (limited to 'spec/unit/node') diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index 144e82e0c..f3772749a 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -1,6 +1,8 @@ #!/usr/bin/env rspec require 'spec_helper' +require 'tmpdir' + require 'puppet/node/environment' require 'puppet/util/execution' @@ -10,10 +12,6 @@ describe Puppet::Node::Environment do Puppet::Node::Environment.clear end - it "should include the Cacher module" do - Puppet::Node::Environment.ancestors.should be_include(Puppet::Util::Cacher) - end - it "should use the filetimeout for the ttl for the modulepath" do Puppet::Node::Environment.attr_ttl(:modulepath).should == Integer(Puppet[:filetimeout]) end @@ -22,10 +20,6 @@ describe Puppet::Node::Environment do Puppet::Node::Environment.attr_ttl(:modules).should == Integer(Puppet[:filetimeout]) end - it "should use the filetimeout for the ttl for the manifestdir" do - Puppet::Node::Environment.attr_ttl(:manifestdir).should == Integer(Puppet[:filetimeout]) - end - it "should use the default environment if no name is provided while initializing an environment" do Puppet.settings.expects(:value).with(:environment).returns("one") Puppet::Node::Environment.new.name.should == :one @@ -109,27 +103,15 @@ describe Puppet::Node::Environment do end end - [:modulepath, :manifestdir].each do |setting| - it "should validate the #{setting} directories" do - path = %w{/one /two}.join(File::PATH_SEPARATOR) - - env = Puppet::Node::Environment.new("testing") - env.stubs(:[]).with(setting).returns path - - env.expects(:validate_dirs).with(%w{/one /two}) - - env.send(setting) - end + it "should validate the modulepath directories" do + real_file = Dir.mktmpdir + path = %W[/one /two #{real_file}].join(File::PATH_SEPARATOR) - it "should return the validated dirs for #{setting}" do - path = %w{/one /two}.join(File::PATH_SEPARATOR) + Puppet[:modulepath] = path - env = Puppet::Node::Environment.new("testing") - env.stubs(:[]).with(setting).returns path - env.stubs(:validate_dirs).returns %w{/one /two} + env = Puppet::Node::Environment.new("testing") - env.send(setting).should == %w{/one /two} - end + env.modulepath.should == [real_file] end it "should prefix the value of the 'PUPPETLIB' environment variable to the module path if present" do diff --git a/spec/unit/node/facts_spec.rb b/spec/unit/node/facts_spec.rb index 6d2b0a7ac..b3a0f92dd 100755 --- a/spec/unit/node/facts_spec.rb +++ b/spec/unit/node/facts_spec.rb @@ -68,10 +68,6 @@ describe Puppet::Node::Facts, "when indirecting" do before do @indirection = stub 'indirection', :request => mock('request'), :name => :facts - # We have to clear the cache so that the facts ask for our indirection stub, - # instead of anything that might be cached. - Puppet::Util::Cacher.expire - @facts = Puppet::Node::Facts.new("me", "one" => "two") end -- cgit From 61df3f7c39d74b82e37f48c3519293406036e1e9 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 21 Jul 2011 21:25:21 -0700 Subject: Don't use non-1.8.5-compatible methods 'Object#tap' and 'Dir.mktmpdir' These methods aren't available until Ruby 1.8.6 (Dir.mktmpdir) and Ruby 1.8.7 (Object#tap). Reviewed-By: Jacob Helwig --- spec/unit/node/environment_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/unit/node') diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index f3772749a..eb20aa7ef 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -104,7 +104,7 @@ describe Puppet::Node::Environment do end it "should validate the modulepath directories" do - real_file = Dir.mktmpdir + real_file = tmpdir('moduledir') path = %W[/one /two #{real_file}].join(File::PATH_SEPARATOR) Puppet[:modulepath] = path -- cgit From 9279d0954eb20d75e18a666fd572b5492e157608 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 22 Jul 2011 12:44:15 -0700 Subject: Fix issue with forward and backslashes in Windows paths The environment validates its modulepath and manifestdir settings, but it uses Dir.getwd to convert a relative path into an absolute path. The problem is that on Windows, Dir.getwd returns a path with backslashes. (Interestingly this only happens when puppet is loaded, not in irb for example.) And since we do not yet support backslashes in Windows paths or UNC paths, the directory is not included in the environment. For the time being, I am using File.expand_path to normalize the path. It has the side-effect of converting backslashes to forward slashes. This is sufficient to work around backslashes in Dir.getwd. In the near future, I will be refactoring how paths are split, validated, tested, etc, and I have a REMIND in place to fix the environment. But as a result of this change it exposed a bug in our rdoc parser dealing with the finding the root of a path. The parser assumed that the root was '/', but caused an infinite loop when passed a Windows path. I added a test for this case, which is only run on Windows, because on Unix File.dirname("C:/") == '.'. After all of that, I had to disable one of the rdoc spec tests, because it attempted to reproduce a specific bug, which caused rdoc to try to create a directory of the form: C:/.../files/C:/.... Of course, this fails because ':' is not a valid filename character on Windows. Paired-with: Nick Lewis Reviewed-by: Jacob Helwig --- spec/unit/node/environment_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/unit/node') diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index eb20aa7ef..78d383440 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -144,7 +144,7 @@ describe Puppet::Node::Environment do FileTest.stubs(:directory?).returns true env = Puppet::Node::Environment.new("testing") - two = File.join(Dir.getwd, "two") + two = File.expand_path(File.join(Dir.getwd, "two")) env.validate_dirs([@path_one, 'two']).should == [@path_one, two] end end -- cgit