rubocop-rails icon indicating copy to clipboard operation
rubocop-rails copied to clipboard

Suggested cop: Prefer built-in time helpers to Timecop

Open andyw8 opened this issue 7 years ago • 4 comments

Most of Timecop's behaviour is now built into Rails.

https://api.rubyonrails.org/v5.1/classes/ActiveSupport/Testing/TimeHelpers.html

Related: https://github.com/rubocop-hq/rspec-style-guide/issues/71

andyw8 avatar Nov 30 '18 16:11 andyw8

~This can be closed~. The cop moved to another extension https://github.com/rubocop/rubocop-rspec_rails/pull/48

pirj avatar Aug 07 '24 23:08 pirj

But not everybody uses RSpec and, afaik, rails new defaults to minitest. So adding this to rubocop-rails over ruboyopc-rspec_rails seems highly desirable

TobiasBales avatar Jan 27 '25 12:01 TobiasBales

Fair enough!

Actually, a cop targeting minitest would be be a better fit, as TimeHelpers are (to my best understanding) always included, and this means that it tears down automatically. Which is, unfortunately, not so simple with RSpec.

pirj avatar Jan 27 '25 17:01 pirj

cop targeting minitest would be be a better fit

What do you mean by this?


Context

Enforcing TimeHelpers over Timecop is indeed straightforward when the test is using ActiveSupport::TestCase (or some subclass), as it includes TimeHelpers, relies on the after_teardown Minitest lifecycle hook (which makes sense, as TestCase inherits directly from Minitest::Test).

For RSpec specs using rails_helper, the Minitest compatibility layer is present, so after_teardown runs and using TimeHelpers is safe, but as you allude to, most of the uncertainty that held up #38 was dealing with the edge case of specs using spec_helper only, which does not load the compatibility layer (unless the host app does so), meaning TimeHelpers does not clean up after itself.

So yes, not having to think about RSpec dramatically simplifies things, but given that Minitest is the default (via ActiveSupport::TestCase), AFAIK there is no separate rubocop-minitest_rails equivalent to rubocop-rspec_rails.

The simplest thing would probably be for the cop to be disabled by default if the project is using RSpec, but expose a config to override that if the project maintainer chooses to do so (e.g. if they add the Minitest lifecycle hooks compatibility layer to their spec_helper).

Alternatively, rubocop-rails could add the cop, and rubocop-rspec_rails could disable it by default.


But unless something has changed, the conclusion in #38 (although I still disagree) was that a Rails/Timecop cop is not appropriate for rubocop-rails[^1], and I don't know that anyone wants to create/maintain a gem[^2] just for this one cop, so I don't see a way forward.

[^1]: I will gladly re-open the PR if this changes. [^2]: Neither rubocop-timecop nor rubocop-rails-timecop seem appropriate, as they would have poor discoverability, and do not fit into the pattern of rubocop-timecop provides cops for users of Timecop, as its only cop would be one that says not to use Timecop.

sambostock avatar Jan 29 '25 22:01 sambostock