From 17d3fd316551660ad19251b969540dc304a9a48c Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 25 Oct 2005 18:14:09 +0000 Subject: 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 | 2 - activerecord/lib/active_record/base.rb | 1 + activerecord/lib/active_record/fixtures.rb | 188 ++++++++++-------------- activerecord/test/associations_go_eager_test.rb | 2 +- activerecord/test/conditions_scoping_test.rb | 1 + activerecord/test/fixtures_test.rb | 81 ---------- activerecord/test/pk_test.rb | 1 + 7 files changed, 82 insertions(+), 194 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 293c1f6f26..d69de4e618 100644 --- a/activerecord/CHANGELOG +++ b/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 ] * Clarify semantics of ActiveRecord::Base#respond_to? #2560 [skaes@web.de] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index e6ccfd4659..3ded347178 100755 --- a/activerecord/lib/active_record/base.rb +++ b/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 diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 260c3f97b1..a798c6bb02 100755 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -1,7 +1,6 @@ require 'erb' require 'yaml' require 'csv' -require 'set' module YAML #:nodoc: class Omap #:nodoc: @@ -123,7 +122,7 @@ end # ... # # By adding a "fixtures" method to the test case and passing it a list of symbols (only one is shown here tho), we trigger -# the testing environment to automatically load the appropriate fixtures into the database before each test. +# the testing environment to automatically load the appropriate fixtures into the database before each test. # To ensure consistent data, the environment deletes the fixtures before running the load. # # In addition to being available in the database, the fixtures are also loaded into a hash stored in an instance variable @@ -149,7 +148,7 @@ end # self.use_instantiated_fixtures = false # # - to keep the fixture instance (@web_sites) available, but do not automatically 'find' each instance: -# self.use_instantiated_fixtures = :no_instances +# self.use_instantiated_fixtures = :no_instances # # Even if auto-instantiated fixtures are disabled, you can still access them # by name via special dynamic methods. Each method has the same name as the @@ -181,37 +180,37 @@ end # # = Transactional fixtures # -# TestCases can use begin+rollback to isolate their changes to the database instead of having to delete+insert for every test case. +# TestCases can use begin+rollback to isolate their changes to the database instead of having to delete+insert for every test case. # They can also turn off auto-instantiation of fixture data since the feature is costly and often unused. # # class FooTest < Test::Unit::TestCase # self.use_transactional_fixtures = true # self.use_instantiated_fixtures = false -# +# # fixtures :foos -# +# # def test_godzilla # assert !Foo.find(:all).empty? # Foo.destroy_all # assert Foo.find(:all).empty? # end -# +# # def test_godzilla_aftermath # assert !Foo.find(:all).empty? # end # end -# -# If you preload your test database with all fixture data (probably in the Rakefile task) and use transactional fixtures, +# +# If you preload your test database with all fixture data (probably in the Rakefile task) and use transactional fixtures, # then you may omit all fixtures declarations in your test cases since all the data's already there and every case rolls back its changes. # -# In order to use instantiated fixtures with preloaded data, set +self.pre_loaded_fixtures+ to true. This will provide +# In order to use instantiated fixtures with preloaded data, set +self.pre_loaded_fixtures+ to true. This will provide # access to fixture data for every table that has been loaded through fixtures (depending on the value of +use_instantiated_fixtures+) # -# When *not* to use transactional fixtures: -# 1. You're testing whether a transaction works correctly. Nested transactions don't commit until all parent transactions commit, -# particularly, the fixtures transaction which is begun in setup and rolled back in teardown. Thus, you won't be able to verify -# the results of your transaction until Active Record supports nested transactions or savepoints (in progress.) -# 2. Your database does not support transactions. Every Active Record database supports transactions except MySQL MyISAM. +# When *not* to use transactional fixtures: +# 1. You're testing whether a transaction works correctly. Nested transactions don't commit until all parent transactions commit, +# particularly, the fixtures transaction which is begun in setup and rolled back in teardown. Thus, you won't be able to verify +# the results of your transaction until Active Record supports nested transactions or savepoints (in progress.) +# 2. Your database does not support transactions. Every Active Record database supports transactions except MySQL MyISAM. # Use InnoDB, MaxDB, or NDB instead. class Fixtures < YAML::Omap DEFAULT_FILTER_RE = /\.ya?ml$/ @@ -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| - Fixtures.instantiate_fixtures(object, fixtures.table_name, fixtures, load_instances) + all_loaded_fixtures.each do |table_name, fixtures| + 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)) - end + 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| - connection.delete "DELETE FROM #{table_name}" - end - - fixtures.each { |f| f.insert_fixtures } + fixtures.reverse.each { |fixture| fixture.delete_existing_fixtures } + fixtures.each { |fixture| fixture.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,85 +415,83 @@ 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 - def 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 + @@already_loaded_fixtures = {} - require_fixture_classes(table_names) - setup_fixture_accessors(table_names) - end + def self.fixtures(*table_names) + table_names = table_names.flatten.map { |n| n.to_s } + self.fixture_table_names |= table_names + require_fixture_classes(table_names) + setup_fixture_accessors(table_names) + end - def require_fixture_classes(table_names=nil) - (table_names || fixture_table_names).each do |table_name| - begin - require table_name.to_s.singularize - rescue LoadError - # Let's hope the developer has included it himself - end + def self.require_fixture_classes(table_names=nil) + (table_names || fixture_table_names).each do |table_name| + begin + 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) - (table_names || fixture_table_names).each do |table_name| - table_name = table_name.to_s.tr('.', '_') - class_eval <<-end_eval, __FILE__, __LINE__ - def #{table_name}(fixture, force_reload = false) - fixture = fixture.to_s - @fixture_cache ||= Hash.new { |h,k| h[k] = Hash.new } - if force_reload or @fixture_cache['#{table_name}'][fixture].nil? - @fixture_cache['#{table_name}'][fixture] = @loaded_fixtures['#{table_name}'][fixture].find - end - @fixture_cache['#{table_name}'][fixture] - end - end_eval + def self.setup_fixture_accessors(table_names=nil) + (table_names || fixture_table_names).each do |table_name| + table_name = table_name.to_s.tr('.','_') + define_method(table_name) do |fixture, *optionals| + force_reload = optionals.shift + @fixture_cache[table_name] ||= Hash.new + @fixture_cache[table_name][fixture] = nil if force_reload + @fixture_cache[table_name][fixture] ||= @loaded_fixtures[table_name][fixture.to_s].find end end + end - def uses_transaction(*methods) - @uses_transaction ||= Set.new - @uses_transaction += methods.flatten.map { |m| m.to_s } - end + def self.uses_transaction(*methods) + @uses_transaction ||= [] + @uses_transaction.concat methods.map { |m| m.to_s } + end - def uses_transaction?(*methods) - @uses_transaction && methods.flatten.all? { |m| @uses_transaction.include?(m.to_s) } - end + def self.uses_transaction?(method) + @uses_transaction && @uses_transaction.include?(method.to_s) 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 if pre_loaded_fixtures && !use_transactional_fixtures - raise RuntimeError, 'pre_loaded_fixtures requires use_transactional_fixtures' + 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! - self.dirty_fixture_table_names |= loaded_fixture_table_names + @@already_loaded_fixtures[self.class] = nil + 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! - # 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| - @loaded_fixtures[f.table_name] = f - f.table_name.to_s - } + def load_fixtures + @loaded_fixtures = {} + fixtures = Fixtures.create_fixtures(fixture_path, fixture_table_names) + unless fixtures.nil? + 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 names + self.class.require_fixture_classes Fixtures.all_loaded_fixtures.keys @@required_fixture_classes = true end Fixtures.instantiate_all_loaded_fixtures(self, load_instances?) diff --git a/activerecord/test/associations_go_eager_test.rb b/activerecord/test/associations_go_eager_test.rb index 3dccc8e65a..c2e903fa2c 100644 --- a/activerecord/test/associations_go_eager_test.rb +++ b/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 diff --git a/activerecord/test/conditions_scoping_test.rb b/activerecord/test/conditions_scoping_test.rb index 4fc9aad64e..b945593b57 100644 --- a/activerecord/test/conditions_scoping_test.rb +++ b/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 diff --git a/activerecord/test/fixtures_test.rb b/activerecord/test/fixtures_test.rb index b5e49999f4..888d743078 100755 --- a/activerecord/test/fixtures_test.rb +++ b/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 diff --git a/activerecord/test/pk_test.rb b/activerecord/test/pk_test.rb index 13adc7eb76..f9cfd6f293 100644 --- a/activerecord/test/pk_test.rb +++ b/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 -- cgit v1.2.3