Merge pull request puppetlabs#497 from daniel-pittman/bug/2.7.x/activ…

…e-record-test-failures-with-3.2.1

Bug/2.7.x/active record test failures with 3.2.1

lib/puppet/rails/inventory_node.rb

@@ -3,19 +3,19 @@
 class Puppet::Rails::InventoryNode < ::ActiveRecord::Base
   has_many :facts, :class_name => "Puppet::Rails::InventoryFact", :foreign_key => :node_id, :dependent => :delete_all
 
-  if Puppet::Util.activerecord_version >= 3.0
-    # Prevents "DEPRECATION WARNING: Base.named_scope has been deprecated, please use Base.scope instead"
-    ActiveRecord::NamedScope::ClassMethods.module_eval { alias :named_scope :scope }
+  if Puppet::Util.activerecord_version < 3.0
+    # For backward compatibility, add the newer name to older implementations.
+    ActiveRecord::NamedScope::ClassMethods.module_eval { alias :scope :named_scope }
   end
 
-  named_scope :has_fact_with_value, lambda { |name,value|
+  scope :has_fact_with_value, lambda { |name,value|
     {
       :conditions => ["inventory_facts.name = ? AND inventory_facts.value = ?", name, value],
       :joins => :facts
     }
   }
 
-  named_scope :has_fact_without_value, lambda { |name,value|
+  scope :has_fact_without_value, lambda { |name,value|
     {
       :conditions => ["inventory_facts.name = ? AND inventory_facts.value != ?", name, value],
       :joins => :facts

spec/lib/puppet_spec/database.rb

@@ -0,0 +1,30 @@
+# This just makes some nice things available at global scope, and for setup of
+# tests to use a real fake database, rather than a fake stubs-that-don't-work
+# version of the same.  Fun times.
+def sqlite?
+  if $sqlite.nil?
+    begin
+      require 'sqlite3'
+      $sqlite = true
+    rescue LoadError
+      $sqlite = false
+    end
+  end
+  $sqlite
+end
+
+def can_use_scratch_database?
+  sqlite? and Puppet.features.rails?
+end
+
+
+# This is expected to be called in your `before :each` block, and will get you
+# ready to roll with a serious database and all.  Cleanup is handled
+# automatically for you.  Nothing to do there.
+def setup_scratch_database
+  dir = PuppetSpec::Files.tmpdir('puppet-sqlite')
+  Puppet[:dbadapter]    = 'sqlite3'
+  Puppet[:dblocation]   = (dir + 'storeconfigs.sqlite').to_s
+  Puppet[:railslog]     = '/dev/null'
+  Puppet::Rails.init
+end

spec/lib/puppet_spec/files.rb

@@ -35,7 +35,8 @@ def make_absolute(path)
     path
   end
 
-  def tmpfile(name)
+  def tmpfile(name) PuppetSpec::Files.tmpfile(name) end
+  def self.tmpfile(name)
     # Generate a temporary file, just for the name...
     source = Tempfile.new(name)
     path = source.path
@@ -49,7 +50,8 @@ def tmpfile(name)
     path
   end
 
-  def tmpdir(name)
+  def tmpdir(name) PuppetSpec::Files.tmpdir(name) end
+  def self.tmpdir(name)
     path = tmpfile(name)
     FileUtils.mkdir_p(path)
     path

spec/spec_helper.rb

@@ -21,6 +21,7 @@ module PuppetSpec
 require 'puppet_spec/files'
 require 'puppet_spec/fixtures'
 require 'puppet_spec/matchers'
+require 'puppet_spec/database'
 require 'monkey_patches/alias_should_to_must'
 require 'monkey_patches/publicize_methods'
 

spec/unit/indirector/catalog/active_record_spec.rb

@@ -1,35 +1,21 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
 
+describe "Puppet::Resource::Catalog::ActiveRecord", :if => can_use_scratch_database? do
+  include PuppetSpec::Files
 
-describe "Puppet::Resource::Catalog::ActiveRecord", :if => Puppet.features.rails? do
   require 'puppet/rails'
 
-  before :all do
-    class Tableless < ActiveRecord::Base
-      def self.columns
-        @columns ||= []
-      end
-      def self.column(name, sql_type=nil, default=nil, null=true)
-        columns << ActiveRecord::ConnectionAdapters::Column.new(name.to_s, default, sql_type.to_s, null)
-      end
-    end
-
-    class Host < Tableless
-      column :name, :string, :null => false
-      column :ip, :string
-      column :environment, :string
-      column :last_compile, :datetime
-    end
+  before :each do
+    require 'puppet/indirector/catalog/active_record'
+    setup_scratch_database
   end
 
-  before do
-    require 'puppet/indirector/catalog/active_record'
-    Puppet.features.stubs(:rails?).returns true
-    Puppet::Rails.stubs(:init)
-    @terminus = Puppet::Resource::Catalog::ActiveRecord.new
+  let :terminus do
+    Puppet::Resource::Catalog::ActiveRecord.new
   end
 
+
   it "should be a subclass of the ActiveRecord terminus class" do
     Puppet::Resource::Catalog::ActiveRecord.ancestors.should be_include(Puppet::Indirector::ActiveRecord)
   end
@@ -39,35 +25,35 @@ class Host < Tableless
   end
 
   describe "when finding an instance" do
-    before do
-      @request = stub 'request', :key => "foo", :options => {:cache_integration_hack => true}
+    let :request do
+      stub 'request', :key => "foo", :options => {:cache_integration_hack => true}
     end
 
     # This hack is here because we don't want to look in the db unless we actually want
     # to look in the db, but our indirection architecture in 0.24.x isn't flexible
     # enough to tune that via configuration.
     it "should return nil unless ':cache_integration_hack' is set to true" do
-      @request.options[:cache_integration_hack] = false
+      request.options[:cache_integration_hack] = false
       Puppet::Rails::Host.expects(:find_by_name).never
-      @terminus.find(@request).should be_nil
+      terminus.find(request).should be_nil
     end
 
     it "should use the Hosts ActiveRecord class to find the host" do
       Puppet::Rails::Host.expects(:find_by_name).with { |key, args| key == "foo" }
-      @terminus.find(@request)
+      terminus.find(request)
     end
 
     it "should return nil if no host instance can be found" do
       Puppet::Rails::Host.expects(:find_by_name).returns nil
 
-      @terminus.find(@request).should be_nil
+      terminus.find(request).should be_nil
     end
 
     it "should return a catalog with the same name as the host if the host can be found" do
       host = stub 'host', :name => "foo", :resources => []
       Puppet::Rails::Host.expects(:find_by_name).returns host
 
-      result = @terminus.find(@request)
+      result = terminus.find(request)
       result.should be_instance_of(Puppet::Resource::Catalog)
       result.name.should == "foo"
     end
@@ -87,70 +73,66 @@ class Host < Tableless
       catalog.expects(:add_resource).with "trans_res1"
       catalog.expects(:add_resource).with "trans_res2"
 
-      @terminus.find(@request)
+      terminus.find(request)
     end
   end
 
   describe "when saving an instance" do
-    before do
-      @host = Host.new(:name => "foo")
-      @host.stubs(:merge_resources)
-      @host.stubs(:save)
-      @host.stubs(:railsmark).yields
-
-      @node = Puppet::Node.new("foo", :environment => "environment")
-      Puppet::Node.indirection.stubs(:find).with("foo").returns(@node)
-
-      Puppet::Rails::Host.stubs(:find_by_name).returns @host
-      @catalog = Puppet::Resource::Catalog.new("foo")
-      @request = Puppet::Indirector::Request.new(:active_record, :save, @catalog)
+    let :catalog do Puppet::Resource::Catalog.new("foo") end
+    let :request do Puppet::Indirector::Request.new(:active_record, :save, catalog) end
+    let :node do Puppet::Node.new("foo", :environment => "environment") end
+
+    before :each do
+      Puppet::Node.indirection.stubs(:find).with("foo").returns(node)
     end
 
     it "should find the Rails host with the same name" do
-      Puppet::Rails::Host.expects(:find_by_name).with("foo").returns @host
-
-      @terminus.save(@request)
+      Puppet::Rails::Host.expects(:find_by_name).with("foo")
+      terminus.save(request)
     end
 
     it "should create a new Rails host if none can be found" do
-      Puppet::Rails::Host.expects(:find_by_name).with("foo").returns nil
-
-      Puppet::Rails::Host.expects(:create).with(:name => "foo").returns @host
-
-      @terminus.save(@request)
+      Puppet::Rails::Host.find_by_name('foo').should be_nil
+      terminus.save(request)
+      Puppet::Rails::Host.find_by_name('foo').should be_valid
     end
 
     it "should set the catalog vertices as resources on the Rails host instance" do
-      @catalog.expects(:vertices).returns "foo"
-      @host.expects(:merge_resources).with("foo")
+      # We need to stub this so we get the same object, not just the same
+      # content, otherwise the expect can't fire. :(
+      host = Puppet::Rails::Host.create!(:name => "foo")
+      Puppet::Rails::Host.expects(:find_by_name).with("foo").returns(host)
+      catalog.expects(:vertices).returns("foo")
+      host.expects(:merge_resources).with("foo")
 
-      @terminus.save(@request)
+      terminus.save(request)
     end
 
     it "should set host ip if we could find a matching node" do
-      @node.stubs(:parameters).returns({"ipaddress" => "192.168.0.1"})
-
-      @terminus.save(@request)
-      @host.ip.should == '192.168.0.1'
+      node.merge("ipaddress" => "192.168.0.1")
+      terminus.save(request)
+      Puppet::Rails::Host.find_by_name("foo").ip.should == '192.168.0.1'
     end
 
     it "should set host environment if we could find a matching node" do
-      @terminus.save(@request)
-      @host.environment.should == "environment"
+      terminus.save(request)
+      Puppet::Rails::Host.find_by_name("foo").environment.should == "environment"
     end
 
     it "should set the last compile time on the host" do
       now = Time.now
-      Time.expects(:now).returns now
+      Time.stubs(:now).returns now
 
-      @terminus.save(@request)
-      @host.last_compile.should == now
+      terminus.save(request)
+      Puppet::Rails::Host.find_by_name("foo").last_compile.should == now
     end
 
     it "should save the Rails host instance" do
-      @host.expects(:save)
+      host = Puppet::Rails::Host.create!(:name => "foo")
+      Puppet::Rails::Host.expects(:find_by_name).with("foo").returns(host)
+      host.expects(:save)
 
-      @terminus.save(@request)
+      terminus.save(request)
     end
   end
 end

spec/unit/indirector/facts/inventory_active_record_spec.rb

@@ -1,39 +1,26 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
-begin
-  require 'sqlite3'
-rescue LoadError
-end
-require 'tempfile'
 require 'puppet/rails'
 
-describe "Puppet::Node::Facts::InventoryActiveRecord", :if => (Puppet.features.rails? and defined? SQLite3) do
+describe "Puppet::Node::Facts::InventoryActiveRecord", :if => can_use_scratch_database? do
+  include PuppetSpec::Files
+
   let(:terminus) { Puppet::Node::Facts::InventoryActiveRecord.new }
 
   before :all do
     require 'puppet/indirector/facts/inventory_active_record'
-    @dbfile = Tempfile.new("testdb")
-    @dbfile.close
   end
 
-  after :all do
+  after :each do
     Puppet::Node::Facts.indirection.reset_terminus_class
-    @dbfile.unlink
   end
 
   before :each do
     Puppet::Node.indirection.reset_terminus_class
     Puppet::Node.indirection.cache_class = nil
 
     Puppet::Node::Facts.indirection.terminus_class = :inventory_active_record
-    Puppet[:dbadapter]  = 'sqlite3'
-    Puppet[:dblocation] = @dbfile.path
-    Puppet[:railslog] = "/dev/null"
-    Puppet::Rails.init
-  end
-
-  after :each do
-    Puppet::Rails.teardown
+    setup_scratch_database
   end
 
   describe "#save" do

spec/unit/indirector/resource/active_record_spec.rb

@@ -1,22 +1,13 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
-
-begin
-  require 'sqlite3'
-rescue LoadError
-end
-
 require 'puppet/rails'
 require 'puppet/node/facts'
 
-describe "Puppet::Resource::ActiveRecord", :if => (Puppet.features.rails? and defined? SQLite3) do
+describe "Puppet::Resource::ActiveRecord", :if => can_use_scratch_database? do
   include PuppetSpec::Files
 
   before :each do
-    dir = Pathname(tmpdir('puppet-var'))
-    Puppet[:vardir]       = dir.to_s
-    Puppet[:dbadapter]    = 'sqlite3'
-    Puppet[:dblocation]   = (dir + 'storeconfigs.sqlite').to_s
+    setup_scratch_database
     Puppet[:storeconfigs] = true
   end
 

spec/unit/parser/collector_spec.rb

@@ -1,11 +1,5 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
-
-begin
-  require 'sqlite3'
-rescue LoadError
-end
-
 require 'puppet/rails'
 require 'puppet/parser/collector'
 
@@ -267,7 +261,7 @@
   end
 end
 
-describe Puppet::Parser::Collector, "when collecting exported resources", :if => (Puppet.features.rails? and defined? SQLite3) do
+describe Puppet::Parser::Collector, "when collecting exported resources", :if => can_use_scratch_database? do
   include PuppetSpec::Files
 
   before do
@@ -287,14 +281,10 @@
 
   context "with storeconfigs enabled" do
     before :each do
-      dir = Pathname(tmpdir('puppet-var'))
-      Puppet[:vardir]       = dir.to_s
-      Puppet[:dbadapter]    = 'sqlite3'
-      Puppet[:dblocation]   = (dir + 'storeconfigs.sqlite').to_s
+      setup_scratch_database
       Puppet[:storeconfigs] = true
       Puppet[:environment]  = "production"
       Puppet[:storeconfigs_backend] = "active_record"
-      Puppet::Rails.init
     end
 
     it "should return all matching resources from the current compile and mark them non-virtual and non-exported" do