Skip to content

Add Cop checking for unsafe use of class methods in tests

Matthias Käppler requested to merge 205382-add-rspec-class-mutation-cop into master

What does this MR do?

We had a failure on master (see #205382 (closed)) where a spec was mutating the ActiveRecord callback chain of model classes in tests, leading to knock-on effects in other tests since those changes persist across test boundaries.

This MR introduces a new Cop, RSpec/ClassMutation which looks for blacklisted method calls in specs that we know to result in unsafe behavior. Currently this list contains only AR related class methods, but it could be expanded on in the future.

The Cop currently yields false negatives, because not all of these method calls result in unsafe behavior. For instance, this is fine:

# in a spec
class MyTestModel
   before_save { ... }
end

It's safe because MyTestModel is specific to the test suite and not used outside of the test defining it. The Cop currently cannot detect such use, because that would require reflecting on the lexical scope and finding out whether we're defining a new class or not. There are myriad ways of doing that however (for instance, with Class.new) that all result in different ASTs, so in the spirit of MVC-ing this change, I suggest to KISS and add manual ignores for now where the Cop is identifying things wrongly.

Does this MR meet the acceptance criteria?

Conformity

Edited by 🤖 GitLab Bot 🤖

Merge request reports