Unit testing code that calls RESTful API

Question

I'm writing a library which calls a RESTful API and then performs some work on the data.

I've architected my code like this:

I have a class (let's call it APIRequestor) that initiates a call to the API via an injected HTTPRequestor library (which basically wraps a cURL request). That class, after retrieving the response, will perform some operations on that data and then return an APIResult object to the caller.

In order to test the APIRequestor, I created a HTTPRequestorMock which extends HTTPRequestor that, rather than wrapping a cURL request, it simply reads a result from a file I have on disk that way I can test different scenarios.

Below is the code for the APIRequestor object:

<?php
namespace APITests;
require_once "PHPUnit/Autoload.php";
require_once "../classes/APIRequestor.php";
require_once 'HTTPRequestorMock.php';
class APIRequestor extends \PHPUnit_Framework_TestCase {
 public function setUp() {}
 public function tearDown() {}
 public function getMockedRequestor() {
 $requestor = new \APIClasses\APIRequestor();
 $requestor->HTTPLibrary = new HTTPRequestorMock();
 return $requestor;
 }
 public function testResponseIDsNoResults() {
 $requestor = $this->getMockedRequestor();
 $requestor->HTTPLibrary->overrideFilepath = "./mock_assets/ResponseIDsNoResults.xml";
 $responseIDs = $requestor->getResponseIDsSinceResponseID(0);
 $this->assertTrue(count($responseIDs) === 0, "More than zero results returned.");
 }
}

1. Is this a good design?
2. Can I improve this design in order to make future additions more testable?
3. Is this the common practice for removing a dependency on an external server for the sake of my unit tests?

• zerkms: "I created a HTTPRequestorMock which extends HTTPRequestor" --- why don't you use native $this->getMock phpunit thing?
• Jesse Bunch: I could use that. I originally figured it'd be cleaner to just override the method in the subclass.
• zerkms: not sure about "cleaner" but for me it's much easier to see the test case and mock setup near, so I could understand what's happening without requirement to explore another class.
• rdlowrey: While I applaud your good intentions in testing your code (and +1'd to show it), the "is this a good design" questions are generally more appropriate on programmers.stackexchange.com ... also, I agree with @zerkms that you're best served to learn and utilize the phpunit mocking capabilities. Tests are at least as much for others as they are for you -- and other people will immediately understand your tests if you use a common API. Maybe you could tweak your question a bit so it doesn't ask for qualitative evaluation?
• Jesse Bunch: Roger to the both of you. This is the feedback I need.

Answer 1

Yes, it looks like a reasonable design and approach.

At the nit-pick level getMockedRequestor should be private or protected, to document that it is a helper for the tests. Also, if every test is going to be calling it, some people might move that code into setUp(). But that only saves one line ($requestor = $this->getMockedRequestor();) in each unit test, so I'd keep it where it is: I like to avoid using member variables, but that is personal preference.

You could use the phpUnit mock interface, but you don't have to. It gives you some very useful things, such as when you say you expect a function to be called once with "Hello World":

$requestor->expects($this->once())
 ->method('afunction')
 ->with($this->equalTo('Hello World'));

Then it will not only complain if afunction() never gets called, but also if it gets called twice.

But structure like this also brings restrictions. Have a poke around some of the phpunit mock questions on StackOverflow to see if you will hit them.

• Jesse Bunch: Thanks for your help. I had already implemented some of the things you mentioned based on things mentioned on Stack Overflow so it was good to hear confirmation here.