Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails test are failing on fixture loading #672

Closed
juice opened this issue Jan 28, 2019 · 15 comments
Closed

Rails test are failing on fixture loading #672

juice opened this issue Jan 28, 2019 · 15 comments

Comments

@juice
Copy link

juice commented Jan 28, 2019

Hello, we tried to migrate the current master into our project, but our tests are failing.

I created a tiny test project with one table and one field.

execute: rails test

https://github.com/juice/mssql-test

It looks like the fixtures cannot be inserted into the table.

I added all options from our project, like:
ActiveRecord::ConnectionAdapters::SQLServerAdapter.use_output_inserted = false

But it doesn't matter, simple tests are failing anyway. It works with Rails 5.1 and previous versions of the adapter.

Here the same tests in a working branch with Rails 5.1:
https://github.com/juice/mssql-test/tree/working

@wpolicarpo
Copy link
Member

wpolicarpo commented Jan 28, 2019

I couldn't reproduce the error locally even with your test project (thanks for that, btw).

Can you tell us which versions you have for SQL Server, tiny_tds and freetds?

Also, what's the compatibility level of your database?

@juice
Copy link
Author

juice commented Jan 28, 2019

The master branch in NOT failing in your case? Interesting.

FreeTDS: 1.00.109
TinyTDS: 2.1.2
MSSQL Server 2012 (110) (11.0.6607)

@juice
Copy link
Author

juice commented Jan 28, 2019

@wpolicarpo Btw, the adapter is working, just the tests are failing.

In Rails 5.1 the SET IDENTITY_INSERT is set to ON before a fixture insert is executed.
In Rails 5.2 the execution is done with the ` Backquote.

Rails 5.1

   (0.1ms)  BEGIN
  Fixture Delete (1.6ms)  DELETE FROM [StoragePolicy]; SELECT @@ROWCOUNT AS AffectedRows
  SQL (0.6ms)  SET IDENTITY_INSERT [StoragePolicy] ON
  Fixture Insert (1.6ms)  INSERT INTO [StoragePolicy] ([sp_ID], [sp_sl_ID], [sp_c_ID], [sp_Name], [sp_Description], [sp_CopyToArchive], [sp_CopyToArchiveAfterDays], [sp_RemoveFromWorkspace], [sp_RemoveFromWorkspaceAfterDays], [sp_ShortName], [sp_IsExplicitContentPolicy]) VALUES (1, 1, 1, N'Test', N'Test Test Test', 1, 20, 1, 20, N'test', 0)
  SQL (0.5ms)  SET IDENTITY_INSERT [StoragePolicy] OFF

Rails 5.2

TypeError: can't quote ActiveRecord::ConnectionAdapters::SQLServer::Type::Data

ActiveRecord::StatementInvalid:         ActiveRecord::StatementInvalid: TinyTds::Error: Incorrect syntax near '`'.: INSERT INTO `StoragePolicy` (`sp_ID`, `sp_sl_ID`, `sp_c_ID`, `sp_Name`, `sp_Description`, `sp_CopyToArchive`, `sp_CopyToArchiveAfterDays`, `sp_RemoveFromWorkspace`, `sp_RemoveFromWorkspaceAfterDays`, `sp_ShortName`, `sp_IsExplicitContentPolicy`) VALUES (1, 1, 1, 'Test', 'Test Test Test', TRUE, 20, TRUE, 20, 'test', FALSE), (2, 1, 1, 'Test', 'Test Test Test', TRUE, 20, TRUE, 20, 'test', FALSE)


