FIX: Flaky spec due to incorrect Rack response body (#24640)

Why was the problem?

ActiveRecord's query cache for the connection pool wasn't disabled after the
`with a fake provider runs 'other_phase' for enabled auth methods` test
in `omniauth_callbacks_controller_spec.rb` was run. This was because the
Rack response body in `FakeAuthenticator::Strategy::other_phase` did not
adhere to the expected Rack body format which is "typically an Array of
String instances". Because this expectation was broken, it cascaded the
problem down where it resulted in the ActiveRecord's query cache for the
connection pool not being disabled as it normally should when the
response body is closed.

When the query cache is left enabled, common assertions pattern in RSpec
like `expect { something }.to change { Group.count }` will fail since
the query cache is enabled and the call first call to `Group.count` will
cache the result to be reused later on.

To see the bug in action, one can run the following command:

`bundle exec rspec --seed 44747
spec/requests/omniauth_callbacks_controller_spec.rb:1150
spec/models/group_spec.rb:283`
This commit is contained in:
Alan Guo Xiang Tan 2023-11-30 10:49:55 +08:00 committed by GitHub
parent c58cd697d2
commit 50bafd48cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 5 additions and 5 deletions

View File

@ -3048,19 +3048,19 @@ RSpec.describe User do
it "tracks old user record correctly" do
expect do user.update_ip_address!("127.0.0.1") end.to change {
UserIpAddressHistory.uncached { UserIpAddressHistory.where(user_id: user.id).count }
UserIpAddressHistory.where(user_id: user.id).count
}.by(1)
freeze_time 10.minutes.from_now
expect do user.update_ip_address!("0.0.0.0") end.to change {
UserIpAddressHistory.uncached { UserIpAddressHistory.where(user_id: user.id).count }
UserIpAddressHistory.where(user_id: user.id).count
}.by(1)
freeze_time 11.minutes.from_now
expect do user.update_ip_address!("127.0.0.1") end.to_not change {
UserIpAddressHistory.uncached { UserIpAddressHistory.where(user_id: user.id).count }
UserIpAddressHistory.where(user_id: user.id).count
}
expect(
@ -3070,7 +3070,7 @@ RSpec.describe User do
freeze_time 12.minutes.from_now
expect do user.update_ip_address!("0.0.0.1") end.not_to change {
UserIpAddressHistory.uncached { UserIpAddressHistory.where(user_id: user.id).count }
UserIpAddressHistory.where(user_id: user.id).count
}
expect(UserIpAddressHistory.where(user_id: user.id).pluck(:ip_address).map(&:to_s)).to eq(

View File

@ -1108,7 +1108,7 @@ RSpec.describe Users::OmniauthCallbacksController do
class Strategy
include OmniAuth::Strategy
def other_phase
[418, {}, "I am a teapot"]
[418, {}, ["I am a teapot"]]
end
end