From 77ca2815f5c5fc20a9ca7fa4cdd16d0c4b908682 Mon Sep 17 00:00:00 2001
From: Yves Senn <yves.senn@gmail.com>
Date: Mon, 3 Sep 2012 20:30:43 +0200
Subject: rewrite inheritance tests with a custom inheritance_column

previously the tests with and without a custom `inheritance_column`
used the same models. Since the model then has both fields this can lead
to false positives.
---
 activerecord/test/cases/inheritance_test.rb | 86 +++++++++++++----------------
 activerecord/test/fixtures/vegetables.yml   | 13 ++++-
 activerecord/test/models/vegetables.rb      | 10 ++++
 activerecord/test/schema/schema.rb          |  1 +
 4 files changed, 61 insertions(+), 49 deletions(-)

(limited to 'activerecord/test')

diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb
index cc9a93c16f..ca5b24b253 100644
--- a/activerecord/test/cases/inheritance_test.rb
+++ b/activerecord/test/cases/inheritance_test.rb
@@ -123,9 +123,10 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_inheritance_find
-    switch_to_alt_inheritance_column
-    test_inheritance_find
-    switch_to_default_inheritance_column
+    assert_kind_of Cucumber, Vegetable.find(1)
+    assert_kind_of Cucumber, Cucumber.find(1)
+    assert_kind_of Cabbage, Vegetable.find(2)
+    assert_kind_of Cabbage, Cabbage.find(2)
   end
 
   def test_alt_becomes_works_with_sti
@@ -142,9 +143,9 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_inheritance_find_all
-    switch_to_alt_inheritance_column
-    test_inheritance_find_all
-    switch_to_default_inheritance_column
+    companies = Vegetable.all.merge!(:order => 'id').to_a
+    assert_kind_of Cucumber, companies[0]
+    assert_kind_of Cabbage, companies[1]
   end
 
   def test_inheritance_save
@@ -157,9 +158,11 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_inheritance_save
-    switch_to_alt_inheritance_column
-    test_inheritance_save
-    switch_to_default_inheritance_column
+    cabbage = Cabbage.new(:name => 'Savoy')
+    cabbage.save!
+
+    savoy = Vegetable.find(cabbage.id)
+    assert_kind_of Cabbage, savoy
   end
 
   def test_inheritance_condition
@@ -169,9 +172,9 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_inheritance_condition
-    switch_to_alt_inheritance_column
-    test_inheritance_condition
-    switch_to_default_inheritance_column
+    assert_equal 4, Vegetable.count
+    assert_equal 1, Cucumber.count
+    assert_equal 3, Cabbage.count
   end
 
   def test_finding_incorrect_type_data
@@ -180,9 +183,8 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_finding_incorrect_type_data
-    switch_to_alt_inheritance_column
-    test_finding_incorrect_type_data
-    switch_to_default_inheritance_column
+    assert_raise(ActiveRecord::RecordNotFound) { Cucumber.find(2) }
+    assert_nothing_raised   { Cucumber.find(1) }
   end
 
   def test_update_all_within_inheritance
@@ -193,9 +195,9 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_update_all_within_inheritance
-    switch_to_alt_inheritance_column
-    test_update_all_within_inheritance
-    switch_to_default_inheritance_column
+    Cabbage.update_all "name = 'the cabbage'"
+    assert_equal "the cabbage", Cabbage.first.name
+    assert_equal ["my cucumber"], Cucumber.all.map(&:name).uniq
   end
 
   def test_destroy_all_within_inheritance
@@ -205,9 +207,9 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_destroy_all_within_inheritance
-    switch_to_alt_inheritance_column
-    test_destroy_all_within_inheritance
-    switch_to_default_inheritance_column
+    Cabbage.destroy_all
+    assert_equal 0, Cabbage.count
+    assert_equal 1, Cucumber.count
   end
 
   def test_find_first_within_inheritance
@@ -217,9 +219,9 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_find_first_within_inheritance
-    switch_to_alt_inheritance_column
-    test_find_first_within_inheritance
-    switch_to_default_inheritance_column
+    assert_kind_of Cabbage, Vegetable.all.merge!(:where => "name = 'his cabbage'").first
+    assert_kind_of Cabbage, Cabbage.all.merge!(:where => "name = 'his cabbage'").first
+    assert_nil Cucumber.all.merge!(:where => "name = 'his cabbage'").first
   end
 
   def test_complex_inheritance
@@ -233,9 +235,13 @@ class InheritanceTest < ActiveRecord::TestCase
   end
 
   def test_alt_complex_inheritance
