MappingDriverChain: the default driver wasn't called for getAllClassNames() #272

Merged
merged 2 commits into from Apr 16, 2013

Projects

None yet

3 participants

@mnapoli
Contributor
mnapoli commented Apr 16, 2013

MappingDriverChain::getAllClassNames() would call and merge sub-drivers getAllClassNames() results, but not for the default driver.

I added a test that reproduced the problem, and then fixed it.

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DCOM-188

We use Jira to track the state of pull requests and the versions they got
included in.

@Ocramius Ocramius and 1 other commented on an outdated diff Apr 16, 2013
.../Tests/Common/Persistence/Mapping/ChainDriverTest.php
+ $companyDriver = $this->getMock('Doctrine\Common\Persistence\Mapping\Driver\MappingDriver');
+ $defaultDriver = $this->getMock('Doctrine\Common\Persistence\Mapping\Driver\MappingDriver');
+ $chain = new MappingDriverChain();
+
+ $companyDriver->expects($this->once())
+ ->method('getAllClassNames')
+ ->will($this->returnValue(array()));
+
+ $defaultDriver->expects($this->once())
+ ->method('getAllClassNames')
+ ->will($this->returnValue(array()));
+
+ $chain->setDefaultDriver($defaultDriver);
+ $chain->addDriver($companyDriver, 'Doctrine\Tests\Models\Company');
+
+ $chain->getAllClassNames();
Ocramius
Ocramius Apr 16, 2013 Owner

No assertion?

mnapoli
mnapoli Apr 16, 2013 Contributor

The assertion is $defaultDriver->expects($this->once()), which fails in the master branch (which cause the test to fail).

Ocramius
Ocramius Apr 16, 2013 Owner

Yes, but this doesn't check that the class is in the result ;)

mnapoli
mnapoli Apr 16, 2013 Contributor

Yep :) done

@Ocramius Ocramius and 1 other commented on an outdated diff Apr 16, 2013
.../Tests/Common/Persistence/Mapping/ChainDriverTest.php
@@ -122,9 +122,29 @@ public function testDefaultDriver()
$this->assertTrue($chain->isTransient($entityClassName));
$this->assertFalse($chain->isTransient($managerClassName));
}
+
+ public function testDefaultDriverGetAllClassNames()
+ {
+ $companyDriver = $this->getMock('Doctrine\Common\Persistence\Mapping\Driver\MappingDriver');
+ $defaultDriver = $this->getMock('Doctrine\Common\Persistence\Mapping\Driver\MappingDriver');
+ $chain = new MappingDriverChain();
+
+ $companyDriver->expects($this->once())
+ ->method('getAllClassNames')
+ ->will($this->returnValue(array()));
+
+ $defaultDriver->expects($this->once())
+ ->method('getAllClassNames')
+ ->will($this->returnValue(array()));
Ocramius
Ocramius Apr 16, 2013 Owner

You should add items here

mnapoli
mnapoli Apr 16, 2013 Contributor

See my other answer, but there is no need, the test is about the $defaultDriver->expects($this->once())

mnapoli
mnapoli Apr 16, 2013 Contributor

Actually you make a point: the test proves that getAllClassNames() is called, but not that the results are returned. I'll improve the test.

@Ocramius Ocramius merged commit 3506be7 into doctrine:master Apr 16, 2013

1 check failed

default The Travis build could not complete due to an error
Details
@mnapoli mnapoli deleted the myclabs:ChainDriverFix branch Sep 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment