From 14e6bbb149a7045d73d34deed2c8ef4a47e4233f Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sun, 12 Jun 2011 18:09:09 +0100 Subject: Refactor tests to be less brittle --- .../associations/has_many_associations_test.rb | 83 +++++++++++++++------- activerecord/test/cases/helper.rb | 7 +- activerecord/test/models/company.rb | 12 ++++ 3 files changed, 74 insertions(+), 28 deletions(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 79b56d81df..e16a573a1f 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -538,15 +538,27 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_transactions_when_adding_to_persisted - force_signal37_to_load_all_clients_of_firm - Client.expects(:transaction) - companies(:first_firm).clients_of_firm.concat(Client.new("name" => "Natural Company")) + good = Client.new(:name => "Good") + bad = Client.new(:name => "Bad", :raise_on_save => true) + + begin + companies(:first_firm).clients_of_firm.concat(good, bad) + rescue Client::RaisedOnSave + end + + assert !companies(:first_firm).clients_of_firm(true).include?(good) end def test_transactions_when_adding_to_new_record - Client.expects(:transaction).never - firm = Firm.new - firm.clients_of_firm.concat(Client.new("name" => "Natural Company")) + prev_ignored_sql = ActiveRecord::SQLCounter.ignored_sql + ActiveRecord::SQLCounter.ignored_sql = [] + + assert_no_queries do + firm = Firm.new + firm.clients_of_firm.concat(Client.new("name" => "Natural Company")) + end + ensure + ActiveRecord::SQLCounter.ignored_sql = prev_ignored_sql end def test_new_aliased_to_build @@ -791,18 +803,31 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_transaction_when_deleting_persisted - force_signal37_to_load_all_clients_of_firm - client = companies(:first_firm).clients_of_firm.create("name" => "Another Client") - Client.expects(:transaction) - companies(:first_firm).clients_of_firm.delete(client) + good = Client.new(:name => "Good") + bad = Client.new(:name => "Bad", :raise_on_destroy => true) + + companies(:first_firm).clients_of_firm = [good, bad] + + begin + companies(:first_firm).clients_of_firm.destroy(good, bad) + rescue Client::RaisedOnDestroy + end + + assert_equal [good, bad], companies(:first_firm).clients_of_firm(true) end def test_transaction_when_deleting_new_record - client = Client.new("name" => "New Client") - firm = Firm.new - firm.clients_of_firm << client - Client.expects(:transaction).never - firm.clients_of_firm.delete(client) + prev_ignored_sql = ActiveRecord::SQLCounter.ignored_sql + ActiveRecord::SQLCounter.ignored_sql = [] + + assert_no_queries do + firm = Firm.new + client = Client.new("name" => "New Client") + firm.clients_of_firm << client + firm.clients_of_firm.destroy(client) + end + ensure + ActiveRecord::SQLCounter.ignored_sql = prev_ignored_sql end def test_clearing_an_association_collection @@ -1139,21 +1164,29 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_transactions_when_replacing_on_persisted - firm = Firm.find(:first, :order => "id") - firm.clients = [companies(:first_client)] - assert firm.save, "Could not save firm" - firm.reload + good = Client.new(:name => "Good") + bad = Client.new(:name => "Bad", :raise_on_save => true) - Client.expects(:transaction) - firm.clients_of_firm = [Client.new("name" => "Natural Company")] + companies(:first_firm).clients_of_firm = [good] + + begin + companies(:first_firm).clients_of_firm = [bad] + rescue Client::RaisedOnSave + end + + assert_equal [good], companies(:first_firm).clients_of_firm(true) end def test_transactions_when_replacing_on_new_record - firm = Firm.new - firm.clients_of_firm << Client.new("name" => "Natural Company") + prev_ignored_sql = ActiveRecord::SQLCounter.ignored_sql + ActiveRecord::SQLCounter.ignored_sql = [] - Client.expects(:transaction).never - firm.clients_of_firm = [Client.new("name" => "New Client")] + assert_no_queries do + firm = Firm.new + firm.clients_of_firm = [Client.new("name" => "New Client")] + end + ensure + ActiveRecord::SQLCounter.ignored_sql = prev_ignored_sql end def test_get_ids diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 85c20179ec..aa5f1f528f 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -58,11 +58,12 @@ end module ActiveRecord class SQLCounter - IGNORED_SQL = [/^PRAGMA (?!(table_info))/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /^SHOW max_identifier_length/, /^BEGIN/, /^COMMIT/] + cattr_accessor :ignored_sql + self.ignored_sql = [/^PRAGMA (?!(table_info))/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /^SHOW max_identifier_length/, /^BEGIN/, /^COMMIT/] # FIXME: this needs to be refactored so specific database can add their own # ignored SQL. This ignored SQL is for Oracle. - IGNORED_SQL.concat [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im] + ignored_sql.concat [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im] def initialize $queries_executed = [] @@ -74,7 +75,7 @@ module ActiveRecord # FIXME: this seems bad. we should probably have a better way to indicate # the query was cached unless 'CACHE' == values[:name] - $queries_executed << sql unless IGNORED_SQL.any? { |r| sql =~ r } + $queries_executed << sql unless self.class.ignored_sql.any? { |r| sql =~ r } end end end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index e0b30efd51..c1f7a4171a 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -124,6 +124,18 @@ class Client < Company has_many :accounts, :through => :firm belongs_to :account + class RaisedOnSave < RuntimeError; end + attr_accessor :raise_on_save + before_save do + raise RaisedOnSave if raise_on_save + end + + class RaisedOnDestroy < RuntimeError; end + attr_accessor :raise_on_destroy + before_destroy do + raise RaisedOnDestroy if raise_on_destroy + end + # Record destruction so we can test whether firm.clients.clear has # is calling client.destroy, deleting from the database, or setting # foreign keys to NULL. -- cgit v1.2.3