Fixed client spec tests for changes to client run (only one save, call to environments endpoint for run_list expansion)

 * also refactored main "run" spec

Fixed run_list spec tests for changes to exceptions thrown from CookbookVersionSelector

Fixed typo in rethrowing DepSelector::Exceptions::NoSolutionExists -> Chef::Exceptions::CookbookVersionSelection::UnsatisfiableRunListItem

Removed Node#reset_defaults_and_overrides, as it was a leaky API only used
in combination with Node#expand!. Its functionality was merged into Node#expand!

Added emacs backup files, *~, to .gitignore

11 files changed, 89 insertions, 74 deletions

diff --git a/.gitignore b/.gitignore
index 372ade651f..ee6370db10 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,3 +35,4 @@ chef-expander/spec/fixtures/expander.log
 features/data/cookbooks/transfer_remote_files/metadata.json
 features/data/cookbooks/synchronize_deps/metadata.json
 */tags
+*~
diff --git a/chef/lib/chef/client.rb b/chef/lib/chef/client.rb
index 2d448f5be7..811fd125af 100644
--- a/chef/lib/chef/client.rb
+++ b/chef/lib/chef/client.rb
@@ -3,7 +3,7 @@
 # Author:: Christopher Walters (<cw@opscode.com>)
 # Author:: Christopher Brown (<cb@opscode.com>)
 # Author:: Tim Hinderliter (<tim@opscode.com>)
-# Copyright:: Copyright (c) 2008-2010 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2011 Opscode, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -240,8 +240,7 @@ class Chef
 
       Chef::Log.info("Run List is [#{@node.run_list}]")
       Chef::Log.info("Run List expands to [#{@expanded_run_list.join(', ')}]")
-      @node.reset_defaults_and_overrides
-
+ 
       @run_status = Chef::RunStatus.new(@node)
 
       @node
diff --git a/chef/lib/chef/cookbook_version_selector.rb b/chef/lib/chef/cookbook_version_selector.rb
index 45134e101a..1fd3e65815 100644
--- a/chef/lib/chef/cookbook_version_selector.rb
+++ b/chef/lib/chef/cookbook_version_selector.rb
@@ -128,9 +128,7 @@ class Chef
 
           raise Chef::Exceptions::CookbookVersionSelection::InvalidRunListItems.new(message, non_existent_cookbooks, cookbooks_with_no_matching_versions)
         rescue DepSelector::Exceptions::NoSolutionExists => e
-          #message = "Could not resolve run_list due to constraint: #{e.unsatisfiable_solution_constraint}."
-
-          raise Chef::Exceptions::CookbookVersionSelect::UnsatisfiableRunListItem.new(filter_dep_selector_message(e.message), e.unsatisfiable_solution_constraint, e.disabled_non_existent_packages, e.disabled_most_constrained_packages)
+          raise Chef::Exceptions::CookbookVersionSelection::UnsatisfiableRunListItem.new(filter_dep_selector_message(e.message), e.unsatisfiable_solution_constraint, e.disabled_non_existent_packages, e.disabled_most_constrained_packages)
         end
 
 
diff --git a/chef/lib/chef/node.rb b/chef/lib/chef/node.rb
index a9300c2350..befc1c5075 100644
--- a/chef/lib/chef/node.rb
+++ b/chef/lib/chef/node.rb
@@ -3,7 +3,7 @@
 # Author:: Christopher Brown (<cb@opscode.com>)
 # Author:: Christopher Walters (<cw@opscode.com>)
 # Author:: Tim Hinderliter (<tim@opscode.com>)
-# Copyright:: Copyright (c) 2008-2010 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2011 Opscode, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -406,15 +406,8 @@ class Chef
       attrs
     end
 
-    # Clear defaults and overrides, so that any deleted attributes between runs are
-    # still gone.
-    def reset_defaults_and_overrides
-      @default_attrs = Mash.new
-      @override_attrs = Mash.new
-    end
-
-    # Expands the node's run list and deep merges the default and
-    # override attributes. Also applies stored attributes (from json provided
+    # Expands the node's run list and sets the default and override
+    # attributes. Also applies stored attributes (from json provided
     # on the command line)
     #
     # Returns the fully-expanded list of recipes.
