Permalink
Browse files

Fix getMockForModel() using the incorrect datasource.

Because getMockForModel() does not go through the test datasource
injection in ClassRegistry::init() we need to duplicate the basics of
that logic here. Thankfully, we already have a mock so we can do that
datasource switching without reflection. Of course this means there will
be limitations to how/when this will work, but I feel those scenarios
can probably be solved by not using mocks, or by mocking out the
problematic methods. This set of changes makes getMockForModel() work
with secondary datasources, as one would expect it to do, but I'm not
sure it ever did.

Refs #4694
  • Loading branch information...
markstory committed Oct 13, 2014
1 parent ecf0307 commit 9b9e886df69d4fda9ded9daffc8dfe9b3752af5f
Showing with 57 additions and 9 deletions.
  1. +46 −8 lib/Cake/Test/Case/TestSuite/CakeTestCaseTest.php
  2. +11 −1 lib/Cake/TestSuite/CakeTestCase.php
@@ -17,9 +17,25 @@
* @since CakePHP v 1.2.0.4487
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
App::uses('CakePlugin', 'Core');
App::uses('Controller', 'Controller');
App::uses('CakeHtmlReporter', 'TestSuite/Reporter');
App::uses('Model', 'Model');
/**
* Secondary Post stub class.
*/
class SecondaryPost extends Model {
/**
* @var string
*/
public $useTable = 'posts';
/**
* @var string
*/
public $useDbConfig = 'secondary';
}
/**
* CakeTestCaseTest
@@ -391,9 +407,9 @@ public function testAssertTextNotContains() {
*/
public function testGetMockForModel() {
App::build(array(
'Model' => array(
CAKE . 'Test' . DS . 'test_app' . DS . 'Model' . DS
)
'Model' => array(
CAKE . 'Test' . DS . 'test_app' . DS . 'Model' . DS
)
), App::RESET);
$Post = $this->getMockForModel('Post');
@@ -408,23 +424,45 @@ public function testGetMockForModel() {
$this->assertInternalType('array', $Post->find('all'));
}
/**
* Test getMockForModel on secondary datasources.
*
* @return void
*/
public function testGetMockForModelSecondaryDatasource() {
App::build(array(
'Plugin' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS),
'Model/Datasource/Database' => array(
CAKE . 'Test' . DS . 'test_app' . DS . 'Model' . DS . 'Datasource' . DS . 'Database' . DS
)
), App::RESET);
CakePlugin::load('TestPlugin');
ConnectionManager::create('test_secondary', [
'datasource' => 'Database/TestLocalDriver'
]);
$post = $this->getMockForModel('SecondaryPost', array('save'));
$this->assertEquals('test_secondary', $post->useDbConfig);
ConnectionManager::drop('test_secondary');
}
/**
* test getMockForModel() with plugin models
*
* @return void
*/
public function testGetMockForModelWithPlugin() {
App::build(array(
'Plugin' => array(
CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS
)
'Plugin' => array(
CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS
)

This comment has been minimized.

Show comment
Hide comment
@dereuromark

dereuromark Nov 21, 2014

Member

The latter was correct actually
But nitpicking

@dereuromark

dereuromark Nov 21, 2014

Member

The latter was correct actually
But nitpicking

), App::RESET);
CakePlugin::load('TestPlugin');
$this->getMockForModel('TestPlugin.TestPluginAppModel');
$TestPluginComment = $this->getMockForModel('TestPlugin.TestPluginComment');
$result = ClassRegistry::init('TestPlugin.TestPluginComment');
$this->assertInstanceOf('TestPluginComment', $result);
$this->assertEquals('test', $result->useDbConfig);
$TestPluginComment = $this->getMockForModel('TestPlugin.TestPluginComment', array('save'));
@@ -445,7 +483,7 @@ public function testGetMockForModelWithPlugin() {
* @return void
*/
public function testGetMockForModelModel() {
$Mock = $this->getMockForModel('Model', array('save'), array('name' => 'Comment'));
$Mock = $this->getMockForModel('Model', array('save', 'setDataSource'), array('name' => 'Comment'));
$result = ClassRegistry::init('Comment');
$this->assertInstanceOf('Model', $result);
@@ -722,13 +722,23 @@ public function getMockForModel($model, $methods = array(), $config = array()) {
list($plugin, $name) = pluginSplit($model, true);
App::uses($name, $plugin . 'Model');
$config = array_merge((array)$config, array('name' => $name));
unset($config['ds']);
if (!class_exists($name)) {
throw new MissingModelException(array($model));
}
$mock = $this->getMock($name, $methods, array($config));
$availableDs = array_keys(ConnectionManager::enumConnectionObjects());
if ($mock->useDbConfig === 'default') {
$mock->setDataSource('test');

This comment has been minimized.

Show comment
Hide comment
@DIDoS

DIDoS Nov 20, 2014

Contributor

This does not work if the data source "default" does not exist because setDataSource is trying to get the prefix from the "old" datasource.
Also there should be a check if "useTable" === false. If that is the case no ds should be set.

@DIDoS

DIDoS Nov 20, 2014

Contributor

This does not work if the data source "default" does not exist because setDataSource is trying to get the prefix from the "old" datasource.
Also there should be a check if "useTable" === false. If that is the case no ds should be set.

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Nov 20, 2014

Member

Pull requests to fix the bugs would be great, or issues describing how to reproduce the issue.

@markstory

markstory Nov 20, 2014

Member

Pull requests to fix the bugs would be great, or issues describing how to reproduce the issue.

This comment has been minimized.

Show comment
Hide comment
@hakito

hakito Jan 4, 2015

I can confirm the behavior described by DIDoS and even worse. setDataSource will cause a connect to the default datasource, which is definitely something i don't want in test code.

So two ways to reproduce the problem:

  • Remove the default config from database.php
    This will cause a MissingDatasourceConfigException: The datasource configuration "default" was not found in database.php
  • Set invalid credentials for default connection in database.php
    This will cause a MissingConnectionException: Database connection "Mysql" is missing, or could not be created.
@hakito

hakito Jan 4, 2015

I can confirm the behavior described by DIDoS and even worse. setDataSource will cause a connect to the default datasource, which is definitely something i don't want in test code.

So two ways to reproduce the problem:

  • Remove the default config from database.php
    This will cause a MissingDatasourceConfigException: The datasource configuration "default" was not found in database.php
  • Set invalid credentials for default connection in database.php
    This will cause a MissingConnectionException: Database connection "Mysql" is missing, or could not be created.
}
if ($mock->useDbConfig !== 'test' && in_array('test_' . $mock->useDbConfig, $availableDs)) {
$mock->setDataSource('test_' . $mock->useDbConfig);
}
ClassRegistry::removeObject($name);
ClassRegistry::addObject($name, $mock);
return $mock;

0 comments on commit 9b9e886

Please sign in to comment.