Unit test Issue on getting value

Question

I am creating a unit test for my simple method, this method get the value from requestHeader

here is my class which I want to create test class

namespace Vendor\Module\Helper;

use Magento\Framework\App\Helper\AbstractHelper;
use Magento\Framework\App\Helper\Context;
use Magento\Framework\App\Request\Http;

class Data extends AbstractHelper
{

    const TEST_VALUE = 'KEYUR';

      public function __construct(
        Context $context,        
        Http $request        
    )
    {        
        $this->request = $request;        
        parent::__construct($context);
    }

    // this is the method which I want to write a test case

  public function getTestValue()
    {
        return $this->request->getHeader(self::TEST_VALUE);
    }

}

and here is my test class,

namespace Vendor\Module\Test\Unit\Helper;

use Magento\Framework\App\Request\Http;
use Vendor\Module\Helper\Data;
use Magento\Framework\App\Helper\Context;
use Zend\Http\Header\HeaderInterface as HttpHeaderInterface;

class DataTest extends \PHPUnit\Framework\TestCase
{
    protected $testValue = 'KEYUR';

     /**
     * @var requestHeaderMock|\PHPUnit_Framework_MockObject_MockObject
     */
    protected $requestHeaderMock;

     /**
     * @var Data
     */
    protected $myHelperMock;

     /**
     * @var HttpHeaderInterface
     */
    protected $httpHeader;

     protected function setUp()
    {
        $this->contextMock = $this->createMock(Context::class);        
        $this->requestMock = $this->createMock(Http::class);        
        $this->httpHeader = $this->createMock(HttpHeaderInterface::class);
        $this->requestMock->method('getHeader')->willReturn($this->httpHeader);

        $this->myHelperMock = new Data(
            $this->contextMock,            
            $this->requestMock            
        );
    }

    public function testgetTestValue()
    {
        $this->requestMock
            ->expects($this->atLeastOnce())
            ->method('getHeader')
            ->with(Data::TEST_VALUE)
            ->willReturn($this->testValue);

        $this->assertTrue($this->testValue == $this->myHelperMock->getTestValue());
    }


}

But my test case fail with Failed asserting that false is true. I do not know why ? Can someone put some shade on this ?

Answer 1

The problem lies in this line in setUp:

    $this->requestMock->method('getHeader')->willReturn($this->httpHeader);

Here you instruct the request mock to return your HttpHeaderInterface mock all the time.

Later, in the test, you write

    $this->requestMock
        ->expects($this->atLeastOnce())
        ->method('getHeader')
        ->with(Data::TEST_VALUE)
        ->willReturn($this->testValue);

but this does not have effect anymore. If you remove the line from setUp, your test should pass. But it does not make much sense, since getHeader should return a HttpHeaderInterface, not a string.

---

Now, as promised, a constructive answer:

You are probably just trying out unit testing with a a seemingly simple example, but you are running into problems because:

1. your code is more complex than it could be
2. your test code is tightly coupled with the implementation

Let me show you how you can make this more testable and get tests with more value:

First of all, there is no need to extend the AbstractHelper with its huge Context dependency for this, or for any custom code (I wrote about "Stop Using Helpers" a while ago, you might want to read this). Instead, write small classes that only depend on what they need. If you need one or more methods to access HTTP request headers, write a dedicated request class for it. You may still call it "helper", that's up to you. I wouldn't :)

"only depend on what you need" also applies to classes and interfaces. If the class you are using has an interface and you only use methods from that interface, depend on the interface. In this case, Magento\Framework\App\Request\Http does not have an interface with getHeader(), but we still can take the most abstract type with this method. This is the base class \Zend\Http\Request where getHeader() is defined. Use this type as constructor argument, it will make testing easier:

namespace Vendor\Module\Helper;

use Zend\Http\Request;

class RequestHeaders
{

    const TEST_VALUE = 'KEYUR';

    public function __construct(
        Request $request        
    )
    {        
        $this->request = $request;        
    }

    public function getTestValue()
    {
        return $this->request->getHeader(self::TEST_VALUE);
    }

}

This means that you need to set a preference for the argument in di.xml (not relevant for unit testing, but for integration testing and to actually make it work):

<type name="Vendor\Module\Helper\RequestHeaders">
    <arguments>
        <argument name="request" xsi:type="object">Magento\Framework\App\Request\Http</argument>
    </arguments>
</type>

The nice thing about the Request base class is, that it is a quite self-contained value object, so we can use a real instance in testing instead of mocking everything. This will lead to more stable tests

This is how I would write a test for this class:

namespace Vendor\Module\Test\Unit\Helper;

use Zend\Http\Request;
use Zend\Http\Headers;
use Vendor\Module\Helper\RequestHeaders;

class RequestHeadersTest extends \PHPUnit\Framework\TestCase
{
    public function testReturnsTestValueFromHttpHeader()
    {
        $request = new Request();
        $request->setHeaders(Headers::fromString("KEYUR=Foobar"));
        $requestHeaders = new RequestHeaders($request);
        $this->assertEquals('Foobar', $requestHeaders->getTestValue());
    }
}

Some would argue that this test is not isolated enough because it uses classes from the Zend\Http package without mocking them. But by using getHeader(), the code is coupled to that implementation, so why pretent it doesn't and make make testing unnecessary complicated? On the plus side, your test is now less coupled to your implementation. It does not assume anything about method calls anymore, it's plainly about input and output now. This makes it more stable, as mentioned before, and also more open to refactoring.