@@ -425,15 +418,17 @@ class Chef
     # on-demand generation of default_attrs and override_attrs,
     # invalidated only when run_list is mutated?
     def expand!(data_source = 'server')
-      # This call should only be called on a chef-client run if you're going to save it later
+      # This call should only be called on a chef-client run
       expansion = run_list.expand(chef_environment, data_source)
       raise Chef::Exceptions::MissingRole if expansion.errors?
 
+      @default_attrs = Mash.new
+      @override_attrs = Mash.new
+
       self[:tags] = Array.new unless attribute?(:tags)
-      @default_attrs = Chef::Mixin::DeepMerge.merge(default_attrs, expansion.default_attrs)
+      @default_attrs = expansion.default_attrs
       environment_attrs = chef_environment == "_default" ? {} : Chef::Environment.load(chef_environment).attributes
-      overrides_before_environment = Chef::Mixin::DeepMerge.merge(override_attrs, expansion.override_attrs)
-      @override_attrs = Chef::Mixin::DeepMerge.merge(overrides_before_environment, environment_attrs)
+      @override_attrs = Chef::Mixin::DeepMerge.merge(expansion.override_attrs, environment_attrs)
       @automatic_attrs[:recipes] = expansion.recipes
       @automatic_attrs[:roles] = expansion.roles
 
diff --git a/chef/lib/chef/run_list.rb b/chef/lib/chef/run_list.rb
index bd0f5fc457..5b06299e01 100644
--- a/chef/lib/chef/run_list.rb
+++ b/chef/lib/chef/run_list.rb
@@ -4,7 +4,7 @@
 # Author:: Tim Hinderliter (<tim@opscode.com>)
 # Author:: Christopher Walters (<cw@opscode.com>)
 # Author:: Seth Falcon (<seth@opscode.com>)
-# Copyright:: Copyright (c) 2008-2010 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2011 Opscode, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -130,6 +130,9 @@ class Chef
     end
     alias :delete :remove
 
+    # Expands this run_list: recursively expand roles into their included
+    # recipes.
+    # Returns a RunListExpansion object.
     def expand(environment, data_source='server', expansion_opts={})
       expansion = expansion_for_data_source(data_source, expansion_opts)
       expansion.expand(environment)
diff --git a/chef/lib/chef/run_list/run_list_expansion.rb b/chef/lib/chef/run_list/run_list_expansion.rb
index fac5fa596f..00733db376 100644
--- a/chef/lib/chef/run_list/run_list_expansion.rb
+++ b/chef/lib/chef/run_list/run_list_expansion.rb
@@ -1,6 +1,7 @@
 #
 # Author:: Daniel DeLeo (<dan@opscode.com>)
-# Copyright:: Copyright (c) 2010 Opscode, Inc.
+# Author:: Tim Hinderliter (<tim@opscode.com>)
+# Copyright:: Copyright (c) 2010, 2011 Opscode, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -31,6 +32,8 @@ class Chef
 
       attr_reader :run_list_items
 
+      # A VersionedRecipeList of recipes. Populated only after #expand
+      # is called.
       attr_reader :recipes
 
       attr_reader :default_attrs
@@ -105,6 +108,10 @@ class Chef
         @applied_roles[role_name] = true
       end
 
+      # Returns an array of role names that were expanded; this
+      # includes any roles that were in the original, pre-expansion
+      # run_list as well as roles processed during
+      # expansion. Populated only after #expand is called.
       def roles
         @applied_roles.keys
       end
diff --git a/chef/lib/chef/shef/shef_session.rb b/chef/lib/chef/shef/shef_session.rb
index 237f483fe5..8b34440ba9 100644
--- a/chef/lib/chef/shef/shef_session.rb
+++ b/chef/lib/chef/shef/shef_session.rb
@@ -135,7 +135,7 @@ module Shef
 
     def rebuild_context
       @run_context = Chef::RunContext.new(@node, {}) # no recipes
-      @run_context.load
+      @run_context.load([]) # no recipe names
     end
 
     private
@@ -231,7 +231,6 @@ module Shef
       ohai_data = @ohai.data.merge(@node.automatic_attrs)
 
       @node.consume_external_attrs(ohai_data,nil)
