Bug in PHPUnit test (null should be array)

[syno-dlr]

Hello all,

I just launched the standard PHPUnit test (without the dev:tests:run command) :

php <magentodir>/vendor/phpunit/phpunit/phpunit --debug

I am in <magentodir>/dev/tests/unit directory.

There is a fatal error on Magento\DownloadableImportExport\Model\Import\Product\Type\Downloadable file. On function fillDataLink, there is a fillExistOptions call where $existingOptions is null. It should be an array. It's the same in fillDataTitleLink method.

I just add an ugly fix with a is_null check for $existingOptions and set it to an empty array and it works.

I think the best way should be to change the fetchAll method to always return an empty array, if no data is retrieved in database.

Is there something I did wrong ?

Damien.

[syilmaz]

What's the status on this @okorshenko?
When we run the unit tests in Magento 2.0.6 we get the same error as mentioned above, coupled with a lot of other warnings.

[Vinai]

Dug into this yesterday, and interestingly this is caused with PHP 7 by a state leakage via \Magento\Setup\Module\Di\Code\Reader\Decorator\Directory::__construct() setting an error handler.

public function __construct(
    \Magento\Setup\Module\Di\Compiler\Log\Log $log,
    \Magento\Framework\Code\Reader\ClassReader $classReader,
    \Magento\Setup\Module\Di\Code\Reader\ClassesScanner $classesScanner,
    \Magento\Framework\Code\Validator $validator,
    $generationDir
) {
    $this->log = $log;
    $this->classReader = $classReader;
    $this->classesScanner = $classesScanner;
    $this->validator = $validator;
    $this->generationDir = $generationDir;

    set_error_handler([$this, 'errorHandler'], E_STRICT);
}

Under PHP 5.6 this does not cause problems, only with PHP 7.

Minimum steps to reproduce:

• Create a testsuite with only two tests:

<testsuite name="Debug Unit Tests">
    <file>../../../setup/src/Magento/Setup/Test/Unit/Module/Di/Code/Reader/InstancesNamesList/DirectoryTest.php</file>
    <file>../../../vendor/magento/module-downloadable-import-export/Test/Unit/Model/Import/Product/Type/DownloadableTest.php</file>
</testsuite>

• Run the testsuite with PHP 7 (I'm on 7.0.6, but it happened on earlier versions, too)

Possible Solution

A reliable fix is to restore the previous error handler in a \Magento\Setup\Test\Unit\Module\Di\Code\Reader\InstancesNamesList\DirectoryTest::tearDown() method.

protected function tearDown()
{
    restore_error_handler();
}

I've tried to use the __destruct() method on the Directory instance, but it seems that isn't called early enough. Even though the testsuite config above runs successfully when restoring the error handler in the destructor, the error still happens when running the full Magento testsuite.
Simply calling restore_error_handler() in the tearDown() makes the whole testsuite pass (on Magento CE 2.0.6, installed with composer create-project).

[rossluk]

It's not only PHP 7 problem. I've had the same with PHP 5.6.17 (but with Zend Opcache v7.0.6-dev). "tearDown" is good solution, thanks.

[piotrekkaminski]

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.