Skip to content

Conversation

@ttskch
Copy link
Contributor

@ttskch ttskch commented Oct 9, 2025

Closes #786

… registered in the service container when using `make:factory --test`
@ttskch ttskch force-pushed the docs/factory-test branch from 8635d76 to 653cd13 Compare October 9, 2025 02:35
Copy link
Member

@nikophil nikophil left a comment

Choose a reason for hiding this comment

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

this is very relevant!

I don't know if you dug into some old PRs, but this is very related to this comment. Thus, I think this fixes #786

I'm even wondering if we should not add this to the Foundry's recipe.
Actually, I think we should do this in config/packages/zenstruck_foundry.yaml, but I'm not 100% convinced, WDYT?

@kbond any thoughts? We talked about this few months ago

docs/index.rst Outdated
Using ``make:factory --test`` will generate the factory in ``tests/Factory``.

If your entity has some properties with Doctrine relationships, the factories for the related entities must be registered
in the service container so that the maker command can find them. To do that, add the following to your ``services.yaml``.
Copy link
Member

@nikophil nikophil Oct 9, 2025

Choose a reason for hiding this comment

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

I don't think the problem is about "entities [which] has some properties with Doctrine relationships"

I think the problem this resolves is something like:

When using --test flag, we're still dealing with dev environment. And because we want the container to know about our factories, we need to declare them as services even if they are in the test directory. To do that, add the following to your services.yaml

don't hesitate to rephrase, this problem is hard to explain in a meaningful way

@ttskch
Copy link
Contributor Author

ttskch commented Oct 10, 2025

@nikophil

Oh, I thought I'd looked into the old issue/PR, but I overlooked it.

You're right. I also think that registering files under tests/ in the service container in the dev environment is a strange workaround. (I don't think a fundamental solution will be possible anytime soon, so I wanted to at least document it to clear up any current user confusion.)

Essentially, I think the solution outlined in your comment below would be preferable.

Or maybe we should add a configuration node in which users can declare where they store their factories, so that we can tag them (or maybe no need to tag, at least add them in another way to the FactoryMap)

In my opinion, ideally, the make:factory command shouldn't use service container tags as a way to obtain a list of existing factories.

The basic premise is that the --test and the --namespace options are very useful and shouldn't be abolished. Given that, it's natural to assume that "existing factories" can exist in both src/ and tests/ (or another folder that's only loaded in other environments).

Therefore, the way make:factory obtains the list of existing factories should be decoupled from the service container.

In my opinion, simply hard-coding the list of folders in zenstruck_foundry.yaml would be sufficient. What do you think?

@nikophil
Copy link
Member

In my opinion, simply hard-coding the list of folders in zenstruck_foundry.yaml would be sufficient. What do you think?

not sure to agree, loading classes based on their path brings some boilerplate code which I'd prefer to not add here (is this psr4 ? psr0 ? etc...)

I think the best solution (well... the least worth solution 😅) is to document this the way you've already done it.

Please could you adapt the PR with the following:

  • change the wording to something like suggested in
  • add it as a .. tip::
  • I do think we should advocate to add this to packages/zenstruck_foundry.yml I even think we could add a tip in the recipe, maybe as some commented code, with a link to the documentation

thanks!

(hey this PR is the 1000th! 🎉)

@ttskch
Copy link
Contributor Author

ttskch commented Oct 10, 2025

@nikophil I've updated this PR and also submitted a PR to recipe.

Note that since we assume that the configuration will be written in zenstruck_foundry.yaml rather than services.yaml, the value of the resource field has been changed from '../tests/Factory/' to '../../tests/Factory/', and the autowire and autoconfigure settings have been moved under the App\Tests\Factory\ key instead of _defaults.

(Congrats on 1,000 (PR|Issue)s 🎊)

@ttskch ttskch force-pushed the docs/factory-test branch from d7ba72d to f4c759a Compare October 10, 2025 08:28
@nikophil
Copy link
Member

Can't we use %kernel.root-dir%?

@ttskch
Copy link
Contributor Author

ttskch commented Oct 10, 2025

@nikophil Surely! (It's %kernel.project_dir% though) I fixed it like that👍

@nikophil nikophil merged commit 81eacf5 into zenstruck:2.x Oct 10, 2025
73 checks passed
@nikophil
Copy link
Member

thanks @ttskch

@ttskch ttskch deleted the docs/factory-test branch October 10, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

[make:factory] Problems with --test flag

2 participants