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

Enhancement: provide a mechanism to use the faker pytest fixture Faker instance #441

Open
strangemonad opened this issue Nov 17, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@strangemonad
Copy link

strangemonad commented Nov 17, 2023

Summary

Faker comes with a built-in pytest fixture. For both efficiency sake and control over the faker instance customization (e.g. custom faker providers, locales and randomness) it would be nice if there were a way to use that shared instance.

Basic Example

The approach I'm imagining is either as an extra option to pytest_plugin.registry_factory(... use_fake_fixture=True) or as a completely separate pytest plugin method.

In that case, when the Factory class doesn't define its own __faker__ instance, the pytest faker fixture would be used.

e.g.

@pytest.fixture(scope="session", autouse=True)
def faker_seed() -> int:
    return 42

@pytest.fixture(scope="session", autouse=True)
def faker_session_locale():
    return ['en_US']

@pytest.fixture(autouse=True)
def customize_faker(faker: Faker):
    faker.add_provider(MyCustomProvider)


class MyPersonFactory(ModelFactory[Person]):
    __model__ = Person

register_fixture(PersonFactory, use_faker_fixture=True)

Drawbacks and Impact

No response

Unresolved questions

  • Ideally, BaseFactory could be changed to set the default __faker__ = Faker() instance to a sentinel value to avoid creating extra Faker instances.
  • Faker's pytest plugin by default exposes a function scope fixture but caches the singleton Faker instance (although that behavior can be modified e.g. to get a fresh faker instance per function to test different locales.
  • I haven't though through all the implications of how this would interact with the function scope custom locale faker instances. I'm assuming the implementation would set the faker instance class var once (when register is called) rather that resolved during each pytest function invocation.

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh Litestar dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@strangemonad strangemonad added the enhancement New feature or request label Nov 17, 2023
@guacs
Copy link
Member

guacs commented Nov 17, 2023

I think this can be added without too much trouble. As for your point regarding the scopes, I think it should be fine if we just depend on the faker fixture in the fixture of the factory.

@strangemonad is this something you'd be interested to work on?

@guacs
Copy link
Member

guacs commented Nov 17, 2023

@strangemonad I've edited the initial description to end the code block at the right place.

@guacs
Copy link
Member

guacs commented Nov 17, 2023

I think this can be added without too much trouble. As for your point regarding the scopes, I think it should be fine if we just depend on the faker fixture in the fixture of the factory.

@strangemonad is this something you'd be interested to work on?

To provide a bit more clarity on this, I think the fixture that we register with pytest would look something like this:

@pytest.fixture
def my_factory_fixture(faker: Faker) -> MyFactory:
    MyFactory.__faker__ = faker
    
    return MyFactory

@strangemonad
Copy link
Author

@guacs

  1. It's potentially something I could work on but I don't know that I can commit any work on this on my end for the next month or so
  2. As for registering. I wonder if it might not make more sense to look at using the shared faker instance in the BaseFactory when you've enabled pytest-faker? A few thoughts
  • If you only register MyFactory.__faker__ you can't use that faker instance outside of class methods e.g. if I wanted to do something like Use(ModelFactory.__faker__...).
  • Faker recommends just using a single shared instance to reduce the cost overhead of multiple instances.

@guacs
Copy link
Member

guacs commented Nov 21, 2023

@guacs

  1. It's potentially something I could work on but I don't know that I can commit any work on this on my end for the next month or so
  2. As for registering. I wonder if it might not make more sense to look at using the shared faker instance in the BaseFactory when you've enabled pytest-faker? A few thoughts
  • If you only register MyFactory.__faker__ you can't use that faker instance outside of class methods e.g. if I wanted to do something like Use(ModelFactory.__faker__...).
  • Faker recommends just using a single shared instance to reduce the cost overhead of multiple instances.

Regarding point one, that's completely fine. I'll try to work on this if I get time, but feel free to let me know if you want to work on it.

As for the second point, are you proposing to make polyfactory use the faker fixture automatically instead of the user having to say to use the fixture when registering the factory?

Also, I like the idea of setting it on the BaseFactory rather than the given factory, but what if the user wants to use the faker fixture only on the factory they registered for some reason?

@strangemonad
Copy link
Author

Also, I like the idea of setting it on the BaseFactory rather than the given factory, but what if the user wants to use the faker fixture only on the factory they registered for some reason?

Yes, exactly, I think that's one of the things that still needs hashing out. Overall I think the following feel right:

  • If I opt in to registering polyfactory with pytest AND the pytest-faker plugin (and related fixtures) are configured then setting the BaseFactory.__faker__ to be that shared instance feels consistent with expectations and guidance from faker to use a single shared instance by default.
  • I think I'm still proposing to some sort of opt-in or polyfactory-pytest plugin mechanism that controls this. I could imagine some folks already configure a pytest faker fixture and use polyfactory as it's currently setup and automatically switching might cause unexpected behavior (e.g. different sampling order from a seeded rng).

For scoping / multiple specially configured faker instances, I think there are a few options:

  • Continue doing it as it's currently done, set an explicit faker instance in those cases you don't want a default. In that case, if you want to register the factory as a fixture, you'd have to opt out of (re-)setting the session-scoped faker fixture.
  • faker-pytest does support a mix of using a session scoped fixture that returns the singleton faker and per-function that use parameterized fixtures to return a custom faker instance (that's cached I believe). I think a really slick feature would be to hook into that to grab the current faker fixture. That might entail either a pytest fixture that takes in the request to get the currently scoped faker. I'm not 100% sure, that would require some investigation.
  • Maybe a good in-between (and possibly a useful building block for any test runner integration) is to allow __faker__ to be an instance or a callable that returns the required instance when needed? I'd imagine that's more of building block or advanced configuration and most people shouldn't need to fiddle with that.

@guacs
Copy link
Member

guacs commented Nov 22, 2023

If I opt in to registering polyfactory with pytest AND the pytest-faker plugin (and related fixtures) are configured then setting the BaseFactory.faker to be that shared instance

I like this idea. I'm just not a 100% sure on how the API would look like while also allowing users to use their own custom faker instances and maintaining backwards compatibility. I think getting the current faker fixture scoped to the function should be possible as well with some approach like you suggested. However, I agree, this would require some investigation and some experimentation to really figure out how to approach this.

if you want to register the factory as a fixture, you'd have to opt out of (re-)setting the session-scoped faker fixture

I think it should be the other way around. You'd have to opt in to use the session-scoped fixture in order to maintain backwards compatibility.

allow faker to be an instance or a callable that returns the required instance when needed

I think there are two issues with this idea:

  • Backwards compatibility again :P A lot of the methods work under the assumption that the __faker__ value to be an instance. So, if we allow it to be a callable as well users would need to change their code to check if __faker__ is an instance of Faker or a callable when they're using the __faker__ for anything.
  • I think this might cause a performance degradation as well because there are a lot of calls to get_provider_map. If a check has to be done each time to see if it's an instance or a callable, and then get the faker by calling the callable if needed, I think it will have a noticeable impact. It might work if we can somehow cache the return of get_provider_map (this does have a pretty nice performance boost if you're creating a large amount of data from my testing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants