6

This question came to mind after I've seen the edits made by Vinai to my answer here.
Let's say we have a class to test with a constructor like this

namespace Some\Namespace;

class SomeClass
{
    public function __construct(
        \Product $product,
        \Category $category
    ) {
        $this->product = $product;
        $this->category = $category;
    }
}

I've seen in the core Magento 2 unit tests this way to mock it:

$objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
$productMock = $this->getMockBuilder('Product')
    ->disableOriginalConstructor()
    ->getMock();
$categoryMock = $this->getMockBuilder('Category')
    ->disableOriginalConstructor()
    ->getMock();
$model = $objectManager->getObject(
    'Some\Namespace\SomeClass',
    [
         'product' => $product,
         'category' => $category
    ]
);

And this works. I've used similar approaches in my own tests.
But Vinai made an edit (and I trust that this edit works).

This

$this->getMockBuilder('Product')

Became this

$this->getMockBuilder(\Product::class).  

What's the difference? (I know what ::class means and does so don't explain this).

Then this:

$model = $objectManager->getObject(
    'Some\Namespace\SomeClass',
    [
         'product' => $product,
         'category' => $category
    ]
);

became this:

$model = new \Some\Namespace\SomeClass($product, $category);

When should I use the object manager helper to instantiate classes and when should I just use new? Or why should I use one instead of the other?

  • The reason I edited your answer was not because your answer was wrong, but just to try and promote better practices for writing tests. – Vinai Jun 23 '16 at 10:01
  • @Vinai. I figured that out. That's why I asked this question and tagged it with coding-standards. – Marius Jun 23 '16 at 10:23
  • Thank you Marius, great service! (I know this kind of "thanks" comments are discouraged but I wanted to write this nevertheless) – Vinai Jun 23 '16 at 10:25
5

Instantiating the class under test

Unit tests should be a simple as possible because that helps with understanding the code being tested as well as debugging.

Lets have a look at the purpose of the \Magento\Framework\TestFramework\Unit\Helper\ObjectManager. In effect, all it does is create simple dummies automatically.

A complex wrapper like that is sometimes useful when testing formerly untested legacy code.
It can also help hiding parameters that are not required for a specific test so it is easier to focus on what is being tested.
But there are better ways to do that, like moving the instantiation of the class under test into the setUp method, or using a utility method for instantiation.

If all dependencies are created in the test explicitly, the helper adds zero value. In fact, I consider it harmful, since it adds one more layer of complexity and indirection.
It requires a developer to know what it does. In comparison, the new keyword is a lot simpler.

I'm not saying that there is no use case for the helper, but using it "just because" goes against the principle of unit tests being as simple as possible.

The core team does not always follow these principles. I believe that is mainly because many of their unit tests where written post-fact for legacy Magento 1 code.
Many of the early tests they wrote are somewhat rigid. I believe they have learned a lot in regards to writing unit tests over time. Probably they would write many differently nowadays.
What I'm trying to say is that it isn't a good idea to take the core tests as examples of "how to write a good unit test".

Using the ::class constant

I'll add this to the answer even though you said to leave it out because some Magento developers might not yet be familiar with the ::class constant available since PHP 5.5.

Favoring the ::class constants over string class names is a good idea because it makes IDEs smarter.
With the ::class constant the IDE can warn me if I have a typo in the class name.
It will also apply refactorings to the class name or the class namespace automatically, so it helps keeping the code in good shape.

Using strings as class names does not offer these benefits. They have no advantage over the ::class constant.

I am aware the core team uses string class names, but that is legacy from back in the day when Magento 2 was still supporting PHP 5.4 (see https://github.com/magento/magento2/issues/4068).

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.