Skip to content
Snippets Groups Projects
Verified Commit 8decab7a authored by Stan Hu's avatar Stan Hu
Browse files

Disable Sidekiq worker open transaction check in JS specs

When transactional tests are enabled, Rails ensures that all threads
use the same connection for accessing the database. However this may
cause a false positive with the Sidekiq worker open transaction
check because:

1. A browser may have multiple requests in-flight.
2. A transaction may start in the middle of another request. For example,
an update to a merge request column initiates a transaction via SAVEPOINT.

To avoid false positives, disable this check for JavaScript feature
specs run. The alternative is to use the deletion strategy, but that
would significantly slow tests.

Relates to #478601
parent 6a195d48
No related branches found
No related tags found
1 merge request!163832Disable Sidekiq worker open transaction check in JS specs
......@@ -13,6 +13,16 @@ def self.skipping_transaction_check(&block)
end
def self.skip_transaction_check
# When transactional tests are in use, Rails calls
# ConnectionPool#lock_thread= to ensure all application threads
# get the same connection so they can all see the data in the
# uncommited transaction. If Puma is in use, check the state of
# the lock thread.
if ::Rails.env.test?
lock_thread = ::ApplicationRecord.connection_pool.instance_variable_get(:@lock_thread)
return true if lock_thread && lock_thread[:sidekiq_worker_skip_transaction_check]
end
Thread.current[:sidekiq_worker_skip_transaction_check]
end
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe 'Sidekiq::Worker' do
RSpec.describe 'Sidekiq::Worker', feature_category: :shared do
shared_examples_for 'a forbiddable operation within a transaction' do
it 'allows the operation outside of a transaction' do
expect { operation }.not_to raise_error
......@@ -14,7 +14,7 @@
end
end
it 'allows the oepration within a transaction if skipped' do
it 'allows the operation within a transaction if skipped' do
Sidekiq::Worker.skipping_transaction_check do
ApplicationRecord.transaction do
expect { operation }.not_to raise_error
......@@ -22,6 +22,20 @@
end
end
it 'allows the operation if lock thread is set' do
Sidekiq::Worker.skipping_transaction_check do
thread = Thread.new do
Thread.current.abort_on_exception = true
ApplicationRecord.transaction do
expect { operation }.not_to raise_error
end
end
thread.join
end
end
it 'forbids the operation if it is within a Ci::ApplicationRecord transaction' do
Ci::Pipeline.transaction do
expect { operation }.to raise_error(Sidekiq::Worker::EnqueueFromTransactionError)
......
......@@ -158,6 +158,21 @@
end
end
# When transactional tests are enabled, Rails ensures that all threads
# use the same connection for accessing the database. However this may
# cause a false positive with the Sidekiq worker open transaction
# check because:
#
# 1. A browser may have multiple requests in-flight.
# 2. A transaction may start in the middle of another request. For example,
# an update to a merge request column initiates a transaction via SAVEPOINT.
#
# To avoid false positives, disable this check. The alternative is to
# use the deletion strategy, but that would significantly slow tests.
config.around(:each, :js) do |example|
Sidekiq::Worker.skipping_transaction_check { example.run }
end
# The :capybara_ignore_server_errors metadata means unhandled exceptions raised
# by the application under test will not necessarily fail the server. This is
# useful when testing conditions that are expected to raise a 500 error in
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment