Skip to content

Instantly share code, notes, and snippets.

@mvriel
Created October 30, 2012 17:14
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save mvriel/3981617 to your computer and use it in GitHub Desktop.
Save mvriel/3981617 to your computer and use it in GitHub Desktop.
Class IpRange
{
public function setStartingAddress($startingAddress)
{
InputValidator::assertValidAddress($startingAddress);
$this->startingAddress = (int) $startingAddress;
return $this;
}
}
@mvriel
Copy link
Author

mvriel commented Oct 30, 2012

The above method setStartingAddress is impossible to accurately test due to the static call to assertValidAddress.

Try to follow along:

  1. When writing a test, what are we going to test here?
    1. Whether the startingAddress property is set with the provided value
    2. That the current object is returned
    3. That an exception is thrown in case an invalid value is provided
  2. Let's focus on use-case '3'; how are we going to test this?
  3. The only solution would be to test whether all rainy day scenario's of the assertValidAddress method produce an exception

WAIT: let's repeat: test whether all rainy day scenario's of the assertValidAddress method

This is the unit test contents for that method; why should I even WANT to test that here again? Can't we trust that the InputValidator::assertValidAddress does what it is supposed to do? Doesn't this lead to duplication in every location I use that assertion?

You do not want to test this; you want to assert that the assertValidAddress method of the InputValidator is called. How are you going to do that?

You can't. It is a static.

Conclusion: you either

  1. Duplicate rainy day scenarios across your unit tests
  2. Do not test rainy day scenarios (bad test!)
  3. Test that your validator is invoked by being able to stub and inject your validation class and make sure that validation class has unit tests.

@mvriel
Copy link
Author

mvriel commented Oct 30, 2012

Clarification; by stubbing I mean having a non-functional mock with which you can measure whether a method of it is called, see: http://www.phpunit.de/manual/3.0/en/mock-objects.html

@weierophinney
Copy link

Well, you test the behaviors you expect; it's just that one of those behaviors is encapsulated in the static method call. That said, you'd likely need a shared data provider for your individual test cases to ensure that the various behaviors are matched to inputs.

It'd be no different than if you used extension to encapsulate the assertion method.

@andriesss
Copy link

I think you are overstating the value of a mock here. YAGNI, right? With PHP 5.4 I would have used a trait...

Let's say I ditch the "staticness" of the InputValidator class... without dependency injection, how would I be able to check if the method was invoked?
Keep in mind that the implementation that the snippet is taken from is from the Modbus protocol, so it is fixed. Allowing dependency injection for the validator class, just for the sake of being able to mock it, and see if it is called seems like a bad way to go too.

Practical vs academical.

@mvriel
Copy link
Author

mvriel commented Oct 30, 2012

Hmm.. I generally test my abstract classes individually and then in child classes I stub the child class and only stub the expected abstract calls.

Example:

class Super
{
    protected function assertTrue()
    {
        ...
    }
}

class Child extends Super
{
    public function valid()
    {
        return $this->assertTrue();
    }
}

I would test the above using the following test:

class ChildTestCase
{
    function testValid()
    {
        $fixture = $this->getMock('Child', array('assertTrue'));
        $fixture->expects($this->once())->method('assertTrue')->will($this->returnValue(true));

        $this->assertTrue($fixture->valid());
    }

    function testValidWithInvalidInput()
    {
        $fixture = $this->getMock('Child', array('assertTrue'));
        $fixture->expects($this->once())->method('assertTrue')->will($this->returnValue(false));

        $this->assertFalse($fixture->valid());
    }
}

This test case only tests the actual behaviour of valid; I consider the behaviour of the assertTrue() to be unimportant to my unit test since that is governed by my abstract class (which is tested in a single location).

@mvriel
Copy link
Author

mvriel commented Oct 30, 2012

I think you are overstating the value of a mock here. YAGNI, right? With PHP 5.4 I would have used a trait...

I don't see how a trait would make any difference; a trait adds the same testing issues as a static, global function, singleton or any other piece of global state

@mvriel
Copy link
Author

mvriel commented Oct 30, 2012

Let's say I ditch the "staticness" of the InputValidator class... without dependency injection, how would I be able to check if the method was invoked?

That depends on how you make your validation methods visible; when using extension you can Stub your fixture as shown in the previous example. As a trait? You can't. Traits are effectively global state as well. SOLID has Dependency Inversion as a principle because it allows for things like testability.

@andriesss
Copy link

Extension is not an option in my case, given multiple inheritance is (unfortunately) not a language feature of PHP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment