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

Date.today causes timezone issues in Rails. #2093

Closed
schwern opened this issue Jul 28, 2020 · 6 comments · Fixed by #2791
Closed

Date.today causes timezone issues in Rails. #2093

schwern opened this issue Jul 28, 2020 · 6 comments · Fixed by #2791

Comments

@schwern
Copy link

schwern commented Jul 28, 2020

Describe the bug

Any Faker method which uses Date.today in Ruby On Rails is potentially off by a day. There are similar potential issues for Time.now and DateTime.now.

To Reproduce

In Rails console, set a time zone which is in a different day. For example, right now it is July 27th in my local time zone, but July 28th in UTC. Date.today will return July 27th (local date), but Date.current will return July 28th (application's date).

[47] pry(main)> Time.zone = "UTC"
=> "UTC"
[53] pry(main)> Date.today
=> Mon, 27 Jul 2020
[54] pry(main)> Date.current
=> Tue, 28 Jul 2020

Now functions like Faker::Date.forward will be off by one with respect to the application's time zone.

[60] pry(main)> Faker::Date.forward(days: 1)
=> Tue, 28 Jul 2020

Expected behavior

I expect Faker::Date.forward(days: 1) to return Wed, 29 Jul 2020 which is 1 day forward in the application's time zone.

Additional context

This has been causing mysterious edge-case problems when using Faker data to test Rails validations. For example, Faker::Date.birthday(min_age: 15, max_age: 18) will occassionally fall outside the validation range of on_or_before: -> { 15.years.ago } causing a false test failure.

See https://thoughtbot.com/blog/its-about-time-zones for more detail about which Time and Date methods are safe under Rails.

Perhaps a config option could be added to have Faker use Rails-safe methods such as Date.current and Time.current. Or it could auto-detect a Rails environment.

@Selectus2
Copy link

If I make the changes as mentioned in the resource given above i.e. if I change Date.today to Date.current will you accept my PR

@robinator
Copy link

I'm wondering why this was marked as completed when it's still an issue with the gem? Is there workaround for rails users or something I should be doing instead? I am also happy to put a patch up if that's better.

@thdaraujo
Copy link
Contributor

thdaraujo commented Apr 12, 2023

If I make the changes as mentioned in the resource given above i.e. if I change Date.today to Date.current will you accept my PR (@Selectus2)

I think this is not the way to go because we don't assume you have Rails installed.

But we do use the to_date ActiveSupport method when available here:

date = date.to_date if date.respond_to?(:to_date)

And .to_date should take into consideration the timezone.

So maybe Faker::Date.forward should call get_date_object instead of Date.today and the problem would be solved.

I'm wondering why this was marked as completed when it's still an issue with the gem? Is there workaround for rails users or something I should be doing instead? I am also happy to put a patch up if that's better. (@robinator)

I believe this issue was closed because it was stale.

A possible workaround would be to set the TZ variable on an initializer. Ruby will then pull the timezone from the OS, for example:

> Faker::Date.forward(days: 1).to_time
=> 2023-04-13 00:00:00 -0700
> ENV['TZ'] = Time.zone = 'UTC'
=> "UTC"
> Faker::Date.forward(days: 1).to_time
=> 2023-04-13 00:00:00 +0000

But feel free to submit a patch with reproduction steps or a test and I'll review it! Thanks!

@thdaraujo
Copy link
Contributor

reopening for further investigation - see #2093 (comment)

@stefannibrasil
Copy link
Contributor

thanks @thdaraujo for helping with this!

I'm wondering why this was marked as completed when it's still an issue with the gem?

@robinator I closed it without pasting the comment, my bad. This is the message that was supposed to have been shared:

"Hey, folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the faker-ruby team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!"

@thdaraujo
Copy link
Contributor

assigned to @luciagirasoles

luciagirasoles added a commit to luciagirasoles/faker that referenced this issue Jun 22, 2023
luciagirasoles added a commit to luciagirasoles/faker that referenced this issue Jun 25, 2023
luciagirasoles added a commit to luciagirasoles/faker that referenced this issue Jun 25, 2023
luciagirasoles added a commit to luciagirasoles/faker that referenced this issue Jun 29, 2023
luciagirasoles added a commit to luciagirasoles/faker that referenced this issue Jul 6, 2023
thdaraujo pushed a commit that referenced this issue Jul 6, 2023
*Allow set from as String or Date type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants