Added possibility to load fixtures by bundle name #41

Open
wants to merge 1 commit into
from

Projects

None yet

8 participants

@biozshock

Feature allows loading fixtures from bundles.

app/console doctrine:fixtures:load --fixtures=@AcmeDemoBundle

will load /path/to/src/Acme/DemoBundle/DataFixtures/ORM

@biozshock

Found this pull request is still open. Is there are something i need to change in order to merge this?

@dbu
Member
dbu commented Nov 1, 2012

+1

@nchaulet

+1

@oltodo
oltodo commented Jan 22, 2013

+1

@Pawka
Pawka commented Apr 10, 2013

👍

@ogizanagi

👍

@lavoiesl
Member

Could you rebase your PR?

@lavoiesl lavoiesl added the Needs work label Mar 23, 2015
@biozshock

Huh.. Almost 3 years :) On top of which branch to rebase? I think 2.0 so more people can use this now?

@stof
Member
stof commented Mar 23, 2015

On top of master. The 2.0 branch is an abandonned branch for Symfony 2.0 (which is long dead)

@biozshock

Huh. Thought that are branches for Symfony 2 and 3.
Rebased.

@lavoiesl
Member

@biozshock, sorry, would it be possible to add a test for this as well?

@lavoiesl lavoiesl added Needs tests and removed Needs work labels Mar 23, 2015
@lavoiesl lavoiesl and 1 other commented on an outdated diff Mar 24, 2015
composer.json
"doctrine/doctrine-bundle": "~1.0",
+ "doctrine/orm": "~2.0",
@lavoiesl
lavoiesl Mar 24, 2015 Member

This bundle should not depend on ORM, add it as require-dev if you must, like in the library: https://github.com/doctrine/data-fixtures/blob/master/composer.json

@biozshock
biozshock Mar 24, 2015

FYI: This code will not work without ORM as it has a hard dependency on Doctrine\ORM\EntityManagerInterface in \Doctrine\Common\DataFixtures\Purger\ORMPurger.
But this is up to you :)

@lavoiesl
lavoiesl Mar 24, 2015 Member

This bundle was working before your modification and your modification is not adding a specific dependency on ORMPurger. Thus, I would advise not to modify this. If this is to be changed, it would need to be in a separate PR.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/bootstrap.php
@@ -0,0 +1,3 @@
+<?php
+
+$loader = require_once(__DIR__ . '/../vendor/autoload.php');
@lavoiesl
lavoiesl Mar 24, 2015 Member

Missing newline

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Fixtures/Entity/CheckIt.php
+ */
+ public function getEmail()
+ {
+ return $this->email;
+ }
+
+ /**
+ * @param string $email
+ */
+ public function setEmail($email)
+ {
+ $this->email = $email;
+ }
+
+
+}
@lavoiesl
lavoiesl Mar 24, 2015 Member

Missing newline

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Fixtures/DataFixtures/ORM/CheckLoad.php
@@ -0,0 +1,28 @@
+<?php
+/**
+ * Created by PhpStorm.
@lavoiesl
lavoiesl Mar 24, 2015 Member

Wrong copyright header, remove it or add the standard Doctrine header.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Fixtures/DataFixtures/ORM/CheckLoad2.php
@@ -0,0 +1,28 @@
+<?php
+/**
+ * Created by PhpStorm.
@lavoiesl
lavoiesl Mar 24, 2015 Member

Wrong copyright header, remove it or add the standard Doctrine header.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Fixtures/DataFixtures/ORM/Product/CheckLoad3.php
@@ -0,0 +1,28 @@
+<?php
+/**
+ * Created by PhpStorm.
@lavoiesl
lavoiesl Mar 24, 2015 Member

Wrong copyright header, remove it or add the standard Doctrine header.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Fixtures/Entity/CheckIt.php
@@ -0,0 +1,66 @@
+<?php
+/**
+ * Created by PhpStorm.
@lavoiesl
lavoiesl Mar 24, 2015 Member

Wrong copyright header, remove it or add the standard Doctrine header.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Command/LoadDataFixturesDoctrineCommandTest.php
+
+ return $this->kernel;
+ }
+
+ private function getBundle($path = '/../Fixtures')
+ {
+ $bundle = $this->getMock('Symfony\Component\HttpKernel\Bundle\BundleInterface');
+ $bundle
+ ->expects($this->any())
+ ->method('getPath')
+ ->will($this->returnValue(realpath(__DIR__ . $path)))
+ ;
+
+ return $bundle;
+ }
+}
@lavoiesl
lavoiesl Mar 24, 2015 Member

Missing newline

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Command/LoadDataFixturesDoctrineCommandTest.php
+ ->method('getBundle')
+ ->will($this->returnValue($this->getBundle()))
+ ;
+ $this->kernel
+ ->expects($this->any())
+ ->method('getBundles')
+ ->will($this->returnValue(array($this->getBundle())))
+ ;
+
+ return $this->kernel;
+ }
+
+ private function getBundle($path = '/../Fixtures')
+ {
+ $bundle = $this->getMock('Symfony\Component\HttpKernel\Bundle\BundleInterface');
+ $bundle
@lavoiesl
lavoiesl Mar 24, 2015 Member

Expected formatting is:

$bundle->expects($this->any())
    ->method('getPath')
    ->will($this->returnValue(realpath(__DIR__ . $path)));
@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Command/LoadDataFixturesDoctrineCommandTest.php
+ ->will($this->returnCallback(function($callable) use ($em) {
+ call_user_func($callable, $em);
+ }));
+
+ return $container;
+ }
+
+ private function getMockKernel()
+ {
+ $this->kernel = $this->getMock('Symfony\Component\HttpKernel\KernelInterface');
+ $this->kernel
+ ->expects($this->any())
+ ->method('getBundle')
+ ->will($this->returnValue($this->getBundle()))
+ ;
+ $this->kernel
@lavoiesl
lavoiesl Mar 24, 2015 Member

formatting, see below

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Command/LoadDataFixturesDoctrineCommandTest.php
+ ->will($this->returnValue($doctrineEventMock));
+
+ $em = $this->emMock;
+ $this->emMock->expects($this->any())
+ ->method('transactional')
+ ->will($this->returnCallback(function($callable) use ($em) {
+ call_user_func($callable, $em);
+ }));
+
+ return $container;
+ }
+
+ private function getMockKernel()
+ {
+ $this->kernel = $this->getMock('Symfony\Component\HttpKernel\KernelInterface');
+ $this->kernel
@lavoiesl
lavoiesl Mar 24, 2015 Member

formatting, see below

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Command/LoadDataFixturesDoctrineCommandTest.php
+ public function fixturesProvider()
+ {
+ return array(
+ array(__DIR__ . '/../Fixtures/DataFixtures/ORM', array('ORM\CheckLoad', 'ORM\CheckLoad2', 'ORM\Product\CheckLoad3')),
+ array('@AcmeTestBundle', array('ORM\CheckLoad', 'ORM\CheckLoad2', 'ORM\Product\CheckLoad3')),
+ array('@AcmeBundle/Product', array('ORM\Product\CheckLoad3')),
+ );
+ }
+
+ /**
+ * @depends testFixtureLoad
+ * @expectedException \InvalidArgumentException
+ */
+ public function testNoFixtures()
+ {
+ $this->testFixtureLoad('hoho', array());
@lavoiesl
lavoiesl Mar 24, 2015 Member

use common strings like 'nonexistant' or 'foo'

@lavoiesl lavoiesl commented on the diff Mar 24, 2015
Command/LoadDataFixturesDoctrineCommand.php
$dirOrFile = $input->getOption('fixtures');
if ($dirOrFile) {
- $paths = is_array($dirOrFile) ? $dirOrFile : array($dirOrFile);
+
+ $paths = array();
+ $pathsParams = is_array($dirOrFile) ? $dirOrFile : array($dirOrFile);
+ foreach ($pathsParams as $path) {
+ if (preg_match('/^\@([^\/\\\\]+)(.*)$/', $path, $matches)) {
+ $bundle = $this->getApplication()->getKernel()->getBundle($matches[1]);
@lavoiesl
lavoiesl Mar 24, 2015 Member

When the bundle does not exist (or typo), is it handled properly? There should be a test case for that.

@biozshock
biozshock Mar 24, 2015

Out of scope of responsibility for this code. This should be handled by Kernel. But will add a testcase which mimic this behaviour.

@lavoiesl
lavoiesl Mar 24, 2015 Member

Excellent, this is exactly what I was looking for.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
composer.json
@@ -22,9 +22,14 @@
"require": {
"php": ">=5.3.2",
"symfony/doctrine-bridge": "~2.1",
+ "symfony/console": "~2.1",
@lavoiesl
lavoiesl Mar 24, 2015 Member

This bundle should not depend on Console, add it as require-dev if you must.

@lavoiesl lavoiesl and 1 other commented on an outdated diff Mar 24, 2015
Command/LoadDataFixturesDoctrineCommand.php
@@ -100,7 +115,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
$purger = new ORMPurger($em);
$purger->setPurgeMode($input->getOption('purge-with-truncate') ? ORMPurger::PURGE_MODE_TRUNCATE : ORMPurger::PURGE_MODE_DELETE);
$executor = new ORMExecutor($em, $purger);
- $executor->setLogger(function($message) use ($output) {
+ $executor->setLogger(function ($message) use ($output) {
@lavoiesl
lavoiesl Mar 24, 2015 Member

Sorry to be nitpick, but this shouldn’t be modified.

@lavoiesl
lavoiesl Mar 24, 2015 Member

I understand, but this is not the job of this PR.

@lavoiesl lavoiesl commented on an outdated diff Mar 24, 2015
Tests/Command/LoadDataFixturesDoctrineCommandTest.php
+ $container->expects($this->any())
+ ->method('getKernel')
+ ->will($this->returnValue($this->getMockKernel()));
+
+ $doctrineMock = $this->getMock('Doctrine\Common\Persistence\ManagerRegistry');
+ $container->expects($this->any())
+ ->method('get')
+ ->with('doctrine')
+ ->will($this->returnValue($doctrineMock));
+
+ $this->emMock = $this->getMock('Doctrine\ORM\EntityManagerInterface');
+ $doctrineMock->expects($this->any())
+ ->method('getManager')
+ ->will($this->returnValue($this->emMock));
+
+ $doctrineEventMock = $this->getMock('\Doctrine\Common\EventManager');
@lavoiesl
lavoiesl Mar 24, 2015 Member

For consistency, please remove leading slash

@biozshock

@lavoiesl I think it's fine to merge this now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment