Cop idea: Dealing with Time

[ydakuka]

Reference: https://rspec.rubystyle.guide/#dealing-with-time

Actual behavior

I have the code:

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe InvoiceReminder do
  subject(:time_with_offset) { described_class.new.get_offset_time }

  it 'offsets the time 2 days into the future' do
    current_time = Time.current
    allow(Time).to receive(:current).and_return(current_time)
    expect(time_with_offset).to eq(current_time + 2.days)
  end
end

I run rubocop and don't get any offences:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/invoice_reminder_spec.rb 
Inspecting 1 file
.

1 file inspected, no offenses detected

Expected behavior

I expected to receive a suggestion to use timecop when rubocop had detected expressions like allow(Time).to receive(:current).and_return(current_time).

Rubocop

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop -V
1.54.1 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 2.7.8) [x86_64-linux]
  - rubocop-capybara 2.18.0
  - rubocop-factory_bot 2.23.1
  - rubocop-performance 1.18.0
  - rubocop-rails 2.20.2
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.22.0
  - rubocop-thread_safety 0.5.1

[ydah]

Perhaps this issue would be better handled by rubocop-rspec.
Can someone from the RuboCop Core team please move this issue to rubocop-rspec?

[ydakuka]

This rule should respect the following rule for rails: https://rails.rubystyle.guide/#freeze-time

[ydah]

This rule should respect the following rule for rails: https://rails.rubystyle.guide/#freeze-time

I don't think so. Because RuboCop RSpec users don't necessarily use Rails.

[ydakuka]

I don't think so. Because RuboCop RSpec users don't necessarily use Rails.

I mean there should be a separate cop for rails that forces the use of built-in helpers instead of Timecop.

[pirj]

Thanks for bringing this up.
It is wip here rubocop/rubocop-rails#38

[ydakuka]

@pirj ok, but

RuboCop RSpec users don't necessarily use Rails.

[pirj]

That’s right. We can’t suggest using ActiveSupport.

Should we recommend Timecop when time is stubbed? I don’t know.

[ydakuka]

Should we recommend Timecop when time is stubbed? I don’t know.

I don't know either, but this is what was recommended by the rspec style guide.

[pirj]

Idk if this still stands. What are the consequences if stubbing time manually?

[ydakuka]

I see that this rule has been added in the first commit, but then it got the modern syntax with the RSpec 3 release.

I think this rule was relevant only for the old syntax. explanation

[ydakuka]

I've tried the old syntax and rubocop didn't get any offenses (except Rails/TimeZone):

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe InvoiceReminder do
  subject(:reminder) { described_class.new }

  it 'offsets the time 2 days into the future' do
    current_time = Time.now # rubocop:disable Rails/TimeZone
    Time.stub(:now).and_return(current_time)
    reminder.get_offset_time.should be_the_same_time_as(current_time + 2.days)
  end
end

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/invoice_reminder_spec.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

[pirj]

We obviously have bo intention to support the legacy RSpec 2 syntax.

But my question has a different. Why not stub time manually?
Timecop stabs Time/Date/DateTime in a single statement. Is that all?
Is stubbing ‘now’ sufficient to affect ‘current’ etc?

I don’t have an answer to this. In any case, I’m opposed to recommend Timecop, as those thread safety issues I’ve been researching were quite nasty.

Do you have an action plan in mind, @ydakuka ?