From e201a09959f48a4e68562d53b91b18a04acae3ce Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 12 Feb 2016 11:47:44 -0700 Subject: [PATCH] spec: fix broken transactions from preventing error reporting this is only necessary in test, when we have the fixture transaction also adjust our detection of "real" transaction in test Change-Id: I81960f62f9656d709face95cf1288c397d0f5773 Reviewed-on: https://gerrit.instructure.com/72161 Tested-by: Jenkins Reviewed-by: Simon Williams Product-Review: Cody Cutrer QA-Review: Cody Cutrer --- config/initializers/active_record.rb | 5 +++-- spec/spec_helper.rb | 30 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/config/initializers/active_record.rb b/config/initializers/active_record.rb index 450a8c4c59a..782e17f8614 100644 --- a/config/initializers/active_record.rb +++ b/config/initializers/active_record.rb @@ -699,7 +699,8 @@ ActiveRecord::Relation.class_eval do (connection.adapter_name == 'PostgreSQL' && (Shackles.environment == :slave || connection.readonly? || - connection.open_transactions > (Rails.env.test? ? 1 : 0))) + (!Rails.env.test? && connection.open_transactions > 0) || + in_transaction_in_test?)) end def find_in_batches_with_cursor(options = {}) @@ -737,7 +738,7 @@ ActiveRecord::Relation.class_eval do # changing the name (and source location) of this method transaction_regex = /^#{Regexp.escape(transaction_method)}:\d+:in `transaction/.freeze # transactions due to spec fixtures are _not_in the callstack, so we only need to find 1 - !!caller.find { |s| s =~ transaction_regex } + !!caller.find { |s| s =~ transaction_regex && !s.include?('spec_helper.rb') } end def find_in_batches_with_temp_table(options = {}) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d916b062f57..d3a23592e3b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -55,6 +55,36 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } ActionView::TestCase::TestController.view_paths = ApplicationController.view_paths +# this makes sure that a broken transaction becomes functional again +# by the time we hit rescue_action_in_public, so that the error report +# can be recorded +ActionController::Base.set_callback(:process_action, :around, ->(_r, block) do + exception = nil + ActiveRecord::Base.transaction(joinable: false, requires_new: true) do + begin + if Rails.version < '5' + # that transaction didn't count as a "real" transaction within the test + test_open_transactions = ActiveRecord::Base.connection.instance_variable_get(:@test_open_transactions) + ActiveRecord::Base.connection.instance_variable_set(:@test_open_transactions, test_open_transactions.to_i - 1) + begin + block.call + ensure + ActiveRecord::Base.connection.instance_variable_set(:@test_open_transactions, test_open_transactions) + end + else + block.call + end + rescue ActiveRecord::StatementInvalid + # these need to properly roll back the transaction + raise + rescue + # anything else, the transaction needs to commit, but we need to re-raise outside the transaction + exception = $! + end + end + raise exception if exception +end) + module RSpec::Core::Hooks class AfterContextHook < Hook def run(example)