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!
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:
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 blockdescribe '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
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