-      @node.reset_defaults_and_overrides
 
       @node
     end
diff --git a/chef/spec/unit/client_spec.rb b/chef/spec/unit/client_spec.rb
index 09e6207f59..07f6925f57 100644
--- a/chef/spec/unit/client_spec.rb
+++ b/chef/spec/unit/client_spec.rb
@@ -52,23 +52,17 @@ describe Chef::Client do
 
   describe "run" do
     it "should identify the node and run ohai, then register the client" do
-
-      mock_chef_rest_for_node = OpenStruct.new({ })
-      mock_chef_rest_for_client = OpenStruct.new({ })
-      mock_couchdb = OpenStruct.new({ })
-
-      Chef::CouchDB.stub(:new).and_return(mock_couchdb)
+      mock_chef_rest_for_node = mock("Chef::REST (node)")
+      mock_chef_rest_for_client = mock("Chef::REST (client)")
+      mock_chef_rest_for_node_save = mock("Chef::REST (node save)")
+      mock_chef_runner = mock("Chef::Runner")
 
       # --Client.register
-      #   Use a filename we're sure doesn't exist, so that the registration
-      #   code creates a new client.
-      temp_client_key_file = Tempfile.new("chef_client_spec__client_key")
-      temp_client_key_file.close
-      FileUtils.rm(temp_client_key_file.path)
-      Chef::Config[:client_key] = temp_client_key_file.path
+      #   Make sure Client#register thinks the client key doesn't
+      #   exist, so it tries to register and create one.
+      File.should_receive(:exists?).with(Chef::Config[:client_key]).exactly(1).times.and_return(false)
 
       #   Client.register will register with the validation client name.
-      Chef::REST.should_receive(:new).with(Chef::Config[:chef_server_url]).at_least(1).times.and_return(mock_chef_rest_for_node)
       Chef::REST.should_receive(:new).with(Chef::Config[:client_url], Chef::Config[:validation_client_name], Chef::Config[:validation_key]).and_return(mock_chef_rest_for_client)
       mock_chef_rest_for_client.should_receive(:register).with(@fqdn, Chef::Config[:client_key]).and_return(true)
       #   Client.register will then turn around create another
@@ -78,28 +72,29 @@ describe Chef::Client do
 
       # --Client.build_node
       #   looks up the node, which we will return, then later saves it.
-      mock_chef_rest_for_node.should_receive(:get_rest).with("nodes/#{@fqdn}").and_return(@node)
-      mock_chef_rest_for_node.should_receive(:put_rest).with("nodes/#{@fqdn}", @node).exactly(2).times.and_return(@node)
+      Chef::Node.should_receive(:find_or_create).with(@fqdn).and_return(@node)
 
-      ## Node expansion w/ environments
-      environment = mock("default Chef::Environment with empty attributes", :attributes => {})
-      Chef::Environment.stub!(:load).and_return(environment)
+      # --Client.setup_run_context
+      # ---Client.sync_cookbooks -- downloads the list of cookbooks to sync
+      mock_chef_rest_for_node.should_receive(:post_rest).with("environments/_default/cookbook_versions", {:run_list => []}).and_return({})
 
-      # --Client.sync_cookbooks -- downloads the list of cookbooks to sync
-      #
+      # --Client.converge
+      Chef::Runner.should_receive(:new).and_return(mock_chef_runner)
+      mock_chef_runner.should_receive(:converge).and_return(true)
 
-      # after run, check proper mutation of node
-      # e.g., node.automatic_attrs[:platform], node.automatic_attrs[:platform_version]
-      Chef::Config.node_path(File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "nodes")))
-      Chef::Config.cookbook_path(File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "cookbooks")))
+      # --Client.save_updated_node
+      Chef::REST.should_receive(:new).with(Chef::Config[:chef_server_url]).and_return(mock_chef_rest_for_node_save)
+      mock_chef_rest_for_node_save.should_receive(:put_rest).with("nodes/#{@fqdn}", @node).and_return(true)
 
-      @client.stub!(:sync_cookbooks).and_return({})
       @client.should_receive(:run_started)
       @client.should_receive(:run_completed_successfully)
+
+
+      # This is what we're testing.
       @client.run
 
 
