CODEWITHSUNDEEP
phpunit-testingif-statementphpunit
Arrowkill
Arrowkill

Reputation: 188

How to Unit Test an if Statement in PHP

I am currently attempting to unit test a get method with an if statement inside. I am not sure how to go about testing this method and I am not finding much information that is helpful. 

/**
 * @var Info
 */
private $info = null;

# Not pertinent information skipped

public function getInfo()
{
    if(!$this->Info) {
        $this->Info = InfoFacade::getInfoByToken($this->getToken());
    }

    return $this->Info;
}

Then in the class InfoFacade there is a function that grabs a set of information and returns itself.

My understanding is that I need to use the shouldreceive and shouldnotreceive, but I am not sure if I should use a dataProvider and if so how to properly apply it to this situation.

My question is how do I test for an if statement like this, and if a dataProvider is the best way like I am led to believe, how do I properly set up the dataProvider?

Upvotes: 0

Views: 3560

Answers (2)

hakre
hakre

Reputation: 197757

The given answer is correct in it's introduction, I'd like to add another aspect, especially after reading your comment beneath it with the question about mocking.

You're asking about testing an if statement inside a public method.

It is important for that case that the if statement is actually an internal detail of the public method getInfo(), the unit test should not really care about that implementation detail. The implementation detail is that the subject under test (SUT) will obtain the info-data on first call (lazy loading) and store it internally for future reference.

This means that the public interface you have is a wrapper around some implementation detail (how that object aquires the data is that objects buisiness, but none of the business of your test).

Now - as you write - for a unit test you'd like to test for all paths in that method. This requires to call the method twice. In the first call you get data back and in the second call that data must be the same. So technically all you need to do is to call that method twice to test the internals.

$foo = new Foo();
$expected = $foo->getInfo();
$actual = $foo->getInfo();
$this->assertSame($expected, $actual);

However to test the unit, you actually only need to call the method once to know that it works as you can expect that the private property won't change as it is private (properly encapsulated) and you actually don't want to care if you use that object.

So actually one could argue that you don't want to test the if clause as that is an implementation detail.

Note: When you would measure the code coverage you would see all lines covered with the single call already (albeit not all paths have been covered by the single call). This just shows that 100% coverage must not mean you covered all paths in isolation.

It is my personal understanding that for a unit-test it should be enough to call the method once in a test and assert that it won't return NULL.

$foo = new Foo();
$this->assertNotNull($foo->getInfo());

You could make this more strict by asserting more exactly of what you expect that method to return (e.g. some concrete type not just non-NULL), but for the unit-test this should already be enough.

The key point is here, that you first-of-all test the public interface of the SUT. You only need to test for details if there is a regression with a specific implementation. That belongs into another test so that you can remove it after a year, b/c the test was only necessary to fix a regression and after some more time should not be necessary any longer at all.

And better then: For that Facade in use, you probably want to test if the Facade is working, that is calling that global function and check for the expected outcome. This is in another test for that Facade, not for the object collaborating with it.

Otherwise you can mock things to death just for the sake of it, but it's better to keep things simple in testing as it is often the case in programming. Otherwise there is always tendency to over-engineer things, which can cause harm in testing. You would turn public interface ad absurdum, but instead you want to rely on them. To have trust in relying on them, therefore is that test.

As an additional note, consider that your tests test for the things even if the implementation details of a method change. This is probably a good description, you want to change things over time and the existing tests should report to you if the changes didn't change any of the (specified by the unit-tests) intention.

And another side-note: As you're having the initialize-property-if-null-on-get, you can also inline this with PHP:

public function getInfo()
{
    return $this->Info 
        ?: $this->Info = InfoFacade::getInfoByToken($this->getToken());
}

or:

public function getInfo()
{
    return $this->Info 
        ?? $this->Info = InfoFacade::getInfoByToken($this->getToken());
}

See the =, ternary (first example) or null-coalesce operator (second example) (please refere to the PHP manual for all these). For a comparison of the last two see as well

Upvotes: 1

Isaac Hildebrandt
Isaac Hildebrandt

Reputation: 1026

The idea of unit testing is that you have a known state before the test and an expected state after the test. Here, you're expecting the state to only be changed the first time getInfo is called.

To run this test, you'll start with a state where Info is null. Then you can call the method and check if the returned info is what you expected to be returned by the facade. Then change what's in the facade and call getInfo again. What's returned should be the same as from the first call, not whatever you changed the facade to.

Upvotes: 0

Related Questions