r2736@asus: jeremy | 2005-10-24 17:08:12 -0700

Test for eager associations with limits should not assume that records are ordered by id.
 r2737@asus:  jeremy | 2005-10-24 19:06:09 -0700
 Fail fast if invalid primary key column.
 r2746@asus:  jeremy | 2005-10-25 15:37:28 -0700
 Begin rollback of fixture delete order.  Its solves a problem for 1% of users who already have a workaround while severely slowing down the other 99%.
 r2747@asus:  jeremy | 2005-10-25 16:03:01 -0700
 Rollback the rest.
 r2748@asus:  jeremy | 2005-10-25 16:06:26 -0700
 Re-add fixtures declaration to conditions scoping test
 r2749@asus:  jeremy | 2005-10-25 16:09:03 -0700
 Re-add fixtures declaration to finder test
 r2750@asus:  jeremy | 2005-10-25 16:13:50 -0700
 Don't assume keyboards table is empty


git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2730 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,7 +1,5 @@
 *SVN*
 
-* Keep closer tabs on dirty, loaded, and declared fixtures.  #2404 [ryand-ruby@zenspider.com]
-
 * Map Active Record time to SQL TIME.  #2575, #2576 [Robby Russell <robby@planetargon.com>]
 
 * Clarify semantics of ActiveRecord::Base#respond_to?  #2560 [skaes@web.de]

activerecord/lib/active_record/base.rb

@@ -1113,6 +1113,7 @@ module ActiveRecord #:nodoc:
       def id
         attr_name = self.class.primary_key
         column    = column_for_attribute(attr_name)
+        raise ActiveRecordError, "No such primary key column #{attr_name} for table #{table_name}" if column.nil?
         define_read_method(:id, attr_name, column) if self.class.generate_read_methods
         (value = @attributes[attr_name]) && column.type_cast(value)
       end

activerecord/lib/active_record/fixtures.rb

@@ -1,7 +1,6 @@
 require 'erb'
 require 'yaml'
 require 'csv'
-require 'set'
 
 module YAML #:nodoc:
   class Omap #:nodoc:
@@ -230,54 +229,41 @@ class Fixtures < YAML::Omap
   end
 
   def self.instantiate_all_loaded_fixtures(object, load_instances=true)
-    all_loaded_fixtures.each do |fixtures|
+    all_loaded_fixtures.each do |table_name, fixtures|
-      Fixtures.instantiate_fixtures(object, fixtures.table_name, fixtures, load_instances)
+      Fixtures.instantiate_fixtures(object, table_name, fixtures, load_instances)
     end
   end
   
   cattr_accessor :all_loaded_fixtures
-  self.all_loaded_fixtures = []
+  self.all_loaded_fixtures = {}
 
   def self.create_fixtures(fixtures_directory, *table_names)
-    table_names = table_names.flatten
+    table_names = table_names.flatten.map { |n| n.to_s }
     connection = block_given? ? yield : ActiveRecord::Base.connection
 
     ActiveRecord::Base.logger.silence do
+      fixtures_map = {}
       fixtures = table_names.map do |table_name|
-        Fixtures.new(connection, File.split(table_name.to_s).last, File.join(fixtures_directory, table_name.to_s))
+        fixtures_map[table_name] = Fixtures.new(connection, File.split(table_name.to_s).last, File.join(fixtures_directory, table_name.to_s))
       end               
+      all_loaded_fixtures.merge! fixtures_map  
 
       connection.transaction do
-        table_names.reverse.each do |table_name|
+        fixtures.reverse.each { |fixture| fixture.delete_existing_fixtures }
-          connection.delete "DELETE FROM #{table_name}"
+        fixtures.each { |fixture| fixture.insert_fixtures }
-        end
-
-        fixtures.each { |f| f.insert_fixtures }
 
+        # Cap primary key sequences to max(pk).
         if connection.respond_to?(:reset_pk_sequence!)
-          table_names.flatten.each do |table_name|
+          table_names.each do |table_name|
             connection.reset_pk_sequence!(table_name)
           end
         end
       end
 
-      all_loaded_fixtures.concat fixtures
       return fixtures.size > 1 ? fixtures : fixtures.first
     end
   end
 
-  def self.delete_fixtures(table_names)
-    ActiveRecord::Base.silence do
-      connection = block_given? ? yield : ActiveRecord::Base.connection
-
-      connection.transaction do
-        table_names.reverse.each do |table_name|
-          connection.delete "DELETE FROM #{table_name}"
-        end
-      end
-    end
-  end
-
 
   attr_reader :table_name
 
@@ -429,68 +415,59 @@ module Test #:nodoc:
   module Unit #:nodoc:
     class TestCase #:nodoc:
       cattr_accessor :fixture_path
-      cattr_accessor :dirty_fixture_table_names
-      cattr_accessor :loaded_fixture_table_names
       class_inheritable_accessor :fixture_table_names
       class_inheritable_accessor :use_transactional_fixtures
       class_inheritable_accessor :use_instantiated_fixtures   # true, false, or :no_instances
       class_inheritable_accessor :pre_loaded_fixtures
 
-      self.dirty_fixture_table_names = []
-      self.loaded_fixture_table_names = []
       self.fixture_table_names = []
       self.use_transactional_fixtures = false
       self.use_instantiated_fixtures = true
       self.pre_loaded_fixtures = false
 
-      class << self
+      @@already_loaded_fixtures = {}
-        def fixtures(*table_names)
+
+      def self.fixtures(*table_names)
         table_names = table_names.flatten.map { |n| n.to_s }
         self.fixture_table_names |= table_names
-          self.dirty_fixture_table_names |= table_names
-
         require_fixture_classes(table_names)
         setup_fixture_accessors(table_names)
       end
 
-        def require_fixture_classes(table_names=nil)
+      def self.require_fixture_classes(table_names=nil)
         (table_names || fixture_table_names).each do |table_name| 
           begin
-              require table_name.to_s.singularize
+            require Inflector.singularize(table_name.to_s)
           rescue LoadError
             # Let's hope the developer has included it himself
           end
         end
       end
 
-        def setup_fixture_accessors(table_names = nil)
+      def self.setup_fixture_accessors(table_names=nil)
         (table_names || fixture_table_names).each do |table_name|
           table_name = table_name.to_s.tr('.','_')
-            class_eval <<-end_eval, __FILE__, __LINE__
+          define_method(table_name) do |fixture, *optionals|
-              def #{table_name}(fixture, force_reload = false)
+            force_reload = optionals.shift
-                fixture = fixture.to_s
+            @fixture_cache[table_name] ||= Hash.new
-                @fixture_cache ||= Hash.new { |h,k| h[k] = Hash.new }
+            @fixture_cache[table_name][fixture] = nil if force_reload
-                if force_reload or @fixture_cache['#{table_name}'][fixture].nil?
+            @fixture_cache[table_name][fixture] ||= @loaded_fixtures[table_name][fixture.to_s].find
-                  @fixture_cache['#{table_name}'][fixture] = @loaded_fixtures['#{table_name}'][fixture].find
           end
-                @fixture_cache['#{table_name}'][fixture]
-              end
-            end_eval
         end
       end
 
-        def uses_transaction(*methods)
+      def self.uses_transaction(*methods)
-          @uses_transaction ||= Set.new
+        @uses_transaction ||= []
-          @uses_transaction += methods.flatten.map { |m| m.to_s }
+        @uses_transaction.concat methods.map { |m| m.to_s }
       end
 
-        def uses_transaction?(*methods)
+      def self.uses_transaction?(method)
-          @uses_transaction && methods.flatten.all? { |m| @uses_transaction.include?(m.to_s) }
+        @uses_transaction && @uses_transaction.include?(method.to_s)
-        end
       end
 
       def use_transactional_fixtures?
-        use_transactional_fixtures && !self.class.uses_transaction?(method_name)
+        use_transactional_fixtures &&
+          !self.class.uses_transaction?(method_name)
       end
 
       def setup_with_fixtures
@@ -498,16 +475,23 @@ module Test #:nodoc:
           raise RuntimeError, 'pre_loaded_fixtures requires use_transactional_fixtures' 
         end
 
+        @fixture_cache = Hash.new
+
         # Load fixtures once and begin transaction.
         if use_transactional_fixtures?
-          reload_fixtures! unless @loaded_fixtures and dirty_fixture_table_names.empty?
+          if @@already_loaded_fixtures[self.class]
+            @loaded_fixtures = @@already_loaded_fixtures[self.class]
+          else
+            load_fixtures
+            @@already_loaded_fixtures[self.class] = @loaded_fixtures
+          end
           ActiveRecord::Base.lock_mutex
           ActiveRecord::Base.connection.begin_db_transaction
 
         # Load fixtures for every test.
         else
-          reload_fixtures!
+          @@already_loaded_fixtures[self.class] = nil
-          self.dirty_fixture_table_names |= loaded_fixture_table_names
+          load_fixtures
         end
 
         # Instantiate fixtures for every test if requested.
@@ -522,10 +506,6 @@ module Test #:nodoc:
           ActiveRecord::Base.connection.rollback_db_transaction
           ActiveRecord::Base.unlock_mutex
         end
-        unless dirty_fixture_table_names.empty?
-          Fixtures.delete_fixtures(dirty_fixture_table_names)
-          dirty_fixture_table_names.clear
-        end
       end
 
       alias_method :teardown, :teardown_with_fixtures
@@ -552,26 +532,15 @@ module Test #:nodoc:
       end
 
       private
-        def reload_fixtures!
+        def load_fixtures
-          # Clear dirty fixtures and loaded fixtures which were not declared
-          # for this test case.
-          wipe = dirty_fixture_table_names | loaded_fixture_table_names - fixture_table_names
-          Fixtures.delete_fixtures(wipe) unless wipe.empty?
-          dirty_fixture_table_names.clear
-          loaded_fixture_table_names.clear
-
-          case fixtures = Fixtures.create_fixtures(fixture_path, fixture_table_names)
-            when Fixtures
-              loaded_fixture_table_names.push fixtures.table_name.to_s
-              @loaded_fixtures = { fixtures.table_name => fixtures }
-            when Enumerable
           @loaded_fixtures = {}
-              loaded_fixture_table_names.concat fixtures.map { |f|
+          fixtures = Fixtures.create_fixtures(fixture_path, fixture_table_names)
-                @loaded_fixtures[f.table_name] = f
+          unless fixtures.nil?
-                f.table_name.to_s
+            if fixtures.instance_of?(Fixtures)
-              }
+              @loaded_fixtures[fixtures.table_name] = fixtures
             else
-              @loaded_fixtures = {}
+              fixtures.each { |f| @loaded_fixtures[f.table_name] = f }
+            end
           end
         end
 
@@ -582,8 +551,7 @@ module Test #:nodoc:
           if pre_loaded_fixtures
             raise RuntimeError, 'Load fixtures before instantiating them.' if Fixtures.all_loaded_fixtures.empty?
             unless @@required_fixture_classes
-              names = Fixtures.all_loaded_fixtures.map { |fixtures| fixtures.table_name }
+              self.class.require_fixture_classes Fixtures.all_loaded_fixtures.keys
-              self.class.require_fixture_classes names
               @@required_fixture_classes = true
             end
             Fixtures.instantiate_all_loaded_fixtures(self, load_instances?)

activerecord/test/associations_go_eager_test.rb

@@ -96,7 +96,7 @@ class EagerAssociationTest < Test::Unit::TestCase
   end
   
   def test_eager_with_has_many_and_limit
-    posts = Post.find(:all, :include => [ :author, :comments ], :limit => 2)
+    posts = Post.find(:all, :order => 'posts.id asc', :include => [ :author, :comments ], :limit => 2)
     assert_equal 2, posts.size
     assert_equal 3, posts.inject(0) { |sum, post| sum += post.comments.size }
   end

activerecord/test/conditions_scoping_test.rb

@@ -85,6 +85,7 @@ class HasAndBelongsToManyScopingTest< Test::Unit::TestCase
     assert_equal 0, @welcome.categories.find_all_by_type('SpecialCategory').size
     assert_equal 2, @welcome.categories.find_all_by_type('Category').size
   end
+
 end
 
 

activerecord/test/fixtures_test.rb

@@ -86,7 +86,6 @@ class FixturesTest < Test::Unit::TestCase
     secondRow = ActiveRecord::Base.connection.select_one("SELECT * FROM prefix_topics_suffix WHERE author_name = 'Mary'")
     assert_nil(secondRow["author_email_address"])        
   ensure
-    Fixtures.all_loaded_fixtures.delete(topics)
     ActiveRecord::Base.connection.drop_table :prefix_topics_suffix rescue nil
   end
 
@@ -290,83 +289,3 @@ class ForeignKeyFixturesTest < Test::Unit::TestCase
     assert true
   end
 end
-
-
-class FixtureCleanup1Test < Test::Unit::TestCase
-  fixtures :topics
-
-  def test_isolation
-    assert_equal 0, Developer.count
-    assert_equal 2, Topic.count
-  end
-end
-
-class FixtureCleanup2Test < Test::Unit::TestCase
-  fixtures :developers
-
-  def test_isolation
-    assert_equal 0, Topic.count
-    assert_equal 10, Developer.count
-  end
-end
-
-class FixtureCleanup3Test < FixtureCleanup2Test
-  self.use_transactional_fixtures = false
-
-  def test_dirty_fixture_table_names
-    assert_equal %w(developers), dirty_fixture_table_names
-    assert_equal %w(developers), loaded_fixture_table_names
-    assert_equal %w(developers), fixture_table_names
-  end
-end
-
-class FixtureCleanup4Test < FixtureCleanup2Test
-  self.use_transactional_fixtures = true
-
-  def test_dirty_fixture_table_names
-    assert_equal [], dirty_fixture_table_names
-    assert_equal %w(developers), loaded_fixture_table_names
-    assert_equal %w(developers), fixture_table_names
-  end
-end
-
-class FixtureCleanup5Test < FixtureCleanup3Test
-  self.use_instantiated_fixtures = false
-
-  def test_dirty_fixture_table_names
-    assert_equal %w(developers), dirty_fixture_table_names
-    assert_equal %w(developers), loaded_fixture_table_names
-    assert_equal %w(developers), fixture_table_names
-  end
-end
-
-class FixtureCleanup6Test < FixtureCleanup4Test
-  self.use_instantiated_fixtures = true
-
-  def test_dirty_fixture_table_names
-    assert_equal [], dirty_fixture_table_names
-    assert_equal %w(developers), loaded_fixture_table_names
-    assert_equal %w(developers), fixture_table_names
-  end
-end
-
-class FixtureCleanup7Test < Test::Unit::TestCase
-  self.use_transactional_fixtures = false
-  self.use_instantiated_fixtures = true
-
-  def test_dirty_fixture_table_names
-    assert_equal [], dirty_fixture_table_names
-    assert_equal [], loaded_fixture_table_names
-    assert_equal [], fixture_table_names
-  end
-
-  def test_isolation
-    assert_equal 0, Topic.count
-    assert_equal 0, Developer.count
-  end
-end
-
-class FixtureCleanup8Test < FixtureCleanup7Test
-  self.use_transactional_fixtures = true
-  self.use_instantiated_fixtures = true
-end

activerecord/test/pk_test.rb

@@ -24,6 +24,7 @@ class PrimaryKeysTest < Test::Unit::TestCase
   end
 
   def test_customized_primary_key_auto_assigns_on_save
+    Keyboard.delete_all
     keyboard = Keyboard.new(:name => 'HHKB')
     assert_nothing_raised { keyboard.save! }
     assert_equal keyboard.id, Keyboard.find_by_name('HHKB').id