SQL (0.6ms)  BEGIN TRANSACTION
  Fixtures Load (1.0ms)  DELETE FROM [StoragePolicy]
  Fixtures Load (1.3ms)  INSERT INTO `StoragePolicy` (`sp_ID`, `sp_sl_ID`, `sp_c_ID`, `sp_Name`, `sp_Description`, `sp_CopyToArchive`, `sp_CopyToArchiveAfterDays`, `sp_RemoveFromWorkspace`, `sp_RemoveFromWorkspaceAfterDays`, `sp_ShortName`, `sp_IsExplicitContentPolicy`) VALUES (1, 1, 1, 'Test', 'Test Test Test', TRUE, 20, TRUE, 20, 'test', FALSE), (2, 1, 1, 'Test', 'Test Test Test', TRUE, 20, TRUE, 20, 'test', FALSE)
  SQL (0.7ms)  IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION
   (3.2ms)  SELECT s.name, o.name
FROM sys.foreign_keys i
INNER JOIN sys.objects o ON i.parent_object_id = o.OBJECT_ID
INNER JOIN sys.schemas s ON o.schema_id = s.schema_id

@wpolicarpo
Copy link
Member

I cloned your demo project and ran the tests against a 2016 (130) database and all worked fine.

Can you tell us the types for those *_ID columns there?

You can use rails itself if you want: StoragePolicy.columns_hash.

@juice
Copy link
Author

juice commented Jan 28, 2019

Ok, the just managed to try the sample project on a 2016 Server:

MSSQL Server 2016 (130) 13.0.5201

The message is now:

Error:
MyModelsControllerTest#test_should_create_my_model:
TypeError: can't quote ActiveRecord::ConnectionAdapters::SQLServer::Type::Data

You are in the master branch and you executed, "rails db:migrate RAILS_ENV=test" and "rails test"?

This is my output for MyModel.columes_hash:

