fix(loader): don't create fixture dependency if already added

[tomtomau]

I outlined this below on Slack:

Symfony4, DoctrineFixturesBundle 3.4, LiipFunctionalTestingBundle.
I have a Fixture A which depends on Fixture B.
Fixture B has constructor dependency injection going on, so the LiipFunctionalTesting bundle loads that from the container (see here) and calls ->addFixture on the loader.

Tracing it through, I can see here that we're doing a plain ol instantiation of the class, even if the fixture is already loaded - Loader.php#L139

We can also definitely fix this in the LiipFunctionalTestingBundle by creating a custom Loader and overriding the createFixture method to use the Container to resolve it instead, but it also seems logical to me that this package doesn't try to create another fixture if it has already been added.

Let me know your thoughts :)

[jwage]

Right now if you register the same fixture class twice, will it try to execute it twice?

[tomtomau]

@jwage not exactly.

If you try to call ->addFixture on the same fixture twice, it will bail early on line 123.

The specific case I'm trying to catch here is if you have already loaded the dependency, we should not try to create the dependency again and rely on the ->addFixture method checking if we've already got it added.

[alcaeus]

DoctrineFixturesBundle 3.4

Wait, what? The newest release of doctrine/doctrine-fixtures-bundle is 3.0.2, there's no 3.4 release (yet). Taking a look at LiipFunctionalTestBundle, it seems the 2.x release supports this version of the fixtures bundle, so I'll base my analysis on that.

Your analysis is correct in the sense that createFixture does a good ole' instantiation of a fixture class (see Loader:191..194), but the loader included in the fixtures bundle overrides this behavior, with createFixture returning a previously loaded fixture (see SymfonyFixturesLoader:59..73). The problem seems to be the WebTestCase class included in the new version of the FunctionalTestBundle: it either instantiates the ContainerAwareLoader from the Doctrine bridge or the Loader included in doctrine/data-fixtures, but never the new SymfonyFixturesLoader from the new bundle version (see WebTestCase:684..691.

This library wasn't really intended to handle fixtures with constructor dependencies (as evidenced by the createFixture method instantiating the class) with multiple instances of the same fixture class loaded if necessary. I wouldn't change this behavior for now, but this is something we might revisit for version 2.0 of the library to better support the behavior needed by the Symfony bundle and other libraries. For now the fix would be to use the new SymfonyFixturesLoader class and always pass an instance of the fixture class when adding it.