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

Adds FFaker::Date.birthday #542

Merged
merged 6 commits into from
Sep 13, 2023
Merged

Conversation

professor
Copy link
Contributor

@professor professor commented Sep 5, 2023

This adds FFaker::Date.birthday

For our use case, we're looking for realistic looking birthdays for employees at a company. It would be weird in our demo/staging environment to have a 3 year old working at a company, not a good look.

In the case of birthday(min_age: 18, max_age: 18), I would interpret this to mean "I want a date for someone who is 18 years old today" -- this means anyone who just turned 18 today back until last year plus a day. So on 9/6/2023 the range would be 9/7/2004 to 9/6/2005 which would be from:9/6/2023 - 19.years + 1.day to: 9/6/2023 - 18.years

I ran this test 10_000 times to verify correctness:

10_000.times do
   assert_random_between(today.prev_year(43).next_day..today.prev_year(42)) { @tester.birthday(min_age: 42, max_age: 42) }
end

Fixes #540

@marocchino
Copy link
Member

please add :birthday to assert_methods_are_deterministic

test/test_date.rb Outdated Show resolved Hide resolved
lib/ffaker/date.rb Outdated Show resolved Hide resolved
professor and others added 2 commits September 5, 2023 19:57
Co-authored-by: marocchino <marocchino@users.noreply.github.com>
Co-authored-by: marocchino <marocchino@users.noreply.github.com>
test/test_date.rb Outdated Show resolved Hide resolved
lib/ffaker/date.rb Outdated Show resolved Hide resolved
lib/ffaker/date.rb Outdated Show resolved Hide resolved
test/test_date.rb Outdated Show resolved Hide resolved
@professor
Copy link
Contributor Author

Are you ok with me adding TimeCop?

@professor
Copy link
Contributor Author

@marocchino thanks for all your help with this PR. I appreciate it.

Copy link
Member

@marocchino marocchino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@marocchino
Copy link
Member

Are you ok with me adding TimeCop?

Well, Personally, I don't like it. Rails treats it as a legacy that needs to be removed, so I often do the work of removing it.
However, other maintainers may have different thoughts.

@professor
Copy link
Contributor Author

OK, I believe this PR is ready for merging.

@AlexWayfer
Copy link
Contributor

Are you ok with me adding TimeCop?

What for? For tests? I'm OK if it'd help.

Well, Personally, I don't like it.

Why?

Rails treats it as a legacy that needs to be removed, so I often do the work of removing it.

TimeCop doesn't belong to Rails, what the deal with Rails? It's like "FFaker treats RSpec as a legacy". TimeCop is under active development, as I see, and not even v1 for now:

image

@professor
Copy link
Contributor Author

@marocchino anything blocking the merging of this PR?

@marocchino
Copy link
Member

I was expecting another maintainer's approval, but it's okay as is.

@marocchino marocchino merged commit 4aa92e0 into ffaker:main Sep 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Birthday
3 participants