6

So I recently stumbled upon a problem while writing a unit test in Magento 2. You can read more about the problem in question here.

Vinai told me it is generally bad practice to use the object manager in unit tests:

In Unit tests, don't use the ObjectManager at all. Just instantiate your class with new, since all dependencies are test doubles anyway. Less magic makes for easier debugging. Especially like in your case where there is only a single dependency.

Now... I did what he suggested, and I changed my Object-Manager based constructing of the class to a vanilla new-statement and it worked:

So instead of this:

$this->sourceModel = (new ObjectManager($this))
    ->getObject(
        \Custom\Shipping\Model\Config\Source\ProductTypes::class,
        [
            'Config' => $this->configHelperMock
        ]
    );

I did this:

$this->sourceModel = new \Custom\Shipping\Model\Config\Source\ProductTypes($this->configHelperMock);

But I also have another test that needed to stub a helper. This I did like this:

$this->configHelper = (new ObjectManager($this))
    ->getObject(
        \Custom\Shipping\Helper\Config::class,
        [
            'scopeConfig' => $this->scopeConfigMock
        ]
    );

Now if you look at Magentos' AbstractHelper-class, you'll note that it has one constructor argument: Magento\Framework\App\Helper\Context. So I figured what the Object Manager does with above constructor call, is it sets the scopeConfig-parameter of my Context-object to my mocked object. I hope this assumption is correct.

But... to get back to Vinais' statement to not use the Object Manager, how would you do this without the Object Manager? Mock the entire Context object? Or is the Object Manager the correct strategy to use in this particular scenario?

I'm really trying to understand what's the correct way to write unit tests for Magento, because for now it seems to me that:

  • You need to use the Object Manager when you want to mock the content of the context-object
  • You don't need to use the Object Manager in all other scenarios.

Can anyone shine a light on this? The Magento 2 documentation is lacking of a more in-depth explanation on how to write proper unit tests.

10

The short answer is, yes, I would mock the entire Context class.

$mockScopeConfig = $this->getMock(\Magento\Framework\App\Config\ScopeConfigInterface::class);
$mockContext = $this->getMock(\Magento\Framework\App\Helper\Context::class, [], [], '', false);
$mockContext->method('getScopeContext')->willReturn($mockScopeConfig);

$classUnderTest = new \Custom\Shipping\Helper\Config($mockContext);

A bit of background

The base of my reasoning here is that I'm assuming it is good for classes to know as little as possible about other classes, including their parents.
I'm asserting this is correct because less knowledge about other classes means less broken code due to changes in other classes.

Core parent classes that my class under test inherits from are particular problematic because I only want to test code I write (in unit tests). I don't want to test core code.

Because of that I try to keep my code as decoupled as possible from core parent classes.
This means I do not use any dependencies from parent classes, for example, by accessing their protected properties.

If I would be writing the constructor of your helper the way I would like, it would look something like this:

public function __construct(HelperContext $context, ScopeConfigInterface $scopeConfig)
{
    parent::__construct($context);
    $this->myScopeConfig = $scopeConfig;
}

You see, this class is only marginally coupled to the parent dependency. I don't want my class to know about the parent or its dependencies.

Unfortunately however Magento forces us to couple our class to the Context class due to an annoying check during bin/magento setup:di:compile.

Because of this I would write the constructor like this:

public function __construct(HelperContext $context)
{
    parent::__construct($context);
    $this->myScopeConfig = $context->getScopeConfig();
}

If the parent class follows the design principles tell-don't-ask, then my class will not need to call any method of the parent class. Instead, all my class will have to do is implement an abstract method which will be called by the parent at the appropriate time (in pattern lingo this is the template method pattern).

Due to the legacy of Magento 1 however not all Magento 2 code follows this principle.
So sometimes I have to mock parts of a context class or parent dependencies, even if my code doesn't need them. I do so manually though and don't rely on the unit test ObjectManager helper.
(One example of a class that is required is entities extending from \Magento\Framework\Model\AbstractModel.)

Conclusion

Finally, I would like to ask you why you are extending the AbstractHelper in the first place?
Just because core modules do it like that?
Does it provide any real benefit?
If your class only requires the scope config, why not have NO parent class and just depend on the ScopeConfigInterface directly?

That way you avoid a lot of test doubles and potential breakage when core code changes, plus you have less unnecessary overhead during runtime because all the other unneeded dependencies don't have to be instantiated.

  • I have an answer to your question 'why you are extending the AbstractHelper in the first place?'. It turns out, that if you don't extend the abstract helper, you cannot use $this->helper(...) in your template files. Or is the use of $this->helper() also considered bad practice? I personally like to keep all the logic in my block class so that my phtml file only gets it's information (and logic) from that specific class, but sometimes I find that the block class is nothing more than a pass-thru for helper X. – Giel Berkers Oct 14 '16 at 8:33
  • Yes, I think $this->helper() should not be used in templates. Rather add a method to the template that contains the required logic. Here is a nice post from @fschmengler on the topic of helpers: schmengler-se.de/en/2016/09/stop-using-helpers – Vinai Oct 14 '16 at 9:50
  • I think, the construct methods are missing double underscore, please update these @Vinai – Asrar Jan 26 '18 at 13:41
  • Thanks to you @Vinai, for all these invaluable materials and such great shares! – Asrar Jan 26 '18 at 15:41
4

To answer your question I will describe why ObjectManagerhelper class was developed by Magento team: When M2 development started all classes use Mage::* methods and we start increase coverage with unit test. But when we add new constructor injection many test start fails. So, we decide to introduce helper class to avoid the unneeded work.

But now constructor dependencies are stabilized, so helper is not need in most cases

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.