-    switch_to_alt_inheritance_column
-    test_complex_inheritance
-    switch_to_default_inheritance_column
+    king_cole = KingCole.create("name" => "uniform heads")
+    assert_equal king_cole, KingCole.where("name = 'uniform heads'").first
+    assert_equal king_cole, GreenCabbage.all.merge!(:where => "name = 'uniform heads'").first
+    assert_equal king_cole, Cabbage.all.merge!(:where => "name = 'uniform heads'").first
+    assert_equal king_cole, Vegetable.all.merge!(:where => "name = 'uniform heads'").first
+    assert_equal 1, Cabbage.all.merge!(:where => "name = 'his cabbage'").to_a.size
+    assert_equal king_cole, Cabbage.find(king_cole.id)
   end
 
   def test_eager_load_belongs_to_something_inherited
@@ -243,6 +249,11 @@ class InheritanceTest < ActiveRecord::TestCase
     assert account.association_cache.key?(:firm), "nil proves eager load failed"
   end
 
+  def test_alt_eager_loading
+    cabbage = RedCabbage.all.merge!(:includes => :seller).find(4)
+    assert cabbage.association_cache.key?(:seller), "nil proves eager load failed"
+  end
+
   def test_eager_load_belongs_to_primary_key_quoting
     con = Account.connection
     assert_sql(/#{con.quote_table_name('companies')}.#{con.quote_column_name('id')} IN \(1\)/) do
@@ -250,12 +261,6 @@ class InheritanceTest < ActiveRecord::TestCase
     end
   end
 
-  def test_alt_eager_loading
-    switch_to_alt_inheritance_column
-    test_eager_load_belongs_to_something_inherited
-    switch_to_default_inheritance_column
-  end
-
   def test_inherits_custom_primary_key
     assert_equal Subscriber.primary_key, SpecialSubscriber.primary_key
   end
@@ -264,21 +269,6 @@ class InheritanceTest < ActiveRecord::TestCase
     assert_kind_of SpecialSubscriber, SpecialSubscriber.find("webster132")
     assert_nothing_raised { s = SpecialSubscriber.new("name" => "And breaaaaathe!"); s.id = 'roger'; s.save }
   end
-
-  private
-    def switch_to_alt_inheritance_column
-      # we don't want misleading test results, so get rid of the values in the type column
-      Company.all.merge!(:order => 'id').to_a.each do |c|
-        c['type'] = nil
-        c.save
-      end
-      [ Company, Firm, Client].each { |klass| klass.reset_column_information }
-      Company.inheritance_column = 'ruby_type'
-    end
-    def switch_to_default_inheritance_column
-      [ Company, Firm, Client].each { |klass| klass.reset_column_information }
-      Company.inheritance_column = 'type'
-    end
 end
 
 
diff --git a/activerecord/test/fixtures/vegetables.yml b/activerecord/test/fixtures/vegetables.yml
index 82bd607701..b9afbfbb05 100644
--- a/activerecord/test/fixtures/vegetables.yml
+++ b/activerecord/test/fixtures/vegetables.yml
@@ -6,4 +6,15 @@ first_cucumber:
 first_cabbage:
   id: 2
   custom_type: Cabbage
-  name: 'my cabbage'
\ No newline at end of file
+  name: 'my cabbage'
+
+second_cabbage:
+  id: 3
+  custom_type: Cabbage
+  name: 'his cabbage'
+
+red_cabbage:
+  id: 4
+  custom_type: RedCabbage
+  name: 'red cabbage'
+  seller_id: 3
\ No newline at end of file
diff --git a/activerecord/test/models/vegetables.rb b/activerecord/test/models/vegetables.rb
index 59cedfd9f5..1f41cde3a5 100644
--- a/activerecord/test/models/vegetables.rb
+++ b/activerecord/test/models/vegetables.rb
@@ -12,3 +12,13 @@ end
 
 class Cabbage < Vegetable
 end
+
+class GreenCabbage < Cabbage
+end
+
+class KingCole < GreenCabbage
+end
+
+class RedCabbage < Cabbage
+  belongs_to :seller, :class_name => 'Company'
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index eae7a04157..7b37ac3389 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -186,6 +186,7 @@ ActiveRecord::Schema.define do
 
   create_table :vegetables, :force => true do |t|
     t.string :name
+    t.integer :seller_id
     t.string :custom_type
   end
 
-- 
cgit v1.2.3


From 9f494a9a3489cf03b0d6d3c87cb8638c3b867c86 Mon Sep 17 00:00:00 2001
From: Yves Senn <yves.senn@gmail.com>
Date: Mon, 3 Sep 2012 20:38:14 +0200
Subject: test cleanup, remove ruby_type because it's no longer needed

All tests with a custom inheritance_column use the `Vegtable` model.
The field ruby_type on the Company models is no longer needed
---
 activerecord/test/cases/attribute_methods_test.rb | 2 +-
 activerecord/test/cases/base_test.rb              | 2 +-
 activerecord/test/cases/inheritance_test.rb       | 2 +-
 activerecord/test/cases/schema_dumper_test.rb     | 2 +-
 activerecord/test/fixtures/companies.yml          | 6 ------
 activerecord/test/models/company.rb               | 4 ----
 activerecord/test/schema/schema.rb                | 3 +--
 7 files changed, 5 insertions(+), 16 deletions(-)

(limited to 'activerecord/test')

diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index ea58a624a1..d08b157011 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -395,7 +395,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase
 
   def test_query_attribute_with_custom_fields
     object = Company.find_by_sql(<<-SQL).first
-      SELECT c1.*, c2.ruby_type as string_value, c2.rating as int_value
+      SELECT c1.*, c2.type as string_value, c2.rating as int_value
         FROM companies c1, companies c2
        WHERE c1.firm_id = c2.id
          AND c1.id = 2
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index c6f7e8cf0f..9fcee99222 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1735,7 +1735,7 @@ class BasicsTest < ActiveRecord::TestCase
   end
 
   def test_attribute_names
-    assert_equal ["id", "type", "ruby_type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"],
+    assert_equal ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"],
                  Company.attribute_names
   end
 
diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb
index ca5b24b253..8fded9159f 100644
--- a/activerecord/test/cases/inheritance_test.rb
+++ b/activerecord/test/cases/inheritance_test.rb
@@ -288,7 +288,7 @@ class InheritanceComputeTypeTest < ActiveRecord::TestCase
   def test_instantiation_doesnt_try_to_require_corresponding_file
     ActiveRecord::Base.store_full_sti_class = false
     foo = Firm.first.clone
-    foo.ruby_type = foo.type = 'FirmOnTheFly'
+    foo.type = 'FirmOnTheFly'
     foo.save!
 
     # Should fail without FirmOnTheFly in the type condition.
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index 01dd25a9df..92084d3eb6 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -182,7 +182,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
 
   def test_schema_dumps_index_columns_in_right_order
     index_definition = standard_dump.split(/\n/).grep(/add_index.*companies/).first.strip
-    assert_equal 'add_index "companies", ["firm_id", "type", "rating", "ruby_type"], :name => "company_index"', index_definition
+    assert_equal 'add_index "companies", ["firm_id", "type", "rating"], :name => "company_index"', index_definition
   end
 
   def test_schema_dumps_partial_indices
diff --git a/activerecord/test/fixtures/companies.yml b/activerecord/test/fixtures/companies.yml
index a982d3921d..0766e92027 100644
--- a/activerecord/test/fixtures/companies.yml
+++ b/activerecord/test/fixtures/companies.yml
@@ -4,14 +4,12 @@ first_client:
   firm_id: 1
   client_of: 2
   name: Summit
-  ruby_type: Client
   firm_name: 37signals
 
 first_firm:
   id: 1
   type: Firm
   name: 37signals
-  ruby_type: Firm
   firm_id: 1
 
 second_client:
@@ -20,13 +18,11 @@ second_client:
   firm_id: 1
   client_of: 1
   name: Microsoft
-  ruby_type: Client
 
 another_firm:
   id: 4
   type: Firm
   name: Flamboyant Software
-  ruby_type: Firm
 
 another_client:
   id: 5
@@ -34,7 +30,6 @@ another_client:
   firm_id: 4
   client_of: 4
   name: Ex Nihilo
-  ruby_type: Client
 
 a_third_client:
   id: 10
@@ -42,7 +37,6 @@ a_third_client:
   firm_id: 4
   client_of: 4
   name: Ex Nihilo Part Deux
-  ruby_type: Client
 
 rails_core:
   id: 6
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index 75f38d275c..9bdce6e729 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -173,10 +173,6 @@ class Client < Company
   before_destroy :overwrite_to_raise
 
   # Used to test that read and question methods are not generated for these attributes
-  def ruby_type
-    read_attribute :ruby_type
-  end
-
   def rating?
     query_attribute :rating
   end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 7b37ac3389..007349ea87 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -171,7 +171,6 @@ ActiveRecord::Schema.define do
 
   create_table :companies, :force => true do |t|
     t.string  :type
-    t.string  :ruby_type
     t.integer :firm_id
     t.string  :firm_name
     t.string  :name
@@ -181,7 +180,7 @@ ActiveRecord::Schema.define do
     t.string :description, :default => ""
   end
 
-  add_index :companies, [:firm_id, :type, :rating, :ruby_type], :name => "company_index"
+  add_index :companies, [:firm_id, :type, :rating], :name => "company_index"
   add_index :companies, [:firm_id, :type], :name => "company_partial_index", :where => "rating > 10"
 
   create_table :vegetables, :force => true do |t|
-- 
cgit v1.2.3