{"id"=>#<ActiveRecord::ConnectionAdapters::SQLServerColumn:0x00007fda4ac7adb8 @sqlserver_options={:ordinal_position=>1, :is_primary=>true, :is_identity=>true}, @name="id", @table_name="dbo.my_models", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SQLServer::SqlTypeMetadata:0x00007fda4ac7b218 @sqlserver_options={:sqlserver_options=>{:ordinal_position=>1, :is_primary=>true, :is_identity=>true}}, @sql_type="bigint(8)", @type=:integer, @limit=8, @precision=nil, @scale=nil>, @null=false, @default=nil, @default_function=nil, @collation=nil, @comment=nil>, "name"=>#<ActiveRecord::ConnectionAdapters::SQLServerColumn:0x00007fda4ac6f080 @sqlserver_options={:ordinal_position=>2, :is_primary=>false, :is_identity=>false}, @name="name", @table_name="dbo.my_models", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SQLServer::SqlTypeMetadata:0x00007fda4ac6f698 @sqlserver_options={:sqlserver_options=>{:ordinal_position=>2, :is_primary=>false, :is_identity=>false}}, @sql_type="nvarchar(4000)", @type=:string, @limit=4000, @precision=nil, @scale=nil>, @null=true, @default=nil, @default_function=nil, @collation="Latin1_General_CI_AS", @comment=nil>, "created_at"=>#<ActiveRecord::ConnectionAdapters::SQLServerColumn:0x00007fda4ac83a08 @sqlserver_options={:ordinal_position=>3, :is_primary=>false, :is_identity=>false}, @name="created_at", @table_name="dbo.my_models", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SQLServer::SqlTypeMetadata:0x00007fda4ac83f58 @sqlserver_options={:sqlserver_options=>{:ordinal_position=>3, :is_primary=>false, :is_identity=>false}}, @sql_type="datetime", @type=:datetime, @limit=nil, @precision=nil, @scale=nil>, @null=false, @default=nil, @default_function=nil, @collation=nil, @comment=nil>, "updated_at"=>#<ActiveRecord::ConnectionAdapters::SQLServerColumn:0x00007fda4ac83058 @sqlserver_options={:ordinal_position=>4, :is_primary=>false, :is_identity=>false}, @name="updated_at", @table_name="dbo.my_models", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SQLServer::SqlTypeMetadata:0x00007fda4ac83710 @sqlserver_options={:sqlserver_options=>{:ordinal_position=>4, :is_primary=>false, :is_identity=>false}}, @sql_type="datetime", @type=:datetime, @limit=nil, @precision=nil, @scale=nil>, @null=false, @default=nil, @default_function=nil, @collation=nil, @comment=nil>}

@wpolicarpo
Copy link
Member

@juice why do you have both sqlite3 and activerecord-sqlserver-adapter gems on your Gemfile?

Does that work if you remove sqlite3?

@juice
Copy link
Author

juice commented Jan 28, 2019

@wpolicarpo Is there an option to disable the backquotes syntax? "TinyTds::Error: Incorrect syntax near '`'", isn't this the problem?

We use multiple db connections, i tried to get as close as possible to the original project, thats why there is another db connection. I'm looking forward to Rails 6.0 multiple Database connection feature.

What has changed between rails 5.1 and rails 5.2 in the sqlserver adapter so that tests are now failing?

@wpolicarpo
Copy link
Member

@juice first of all, I don't think Rails supports (or will support) the presence of multiple adapters within the same application even with Rails 6 multiple db connection feature, but I might be wrong.

Your tests are failing because Rails changed the way fixtures are loaded with the addition of Arel::InsertManager, which fallbacks to the connection attached to ActiveRecord::Base if none is passed as seen here and here. And because your main database connection is SQLite, InsertManager is trying to build the SQL to insert fixtures using that adapter instead of SQL Server's one.

If the SQL Server adapter was the main connection, this problem would not be happening, but I bet you would have more issues other than these.

Given that, I would suggest:

  1. Check with Rails team if what you want to do is possible (having multiple adapters)
  2. If so, open an issue there explaining what's happening so they can fix the issue (I might help with a PR if they decide it is a bug)

@juice
Copy link
Author

juice commented Feb 5, 2019

@wpolicarpo I have a backtrace here, i hope this helps and thanks for the links to the source. It would be great to fix the broken Fixture code in this Adapter. Everything works together as expected and with Rails 6.0 it will finally backed into the Rails Core. So setting up multiple DB will be a bit easier.

The Adapter is playing nicely with other adapters, but as you mentioned the Problem is in the "insert_fixtures_set" method. After calling "insert_fixtures_set" from the activerecord-sqlserver-adapter, it is falling back to the active_record method "build_fixture_sql". And at this point the data from the SQLServer is mishandled.

Understandably, "TypeError: can't quote ActiveRecord::ConnectionAdapters::SQLServer::Type::Data" will be raised.

BTW, our original Setup is not, sqlite3, it is MySQL in some projects and PostgreSQL in other projects.

TypeError: can't quote ActiveRecord::ConnectionAdapters::SQLServer::Type::Data
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/connection_adapters/abstract/quoting.rb:179:in `_quote'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/connection_adapters/abstract/quoting.rb:18:in `quote'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:799:in `quote'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:181:in `block (2 levels) in visit_Arel_Nodes_ValuesList'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:176:in `each'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:176:in `each_with_index'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:176:in `block in visit_Arel_Nodes_ValuesList'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:173:in `each'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:173:in `each_with_index'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:173:in `visit_Arel_Nodes_ValuesList'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/reduce.rb:15:in `visit'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:815:in `maybe_visit'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/to_sql.rb:134:in `visit_Arel_Nodes_InsertStatement'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/reduce.rb:15:in `visit'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/visitors/reduce.rb:8:in `accept'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb:10:in `accept'
 .rvm/gems/ruby-2.6.0/gems/arel-9.0.0/lib/arel/tree_manager.rb:22:in `to_sql'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:454:in `build_fixture_sql'
 .rvm/gems/ruby-2.6.0/bundler/gems/activerecord-sqlserver-adapter-0dd5e98dadc0/lib/active_record/connection_adapters/sqlserver/database_statements.rb:88:in `block (2 levels) in insert_fixtures_set'
 .rvm/gems/ruby-2.6.0/bundler/gems/activerecord-sqlserver-adapter-0dd5e98dadc0/lib/active_record/connection_adapters/sqlserver/database_statements.rb:87:in `each'
 .rvm/gems/ruby-2.6.0/bundler/gems/activerecord-sqlserver-adapter-0dd5e98dadc0/lib/active_record/connection_adapters/sqlserver/database_statements.rb:87:in `each_slice'
 .rvm/gems/ruby-2.6.0/bundler/gems/activerecord-sqlserver-adapter-0dd5e98dadc0/lib/active_record/connection_adapters/sqlserver/database_statements.rb:87:in `block in insert_fixtures_set'
 .rvm/gems/ruby-2.6.0/bundler/gems/activerecord-sqlserver-adapter-0dd5e98dadc0/lib/active_record/connection_adapters/sqlserver/database_statements.rb:86:in `each'
 .rvm/gems/ruby-2.6.0/bundler/gems/activerecord-sqlserver-adapter-0dd5e98dadc0/lib/active_record/connection_adapters/sqlserver/database_statements.rb:86:in `insert_fixtures_set'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/fixtures.rb:566:in `block in create_fixtures'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/fixtures.rb:558:in `each'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/fixtures.rb:558:in `create_fixtures'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/fixtures.rb:1031:in `load_fixtures'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/fixtures.rb:968:in `setup_fixtures'
 .rvm/gems/ruby-2.6.0/gems/activerecord-5.2.2/lib/active_record/fixtures.rb:860:in `before_setup'
 .rvm/gems/ruby-2.6.0/gems/activesupport-5.2.2/lib/active_support/testing/setup_and_teardown.rb:40:in `before_setup'
 .rvm/gems/ruby-2.6.0/gems/actionpack-5.2.2/lib/action_dispatch/testing/integration.rb:314:in `before_setup'
 .rvm/gems/ruby-2.6.0/gems/railties-5.2.2/lib/rails/test_help.rb:46:in `before_setup'

@wpolicarpo
Copy link
Member

@juice I don't think the issue is in the adapter as I explained before. Your setup is very peculiar and I don't know if Rails would support it out of the box (even Rails 6).

I made a few tests (manually monkey patching ActiveRecord) and managed to make the tests pass, but it has nothing to do with the adapter itself. I understand that it was working on Rails 5.1, but it does not guarantee it was expected or by design.

I'm happy to help with a PR to Rails, but I need to be sure this is intentionally supported by them. Can you open an issue there linking back this thread and we can discuss with them what needs to be done?

@metaskills
Copy link
Contributor

FWIW, I am a little surprised Rails does not find the right constant. It could that 5.2 has a regression. Maybe check rails/rails to be sure but I would expect it to find your right model constant. A few thoughts:

  • Maybe change class MssqlAdapter < ApplicationRecord to class MssqlAdapter < ActiveRecord::Base so Rails can find your constant which inherits from the right thing.
  • If you need a nuclear option, maybe use Rails set_fixture_class.

HTH

@juice
Copy link
Author

juice commented Feb 8, 2019

@metaskills Thanks for your help. Sadly, both tips didn't change the behavior.

@juice
Copy link
Author

juice commented Feb 18, 2019

@wpolicarpo @metaskills
The next major version Rails 6.0 is now in beta. So we are counting weeks here.
I'm sure you guys are working already on a compatible version for Rails 6.0.

Right now the MSSQL Adapter is in a broken state, loading fixtures in tests with multi adapters is not working.

As highlighted by the Rails Maintainers, one of the biggest points on the Roadmap is multi database connection and easier multi database adapter support.

rails/rails#34052

Is there a way to make sure that the original behavior is working again by adding a test?

@wpolicarpo
Copy link
Member

@juice version 6.0.0.rc1 is out.

Would you be able to test it and let us know if the problem still persists?

@juice
Copy link
Author

juice commented May 15, 2020

@wpolicarpo I just did an upgrade to Rails 6.0 and the tests are now working as intended.

Thanks to everyone who helped with the Rails 6.0 PRs.

@juice juice closed this as completed May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants