Skip to content

Commit

Permalink
fix: handle new connection pools in before_all
Browse files Browse the repository at this point in the history
  • Loading branch information
palkan committed Dec 14, 2024
1 parent deb2a04 commit 01a9318
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 9 deletions.
11 changes: 7 additions & 4 deletions .github/workflows/rspec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@ jobs:
strategy:
fail-fast: false
matrix:
ruby: [2.7]
gemfile: ["gemfiles/activerecord6.gemfile"]
ruby: [3.3]
gemfile: ["gemfiles/activerecord8.gemfile"]
db: ["postgres"]
include:
- ruby: 3.3
gemfile: "gemfiles/activerecord8.gemfile"
db: "postgres"
multi_db: "true"
- ruby: 3.3
gemfile: "gemfiles/railsmaster.gemfile"
db: "mysql"
Expand All @@ -39,7 +43,7 @@ jobs:
db: "sqlite-file"
- ruby: 3.2
gemfile: "gemfiles/activerecord7.gemfile"
db: "sqlite"
db: "sqlite-file"
multi_db: "true"
- ruby: 3.1
gemfile: "gemfiles/activerecord7.gemfile"
Expand Down Expand Up @@ -95,4 +99,3 @@ jobs:
- name: Run RSpec
run: |
bundle exec rspec --force-color
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ else
gem "activerecord-jdbcsqlite3-adapter"
end

gem "pg"
gem "activerecord", "~> 7.0"
gem "actionview"
gem "actionpack"
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/activerecord7.gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
source 'https://rubygems.org'

gem "activerecord", "~> 7.0"
gem "activerecord", "~> 8.0"
gem "factory_bot"
gem "fabrication"
gem "sqlite3", "~> 2.0"
Expand Down
11 changes: 11 additions & 0 deletions gemfiles/activerecord8.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
source 'https://rubygems.org'

gem "activerecord", "~> 7.0"
gem "factory_bot"
gem "fabrication"
gem "sqlite3", "~> 2.0"
gem "sidekiq"
gem "timecop"
gem "pg"

gemspec path: '..'
2 changes: 1 addition & 1 deletion lib/test_prof/any_fixture/dump.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def finish(_event, _id, payload)
end

def commit
return unless defined?(:@file)
return unless instance_variable_defined?(:@file)

file.close

Expand Down
21 changes: 21 additions & 0 deletions lib/test_prof/before_all/adapters/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module ActiveRecord
class << self
if ::ActiveRecord::Base.connection.pool.respond_to?(:pin_connection!)
def begin_transaction
subscribe!
::ActiveRecord::Base.connection_handler.connection_pool_list(:writing).each do |pool|
pool.pin_connection!(true)
end
Expand All @@ -19,6 +20,26 @@ def rollback_transaction
::ActiveRecord::Base.connection_handler.connection_pool_list(:writing).each do |pool|
pool.unpin_connection!
end
unsubscribe!
end

def subscribe!
Thread.current[:before_all_connection_subscriber] = ActiveSupport::Notifications.subscribe("!connection.active_record") do |_, _, _, _, payload|
connection_name = payload[:connection_name] if payload.key?(:connection_name)
shard = payload[:shard] if payload.key?(:shard)
next unless connection_name

pool = ::ActiveRecord::Base.connection_handler.retrieve_connection_pool(connection_name, shard: shard)
next unless pool && pool.role == :writing

pool.pin_connection!(true)
end
end

def unsubscribe!
return unless Thread.current[:before_all_connection_subscriber]
ActiveSupport::Notifications.unsubscribe(Thread.current[:before_all_connection_subscriber])
Thread.current[:before_all_connection_subscriber] = nil
end
else
def all_connections
Expand Down
2 changes: 1 addition & 1 deletion lib/test_prof/recipes/minitest/before_all.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def included(base)
if base.respond_to?(:parallelize_teardown)
base.parallelize_teardown do
last_klass = ::Minitest.previous_klass
if last_klass&.respond_to?(:parallelized) && last_klass&.parallelized
if last_klass&.respond_to?(:parallelized) && last_klass.parallelized
last_klass.before_all_executor&.deactivate!
end
end
Expand Down
13 changes: 13 additions & 0 deletions spec/bugs/before_all_any_fixture_multidb_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

# https://github.com/test-prof/test-prof/issues/310
describe "before_all + any_fixture + multidb", type: :integration do
specify "works" do
output = run_rspec(
"before_all_any_fixture_multidb",
chdir: File.join(__dir__, "fixtures")
)

expect(output).to include("0 failures")
end
end
79 changes: 79 additions & 0 deletions spec/bugs/fixtures/before_all_any_fixture_multidb_fixture.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# frozen_string_literal: true

require "action_controller/railtie"
require "action_view/railtie"
require "active_record/railtie"
require "rspec/rails"

require_relative "../../support/ar_models"

RSpec.configure do |config|
config.fixture_path = File.join(__dir__, "fixtures")
config.use_transactional_fixtures = true
end

require "test_prof/recipes/rspec/before_all"
require "test_prof/recipes/rspec/let_it_be"
require "test_prof/recipes/rspec/any_fixture"

RSpec.configure do |config|
config.include TestProf::FactoryBot::Syntax::Methods

config.before(:suite) do
TestProf::AnyFixture.register(:user) { TestProf::FactoryBot.create(:user) }
end
end

# ActiveSupport::Notifications.subscribe("transaction.active_record") do |event|
# puts "[TRANSACTION] #{event.payload[:outcome].upcase} #{event.payload[:connection].inspect} from: #{caller_locations.find { |l| l.to_s.include?(Rails.root.to_s) }&.to_s}"
# end

# TestProf::BeforeAll.configure do |config|
# config.before(:begin) do
# puts "[BEFORE_ALL] connection pools #{::ActiveRecord::Base.connection_handler.connection_pool_list(:writing).map(&:inspect)}"
# end

# config.before(:rollback) do
# puts "[BEFORE_ALL] connection pools before unpin_connection! #{::ActiveRecord::Base.connection_handler.connection_pool_list(:writing).map(&:inspect)}"
# end
# end

describe "let_it_be vs lazy multi db" do
let_it_be(:user) { TestProf::FactoryBot.create(:user) }

# Loading an AR class with a custom DB configuration
# triggers a new connection pool creation that hasn't been tracked by
# before_all...
let!(:dual_comments_class) do
Class.new(ApplicationRecord) do
def self.name
"DualCommentsRecord"
end

self.abstract_class = true

connects_to database: {writing: :comments, reading: :primary} if multi_db?
end.then do |record_class|
Class.new(record_class) do
self.table_name = "comments"

belongs_to :user, dependent: :destroy
end
end
end

specify do
expect(User.count).to eq 2

dual_comments_class.create!(user: user, comment: "Hello!")
end

specify do
expect(TestProf::FactoryBot.create(:comment)).to be_present
expect(Comment.count).to eq 1
end

context "with clean fixture", :with_clean_fixture do
specify { expect(User.count).to eq 0 }
end
end
7 changes: 5 additions & 2 deletions spec/support/ar_models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ def multi_db?
db_comments_path = File.join(TestProf.config.output_dir, "testdb_comments.sqlite")
FileUtils.rm(db_comments_path) if File.file?(db_comments_path)
DB_CONFIG_COMMENTS = {adapter: "sqlite3", database: db_comments_path}
ActiveRecord::Base.configurations = {test: {comments: DB_CONFIG_COMMENTS, posts: DB_CONFIG}}
ActiveRecord::Base.configurations = {test: {primary: DB_CONFIG, comments: DB_CONFIG_COMMENTS}}

class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true

connects_to database: {writing: :posts, reading: :posts}
connects_to database: {writing: :primary, reading: :primary}
end

class CommentsRecord < ApplicationRecord
Expand Down Expand Up @@ -126,6 +126,9 @@ class Comment < ApplicationRecord
end
end

ActiveRecord::Base.establish_connection
CommentsRecord.establish_connection DB_CONFIG_COMMENTS if multi_db?

ActiveRecord::Base.logger =
if ENV["DEBUG"]
Logger.new($stdout)
Expand Down

0 comments on commit 01a9318

Please sign in to comment.