summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeinrich Lee Yu <heinrich@gitlab.com>2019-04-26 23:49:19 +0800
committerHeinrich Lee Yu <heinrich@gitlab.com>2019-07-12 10:39:14 +0800
commitd4a919679a1eb5d3e2aaed4b920e6027d2482971 (patch)
treecdaaeb66e7de69a71994e08dd65478cab364fa24
parentecffca5d92353d55aaf8f984737fa617782310e0 (diff)
downloadgitlab-ce-d4a919679a1eb5d3e2aaed4b920e6027d2482971.tar.gz
Use transactions in JS feature specsjs-specs-transactions
Uses Rails transactional tests instead of DatabaseCleaner transaction strategy because that doesn't work with JS tests
-rw-r--r--config/environments/test.rb2
-rw-r--r--config/initializers/active_record_query_cache.rb3
-rw-r--r--lib/gitlab/database.rb25
-rw-r--r--lib/gitlab/patch/active_record_query_cache.rb39
-rw-r--r--spec/spec_helper.rb12
-rw-r--r--spec/support/database_cleaner.rb27
-rw-r--r--spec/support/db_cleaner.rb4
-rw-r--r--spec/support/shared_contexts/email_shared_context.rb2
8 files changed, 85 insertions, 29 deletions
diff --git a/config/environments/test.rb b/config/environments/test.rb
index e7166882eea..bd35bdd8d5a 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -40,7 +40,7 @@ Rails.application.configure do
# Print deprecation notices to the stderr
config.active_support.deprecation = :stderr
- config.eager_load = false
+ config.eager_load = true
config.cache_store = :null_store
diff --git a/config/initializers/active_record_query_cache.rb b/config/initializers/active_record_query_cache.rb
new file mode 100644
index 00000000000..61505a1edd3
--- /dev/null
+++ b/config/initializers/active_record_query_cache.rb
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+ActiveRecord::ConnectionAdapters::ConnectionPool.prepend Gitlab::Patch::ActiveRecordQueryCache
diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb
index 1177f8ea99e..299a8005f42 100644
--- a/lib/gitlab/database.rb
+++ b/lib/gitlab/database.rb
@@ -284,17 +284,28 @@ module Gitlab
end
# inside_transaction? will return true if the caller is running within a transaction. Handles special cases
- # when running inside a test environment, in which the entire test is running with a DatabaseCleaner transaction
+ # when running inside a test environment, where tests may be wrapped in transactions
def self.inside_transaction?
- ActiveRecord::Base.connection.open_transactions > open_transactions_baseline
+ if Rails.env.test?
+ ActiveRecord::Base.connection.open_transactions > open_transactions_baseline
+ else
+ ActiveRecord::Base.connection.open_transactions > 0
+ end
end
- def self.open_transactions_baseline
- if ::Rails.env.test?
- return DatabaseCleaner.connections.count { |conn| conn.strategy.is_a?(DatabaseCleaner::ActiveRecord::Transaction) }
- end
+ # These methods that access @open_transactions_baseline are not thread-safe.
+ # These are fine though because we only call these in RSpec's main thread. If we decide to run
+ # specs multi-threaded, we would need to use something like ThreadGroup to keep track of this value
+ def self.set_open_transactions_baseline
+ @open_transactions_baseline = ActiveRecord::Base.connection.open_transactions
+ end
+
+ def self.reset_open_transactions_baseline
+ @open_transactions_baseline = 0
+ end
- 0
+ def self.open_transactions_baseline
+ @open_transactions_baseline ||= 0
end
private_class_method :open_transactions_baseline
diff --git a/lib/gitlab/patch/active_record_query_cache.rb b/lib/gitlab/patch/active_record_query_cache.rb
new file mode 100644
index 00000000000..71d66bdbe02
--- /dev/null
+++ b/lib/gitlab/patch/active_record_query_cache.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+# Fixes a bug where the query cache isn't aware of the shared
+# ActiveRecord connection used in tests
+# https://github.com/rails/rails/issues/36587
+
+# To be removed with https://gitlab.com/gitlab-org/gitlab-ce/issues/64413
+
+module Gitlab
+ module Patch
+ module ActiveRecordQueryCache
+ # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ def enable_query_cache!
+ @query_cache_enabled[connection_cache_key(current_thread)] = true
+ connection.enable_query_cache! if active_connection?
+ end
+
+ def disable_query_cache!
+ @query_cache_enabled.delete connection_cache_key(current_thread)
+ connection.disable_query_cache! if active_connection?
+ end
+
+ def query_cache_enabled
+ @query_cache_enabled[connection_cache_key(current_thread)]
+ end
+
+ def active_connection?
+ @thread_cached_conns[connection_cache_key(current_thread)]
+ end
+
+ private
+
+ def current_thread
+ @lock_thread || Thread.current
+ end
+ # rubocop:enable Gitlab/ModuleWithInstanceVariables
+ end
+ end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 62fdc039b5e..0f27616a3a3 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -47,7 +47,7 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }
quality_level = Quality::TestLevel.new
RSpec.configure do |config|
- config.use_transactional_fixtures = false
+ config.use_transactional_fixtures = true
config.use_instantiated_fixtures = false
config.fixture_path = Rails.root
@@ -289,6 +289,16 @@ RSpec.configure do |config|
config.before(:each, :https_pages_disabled) do |_|
allow(Gitlab.config.pages).to receive(:external_https).and_return(false)
end
+
+ # We can't use an `around` hook here because the wrapping transaction
+ # is not yet opened at the time that is triggered
+ config.prepend_before do
+ Gitlab::Database.set_open_transactions_baseline
+ end
+
+ config.append_after do
+ Gitlab::Database.reset_open_transactions_baseline
+ end
end
# add simpler way to match asset paths containing digest strings
diff --git a/spec/support/database_cleaner.rb b/spec/support/database_cleaner.rb
index edd7de94203..f0dd6c52b74 100644
--- a/spec/support/database_cleaner.rb
+++ b/spec/support/database_cleaner.rb
@@ -26,31 +26,22 @@ RSpec.configure do |config|
end
config.append_after(:context) do
- DatabaseCleaner.clean_with(:deletion, cache_tables: false)
+ delete_from_all_tables!
end
- config.before do
- setup_database_cleaner
- DatabaseCleaner.strategy = :transaction
- end
+ config.around(:each, :delete) do |example|
+ self.class.use_transactional_tests = false
- config.before(:each, :js) do
- DatabaseCleaner.strategy = :deletion, { except: deletion_except_tables, cache_tables: false }
- end
+ example.run
- config.before(:each, :delete) do
- DatabaseCleaner.strategy = :deletion, { except: deletion_except_tables, cache_tables: false }
+ delete_from_all_tables!(except: deletion_except_tables)
end
- config.before(:each, :migration) do
- DatabaseCleaner.strategy = :deletion, { cache_tables: false }
- end
+ config.around(:each, :migration) do |example|
+ self.class.use_transactional_tests = false
- config.before do
- DatabaseCleaner.start
- end
+ example.run
- config.append_after do
- DatabaseCleaner.clean
+ delete_from_all_tables!
end
end
diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb
index c69fa322073..08622dff6d9 100644
--- a/spec/support/db_cleaner.rb
+++ b/spec/support/db_cleaner.rb
@@ -1,4 +1,8 @@
module DbCleaner
+ def delete_from_all_tables!(except: nil)
+ DatabaseCleaner.clean_with(:deletion, cache_tables: false, except: except)
+ end
+
def deletion_except_tables
[]
end
diff --git a/spec/support/shared_contexts/email_shared_context.rb b/spec/support/shared_contexts/email_shared_context.rb
index 9d806fc524d..4f5d53f9317 100644
--- a/spec/support/shared_contexts/email_shared_context.rb
+++ b/spec/support/shared_contexts/email_shared_context.rb
@@ -1,5 +1,3 @@
-require 'gitlab/email/receiver'
-
shared_context :email_shared_context do
let(:mail_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
let(:receiver) { Gitlab::Email::Receiver.new(email_raw) }