EcomDev_PHPUnit icon indicating copy to clipboard operation
EcomDev_PHPUnit copied to clipboard

Illegal offset error in EcomDev_PHPUnit_Model_Config->getModelInstance()

Open jsdalton opened this issue 11 years ago • 1 comments

When running a controller test against a Magento installation with Enterprise edition installed I'm bumping into the following test error:

Exception: Warning: Illegal offset type in isset or empty  in /vagrant/.modman/EcomDev_PHPUnit/app/code/community/EcomDev/PHPUnit/Model/Config.php on line 127

That's referencing this function in EcomDev_PHPUnit_Model_Config:

public function getModelInstance($modelClass='', $constructArguments=array())
{
    if (!isset($this->_replaceInstanceCreation['model'][$modelClass])) {
        return parent::getModelInstance($modelClass, $constructArguments);
    }

    return $this->_replaceInstanceCreation['model'][$modelClass];
}

So digging a bit deeper, here's the problem: The model class Enterprise_UrlRewrite_Model_Matcher_Redirect in Enterprise is being requested from getModelInstance() by passing an object as the $modelClass argument rather than a string. So the EcomDev_PHPUnit code above is choking when it tries to specify this object as an array index.

Taking a look at getModelInstance() in Mage_Core_Model_Config (which EcomDev_PHPUnit_Model_Config is extending), it appears the Magento team are accounting for this possibility (on line 1345 for me), by first passing $modelClass through the method getModelClassName() to return a string class name:

$className = $this->getModelClassName($modelClass);

I was able to resolve this issue by patching EcomDev_PHPUnit_Model_Config->getModelInstance() to use that method:

public function getModelInstance($modelClass='', $constructArguments=array())
{
    $className = $this->getModelClassName($modelClass);
    if (!isset($this->_replaceInstanceCreation['model'][$className])) {
        return parent::getModelInstance($modelClass, $constructArguments);
    }

    return $this->_replaceInstanceCreation['model'][$className];
}

This ensures that a string is passed to the _replaceInstanceCreaation['model'] array and thus no error!

I'd be happy to file a pull request with this change if the above makes sense.

Thanks!

jsdalton avatar Feb 10 '14 20:02 jsdalton

I was still having issue after applying the proposed fix. I had to change it to the following to have it working:

    public function getModelInstance($modelClass='', $constructArguments=array())
    {
        $className = $this->getModelClassName($modelClass);
        if (!isset($this->_replaceInstanceCreation['model'][$className])) {
            if(is_string($modelClass) && isset($this->_replaceInstanceCreation['model'][$modelClass])) {
                return $this->_replaceInstanceCreation['model'][$modelClass];
            }
            return parent::getModelInstance($modelClass, $constructArguments);
        }

        return $this->_replaceInstanceCreation['model'][$className];
    }

Thanks!

jpjoao avatar Jul 31 '14 13:07 jpjoao