Tidied up Fixtures for better readability and some error checking [bitsweat]

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

2 files changed, 61 insertions, 42 deletions

diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 80ffe0d523..320b0dbe78 100755
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -30,9 +30,9 @@ require 'active_record/support/inflector'
 #     name: Google
 #     url: http://www.google.com
 #
-# This YAML fixture file includes two fixtures.  Each YAML fixture (ie. record) is given a name and is followed by an 
+# This YAML fixture file includes two fixtures.  Each YAML fixture (ie. record) is given a name and is followed by an
 # indented list of key/value pairs in the "key: value" format.  Records are separated by a blank line for your viewing
-# pleasure.  
+# pleasure.
 #
 # = CSV fixtures
 #
@@ -51,17 +51,17 @@ require 'active_record/support/inflector'
 # need to use a double quote character, you must escape it with another double quote.
 #
 # Another unique attribute of the CSV fixture is that it has *no* fixture name like the other two formats.  Instead, the
-# fixture names are automatically generated by deriving the class name of the fixture file and adding an incrementing 
+# fixture names are automatically generated by deriving the class name of the fixture file and adding an incrementing
 # number to the end.  In our example, the 1st fixture would be called "web_site_1" and the 2nd one would be called
 # "web_site_2".
 #
-# Most databases and spreadsheets support exporting to CSV format, so this is a great format for you to choose if you 
+# Most databases and spreadsheets support exporting to CSV format, so this is a great format for you to choose if you
 # have existing data somewhere already.
-# 
-# = Single-file fixtures 
+#
+# = Single-file fixtures
 #
 # This type of fixtures was the original format for Active Record that has since been deprecated in favor of the YAML and CSV formats.
-# Fixtures for this format are created by placing text files in a sub-directory (with the name of the model) to the directory 
+# Fixtures for this format are created by placing text files in a sub-directory (with the name of the model) to the directory
 # appointed by <tt>Test::Unit::TestCase.fixture_path=(path)</tt> (this is automatically configured for Rails, so you can just
 # put your files in <your-rails-app>/test/fixtures/<your-model-name>/ -- like <your-rails-app>/test/fixtures/web_sites/ for the WebSite
 # model).
@@ -91,19 +91,19 @@ require 'active_record/support/inflector'
 #   class WebSiteTest < Test::Unit::TestCase
 #     def test_web_site_count
 #       assert_equal 2, WebSite.count
-#     end 
+#     end
 #   end
-#  
-# As it stands, unless we pre-load the web_site table in our database with two records, this test will fail.  Here's the 
+#
+# As it stands, unless we pre-load the web_site table in our database with two records, this test will fail.  Here's the
 # easiest way to add fixtures to the database:
 #
 #   ...
 #   class WebSiteTest < Test::Unit::TestCase
 #     fixtures :web_sites # add more by separating the symbols with commas
 #   ...
-#  
+#
 # 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, and 
+# the testing environment to automatically load the appropriate fixtures into the database before each test, and
 # automatically delete them after each test.
 #
 # In addition to being available in the database, the fixtures are also loaded into a hash stored in an instance variable
@@ -137,38 +137,42 @@ require 'active_record/support/inflector'
 # This will create 1000 very simple YAML fixtures.
 #
 # Using ERb, you can also inject dynamic values into your fixtures with inserts like <%= Date.today.strftime("%Y-%m-%d") %>.
-# This is however a feature to be used with some caution. The point of fixtures are that they're stable units of predictable 
+# This is however a feature to be used with some caution. The point of fixtures are that they're stable units of predictable
 # sample data. If you feel that you need to inject dynamic values, then perhaps you should reexamine whether your application
 # is properly testable. Hence, dynamic values in fixtures are to be considered a code smell.
 class Fixtures < Hash
+  DEFAULT_FILTER_RE = /\.ya?ml$/
+
   def self.instantiate_fixtures(object, fixtures_directory, *table_names)
     [ create_fixtures(fixtures_directory, *table_names) ].flatten.each_with_index do |fixtures, idx|
       object.instance_variable_set "@#{table_names[idx]}", fixtures
       fixtures.each { |name, fixture| object.instance_variable_set "@#{name}", fixture.find }
     end
   end
