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

Provide a function to reset faker.unique() #371

Closed
Shinigami92 opened this issue Jan 30, 2022 · 3 comments · Fixed by #800
Closed

Provide a function to reset faker.unique() #371

Shinigami92 opened this issue Jan 30, 2022 · 3 comments · Fixed by #800
Assignees
Labels
c: feature Request for new feature p: 1-normal Nothing urgent

Comments

@Shinigami92
Copy link
Member

Clear and concise description of the problem

Currently unique uses a global constant that remembers which values where already provided back so these will not be provided back a second time.
It seems there is not possibility to reset this or provide a dedicated cache.

Suggested solution

Provide the possibility to create a dedicated store that can be used.

Alternative

No response

Additional context

const found: Record<string, string> = {};

@Shinigami92 Shinigami92 added the s: pending triage Pending Triage label Jan 30, 2022
@Shinigami92 Shinigami92 added this to the v6.2 - New small features milestone Jan 30, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Jan 30, 2022
@Shinigami92 Shinigami92 added the c: feature Request for new feature label Jan 30, 2022
@github-actions github-actions bot removed the s: pending triage Pending Triage label Jan 30, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Feb 10, 2022

IMO we should go for a parameter passed unique store.
The reset is just for backwards compatibility.
In 7.0 we could remove the global store and just use a parameter based one.

@Shinigami92 Shinigami92 self-assigned this Apr 7, 2022
@Shinigami92 Shinigami92 added the p: 1-normal Nothing urgent label Apr 7, 2022
@Shinigami92 Shinigami92 moved this from Todo to In Progress in Faker Roadmap Apr 7, 2022
@Shinigami92 Shinigami92 linked a pull request Apr 7, 2022 that will close this issue
Repository owner moved this from In Progress to Done in Faker Roadmap Apr 22, 2022
@fshafiee
Copy link

fshafiee commented Jun 6, 2022

IMO we should go for a parameter passed unique store.
The reset is just for backwards compatibility.
In 7.0 we could remove the global store and just use a parameter based one.

I have a case to make for the global store.

We are using using factory pattern, and the factory runs in a scope of single instance of the model. Therefore, the factory doesn't know if it's creating multiple instances or just one (it wouldn't help even if it did know).
Here's a sample:

// Book Factory
defineFactory(
  BookEntity,
  (
    faker: Faker,
    context: {
      title?: string
    },
  ) => {
    const book = new BookEntity()
    book.title = context.title || faker.unique(faker.lorem.words)
    return book
  },
)
// Inside a test body
const books = await factory(BookEntity)().createMany(3)

We could pass the faker instance (or the dictionary) as context to the factory, but this would result in unnecessary boilerplate code for something that should be very trivial and easy to achieve. It makes the test writing experience unnecessarily painful.
Ideally, it's the job for the test suite to set the global state in lifecycle hooks of the tests (beforeEach, afterEach, etc.), and the dictionary that is used to track the usage of these values is essentially the same thing as a database. It's keeping a global state and introduces side-effects and just like any other data-store, it's only logical to allow the global dictionary to be reset between tests. So, from the testing standpoint, a global store with ability to reset its state is more desirable. After all, uniqueness is a concept that makes sense in global context and it makes sense to handle it globally.

beforeEach(async () => {
  DB.clear()
  Faker.reset()  // This is a static method, not a instance method
})

For inspiration: https://github.com/faker-ruby/faker#ensuring-unique-values

@Shinigami92
Copy link
Member Author

@fshafiee Please open a new feature request issue or a discussion, but please don't write especially such long answers into already merged and closed historical PRs, as the info you provide will go under here.

@ST-DDT ST-DDT removed this from Faker Roadmap Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants