Creating testcase

nimasdj · October 31, 2019, 7:31am

How to create a testcase for a library class that uses curl? Should the testcase actually try to send and receive data? Please help creating one.

John_Betong · October 31, 2019, 7:40am

Sometime ago I created a Curl function that downloads a URL. The function source code is on the home-page:

http://johns-jokes.com/downloads/sp-d/johnyboy-curl-test/

append " ?url=http://Site-2-Test.net" to index.php

I will leave it up to you to create a class and to experiment with sending data

nimasdj · October 31, 2019, 7:47am

I meant how to create PHPUnit testcase.

rpkamp · October 31, 2019, 8:15am

The proper way to solve this is by using the Dependency Inversion Principle, which states that high level modules should not depend on low level modules, but both should depend on abstractions.

For this case it means that your class should not depend on the library class directly, but it should depend on an abstraction, and the library should implement that abstraction.

I don’t know what the library looks like, but let’s suppose it has a get method that returns a string (for HTTP GET).

The first step is to write an interface

interface HttpClient
{
    public function get(string $url): string;
}

Now instead of injecting the library in your class, inject this HttpClient interface

class YourClass
{
    private $client;
    public function __construct(HttpClient $client)
    {
        $this->client = $client;
    }
}

And then a class that implements the interface and uses the library

interface LibraryHttpClient implements HttpClient
{
    private $libraryClass;

    public function __construct(LibraryClass $libraryClass)
    {
        $this->libraryClass = $libraryClass;
    }

    public function get(string $url): string
    {
        return $this->libraryClass->get($url);
    }
}

And now in your test you can create a fake HttpClient implementation that implements the interface and returns anything you tell it to return. You can code this up yourself or use a mocking framework.

An added advantage of this that if you ever decide you want to swap the library for some other library all you need to do is create a new class that implements the HttpInterface and plug that in to your class instead. No need to change you class at all.

Does that make sense?

nimasdj · October 31, 2019, 8:25pm

I found this repo: https://github.com/phplicengine/bitly
Let’s use it as sample that I can learn about testcase. Can you please help with it?

rpkamp · October 31, 2019, 8:55pm

For that project I would start by doing the following:

Create an interface for PHPLicengine\Api\Api, with all the public methods like get, post, etc. Let’s call it PHPLicengine\Api\ApiInterface
Let PHPLicengine\Api\Api implement PHPLicengine\Api\ApiInterface
Inject PHPLicengine\Api\ApiInterface into classes like PHPLicengine\Service\Bitlink, instead of letting that class extend it.

Then in your tests you can mock PHPLicengine\Api\ApiInterface into the Bitlink class for example, and use that to test it.

As a small example, suppose the PHPLicengine\Api\Api class has just a get method and Bitlink has just the getBitlink method.

So the interface for that small example would look like this

<?php

namespace PHPLicengine\Api;

interface ApiInterface
{
    public function get($url, $params = null, $headers = null);
}

<?php

namespace PHPLicengine\Api;

class Api implements ApiInterface
{
   // normal implementation
   // nothing needs to change here
}

<?php

namespace PHPLicengine\Service;

use PHPLicengine\Api\ApiInterface;

class Bitlink
{
    private $api;
    private $url;

    public function __construct(ApiInterface $api)
    {
        $this->api = $api;
        $this->url = 'https://api-ssl.bitly.com/v4';
    }

    public function getBitlink(string $bitlink) 
    {
        return $this->api->get($this->url . '/bitlinks/'.$bitlink);
    }
}

And now for the test

<?php

use PHPLicengine\Api\ApiInterface;
use PHPLicengine\Service\Bitlink;
use PHPLicengine\Api\ApiInterface;

class BitlinkTest extends TestCase
{
    public function testGetLink()
    {
        $mock = $this->createMock(ApiInterface::class);
        $mock
            ->expects($this->once())
            ->method('get')
            ->with($this->equalTo('https://api-ssl.bitly.com/v4/bitlinks/foobar'));

        $bitlink = new Bitlink($mock);
        $bitlink->getBitlink('foobar');
    }
}

(not tested, so parts might be off slightly and/or there might be typos etc, but this is the method behind it)

nimasdj · November 1, 2019, 3:59am

Thanks a lot. That getBitlink has just a path parameter and in tescase we are testing if it returns a link with a foo path parameter, what if a case has a get query parameter or post query parameter or both has path and query parameter? Please give sample for them so.

rpkamp · November 1, 2019, 6:33am

How about you try it yourself and let me know what you get stuck with

nimasdj · November 1, 2019, 7:41am

I installed the library with composer and did changes you said in classes. Then I added BitlinkTest class you said in this location:
vendor/phplicengine/bitly/lib/PHPLicengine/tests/BitlinkTest.php

then in upper folder than /vendor I used this command:

$ /opt/cpanel/ea-php72/root/usr/bin/php vendor/bin/phpunit --version

and I got the PHPUnit version, so I assume paths are correct.

But when I try to run this:
$ /opt/cpanel/ea-php72/root/usr/bin/php vendor/bin/phpunit --bootstrap vendor/autoload.php vendor/phplicengine/bitly/lib/PHPLicengine/tests/BitlinkTest

I get:
Cannot open file "BitlinkTest.php".

Where and how should I call the testfile?

rpkamp · November 1, 2019, 7:54am

You’re making it way too complicated.
The entire vendor thing is making it hard.

How about you just clone the repository and then work on it?

nimasdj · November 1, 2019, 8:25am

I did clone it, now I have that lib in
bitly/lib/PHPLicengine/Api
bitly/lib/PHPLicengine/Service
etc.
I placed the test in
bitly/lib/PHPLicengine/tests/BitlinkTest.php

Then I used:

$ /opt/cpanel/ea-php72/root/usr/bin/php phpunit --bootstrap vendor/autoload.php bitly/lib/PHPLicengine/tests/BitlinkTest

I am still getting the same error. What to do? And what about namespaces as autoloader has no longer those namespaces as this is not installed via composer? PHPUnit can still find them?

rpkamp · November 1, 2019, 9:01am

Have you tried /opt/cpanel/ea-php72/root/usr/bin/php phpunit --bootstrap vendor/autoload.php bitly/lib/PHPLicengine/tests/BitlinkTest.php?

nimasdj · November 1, 2019, 9:14am

Opps, I discovered the error, it was because of duplicate use line in interface!!
I removed one of them, now I get:
There was 1 warning:

BitlinkTest::testGetLink
Cannot stub or mock class or interface “PHPLicengine\Api\ApiInterface” which does not exist

It guess it means it cannot find the interface because of autoloader as it is not installed via composer but clone?

nimasdj · November 1, 2019, 9:27am

Well I am back to use it in vendor installed by composer now I get:

$ phpunit --bootstrap vendor/autoload.php vendor/phplicengine/bitly/lib/PHPLicengine/tests/BitlinkTest.php
Time: 113 ms, Memory: 10.00 MB

OK (1 test, 1 assertion)

The whole problem was because of adding .php extension, I used only BitlinkTest, but it was not mentioned this in PHPUniit doc, that .php is needed!

Now for normal usage do I need:

$api = new API('ffaaf....');
$bitlink = new Bitlink($api);
$result = $bitlink->getBitlink('bit.ly/34nRNvl');

rather than passing api key directly to Bitlink constructor as it was before? right?

I stuck for cases with query parameter, please help.
PS. As about query params I guess to use ?foo=bar in url in asserts but I am not sure if this is good idea for post methods?

nimasdj · November 1, 2019, 10:40am

I tried this for methods with both path and query parameters. I am stuck please help.

    $mock
        ->expects($this->once())
        ->method('post')
        ->with($this->equalTo('https://api-ssl.bitly.com/v4/bitlinks/foobar'))
        ->with($this->assertArraySubset(['key' => 'value'], ['key' => 'value']));
    $bitlink = new Bitlink($mock);
    $bitlink->getBitlink('foobar', ['key' => 'value']);

rpkamp · November 1, 2019, 10:54am

You’re mixing mock functionality with assertion functionality
You should call with() only once; you’re calling to the method once with two parameters; you’re not calling the method twice every time with a different parameter.

nimasdj · November 1, 2019, 11:25am

    public function testGetMetricsForBitlinkByReferrersByDomains()
    {
        $mock = $this->createMock(ApiInterface::class);
        $mock
            ->expects($this->once())
            ->method('get')
            ->with(
                    $this->equalTo('https://api-ssl.bitly.com/v4/bitlinks/test/referrers_by_domains'),                    
                    $this->identicalTo(['key' => 'value'])
                    );
        $bitlink = new Bitlink($mock);
        $bitlink->getMetricsForBitlinkByReferrersByDomains('test', ['key' => 'value']);
    }

This one worked. Is it a good practice? And constraints are enough or for sure better to use more constraints usings localicalAnd() and add more constraints like IsType() for each parameter?

rpkamp · November 1, 2019, 11:47am

Yes that looks good

isType() doesn’t really add anything, because you’re comparing to hardcoded values; only strings match strings, so it will of course be of the type string.

isType() can be used when you’re not sure of the exact value, but you want to check the type.

For this example logicalAnd is not needed, but it may be useful in other cases. Not sure.

nimasdj · November 1, 2019, 11:52am

Now for normal usage do I need this:

$api = new API('ffaaf....');
$bitlink = new Bitlink($api);
$result = $bitlink->getBitlink('bit.ly/34nRNvl');

rather than passing api key directly to Bitlink constructor as it was before? right?

rpkamp · November 1, 2019, 11:59am

Now the next step I would take for that package, if it were mine, is stop leaking HTTP implementation details from the Bitlink class methods. As a user of Bitlink I am not at all interested that it uses HTTP to get those results, and I really can’t be bothered to go and check HTTP status codes for example.

In my opinion, for example for the method getBitlink should return an object like so:

<?php

namespace PHPLicengine\Service\Dto;
// DTO stands for Data Transfer Object

class Bitlink
{
    /**
     * @var array<string, string>
     */
    public $references;

    /**
     * @var bool
     */
    public $archived;

    /**
     * @var string[]
     */
    public $tags;

     // etc
}

And when something goes wrong (internal server for example) it should throw an exception.

This has the following advantages:

users of the Bitlink class don’t need to know about HTTP anymore
if you ever switch from HTTP to something else you don’t need to change the return type of the Bitlink service
clients of the Bitlink class get an object with typed properties so it is a lot easier to discover what can be done with that, plus it enables auto completion in IDEs like PHPStorm. In the current situation I should probably call getBody() on the result and then I get an array, which is a lot less discoverable and does not offer any guarantees and also no autocompletion.