From 9e11fdb1b06649b8dd500fb229d62cb96430aa3a Mon Sep 17 00:00:00 2001 From: Mark Mzyk Date: Mon, 30 Jan 2012 16:43:01 -0500 Subject: [PATCH] CHEF-2899: Ensure proper roles/node json when activesupport is present Added activesupport to its own group in bundler so it can be loaded on machines for chef compatibility testing --- chef/Gemfile | 1 + chef/lib/chef/node.rb | 3 ++- chef/lib/chef/role.rb | 10 ++++++++-- chef/spec/unit/node_spec.rb | 14 ++++++++++++-- chef/spec/unit/role_spec.rb | 10 +++++++--- chef/tasks/rspec.rb | 14 ++++++++++++++ 6 files changed, 44 insertions(+), 8 deletions(-) diff --git a/chef/Gemfile b/chef/Gemfile index 4e03f813049..c3aa8a95f06 100644 --- a/chef/Gemfile +++ b/chef/Gemfile @@ -3,6 +3,7 @@ source :rubygems gemspec gem "dep_selector", :group => :development, :platform => "ruby" +gem "activesupport", :group => :compat_testing, :platform => "ruby" platforms :mswin, :mingw do gem "ffi", "1.0.9" diff --git a/chef/lib/chef/node.rb b/chef/lib/chef/node.rb index 68cf553f6bb..9486f67142e 100644 --- a/chef/lib/chef/node.rb +++ b/chef/lib/chef/node.rb @@ -495,7 +495,8 @@ def to_json(*a) "chef_type" => "node", "default" => default_attrs, "override" => override_attrs, - "run_list" => run_list.run_list + #Render correctly for run_list items so malformed json does not result + "run_list" => run_list.run_list.map { |item| item.to_s } } result["_rev"] = couchdb_rev if couchdb_rev result.to_json(*a) diff --git a/chef/lib/chef/role.rb b/chef/lib/chef/role.rb index 2575ba68316..c428472f1f6 100644 --- a/chef/lib/chef/role.rb +++ b/chef/lib/chef/role.rb @@ -168,8 +168,14 @@ def to_hash "default_attributes" => @default_attributes, "override_attributes" => @override_attributes, "chef_type" => "role", - "run_list" => run_list, - "env_run_lists" => env_run_lists_without_default + + #Render to_json correctly for run_list items (both run_list and evn_run_lists) + #so malformed json does not result + "run_list" => run_list.run_list.map { |item| item.to_s }, + "env_run_lists" => env_run_lists_without_default.inject({}) do |accumulator, (k, v)| + accumulator[k] = v.map { |x| x.to_s } + accumulator + end } result["_rev"] = couchdb_rev if couchdb_rev result diff --git a/chef/spec/unit/node_spec.rb b/chef/spec/unit/node_spec.rb index 4aca857b9c2..fbe1a4b5253 100644 --- a/chef/spec/unit/node_spec.rb +++ b/chef/spec/unit/node_spec.rb @@ -17,6 +17,7 @@ # require File.expand_path(File.join(File.dirname(__FILE__), "..", "spec_helper")) +require 'ostruct' describe Chef::Node do before(:each) do @@ -543,7 +544,7 @@ end describe "json" do - it "should serialize itself as json" do + it "should serialize itself as json", :json => true do @node.find_file("test.example.com") json = Chef::JSONCompat.to_json(@node) json.should =~ /json_class/ @@ -555,7 +556,16 @@ json.should =~ /run_list/ end - it "should deserialize itself from json" do + it 'should serialze valid json with a run list', :json => true do + #This test came about because activesupport mucks with Chef json serialization + #Test should pass with and without Activesupport + @node.run_list << {"type" => "role", "name" => 'Cthulu'} + @node.run_list << {"type" => "role", "name" => 'Hastur'} + json = Chef::JSONCompat.to_json(@node) + json.should =~ /\"run_list\":\[\"role\[Cthulu\]\",\"role\[Hastur\]\"\]/ + end + + it "should deserialize itself from json", :json => true do @node.find_file("test.example.com") json = Chef::JSONCompat.to_json(@node) serialized_node = Chef::JSONCompat.from_json(json) diff --git a/chef/spec/unit/role_spec.rb b/chef/spec/unit/role_spec.rb index f8c5dd158b7..6615ce6b8da 100644 --- a/chef/spec/unit/role_spec.rb +++ b/chef/spec/unit/role_spec.rb @@ -161,7 +161,7 @@ end end - describe "when serialized as JSON" do + describe "when serialized as JSON", :json => true do before(:each) do @role.name('mars_volta') @role.description('Great band!') @@ -192,6 +192,8 @@ end it "should include 'run_list'" do + #Activesupport messes with Chef json formatting + #This test should pass with and without activesupport @serialized_role.should =~ /"run_list":\["recipe\[one\]","recipe\[two\]","role\[a\]"\]/ end @@ -201,7 +203,9 @@ @serialized_role = Chef::JSONCompat.from_json(Chef::JSONCompat.to_json(@role), :create_additions => false) end - it "includes the per-environment run lists in the " do + it "includes the per-environment run lists" do + #Activesupport messes with Chef json formatting + #This test should pass with and without activesupport @serialized_role["env_run_lists"]["production"].should == ['role[monitoring]', 'role[auditing]', 'role[apache]'] @serialized_role["env_run_lists"]["dev"].should == ["role[nginx]"] end @@ -213,7 +217,7 @@ end end - describe "when created from JSON" do + describe "when created from JSON", :json => true do before(:each) do @role.name('mars_volta') @role.description('Great band!') diff --git a/chef/tasks/rspec.rb b/chef/tasks/rspec.rb index e2ea2f47cbc..40cf8f1f249 100644 --- a/chef/tasks/rspec.rb +++ b/chef/tasks/rspec.rb @@ -39,6 +39,20 @@ t.pattern = FileList['spec/functional/**/*_spec.rb'] end + desc "Run the rspec tests with activesupport loaded" + RSpec::Core::RakeTask.new(:spec_activesupport) do |t| + #require activesupport to make sure it is on the system and fail early if not found + #this has no affect on rspec, which still has to load it itself + begin + require 'active_support/core_ext' + rescue LoadError + raise "ActiveSupport not found on system, it is needed to run these tests" + end + + t.rspec_opts = ['--options', "\"#{CHEF_ROOT}/.rspec\"", "--require active_support/core_ext"] + t.pattern = FileList['spec/unit/**/*_spec.rb'] + end + namespace :spec do desc "Run all specs in spec directory with RCov" RSpec::Core::RakeTask.new(:rcov) do |t|