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

Return values differ between package version #1464

Closed
9 of 10 tasks
VodekKwasniak opened this issue Oct 17, 2022 · 7 comments
Closed
9 of 10 tasks

Return values differ between package version #1464

VodekKwasniak opened this issue Oct 17, 2022 · 7 comments
Assignees
Labels
needs documentation Documentations are needed p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@VodekKwasniak
Copy link

Pre-Checks

Describe the bug

Up to version 7.4 if provided same seed, address and other values generated were predictable, always same, since version 7.5 they are different. See example:

Minimal reproduction code

const {faker} = require("@faker-js/faker");
faker.seed(123);


const addresses = [faker.address.streetAddress(), faker.address.streetAddress()];
console.log(addresses)
// @ v 7.6.0 returns [ '724 Mason Mission', '744 Wuckert Isle' ]
// @ v 7.5.0 returns [ '724 Mason Oval', '744 Wuckert Lake' ]
// @ v 7.4.0 returns [ '724 Maryse Oval', '744 Wuckert Lake' ]
// @ v 7.3.0 returns [ '724 Maryse Oval', '744 Wuckert Lake' ]

Additional Context

No response

Environment Info

System:
    OS: macOS 12.6
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
    Memory: 16.41 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 14.19.1 - ~/.nvm/versions/node/v14.19.1/bin/node
    npm: 8.19.2 - ~/.nvm/versions/node/v14.19.1/bin/npm
  Browsers:
    Brave Browser: 106.1.44.108
    Chrome: 106.0.5249.119
    Firefox: 105.0.3
    Safari: 16.0
  npmPackages:
    @faker-js/faker: 7.6.0 => 7.3.0

Which module system do you use?

  • CJS
  • ESM

Used Package Manager

npm

@VodekKwasniak VodekKwasniak added c: bug Something isn't working s: pending triage Pending Triage labels Oct 17, 2022
@Shinigami92
Copy link
Member

Shinigami92 commented Oct 17, 2022

It's not a bug, but seems we are missing documentation about that

The deterministic can only be within one exact version and change when you upgrade to a different version
All methods depend on the same seed generator and therefor if we change any function in e.g. accessing the generator one time more or less the whole output can change.

@Shinigami92 Shinigami92 added needs documentation Documentations are needed and removed c: bug Something isn't working s: pending triage Pending Triage labels Oct 17, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Oct 17, 2022
@VodekKwasniak
Copy link
Author

Thank you, @Shinigami92 , worth to know, will pin version in my project then :)

@ST-DDT
Copy link
Member

ST-DDT commented Oct 26, 2022

I think I will write a new page regarding reproducibility of our results and other related suggestions/workarounds for this.

@ST-DDT ST-DDT self-assigned this Oct 26, 2022
@ST-DDT ST-DDT moved this from Todo to In Progress in Faker Roadmap Oct 26, 2022
@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Oct 26, 2022
@flipflopsimsommer
Copy link

flipflopsimsommer commented Dec 22, 2022

@Shinigami92 would be nice to have it in the changelog.

I used seeds with faker-js/faker and relied on a non-changing values because i saw this project had tests to ensure them.

The tests go removed with #1653 it seems.

@ST-DDT
Copy link
Member

ST-DDT commented Dec 22, 2022

The tests go removed with #1653 it seems.

No, we still have the tests. Due to their similar nature, we simplified them in a helper utility.

seededTests(faker, 'system', (t) => {

I used seeds with faker-js/faker and relied on a non-changing values

You still can. However, you need to be aware that the values might change if you update faker because we might change the impl or locale entries to address issues such as #1633.
With a fixed version of faker the assumption holds true.

We have plans for explicitly recording and replaying results, but those wont make it into v8.x for now. As that either requires test runner integration/support or manual configuration to ensure max resillience against changes in user tests (swapping the first and last name generation, shouldnt break the test).

As a workaround, you can either not update faker or update faker in a separate commit and update the fixed values if neccessary (should rarely happen).

@flipflopsimsommer
Copy link

Oh 🦆 , so no changelog entry is needed , if this never worked and was never supposed to work.
Thx for adding new pages to help others not making this wrong assumption.

@matthewmayer
Copy link
Contributor

matthewmayer commented Mar 25, 2023

i think this can be closed now, as there is a note about this in new Reproducible Results section about this

#1892

Screenshot 2023-03-25 at 17 15 47

@github-project-automation github-project-automation bot moved this from In Progress to Done in Faker Roadmap Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs documentation Documentations are needed p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants