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

[POC] add integration with Doctrine DataFixtures #3

Closed
wants to merge 1 commit into from

Conversation

kbond
Copy link
Member

@kbond kbond commented Jun 15, 2020

Can use Foundry factories/stories within your Doctrine DataFixtures:

namespace App\DataFixtures;

use App\Tests\Factories\PostFactory;
use App\Tests\Stories\DefaultCategoriesStory;
use Doctrine\Bundle\FixturesBundle\Fixture;
use Doctrine\Persistence\ObjectManager;
use Zenstruck\Foundry\Bridge\FoundryDataFixture;

class AppFixtures extends Fixture
{
    use FoundryDataFixture;

    public function load(ObjectManager $manager)
    {
        DefaultCategoriesStory::load();

        PostFactory::new()->createMany(10);
    }
}

Some potential issues:

  1. Autowiring your fixtures is required because of the @required flag on FoundryDataFixture::register() (probably not a big deal as this is the default)
  2. Leaks tests/ objects into your src/ directory. I'm not sure the best way to deal with this or even if it is a major problem. One solution could be to default the make:factory/make:story commands to create the objects in src/Stories/src/Factories.

Major issue: There are several "global" options that can be set in Foundry:

  • Factory::registerDefaultInstantiator()
  • Factory::registerFaker()
  • Factory::proxyByDefault()
  • Proxy::autoRefreshByDefault()

Currently, in the docs, it is advised to set these in tests/bootstrap.php as I never intended for factories/stories to be used outside the test env.

One solution could be to allow setting these options in your .env file:

# .env/.env.test etc

FOUNDRY_WITHOUT_CONSTRUCTOR=true
FOUNDRY_ALLOW_EXTRA_ATTRIBUTES=true
FOUNDRY_ALWAYS_FORCE_PROPERTIES=true
FOUNDRY_FAKER_LOCALE=fr_FR
FOUNDRY_PROXY_BY_DEFAULT=false
FOUNDRY_PROXY_AUTO_REFRESH=false

I could check the $_SERVER variables when determining defaults.

@kbond kbond mentioned this pull request Jun 15, 2020
24 tasks
@weaverryan
Copy link
Contributor

Autowiring your fixtures is required because of the @required flag on FoundryDataFixture::register() (probably not a big deal as this is the default)

In practice, probably not a big deal... though if you don't have autowiring, then the error won't be great, which is unfortunate.

Because there are no hook points in the fixtures system to "do something before loading fixtures", the only other solution I can think of is to use service decoration... like we could decorate doctrine.fixtures.loader - https://github.com/doctrine/DoctrineFixturesBundle/blob/07c6bd42b7194a4c5b1db67f39eef3af6fc374b8/Resources/config/services.xml#L14 - and then run our setup code before calling the internal loader. That's... a really magic way of handling this. So I'm not sure it's better.

Leaks tests/ objects into your src/ directory. I'm not sure the best way to deal with this or even if it is a major problem. One solution could be to default the make:factory/make:story commands to create the objects in src/Stories/src/Factories.

I was thinking about this too. Moving to src/ seems like an easy win. The downside would be that these classes would be "available" in production... which in practice, means a very marginal impact I think on the autoloader. But maybe worrying about that is "splitting hairs"?

Major issue: There are several "global" options that can be set in Foundry

We could also... do nothing? And say: if you want to set these options, then that's your problem to do it before your first fixture or in some other way?

If there was less staticness in the library, then the normal solution would be to configure these in a YAML file and mutate some service based on this config. In the fixtures, you could fetch this service (in the trait from #2) and pass it to some static configuration method that would call all the static stuff for you. In the test environment, since you're already booting the container there, I think you could do the same? This is only half thought-out. It just occurred to me: "Why is this config challenging?" and "What would a normal bundle do"?

@kbond
Copy link
Member Author

kbond commented Jun 17, 2020

The downside would be that these classes would be "available" in production...

This is the same case for your doctrine fixtures classes but yeah, you probably will have more factory/story classes. You're right, is a very negligible performance hit.

One thought is to default the makers to creat to src/ with a --test flag to create in tests/.

@kbond
Copy link
Member Author

kbond commented Jun 22, 2020

Solved in a better way with #8

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.

2 participants