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

Fix/seeding #1754

Merged
merged 4 commits into from Nov 22, 2022
Merged

Fix/seeding #1754

merged 4 commits into from Nov 22, 2022

Conversation

johntmyers
Copy link
Contributor

fixes #1656

What does this change

This removes re-seeding the Random() instance on every call to _select_factory() which gets called every time a faker method is used. This code was only reached when both conditions are true:

  • A seed is set via Faker.seed()
  • More than one locale is specified

What was wrong

Basically same as above, it is unnecessary to set the Random() instance's seed value every time a faker method is used. The Random() instance is seeded when making the call to Faker.seed() which is sufficient for making sure that the order of the locales selected is the same between subsequent instances of Faker

How this fixes it

Remove the seeding that isn't needed and modified a test that asserts:

  • Generating values when a seed is used and multiple locales are used does not generate the same value over and over
  • Calling Faker.seed() and using a Faker instance in an identical way will produce the same fake output each time

Fixes #...

@fcurella
Copy link
Collaborator

@johntmyers You can run make lint to format the code with our settings.

@johntmyers
Copy link
Contributor Author

@fcurella everything is passing now

Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

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

Thank you!

@fcurella fcurella merged commit 2826e71 into joke2k:master Nov 22, 2022
@johntmyers johntmyers deleted the fix/seeding branch November 22, 2022 18:59
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.

Faker returns one value when using multiple locales
2 participants