skålfyfan
skålfyfan

Reputation: 5281

Best way to generate dynamic tests that don't leak using RSpec (LeakyConstantDeclaration issue)?

Am taking over some Ruby code which includes a fairly large test suite. One thing that's been missing that I'm adding is rubocop to fix some problems. One thing I've noticed is that numerous tests have been set up to be dynamically generated in a way like so:

describe 'some test' do
  SOME_CONSTANT = { some hash }

  SOME_CONSTANT.each do |param1, param2|
    it 'does #{param1} and successfully checks #{param2} do
      # do something with #{param1} or #{param2}
      expect(param2).to eq "blahblah"
    end
  end
end

The issue here is SOME_CONSTANT. With rubocop this fails the RSpec/LeakyConstantDeclaration cop rule. The way these tests are set up these constants can re-assign a global constant by accident and result in random spec failures elsewhere if folks aren't paying attention.

The only workable solution to I've found is to change these constants into instance variables. For example:

describe 'some test' do
  @some_constant = { some hash }

  @some_constant.each do |param1, param2|
    it 'does #{param1} and successfully checks #{param2} do
      # do something with #{param1} or #{param2}
      expect(param2).to eq "blahblah"
    end
  end
end

There is a danger that these instance vars can leak into other it/example specs too (within the same spec file if a single test changes it), but at least it's limited to the individual *_spec.rb files, and won't impact global scope of the entire test suite. This also fixes the RSpec/LeakyConstantDeclaration.

Would anyone have any better suggestions? One that does not use instance variables, and is more modern RSpec friendly? I've tried using let, and let! but the way the tests are setup any variables set this way are only accessible within the it blocks. Have also tried using stub_const in a before(:context) block, but run into the same issue where the stubbed constant is only accessible within the it/example context. I also even tried RSpec.Mocks.with_temporary_scope and same issue. Instance variables seem to be the only thing that works in this set up.

Thanks in advance for any helpful suggestions!

Upvotes: 3

Views: 970

Answers (2)

melcher
melcher

Reputation: 1601

Agree w/ Jay, I try to steer away from dynamic tests and prefer to make them simpler and explicit (if more repetitive). However that may be more of a refactor than you're looking to take on. To only address the issue of variable names I would consider not assigning the data to a variable at all e.g:

[:each, :attribute, :to, :test].each do |attr|
  it "does a thing with #{attr}"...
end

and if it's a larger array/hash:

{ 
  some 
  larger
  hash 
}.each do |param1, param2|
  it 'does #{param1} and successfully checks #{param2}' do
    # do something with #{param1} or #{param2}
    expect(param2).to eq "blahblah"
  end
end

Upvotes: 0

Jay Dorsey
Jay Dorsey

Reputation: 3662

The example you provided feels a little too much like programming, rather than a test; e.g. I'd try to be a little bit more explicit rather than adding a loop of the behaviors.

Two constructs come to mind:

  • Using the let (which I know you said you did), but with additional nesting. IIRC, you might be able to add another describe block outside of your each block
  • Shared examples. This is what I'd try first. I added some pseudocode below
describe 'my test' do
  shared_examples 'does the thing' do |arg1, arg2|
    before { arg1.call }

    it "does #{arg1} and returns #{arg2}" do
      expect(arg2).to eq(true)
    end
  end

  it_behaves_like 'does the thing', Foo.new, bar
  it_behaves_like 'does the thing', Blarge.new, blah
end

You can also combine these with lets and/or a block (and refactor the shared example to reference these rather than passing in the arguments explicitly

it_behaves_like 'does the thing' do
  let(:method) { :foo }
  let(:result) { 42 }
end

Upvotes: 0

Related Questions