Minitest hooks should be respected by default

[sambostock]

What Ruby, Rails and RSpec versions are you using?

Ruby version: 2.6.5
Rails version: 6.0.2.1
RSpec version: 3.9

ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin19]
Rails 6.0.2.1
RSpec 3.9
  - rspec-core 3.9.1
  - rspec-expectations 3.9.0
  - rspec-mocks 3.9.1
  - rspec-rails 3.9.0
  - rspec-support 3.9.2

Observed behaviour

Test helpers provided by Rails should be useable out of the box. Specifically, their Minitest lifecycle hooks should be honoured automatically, by default.

Expected behaviour

Some test helpers do not work out of the box, as their lifecycle hooks are not called.

Specifically, ActiveSupport::Testing::TimeHelpers can be included, and freeze_time will work, but TimeHelpers#after_teardown is not called, meaning it doesn't clean up after itself as it is supposed to.

To get it to work requires including RSpec::Rails::MinitestLifecycleAdapter, which is not obvious, and error prone.

See in-depth discussion and motivation in rubocop/rubocop-rails#38.

Can you provide an example app?

I have constructed an example usage of ActiveSupport::Testing::TimeHelpers#freeze_time in this repository.

The test does not "fail", due to the difficulty of testing test hooks. However, it puts messages which indicate if it is working or not.

TL;DR

describe 'TimeHelpers#freeze_time' do
  it 'works' { freeze_time }
end

The test above doesn't work unless the following is added to spec/rails_helper.rb

  config.include ActiveSupport::Testing::TimeHelpers

This is unfortunate, but acceptable. However, the TimeHelpers#after_teardown hook fails to run unless we also add

config.include RSpec::Rails::MinitestLifecycleAdapter

This means time is left frozen after the test, and violates the contract of freeze_time, which is supposed to always cleanup.

[JonRowe]

👋 We do include this module, and a bunch of others however you must be in a rails example group for it to work. Can you demonstrate that freeze_time doesn't work in any supported type: <type> spec?

Otherwise its the same as expecting freeze_time to work in plain code, you have to be in rails context for it to work.

[sambostock]

Thanks for the info @JonRowe! Forgive my unfamiliarity with RSpec(-Rails). I'm coming from almost exclusively using Minitest, but wanting to make sure that rubocop/rubocop-rails#38 is appropriate for users of RSpec as well.

I have, I believe, created example groups using freeze_time inside type: <type> specs. I have added them as a separate commit to my demo repository.

[:model, ...].each do |type|
  describe "TimeHelpers (type: :#{type})", type: type do
    it 'allows calling freeze_time' do
      freeze_time
    end
  end
end

However, when the specs are run, the following error occurs, for every example:

undefined local variable or method `freeze_time' for #<RSpec::ExampleGroups::TimeHelpers:0x00007fd0191c4288>

As far as I can tell, these examples are in line with the docs, other than file location (though I have tried that too). Let me know if I'm missing something.

[pirj]

@sambostock I believe you have to include ActiveSupport::Testing::TimeHelpers, as rspec-rails might not do it for you, easiest for the sake of testing is:

  describe "TimeHelpers (type: :#{type})", type: type do
+   include ActiveSupport::Testing::TimeHelpers

[JonRowe]

I'd expect @pirj's solution to work out of the box @sambostock if it doesn't let us know?

As an aside I would do:

[:model, ...].each do |type|
  config.include ActiveSupport::Testing::TimeHelpers, type: type
end

If you want it always available, but I'm unsure I want to add this as a default, as I don't personally believe freezing time is a good idea.

[pirj]

@JonRowe I believe it's mostly about recommending ActiveSupport::Testing::TimeHelpers instead of Timecop, and since Timecop has to be added by users explicitly, to be on par it's not necessary at all to include it in rspec-rails' defaults, it's sufficient to mention that in the output.
rubocop/rspec-style-guide#71

[pirj]

@sambostock I've updated your example app in the following way:

# spec/time_helper_support_spec.rb
require 'rails_helper'

describe 'TimeHelpers', type: :model, order: :defined do
  it 'freezes and sets time' do
    freeze_time

    $time = Time.now
    sleep 0.01
    expect(Time.now).to eq($time)
  end

  it 'has a different time' do
    sleep 0.01
    expect($time).not_to be(nil)
    expect(Time.now).not_to eq($time)
  end
end

# spec/rails_helper.rb
-  config.include RSpec::Rails::MinitestLifecycleAdapter

I believe since the cop mentions a caveat to explicitly include TimeHelpers where needed, o on this front we're all good.
Maybe it makes sense to change that note to only include it in Rails types of specs like Jon mentions above?

RSpec.configure do |config|
  %i[channel controller helper job mailer model request routing view feature system mailbox].each do |type|
    config.include ActiveSupport::Testing::TimeHelpers, type: type
  end
end

There doesn't seem to be anything to be fixed on RSpec side.

[enowmbi]

including 'rails_helper' in my spec file worked in my case!