magento / community-features

Magento Features Development is an Initiative to Allows Community Memebers Join to Development of Magento Features
46 stars 18 forks source link

Check existence of source class in TestFramework FactoryGenerator #303

Open HenKun opened 3 years ago

HenKun commented 3 years ago

Would anything speak against a check for the existence of the source class in Magento\Framework\TestFramework\Unit\Autoloader\FactoryGenerator?

The current code looks like:

private function isFactory($className)
    {
        if (!preg_match('/[\\\A-Z]/', substr(ltrim($className), 0, 1))) {
            return false;
        }
        $sourceName = rtrim(substr($className, 0, -strlen('Factory')), '\\');

        return $sourceName . 'Factory' == $className;
    }

If it would be changed to the following, there would be another degree of safety in unit tests, since it checks for the validity of a factory:

private function isFactory($className)
    {
        if (!preg_match('/[\\\A-Z]/', substr(ltrim($className), 0, 1))) {
            return false;
        }
        $sourceName = rtrim(substr($className, 0, -strlen('Factory')), '\\');

        if (!\class_exists($sourceName))
        {
            return false;
        }

        return $sourceName . 'Factory' == $className;
    }

It can also prevent missspelling errors. Currently unit tests can pass although the code won't work in production.

Any opinions on this?

m2-assistant[bot] commented 3 years ago

Hi @HenKun. Thank you for your report. To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this