-      # check that node has been filled in correctly
+      # Post conditions: check that node has been filled in correctly
       @node.automatic_attrs[:platform].should == "example-platform"
       @node.automatic_attrs[:platform_version].should == "example-platform-1.0"
     end
@@ -149,14 +144,33 @@ describe Chef::Client do
 
   describe "build_node" do
     it "should expand the roles and recipes for the node" do
+      @node.run_list << "role[role_containing_cookbook1]"
+      role_containing_cookbook1 = Chef::Role.new
+      role_containing_cookbook1.name("role_containing_cookbook1")
+      role_containing_cookbook1.run_list << "cookbook1"
+
       Chef::Node.should_receive(:find_or_create).and_return(@node)
-      @node.should_receive(:save).and_return(true)
+      #@node.should_receive(:expand!).with('server').and_return(RunListExpansionFromDisk.new(RunListItem.new("cookbook1")))
+
+      # build_node will call Node#expand! with server, which will
+      # eventually hit the server to expand the included role.
+      mock_chef_rest = mock("Chef::REST")
+      mock_chef_rest.should_receive(:get_rest).with("roles/role_containing_cookbook1").and_return(role_containing_cookbook1)
+      Chef::REST.should_receive(:new).and_return(mock_chef_rest)
 
+      # check pre-conditions.
       @node[:roles].should be_nil
       @node[:recipes].should be_nil
+
       @client.build_node
+
+      # check post-conditions.
       @node[:roles].should_not be_nil
+      @node[:roles].length.should == 1
+      @node[:roles].should include("role_containing_cookbook1")
       @node[:recipes].should_not be_nil
+      @node[:recipes].length.should == 1
+      @node[:recipes].should include("cookbook1")
     end
   end
 end
diff --git a/chef/spec/unit/node_spec.rb b/chef/spec/unit/node_spec.rb
index b375b38147..28d2ed11d3 100644
--- a/chef/spec/unit/node_spec.rb
+++ b/chef/spec/unit/node_spec.rb
@@ -336,15 +336,6 @@ describe Chef::Node do
       @ohai_data = {:platform => 'foobuntu', :platform_version => '23.42'}
     end
 
-    it "clears the default and override attributes" do
-      @node.default_attrs["foo"] = "bar"
-      @node.override_attrs["baz"] = "qux"
-      @node.consume_external_attrs(@ohai_data, {})
-      @node.reset_defaults_and_overrides
-      @node.default_attrs.should be_empty
-      @node.override_attrs.should be_empty
-    end
-
     it "sets its platform according to platform detection" do
       @node.consume_external_attrs(@ohai_data, {})
       @node.automatic_attrs[:platform].should == 'foobuntu'
diff --git a/chef/spec/unit/run_context_spec.rb b/chef/spec/unit/run_context_spec.rb
index 56619bf438..6c68bb8f5f 100644
--- a/chef/spec/unit/run_context_spec.rb
+++ b/chef/spec/unit/run_context_spec.rb
@@ -42,7 +42,7 @@ describe Chef::RunContext do
 
   describe "after loading the cookbooks" do
     before do
-      @run_context.load
+      @run_context.load(@node.run_list.expand('_default').recipes)
     end
 
     it "should load all the definitions in the cookbooks for this node" do
diff --git a/chef/spec/unit/run_list_spec.rb b/chef/spec/unit/run_list_spec.rb
index 334dcf4846..0a2b974a14 100644
--- a/chef/spec/unit/run_list_spec.rb
+++ b/chef/spec/unit/run_list_spec.rb
@@ -45,7 +45,6 @@ describe Chef::RunList do
     it "should accept recipes that are unqualified" do
       @run_list << "needy"
       @run_list.should include('recipe[needy]')
-      #@run_list.include?('recipe[needy]').should == true
       @run_list.recipes.include?('needy').should == true
     end
 
@@ -320,12 +319,21 @@ describe Chef::RunList do
       { :name => cookbook_name, :version_constraint => vc }
     end
 
-    def assert_failure(run_list, all_cookbooks, constraints, expected_message)
+    def assert_failure_unsatisfiable_item(run_list, all_cookbooks, constraints, expected_message)
       begin