-  
+
   def self.create_fixtures(fixtures_directory, *table_names)
     connection = block_given? ? yield : ActiveRecord::Base.connection
     old_logger_level = ActiveRecord::Base.logger.level
 
     begin
       ActiveRecord::Base.logger.level = Logger::ERROR
-      fixtures = []
+
+      fixtures = table_names.flatten.map do |table_name|
+        Fixtures.new(connection, table_name.to_s, File.join(fixtures_directory, table_name.to_s))
+      end
+
       connection.transaction do
-        fixtures = table_names.flatten.map do |table_name|
-          Fixtures.new(connection, table_name.to_s, File.join(fixtures_directory, table_name.to_s))
-        end
         fixtures.reverse.each { |fixture| fixture.delete_existing_fixtures }
         fixtures.each { |fixture| fixture.insert_fixtures }
       end
+
       return fixtures.size > 1 ? fixtures : fixtures.first
     ensure
       ActiveRecord::Base.logger.level = old_logger_level
     end
   end
 
-  def initialize(connection, table_name, fixture_path, file_filter = /^\.|CVS|\.yml|\.csv/)
+  def initialize(connection, table_name, fixture_path, file_filter = DEFAULT_FILTER_RE)
     @connection, @table_name, @fixture_path, @file_filter = connection, table_name, fixture_path, file_filter
     @class_name = Inflector.classify(@table_name)
 
@@ -176,23 +180,23 @@ class Fixtures < Hash
   end
 
   def delete_existing_fixtures
-    @connection.delete "DELETE FROM #{@table_name}"
+    @connection.delete "DELETE FROM #{@table_name}", 'Fixture Delete'
   end
 
   def insert_fixtures
     values.each do |fixture|
-      @connection.execute "INSERT INTO #{@table_name} (#{fixture.key_list}) VALUES(#{fixture.value_list})"
+      @connection.execute "INSERT INTO #{@table_name} (#{fixture.key_list}) VALUES (#{fixture.value_list})", 'Fixture Insert'
     end
   end
 
   private
     def read_fixture_files
-      if File.exists?(yaml_file_path)
+      if File.file?(yaml_file_path)
         # YAML fixtures
         YAML::load(erb_render(IO.read(yaml_file_path))).each do |name, data|
           self[name] = Fixture.new(data, @class_name)
         end
-      elsif File.exists?(csv_file_path)
+      elsif File.file?(csv_file_path)
         # CSV fixtures
         reader = CSV::Reader.create(erb_render(IO.read(csv_file_path)))
         header = reader.shift
@@ -202,22 +206,31 @@ class Fixtures < Hash
           row.each_with_index { |cell, j| data[header[j].to_s.strip] = cell.to_s.strip }
           self["#{Inflector::underscore(@class_name)}_#{i+=1}"]= Fixture.new(data, @class_name)
         end
+      elsif File.file?(deprecated_yaml_file_path)
+        raise Fixture::FormatError, ".yml extension required: rename #{deprecated_yaml_file_path} to #{yaml_file_path}"
       else
         # Standard fixtures
-        Dir.entries(@fixture_path).each do |file| 
-          self[file] = Fixture.new(File.join(@fixture_path, file), @class_name) unless file =~ @file_filter
+        Dir.entries(@fixture_path).each do |file|
+          path = File.join(@fixture_path, file)
+          if File.file?(path) and file !~ @file_filter
+            self[file] = Fixture.new(path, @class_name)
+          end
         end
       end
     end
 
     def yaml_file_path
-      @fixture_path + ".yml"
+      "#{@fixture_path}.yml"
+    end
+
+    def deprecated_yaml_file_path
+      "#{@fixture_path}.yaml"
     end
 
     def csv_file_path
       @fixture_path + ".csv"
     end
-    
+
     def yaml_fixtures_key(path)
       File.basename(@fixture_path).split(".").first
     end
@@ -256,25 +269,25 @@ class Fixture #:nodoc:
   def value_list
     @fixture.values.map { |v| ActiveRecord::Base.connection.quote(v).gsub('\\n', "\n").gsub('\\r', "\r") }.join(", ")
   end
-  
+
   def find
     Object.const_get(@class_name).find(self[Object.const_get(@class_name).primary_key])
   end
-  
+
   private
     def read_fixture_file(fixture_file_path)
       IO.readlines(fixture_file_path).inject({}) do |fixture, line|
         # Mercifully skip empty lines.
-        next if line.empty?
+        next if line =~ /^\s*$/
 
         # Use the same regular expression for attributes as Active Record.
         unless md = /^\s*([a-zA-Z][-_\w]*)\s*=>\s*(.+)\s*$/.match(line)
-          raise FormatError, "#{path}: fixture format error at '#{line}'.  Expecting 'key => value'."
+          raise FormatError, "#{fixture_file_path}: fixture format error at '#{line}'.  Expecting 'key => value'."
         end
         key, value = md.captures
 
         # Disallow duplicate keys to catch typos.
-        raise FormatError, "#{path}: duplicate '#{key}' in fixture." if fixture[key]
+        raise FormatError, "#{fixture_file_path}: duplicate '#{key}' in fixture." if fixture[key]
         fixture[key] = value.strip
         fixture
       end
@@ -283,10 +296,10 @@ end
 
 class Test::Unit::TestCase #:nodoc:
   include ClassInheritableAttributes
-  
+
   cattr_accessor :fixture_path
   cattr_accessor :fixture_table_names
-  
+
   def self.fixtures(*table_names)
     write_inheritable_attribute("fixture_table_names", table_names)
   end
@@ -294,7 +307,7 @@ class Test::Unit::TestCase #:nodoc:
   def setup
     instantiate_fixtures(*fixture_table_names) if fixture_table_names
   end
-  
+
   def self.method_added(method_symbol)
     if method_symbol == :setup && !method_defined?(:setup_without_fixtures)
       alias_method :setup_without_fixtures, :setup
@@ -309,7 +322,7 @@ class Test::Unit::TestCase #:nodoc:
     def instantiate_fixtures(*table_names)
       Fixtures.instantiate_fixtures(self, fixture_path, *table_names)
     end
-    
+
     def fixture_table_names
       self.class.read_inheritable_attribute("fixture_table_names")
     end
diff --git a/activerecord/test/fixtures_test.rb b/activerecord/test/fixtures_test.rb
index 5b4bf9a922..a971b9f6f8 100755
--- a/activerecord/test/fixtures_test.rb
+++ b/activerecord/test/fixtures_test.rb
@@ -5,7 +5,7 @@ require 'fixtures/company'
 
 class FixturesTest < Test::Unit::TestCase
   fixtures :topics, :developers, :accounts
-  
+
   FIXTURES = %w( accounts companies customers
                  developers developers_projects entrants
                  movies projects subscribers topics )
@@ -49,34 +49,40 @@ class FixturesTest < Test::Unit::TestCase
   def test_bad_format
     path = File.join(File.dirname(__FILE__), 'fixtures', 'bad_fixtures')
     Dir.entries(path).each do |file|
-      next unless File.file?(file) and file !~ %r(^.|.yaml$)
+      next unless File.file?(file) and file !~ Fixtures::DEFAULT_FILTER_RE
       assert_raise(Fixture::FormatError) {
         Fixture.new(bad_fixtures_path, file)
       }
     end
   end
 
+  def test_deprecated_yaml_extension
+    assert_raise(Fixture::FormatError) {
+      Fixtures.new(nil, 'bad_extension', File.join(File.dirname(__FILE__), 'fixtures'))
+    }
+  end
+
   def test_logger_level_invariant
     level = ActiveRecord::Base.logger.level
     create_fixtures('topics')
     assert_equal level, ActiveRecord::Base.logger.level
   end
-  
+
   def test_instantiation
     topics = create_fixtures("topics")
     assert_kind_of Topic, topics["first"].find
   end
-  
+
   def test_complete_instantiation
     assert_equal 2, @topics.size
     assert_equal "The First Topic", @first.title
   end
-  
+
   def test_fixtures_from_root_yml_with_instantiation
     # assert_equal 2, @accounts.size
     assert_equal 50, @unknown.credit_limit
   end
-  
+
   def test_erb_in_fixtures
     assert_equal 10, @developers.size
     assert_equal "fixture_5", @dev_5.name