rspec-style-guide
rspec-style-guide copied to clipboard
Differentiating between test constants and context variables
I had an interesting discussion on a PR just now. Lately I've taken to using assignment to indicate things that won't change between tests:
describe Something do
# This won't change between contexts, but we want to reuse it between tests
some_test_constant = "some_value"
# This will change between contexts
let(:weather) { "sunny" }
end
However, there's a caveat with this. If you use a method that mutates the constant, this mutation will be persisted across future tests. This isn't the case when you use let, because the state of this variable will be reset between tests. However, it looks to me like variables defined with let will be changed between contexts, so I'm hesitant to use it for things like this, especially for fear of setting off the MultipleMemoizedVariables cop.
Any thoughts here? Does RSpec have something in its kit to address this already, or should something be introduced?
If you use a method that mutates the constant, this mutation will be persisted across future tests
Correct.
variables defined with
letwill be changed between contexts
You can say that is "reset", even though none of this is exactly correct. The following code would explain better how it works:
class C
a = 'hi'
define_method(:p) { puts a }
define_method(:r) { a.reverse! }
end
x = C.new
y = C.new
x.p # => hi
y.p # => hi
x.r
y.p # => ih
# BOOM
let, on the contrary, is defined as a memoized instance method, and it is not shared between different examples (instances x and y).
Speaking of the RSpec/MultipleMemoizedHelpers cop, if you take a look at a commit message that enables the cop by default. There's no one style that fits all. If you feel that writing in the style you are comfortable with triggers RuboCop, it's time to discuss the style with your colleagues and adjust RuboCop settings in .rubocop.yml. The file is there in the first place to let users adjust the defaults to their liking.
Does RSpec have something in its kit to address this already, or should something be introduced?
RSpec doesn't, and will unlikely add any countermeasure.
I'd suggest freezing what is possible, e.g. some_test_constant = "some_value".freeze or by adding frozen_string_literal: true directive. For collections like Array/Hash/Set, .freeze would also work.
In general, there's nothing wrong with this approach, except
- it's tempting to make a constant a constant:
SOME_TEST_CONSTANT = "some_value"
that would leak into the global context.
- it's tempting to overuse it for non-literal constants
some_shared_model = FactoryBot.create(:location, name: 'New-York')
A similar problem with before(:context) hooks is described in rspec-rails docs:
Data created in before(:context) are not rolled back before(:context) hooks are invoked before the transaction is opened. You can use this to speed things up by creating data once before any example in a group is run, however, this introduces a number of complications and you should only do this if you have a firm grasp of the implications.
There is a semi-related WIP cop.
I encourage you to send a PR adding a note to this style guide to be extra careful with purposed-to-be-constant variables defined on the example group level.