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 of issue 218 #219

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix of issue 218 #219

wants to merge 6 commits into from

Conversation

mcosti
Copy link

@mcosti mcosti commented May 1, 2024

#218

This was the most backwards compatible way of implementing it that I could think of.

It works for my usecase scenario, but I am of course open to suggestions.

I would add testing but there are no tests involving Django models yet

@mcosti
Copy link
Author

mcosti commented May 1, 2024

It seems like the fix is not complete.

Somewhere in the partial function model_fixture, the regular fixture name is called, and if the clashing factory does not have the same parameter, it will fail like this:

file /opt/project/tests/finance/subscriptions/test_prices_checkout.py, line 17
  def test_cancel_subscription(subscription: Subscription):
file /usr/local/lib/python3.11/dist-packages/pytest_factoryboy/fixture.py, line 350
  def model_fixture(request: SubRequest, factory_name: str) -> Any:
file /usr/local/lib/python3.11/dist-packages/pytest_factoryboy/fixture.py, line 502
  def subfactory_fixture(request: SubRequest, factory_class: FactoryType) -> Any:
E       fixture 'customer__id' not found

@mcosti
Copy link
Author

mcosti commented Jan 15, 2025

Hi @youtux , it's a-me again! I hope things are good.

This has yet again become relevant for me. I know that the subfactory fixture name is generated using

    fixture = inflection.underscore(get_model_name(factory_class))

So the overriden fixture name is once again relevant.

Consider the following model name:

In [1]: import inflection

In [2]: inflection.underscore("B2BPartner")
Out[2]: 'b2_b_partner'

This makes for a very weird fixture name, but looking at the code of the function, it makes sense.

In my very specific case, with the code of my own PR, this ends up being:

class B2BPartnerFactory(CustomFixtureDjangoModelFactory):
    class Meta:
        model = B2BPartner
        fixture_name = "b2b_partner"


In [4]: inflection.underscore("b2b_partner")
Out[4]: 'b2b_partner'

However, somewhere it's still looking for the regular fixture name, still investigating that.

tests-1  |   def test_retrieve_face_search_request(api_client, face_search_request):
tests-1  | file /usr/local/lib/python3.12/site-packages/pytest_factoryboy/fixture.py, line 354
tests-1  |   def model_fixture(request: SubRequest, factory_name: str) -> Any:
tests-1  | file /usr/local/lib/python3.12/site-packages/pytest_factoryboy/fixture.py, line 506
tests-1  |   def subfactory_fixture(request: SubRequest, factory_class: FactoryType) -> Any:
tests-1  | E       fixture 'b2_b_partner_branch' not found```

What do you think about this? Thank you

@youtux
Copy link
Contributor

youtux commented Jan 15, 2025

Hmm, I think I would prefer instead to change the register decorator, and maybe add a factory_fixture_name parameter instead?

Just thinking out loud, not sure if this would look good or if it would break something or if it's even feasible...

@youtux
Copy link
Contributor

youtux commented Jan 15, 2025

I'm thinking of keeping a mapping between Factories and the factory fixture name using a WeakKeyDictionary "registry" constant in pytest-factoryboy (similar to what I did recently for pytest-bdd: pytest-dev/pytest-bdd#658)

@mcosti
Copy link
Author

mcosti commented Jan 16, 2025

I think that could work. I am only wondering about the following thing:

How would we define which is the "main" fixture? The one that right now would be created by calling register(YourFactory) with no name argument and is by default used for SubFactory.

Would we define it with a boolean? Would we pass the name as a param to SubFactory?

register(AuthorBookFactory)

Results in an author_book fixture, which will also be used by the SubFactory.

register(AuthorBookFactory, factory_fixture_name="authorbook")

How do we make the SubFactory choose this one? Or maybe I am not seeing the whole picture you have in mind.

If that all becomes clear to me, I wouldn't mind working on the code for it.

Thanks for the very quick reply, Alessio!

@youtux
Copy link
Contributor

youtux commented Jan 17, 2025

Good point, I didn't think about it!
Maybe it could be a param of register, like default_fixture _name=..., but I'm not sure I like it.
Let me think about it, I'll get back to you!

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.

2 participants