-
-
Notifications
You must be signed in to change notification settings - Fork 277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unclosed sidekiq testing blocks carrying the testing mode across specs. #1374
Comments
RuboCop focuses on the Ruby programming language. I'm not sure whether this proposal will be accepted, anyway I will transfer it to RuboCop RSpec. |
thanks @koic , ideally it should have been under rubocop/rspec only, My bad. |
Even though it's related to testing, we mostly keep cops that look after the RSpec coding style, and related test libraries like FactoryBot, Capybara, and RSpec Rails with their DSLs. I'm on the fence if to accept such a cop, or to suggest creating a custom cop in a separate repo. Or in some repo that combines RSpec and Sidekiq, but both that I know of are unmaintained (1, 2). I'd suggest a workaround that wouldn't need a cop: # spec/rails_helper.rb
RSpec.configure do |config|
config.before do
Sidekiq::Testing.fake!
end
end Would this be an acceptable solution to prevent leaks? |
@pirj the workaround will not work as if any other testing block goes unclosed in any test, all the other tests will be overridden by unclosed testing block mode. |
Why? To get you going, here's a template: require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
gem "rspec"
gem "sidekiq"
end
require 'rspec'
require 'sidekiq'
require 'sidekiq/testing'
RSpec.configure do |config|
config.before do
Sidekiq::Testing.fake!
end
end
require 'rspec/autorun'
RSpec.describe 'Sidekiq::Testing leak demonstration', order: :defined do
it 'leaks' do
Sidekiq::Testing.inline!
# ...
end
it 'suffers from a leak in some way' do
# expect(...).to ... # => BOOM
end
end In any case, |
oh got it , configuration at project level was wrong for me, i was configuring at project level which was not setting the scope correctly. After doing the configuration correctly its working even without explicit definition at file level. |
Good to know you've sorted it out. |
Problem
Problem is with unclosed sidekiq testing blocks in Rspec.
Explanation
By default Sidekiq testing block is fake and we have configure it explicitly in spec helper as well. Everything is going good, unless someone in other spec file write a test with unclosed
testing block in Disable! mode. With this the mode will be set across the files unless someone explicitly changes it to fake or other mode.
Since disable mode enqueues job to Redis it will start failing when we run spec for the project , for the test cases where someone is not testing anything under the fake! block as it is presumed that it has been set as default
Solution
We raise offense when someone keeps a testing block open.
Bad code:
Good Code:
Approach to fix
I have fixed this problem for our repo by allowing the ast to pass if the souce line ends with do else raise a block.
We can make this configurable as well by setting something like:
allow_unclsoed_sidekiq_testing_blocks: true/false
If the issue gets feature-request tag i am willing to raise a PR to get this feature. ❤️
The text was updated successfully, but these errors were encountered: