Moving the node tests to rspec, and cleaning up the spec of the node, especially WRT the environment.

2 files changed, 108 insertions, 74 deletions

diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb
index 3bbbe5979..e23472ebf 100644
--- a/lib/puppet/node.rb
+++ b/lib/puppet/node.rb
@@ -1,7 +1,24 @@
 # A simplistic class for managing the node information itself.
 class Puppet::Node
-    attr_accessor :name, :classes, :parameters, :environment, :source, :ipaddress, :names
+    attr_accessor :name, :classes, :parameters, :source, :ipaddress, :names
     attr_reader :time
+    attr_writer :environment
+
+    # Do not return environments tha are empty string, and use
+    # explicitly set environments, then facts, then a central env
+    # value.
+    def environment
+        unless @environment and @environment != ""
+            if env = parameters["environment"] and env != ""
+                @environment = env
+            elsif env = Puppet[:environment] and env != ""
+                @environment = env
+            else
+                @environment = nil
+            end
+        end
+        @environment
+    end
 
     def initialize(name, options = {})
         @name = name
@@ -21,11 +38,7 @@ class Puppet::Node
 
         @parameters = options[:parameters] || {}
 
-        unless @environment = options[:environment] 
-            if env = Puppet[:environment] and env != ""
-                @environment = env
-            end
-        end
+        @environment = options[:environment] 
 
         @time = Time.now
     end
diff --git a/spec/unit/other/node.rb b/spec/unit/other/node.rb
index b3f12d11d..a29fb23cf 100755
--- a/spec/unit/other/node.rb
+++ b/spec/unit/other/node.rb
@@ -1,75 +1,96 @@
 #!/usr/bin/env ruby
 
-$:.unshift("../lib").unshift("../../lib") if __FILE__ =~ /\.rb$/
-
-require 'mocha'
-require 'puppettest'
-require 'puppet/node'
-
-class TestNode < Test::Unit::TestCase
-    include PuppetTest
-    Node = Puppet::Node
-
-    # Make sure we get all the defaults correctly.
-    def test_initialize
-        node = nil
-        assert_nothing_raised("could not create a node without classes or parameters") do
-            node = Node.new("testing")
-        end
-        assert_equal("testing", node.name, "Did not set name correctly")
-        assert_equal({}, node.parameters, "Node parameters did not default correctly")
-        assert_equal([], node.classes, "Node classes did not default correctly")
-        assert_instance_of(Time, node.time, "Did not set the creation time")
-
-        # Now test it with values for both
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+describe Puppet::Node, " when initializing" do
+    before do
+        @node = Puppet::Node.new("testnode")
+    end
+
+    it "should set the node name" do
+        @node.name.should == "testnode"
+    end
+
+    it "should default to an empty parameter hash" do
+        @node.parameters.should == {}
+    end
+
+    it "should default to an empty class array" do
+        @node.classes.should == []
+    end
+
+    it "should note its creation time" do
+        @node.time.should be_instance_of(Time)
+    end
+
+    it "should accept parameters passed in during initialization" do
         params = {"a" => "b"}
+        @node = Puppet::Node.new("testing", :parameters => params)
+        @node.parameters.should == params
+    end
+
+    it "should accept classes passed in during initialization" do
         classes = %w{one two}
-        assert_nothing_raised("could not create a node with classes and parameters") do
-            node = Node.new("testing", :parameters => params, :classes => classes)
-        end
-        assert_equal("testing", node.name, "Did not set name correctly")
-        assert_equal(params, node.parameters, "Node parameters did not get set correctly")
-        assert_equal(classes, node.classes, "Node classes did not get set correctly")
-
-        # And make sure a single class gets turned into an array
-        assert_nothing_raised("could not create a node with a class as a string") do
-            node = Node.new("testing", :classes => "test")
-        end
-        assert_equal(%w{test}, node.classes, "A node class string was not converted to an array")
-
-        # Make sure we get environments
-        assert_nothing_raised("could not create a node with an environment") do
-            node = Node.new("testing", :environment => "test")
-        end
-        assert_equal("test", node.environment, "Environment was not set")
-
-        # Now make sure we get the default env
-        Puppet[:environment] = "prod"
-        assert_nothing_raised("could not create a node with no environment") do
-            node = Node.new("testing")
-        end
-        assert_equal("prod", node.environment, "Did not get default environment")
-
-        # But that it stays nil if there's no default env set
-        Puppet[:environment] = ""
-        assert_nothing_raised("could not create a node with no environment and no default env") do
-            node = Node.new("testing")
-        end
-        assert_nil(node.environment, "Got a default env when none was set")
-
-    end
-
-    # Verify that the node source wins over facter.
-    def test_fact_merge
-        node = Node.new("yay", :parameters => {"a" => "one", "b" => "two"})
-
-        assert_nothing_raised("Could not merge parameters") do
-            node.fact_merge("b" => "three", "c" => "yay")
-        end
-        params = node.parameters
-        assert_equal("one", params["a"], "Lost nodesource parameters in parameter merge")
-        assert_equal("two", params["b"], "Overrode nodesource parameters in parameter merge")
-        assert_equal("yay", params["c"], "Did not get facts in parameter merge")
+        @node = Puppet::Node.new("testing", :classes => classes)
+        @node.classes.should == classes
+    end
+
+    it "should always return classes as an array" do
+        @node = Puppet::Node.new("testing", :classes => "myclass")
+        @node.classes.should == ["myclass"]
+    end
+
+    it "should accept the environment during initialization" do
+        @node = Puppet::Node.new("testing", :environment => "myenv")
+        @node.environment.should == "myenv"
+    end
+end
+
+describe Puppet::Node, " when returning the environment" do
+    before do
+        @node = Puppet::Node.new("testnode")
+    end
+
+    it "should return the 'environment' fact if present and there is no explicit environment" do
+        @node.parameters = {"environment" => "myenv"}
+        @node.environment.should == "myenv"
+    end
+
+    it "should return the central environment if there is no environment fact nor explicit environment" do
+        Puppet.config.expects(:[]).with(:environment).returns(:centralenv)
+        @node.environment.should == :centralenv
+    end
+
+    it "should not use an explicit environment that is an empty string" do
+        @node.environment == ""
+        @node.environment.should be_nil
+    end
+
+    it "should not use an environment fact that is an empty string" do
+        @node.parameters = {"environment" => ""}
+        @node.environment.should be_nil
+    end
+
+    it "should not use an explicit environment that is an empty string" do
+        Puppet.config.expects(:[]).with(:environment).returns(nil)
+        @node.environment.should be_nil
     end
 end
 
+describe Puppet::Node, " when merging facts" do
+    before do
+        @node = Puppet::Node.new("testnode")
+    end
+
+    it "should prefer parameters already set on the node over facts from the node" do
+        @node.parameters = {"one" => "a"}
+        @node.fact_merge("one" => "c")
+        @node.parameters["one"].should == "a"
+    end
+
+    it "should add passed parameters to the parameter list" do
+        @node.parameters = {"one" => "a"}
+        @node.fact_merge("two" => "b")
+        @node.parameters["two"].should == "b"
+    end
+end