-        Chef::CookbookVersionSelector.constrain(all_cookbooks, run_list.recipes.with_version_constraints)
-        fail "Should have raised a Chef::Exceptions::CookbookVersionConflict exception"
-      rescue Chef::Exceptions::CookbookVersionConflict => cvc
-        cvc.message.should include(expected_message)
+        Chef::CookbookVersionSelector.constrain(all_cookbooks, constraints)
+        fail "Should have raised a Chef::Exceptions::CookbookVersionSelection::UnsatisfiableRunListItem exception"
+      rescue Chef::Exceptions::CookbookVersionSelection::UnsatisfiableRunListItem => urli
+        urli.message.should include(expected_message)
+      end
+    end
+
+    def assert_failure_invalid_items(run_list, all_cookbooks, constraints, expected_message)
+      begin
+        Chef::CookbookVersionSelector.constrain(all_cookbooks, constraints)
+        fail "Should have raised a Chef::Exceptions::CookbookVersionSelection::InvalidRunListItems exception"
+      rescue Chef::Exceptions::CookbookVersionSelection::InvalidRunListItems => irli
+        irli.message.should include(expected_message)
       end
     end
 
@@ -379,12 +387,12 @@ describe Chef::RunList do
 
     it "should fail to find a solution when a run list item is constrained to a range that includes no cookbooks" do
       constraints = [vc_maker("d", "> 5.0")]
-      assert_failure(@run_list, @all_cookbooks, constraints, "Run list item (d > 5.0.0) does not match any versions")
+      assert_failure_invalid_items(@run_list, @all_cookbooks, constraints, "Run list contains invalid items: no versions match the constraints on cookbook d.")
     end
 
     it "should fail to find a solution when a run list item's dependency is constrained to a range that includes no cookbooks" do
       constraints = [vc_maker("g", nil)]
-      assert_failure(@run_list, @all_cookbooks, constraints, "Unable to satisfy constraints on cookbook d due to run list item (g >= 0.0.0)")
+      assert_failure_unsatisfiable_item(@run_list, @all_cookbooks, constraints, "Unable to satisfy constraints on cookbook d due to run list item (g >= 0.0.0)")
     end
 
     it "selects 'd 2.1.0' given constraint 'd > 1.2.3'" do
@@ -408,19 +416,19 @@ describe Chef::RunList do
       cookbooks["d"].version.should == "1.1.0"
     end
 
-    it "raises CookbookVersionConflict for an unknown cookbook in the run list" do
+    it "raises InvalidRunListItems for an unknown cookbook in the run list" do
       constraints = [vc_maker("nosuch", "1.0.0")]
-      assert_failure(@run_list, @all_cookbooks, constraints, "Run list item (nosuch = 1.0.0) specifies a cookbook that does not exist in the dependency graph")
+      assert_failure_invalid_items(@run_list, @all_cookbooks, constraints, "Run list contains invalid items: no such cookbook nosuch.")
     end
 
     it "raises CookbookVersionConflict for an unknown cookbook in a cookbook's dependencies" do
       constraints = [vc_maker("depends_on_nosuch", "1.0.0")]
-      assert_failure(@run_list, @all_cookbooks, constraints, "Unable to satisfy constraints on cookbook nosuch, which does not exist, due to run list item (depends_on_nosuch = 1.0.0)")
+      assert_failure_unsatisfiable_item(@run_list, @all_cookbooks, constraints, "Unable to satisfy constraints on cookbook nosuch, which does not exist, due to run list item (depends_on_nosuch = 1.0.0). Run list items that may result in a constraint on nosuch: [(depends_on_nosuch = 1.0.0) -> (nosuch >= 0.0.0)]")
     end
 
-    it "raises CookbookVersionConflict for direct conflict" do
+    it "raises UnsatisfiableRunListItem for direct conflict" do
       constraints = [vc_maker("d", "= 1.1.0"), vc_maker("d", ">= 2.0")]
-      assert_failure(@run_list, @all_cookbooks, constraints, "Unable to satisfy constraints on cookbook d due to run list item (d >= 2.0.0)")
+      assert_failure_unsatisfiable_item(@run_list, @all_cookbooks, constraints, "Unable to satisfy constraints on cookbook d due to run list item (d >= 2.0.0)")
     end
 
     describe "should solve regardless of constraint order" do