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

Make doctrine.fixtures.loader public #222

Closed
wants to merge 1 commit into from
Closed

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Dec 4, 2017

I'm trying to get https://github.com/BehatExtension/DoctrineDataFixturesExtension to work with version 3.0 of this bundle.

It would help if this service was public, so that we can use this service to load all fixtures with all the dependencies.

Related issue

BehatExtension/DoctrineDataFixturesExtension#3

@ruudk
Copy link
Contributor Author

ruudk commented Dec 4, 2017

Tests are failing because something else. Master seems broken.

@ruudk
Copy link
Contributor Author

ruudk commented Dec 4, 2017

Master is failing because a wrong merge in #216 (comment)

@ruudk
Copy link
Contributor Author

ruudk commented Dec 4, 2017

Rebased on top of #223 to show that tests pass :)

@alcaeus
Copy link
Member

alcaeus commented Dec 4, 2017

@ruudk can you rebase on top of master please? Thanks!

@ruudk
Copy link
Contributor Author

ruudk commented Dec 4, 2017

@alcaeus Done!

@alcaeus
Copy link
Member

alcaeus commented Dec 5, 2017

Thanks!

FWIW, I can't figure out why we need to expose the loader for use in the extension you mentioned. Looking at the service config, this might be a good opportunity to change the signature of the loader: instead of fetching services from the container, inject them properly using the service config. I don't see why we should expose services to the outside to cater to extensions that don't properly use the service container.

@ruudk
Copy link
Contributor Author

ruudk commented Dec 5, 2017

@alcaeus Hmm, interesting. I'm gonna try to see if I can achieve it in a different way.

@ruudk
Copy link
Contributor Author

ruudk commented Dec 5, 2017

My plan was to do the following:

  • When DoctrineFixturesBundle 3.0.2+ is installed, then grab all services from the Container tagged doctrine.fixture.orm. The issue is that the tags are only available during the building of the container.

Since this Behat extension has no way to hook into that point (using an compiler pass), it becomes extremely hard to do this.

Do you have another idea?

@ruudk
Copy link
Contributor Author

ruudk commented Dec 5, 2017

Or did you mean something like this: BehatExtension/DoctrineDataFixturesExtension#4

@alcaeus
Copy link
Member

alcaeus commented Dec 5, 2017

Even if you inject the loader, I wouldn't make the assumption that you want the behat extension to load the default fixtures.

@ruudk
Copy link
Contributor Author

ruudk commented Dec 5, 2017

Any recommendations? My PR is not possible because Behat (+ extensions) run in a different container than the tests.

@fabwu
Copy link

fabwu commented Dec 14, 2017

@alcaeus I have a use case for this feature in my functional tests:

Before I execute my tests I would like to load all my fixtures in a sqlite database. I do that in the setUp of KernelTestCase:

class FunctionalTestCase extends KernelTestCase
{
    private static $isDatabaseReady = false;

    protected function setUp()
    {
        if (!self::$isDatabaseReady) {
            self::bootKernel();

            $databaseInitializer = new DatabaseInitializer(self::$kernel->getContainer());
            $databaseInitializer->dropDatabase();
            $databaseInitializer->loadSchema();
            $databaseInitializer->loadFixtures();

            self::$isDatabaseReady = true;
        }
    }
...

Currently I have to look up all my fixtures with the ContainerAwareLoader and filepaths. It would be great if I could fetch the SymfonyFixturesLoader from the container and load my fixtures that way.

Any chance to get that merged?

@alcaeus
Copy link
Member

alcaeus commented Dec 15, 2017

If you need to make a service public, it's much better to add a public alias in a special test.xml service file that's only loaded in your test environment. In that case, you could even make the DatabaseInitializer a service within test.xml that you can fetch from the container. That way you can also properly wire arguments instead of relying on $this->container->get(...).

Given that the original use-case (using the service within the Behat extension) is no longer given (the author also suggested to change the FixtureLoader class to receive proper service arguments, BehatExtension/DoctrineDataFixturesExtension#3 (comment)) I'm closing this pull request.

@alcaeus alcaeus closed this Dec 15, 2017
@ruudk ruudk deleted the patch-2 branch December 15, 2017 14:00
@fabwu
Copy link

fabwu commented Dec 18, 2017

@alcaeus Thanks for the hint. I could solve it with your explanation.

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

Successfully merging this pull request may close these issues.

3 participants