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

Updates to run tests with activerecord 7.1 #2359

Closed
wants to merge 2 commits into from

Conversation

davinlagerroos
Copy link
Contributor

When running the oracle enhanced tests with activerecord v7.1.0 or above I get:

==> Effective ActiveRecord version 7.1.3
rake aborted!
NoMethodError: undefined method `database_version' for nil:NilClass (NoMethodError)

        @raw_connection.database_version
                       ^^^^^^^^^^^^^^^^^
/app/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:699:in `get_database_version'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/schema_cache.rb:374:in `database_version'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/schema_cache.rb:70:in `database_version'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/schema_cache.rb:200:in `database_version'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:871:in `database_version'
/app/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:307:in `supports_fetch_first_n_rows_and_offset?'
/app/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:271:in `arel_visitor'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:159:in `initialize'
/app/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:250:in `initialize'
/app/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:76:in `new'
/app/lib/active_record/connection_adapters/oracle_enhanced_adapter.rb:76:in `oracle_enhanced_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:676:in `public_send'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:676:in `new_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:723:in `checkout_new_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:702:in `try_to_checkout_new_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:654:in `acquire_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:353:in `checkout'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:182:in `connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_adapters/abstract/connection_handler.rb:246:in `retrieve_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_handling.rb:287:in `retrieve_connection'
/usr/local/bundle/bundler/gems/rails-9c50861250dd/activerecord/lib/active_record/connection_handling.rb:254:in `connection'
/app/rakefile:24:in `block in <top (required)>'
/usr/local/bundle/gems/rake-13.1.0/exe/rake:27:in `<top (required)>'
/usr/local/bin/bundle:25:in `load'
/usr/local/bin/bundle:25:in `<main>'
Tasks: TOP => spec => clear
(See full trace by running task with --trace)

Line 24 in the rakefile that is causing the problem is
ActiveRecord::Base.connection.execute_structure_dump(ActiveRecord::Base.connection.full_drop). This appears to be related to the changes in rails/rails#44576 and rails/rails#44591. We no longer have a @raw_connection after calling ActiveRecord::Base.establish_connection(CONNECTION_PARAMS) on line 22, so trying work with ActiveRecord::Base.connection gives us the nil error.

3a288c3 addresses the error in starting the tests by:

  • change get_database_version to use valid_raw_connection which will connect if necessary
  • setting self.lock_thread = nil before the call to super in OracleEnhancedAdapter#initialize. In super they assign a visitor before self.lock_thread = nil. In oracle enhanced assigning a visitor requires a check of database_version, which with the new use of valid_raw_connection requires @lock to be set first.

After those changes, the test start, although with many errors. A couple of the more immediate ones are addressed here as well:

  • Revert "Merge pull request #48188 from eileencodes/revert-48069" rails/rails#48229 requires an internal_exec_query that is called by the abstract adapter's exec_query. Renaming oracle_enhanced's exec_query to internal_exec_query to address this
  • I saw lots of errors with active? because @raw_connection is nil. Using valid_raw_connection in active? results in a stack level too deep, so I adapted he postgres adapter's implementation.

We can't call @raw_connection right away anymore, because it is nil until
it is used after the changes in rails/rails#44576
and rails/rails#44591.

Also we need to have @lock established in order to be able to use
`with_raw_connection`. In the abstract_adapter implementation, that happens
after the arel_visitor is set, but in oracle enhanced, we need to use
`with_raw_connection` to check the database version in order to pick with
visitor to use. Calling lock_thread = nil before super means @lock isi
available to set the arel_visitor.

Rails rails/rails#48229 also requires an internal_exec_query
method that is called by the abstract adpater's exec_query. Naively renaming
the oracle-enhanced's exec_query to internal_exec_query makes the oracle
enhanced tests run.

I also saw errors due to `active?` using @raw_connection. `valid_raw_connection`
ends up calling `active?` via `with_raw_connection` and `verify` so that
can't be used as a replacement for @raw_connection here. Borrowing from
the postgres adapter implementation to get around this.
@andynu
Copy link
Contributor

andynu commented Apr 1, 2024

#2381 is running against rails 7-1-stable with passing tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants