Browse files

Merge pull request #15394 from morgoth/fix-automatic-maintaining-test…

…-schema-for-sql-format

ActiveRecord::Migration.maintain_test_schema! doesn't work with structure.sql

Conflicts:
	activerecord/CHANGELOG.md

Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
1 parent 98237e5 commit 22e9a91189af2c4e6217a888e77f22a23d3247d1 @senny senny committed Jun 12, 2014
View
7 activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+* Fixed automatic maintaining test schema to properly handle sql structure
+ schema format.
+
+ Fixes #15394.
+
+ *Wojciech Wnętrzak*
+
* Correctly extract IPv6 addresses from `DATABASE_URI`: the square brackets
are part of the URI structure, not the actual host.
View
2 activerecord/lib/active_record/tasks/database_tasks.rb
@@ -161,10 +161,12 @@ def load_schema(format = ActiveRecord::Base.schema_format, file = nil)
when :ruby
file ||= File.join(db_dir, "schema.rb")
check_schema_file(file)
+ purge(current_config)
load(file)
when :sql
file ||= File.join(db_dir, "structure.sql")
check_schema_file(file)
+ purge(current_config)
structure_load(current_config, file)
else
raise ArgumentError, "unknown format #{format.inspect}"
View
6 activerecord/lib/active_record/tasks/sqlite_database_tasks.rb
@@ -21,7 +21,11 @@ def drop
FileUtils.rm(file) if File.exist?(file)
end
- alias :purge :drop
+
+ def purge
+ drop
+ create
+ end
def charset
connection.encoding
View
91 railties/test/application/test_test.rb
@@ -67,7 +67,7 @@ def test_failure
assert_match %r{/app/test/unit/failing_test\.rb}, output
end
- test "migrations" do
+ test "ruby schema migrations" do
output = script('generate model user name:string')
version = output.match(/(\d+)_create_users\.rb/)[1]
@@ -104,6 +104,95 @@ class UserTest < ActiveSupport::TestCase
assert !result.include?("create_table(:users)")
end
+ test "sql structure migrations" do
+ output = script('generate model user name:string')
+ version = output.match(/(\d+)_create_users\.rb/)[1]
+
+ app_file 'test/models/user_test.rb', <<-RUBY
+ require 'test_helper'
+
+ class UserTest < ActiveSupport::TestCase
+ test "user" do
+ User.create! name: "Jon"
+ end
+ end
+ RUBY
+
+ app_file 'db/structure.sql', ''
+ app_file 'config/initializers/enable_sql_schema_format.rb', <<-RUBY
+ Rails.application.config.active_record.schema_format = :sql
+ RUBY
+
+ assert_unsuccessful_run "models/user_test.rb", "Migrations are pending"
+
+ app_file 'db/structure.sql', <<-SQL
+ CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL);
+ CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version");
+ CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255));
+ INSERT INTO schema_migrations (version) VALUES ('#{version}');
+ SQL
+
+ app_file 'config/initializers/disable_maintain_test_schema.rb', <<-RUBY
+ Rails.application.config.active_record.maintain_test_schema = false
+ RUBY
+
+ assert_unsuccessful_run "models/user_test.rb", "Could not find table 'users'"
+
+ File.delete "#{app_path}/config/initializers/disable_maintain_test_schema.rb"
+
+ assert_successful_test_run('models/user_test.rb')
+ end
+
+ test "sql structure migrations when adding column to existing table" do
+ output_1 = script('generate model user name:string')
+ version_1 = output_1.match(/(\d+)_create_users\.rb/)[1]
+
+ app_file 'test/models/user_test.rb', <<-RUBY
+ require 'test_helper'
+ class UserTest < ActiveSupport::TestCase
+ test "user" do
+ User.create! name: "Jon"
+ end
+ end
+ RUBY
+
+ app_file 'config/initializers/enable_sql_schema_format.rb', <<-RUBY
+ Rails.application.config.active_record.schema_format = :sql
+ RUBY
+
+ app_file 'db/structure.sql', <<-SQL
+ CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL);
+ CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version");
+ CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255));
+ INSERT INTO schema_migrations (version) VALUES ('#{version_1}');
+ SQL
+
+ assert_successful_test_run('models/user_test.rb')
+
+ output_2 = script('generate migration add_email_to_users')
+ version_2 = output_2.match(/(\d+)_add_email_to_users\.rb/)[1]
+
+ app_file 'test/models/user_test.rb', <<-RUBY
+ require 'test_helper'
+
+ class UserTest < ActiveSupport::TestCase
+ test "user" do
+ User.create! name: "Jon", email: "jon@doe.com"
+ end
+ end
+ RUBY
+
+ app_file 'db/structure.sql', <<-SQL
+ CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL);
+ CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version");
+ CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255), "email" varchar(255));
+ INSERT INTO schema_migrations (version) VALUES ('#{version_1}');
+ INSERT INTO schema_migrations (version) VALUES ('#{version_2}');
+ SQL
+
+ assert_successful_test_run('models/user_test.rb')
+ end
+
private
def assert_unsuccessful_run(name, message)
result = run_test_file(name)

8 comments on commit 22e9a91

@rubys

I'm seeing failures in AWDwR tests on 4-1-stable, starting with this commit:

22e9a91189af2c4e6217a888e77f22a23d3247d1 is the first bad commit
commit 22e9a91189af2c4e6217a888e77f22a23d3247d1
Author: Yves Senn 
Date:   Thu Jun 12 15:29:20 2014 +0200

    Merge pull request #15394 from morgoth/fix-automatic-maintaining-test-schema-for-sql-format
    
    ActiveRecord::Migration.maintain_test_schema! doesn't work with structure.sql
    
    Conflicts:
        activerecord/CHANGELOG.md
    
    Conflicts:
        activerecord/CHANGELOG.md

:040000 040000 038b54d405a60e2883a609a7ee060a7ab57e3d1e 06bb674f7cf21092a7157a9568eda468b16f1568 M  activerecord
:040000 040000 e776d81eb341e097a39cee41eb9fcf3eae8998b5 fa6c4984f0d478075dbd5bca4c03683df8aa30f5 M  railties
bisect run success

Symptoms:

ActiveRecord::StatementInvalid (ArgumentError: prepare called on a closed database:           SELECT name
         FROM sqlite_master
         WHERE type = 'table' AND NOT name = 'sqlite_sequence'
AND name = "schema_migrations"):
 sqlite3 (1.3.9) lib/sqlite3/database.rb:91:in `initialize'
 sqlite3 (1.3.9) lib/sqlite3/database.rb:91:in `new'
 sqlite3 (1.3.9) lib/sqlite3/database.rb:91:in `prepare'
 /home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb:294:in `block in exec_query'
 /home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:452:in `block in log'
 /home/rubys/git/rails/activesupport/lib/active_support/notifications/instrumenter.rb:20:in `instrument'
 /home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:446:in `log'
 /home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb:291:in `exec_query'
 /home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb:372:in `tables'
 /home/rubys/git/rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb:378:in `table_exists?'
 /home/rubys/git/rails/activerecord/lib/active_record/migration.rb:838:in `current_version'
 /home/rubys/git/rails/activerecord/lib/active_record/migration.rb:846:in `needs_migration?'
 /home/rubys/git/rails/activerecord/lib/active_record/migration.rb:392:in `check_pending!'
 /home/rubys/git/rails/activerecord/lib/active_record/migration.rb:379:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
 /home/rubys/git/rails/activesupport/lib/active_support/callbacks.rb:82:in `run_callbacks'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/callbacks.rb:27:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/reloader.rb:73:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/remote_ip.rb:76:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
 /home/rubys/git/rails/railties/lib/rails/rack/logger.rb:38:in `call_app'
 /home/rubys/git/rails/railties/lib/rails/rack/logger.rb:20:in `block in call'
 /home/rubys/git/rails/activesupport/lib/active_support/tagged_logging.rb:68:in `block in tagged'
 /home/rubys/git/rails/activesupport/lib/active_support/tagged_logging.rb:26:in `tagged'
 /home/rubys/git/rails/activesupport/lib/active_support/tagged_logging.rb:68:in `tagged'
 /home/rubys/git/rails/railties/lib/rails/rack/logger.rb:20:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/request_id.rb:21:in `call'
 rack (1.5.2) lib/rack/methodoverride.rb:21:in `call'
 rack (1.5.2) lib/rack/runtime.rb:17:in `call'
 /home/rubys/git/rails/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb:26:in `call'
 rack (1.5.2) lib/rack/lock.rb:17:in `call'
 /home/rubys/git/rails/actionpack/lib/action_dispatch/middleware/static.rb:64:in `call'
 rack (1.5.2) lib/rack/sendfile.rb:112:in `call'
 /home/rubys/git/rails/railties/lib/rails/engine.rb:514:in `call'
 /home/rubys/git/rails/railties/lib/rails/application.rb:144:in `call'
 rack (1.5.2) lib/rack/lock.rb:17:in `call'
 rack (1.5.2) lib/rack/content_length.rb:14:in `call'
 rack (1.5.2) lib/rack/handler/webrick.rb:60:in `service'
 /home/rubys/.rvm/rubies/ruby-2.0.0-p481/lib/ruby/2.0.0/webrick/httpserver.rb:138:in `service'
 /home/rubys/.rvm/rubies/ruby-2.0.0-p481/lib/ruby/2.0.0/webrick/httpserver.rb:94:in `run'
 /home/rubys/.rvm/rubies/ruby-2.0.0-p481/lib/ruby/2.0.0/webrick/server.rb:295:in `block in start_thread'

Note: I'm seeing what looks to me to be quite similar failures on master, but I haven't managed to do a successful bisect.

@morgoth

@senny in stacktrace DatabaseTasks#load_schema method which was modified in this commit is not called, so I'm not sure if this is the reason.

@dhh
Ruby on Rails member

Saw the same issue. Reverted this commit for now.

@senny
Ruby on Rails member

@dhh @rubys I have been looking at the AWDwR tests but couldn't yet figure out what's causing the issue. Is there a way to reproduce this in a sample application? Does it always break rake db:schema:load for MySQL?

I'm currently traveling so I don't have much time to investigate. 😓

/cc @rafaelfranca

@rubys

I've not been able to reproduce the problem on 4-1-stable into a simple: take this application, start the server, and run this command. It seems that some series of steps leaves the database closed, and the next operation goes boom.

I have been able to reproduce a similar looking problem on master. Perhaps solving that one will give some insight into what is going on? See 4dc6b64#commitcomment-6823182

@dhh
Ruby on Rails member
@rafaelfranca
Ruby on Rails member

I'll take a look on this problem tomorrow, if I could not fix it I'll revert to not block anyone and work to fix in a branch.

@rubys

FYI: my problem is with SQLite.

Please sign in to comment.