Skip to content
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

Differentiating between test constants and context variables #127

Open
boardfish opened this issue Oct 31, 2022 · 1 comment
Open

Differentiating between test constants and context variables #127

boardfish opened this issue Oct 31, 2022 · 1 comment

Comments

@boardfish
Copy link
Contributor

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?

@pirj
Copy link
Member

pirj commented Oct 31, 2022

If you use a method that mutates the constant, this mutation will be persisted across future tests

Correct.

variables defined with let will 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

  1. it's tempting to make a constant a constant:
SOME_TEST_CONSTANT = "some_value"

that would leak into the global context.

  1. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants