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

Rails/FreezeTime gives invalid suggestion when travel_to is passed future or past dates #769

Closed
rmm5t opened this issue Sep 10, 2022 · 3 comments · Fixed by #770
Closed
Labels
bug Something isn't working

Comments

@rmm5t
Copy link
Contributor

rmm5t commented Sep 10, 2022

Rails/FreezeTime incorrectly suggests changes when travel_to is used with an argument in the past or the future.

It appears that this cop is assuming that the current date/time is always the first argument, but freeze_time cannot be used in place of travel_to when travel_to is being used for a future or past date.

Expected behavior

When travel_to is used with a past or future date, no suggestion should be made.

Actual behavior

When travel_to is used with a past or future date, the following incorrect suggestion is made:

spec/models/something_spec.rb:60:7: C: [Correctable] Rails/FreezeTime: Use freeze_time instead of travel_to.
      travel_to DateTime.new(2022, 5, 3, 12, 0, 0) do

The autocorrect also incorrectly corrects this to something that is not equivalent:

      freeze_time do

Steps to reproduce the problem

  1. Create a spec with travel_to taking a date/time argument that is not equivalent to the current time:
    travel_to DateTime.new(2022, 5, 3, 12, 0, 0) do
      # something
    end
  2. rubocop --only Rails/FreezeTime

RuboCop version

$ [bundle exec] rubocop -V
1.36.0 (using Parser 3.1.2.1, rubocop-ast 1.21.0, running on ruby 3.0.4) [x86_64-darwin21]
  - rubocop-faker 1.1.0
  - rubocop-performance 1.15.0
  - rubocop-rails 2.16.0
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.12.1
@koic koic added the bug Something isn't working label Sep 12, 2022
koic added a commit to koic/rubocop-rails that referenced this issue Sep 12, 2022
Fixes rubocop#769.

This PR fixes a false positive for `Rails/FreezeTime` when using
`travel_to` with an argument of `DateTime.new` with arguments.
koic added a commit to koic/rubocop-rails that referenced this issue Sep 12, 2022
Fixes rubocop#769.

This PR fixes a false positive for `Rails/FreezeTime` when using
`travel_to` with an argument of `DateTime.new` with arguments.
@koic koic closed this as completed in #770 Sep 13, 2022
koic added a commit that referenced this issue Sep 13, 2022
…ze_time

[Fix #769] Fix a false positive for `Rails/FreezeTime`
@christos
Copy link

This started happening again in 2.17.1 and is still there in 2.17.2 — Shall I open a new issue?

[snip...]: C: [Correctable] Rails/FreezeTime: Use freeze_time instead of travel_to.
[184](https://github.com/lingokids/api/actions/runs/3359574340/jobs/5567735155#step:7:185)
        travel_to Time.new(2019, 4, 3, 12, 30).in_time_zone do
[185](https://github.com/lingokids/api/actions/runs/3359574340/jobs/5567735155#step:7:186)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@koic
Copy link
Member

koic commented Oct 31, 2022

@christos Yeah, Can you open a new issue?

@christos
Copy link

@koic #848

It is not the exact same issue, but similar. Narrowed it down to a chained method invocation in the travel_to arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants