Unclosed sidekiq testing blocks carrying the testing mode across specs.

[krsinghshubham]

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:

     Sidekiq::Testing.inline! 
     3.times { Sidekiq::RandomWorker.enqueue }

Good Code:

    Sidekiq::Testing.disable! do
        3.times { Sidekiq::GlobalWorker.enqueue }
     end

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. ❤️

[koic]

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.

[krsinghshubham]

thanks @koic , ideally it should have been under rubocop/rspec only, My bad.

[pirj]

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?

[krsinghshubham]

@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. In fact we already had the default mode set in RSpec but other dev left one of the other testing mode block opened which failed few of the new tests when we ran RSpec for the whole project.

[pirj]

if any other testing block goes unclosed in any test, all the other tests will be overridden by unclosed testing block mode

Why? config.before (and its default scope is :example/:each) would run before each test. Even if the previous test have set the mode to "inline", the hook (config.before) would set the mode back to "fake" for the new example (test). Can you provide a runnable example that indicates side effects, @krsinghshubham ?

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, rubocop-rspec is not the best place for such a cop, as we exclusively inspect RSpec-related tools, not production-related tools, even if we're talking about their testing-related (not strictly RSpec-, sometimes Minitest-related!) parts.

[krsinghshubham]

It will not leak if we configure it at file level but will leak if we configure the RSpec at project level.

explicit configuration at file

when defined at project level

but totally agree to not include this cop here.

[krsinghshubham]

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.

[pirj]

RSpec.configure do is done in spec/spec_helper.rb/spec/rails_helper.rb, so it's project level. I do not suggest to use RSpec.configure in the beginning of the spec files, I only used it for you to provide a single-file self-contained example.

Good to know you've sorted it out.