PHPUnit: Doing assertions on non-public variables
Asked Answered
A

4

31

Suppose I have a class with a private property and associated public getter and setter. I want to test with PHPUnit that the property gets the correct value after the setter has been used or that the getter returns the correct property.

Of course I can test the setter by using the getter to see that the object is storing the correct value, and vice versa for testing the getter. However, this doesn't guarantee that the private property is the one being set.

Say I had the following class. I created a property, getter and setter. But I made a typo in the property name, so the getter and the setter don't actually manipulate the property they're meant to manipulate

class SomeClass
{
    private 
        $mane = NULL; // Was supposed to be $name but got fat-fingered!

    public function getName ()
    {
        return ($this -> name);
    }

    public function setName ($newName)
    {
        $this -> name = $newName;
        return ($this);
    }
}

If I run the following test

public function testSetName ()
{
    $this -> object -> setName ('Gerald');
    $this -> assertTrue ($this -> object -> getName () == 'Gerald');
}

I would get a pass. However, something very bad has actually happened that I don't want. When setName() is called, it actually creates a new property in the class with the name I thought my private property had, only the one that the setter creates is public! I can demonstrate that with the following code:

$a  = new SomeClass;

$a -> setName('gerald');
var_dump ($a -> getName ());
var_dump ($a -> name);

It would output:

string(6) "gerald"

string(6) "gerald"

Is there any way I can access the private properties from PHPUnit so I can write tests that make sure that the properties I think are being get and set actually really are being get and set?

Or is there some other thing I should be doing in a test to catch problems like this without trying to get access to the private state of the object under test?

Ashelyashen answered 19/1, 2012 at 15:42 Comment(1)
I'm not sure if this works with private variables. phpunit.de/manual/current/en/…Rhythmist
H
27

For testing properties, I'd make the same arguments I make then talking about testing private methods.

You usually don't want to do this.

It's about testing observable behavior.

If you rename all your properties or decide to store them into an array you should not need to adapt your tests at all. You want your tests to tell you that everything still works! When you need to change the tests to make sure everything still works you lose all the benefits as you also could make an error changing the tests.

So, all in all, you lose the value of you test suite!


Just testing the get/set combinations would be ok enough but usually not every setter should have a getter and just creating them for testing is not a nice thing ether.

Usually, you set some stuff and then tell the method to DO (behavior) something. Testing for that (that the class does what is should do) is the best option for testing and should make testing the properties superfluous.


If you really want to do that there is the setAccessible functionality in PHP reflections API but I can't make up an example where I find this desirable

Finding unused properties to catch bugs / issues like this one:

The PHP Mess Detector As a UnusedPrivateField Rule

class Something
{
    private static $FOO = 2; // Unused
    private $i = 5; // Unused
    private $j = 6;
    public function addOne()
    {
        return $this->j++;
    }
}

This will generate two warnings for you because the variables are never accessed

Hailstone answered 19/1, 2012 at 16:24 Comment(3)
If there's some way of catching errors like the one described in the question I'd like to know what it is though.Ashelyashen
@vascowhite: Doesn't seem to (Netbeans 7.0)Ashelyashen
@Ashelyashen I've expanded the question a little showing a tool that can help you finding those issuesHailstone
S
43

You can also use Assert::assertAttributeEquals('value', 'propertyName', $object).

See https://github.com/sebastianbergmann/phpunit/blob/3.7/PHPUnit/Framework/Assert.php#L490

Supplicatory answered 3/10, 2013 at 13:8 Comment(5)
Or use Assert::readAttribute($actualClassOrObject, $actualAttributeName) which is used by Assert::assertAttributeEquals()Lobel
This method is deprecated now and testing private/public methods is anti-practice (according to PHPunit contributors).Flock
@Flock I think you mean private/protected methods, as testing public methods is sort of the point of testing ;)Vomiturition
"anti-practice" depends on the context... This is very useful when checking that an array of options, for example, has been properly set from a json string (presumably that comes from the database or from an external source), for example.Labialize
It is now beyond "anti-practice." readAttribute() and related methods are deprecated and will be removed in PHPUnit 9.Bespangle
H
27

For testing properties, I'd make the same arguments I make then talking about testing private methods.

You usually don't want to do this.

It's about testing observable behavior.

If you rename all your properties or decide to store them into an array you should not need to adapt your tests at all. You want your tests to tell you that everything still works! When you need to change the tests to make sure everything still works you lose all the benefits as you also could make an error changing the tests.

So, all in all, you lose the value of you test suite!


Just testing the get/set combinations would be ok enough but usually not every setter should have a getter and just creating them for testing is not a nice thing ether.

Usually, you set some stuff and then tell the method to DO (behavior) something. Testing for that (that the class does what is should do) is the best option for testing and should make testing the properties superfluous.


If you really want to do that there is the setAccessible functionality in PHP reflections API but I can't make up an example where I find this desirable

Finding unused properties to catch bugs / issues like this one:

The PHP Mess Detector As a UnusedPrivateField Rule

class Something
{
    private static $FOO = 2; // Unused
    private $i = 5; // Unused
    private $j = 6;
    public function addOne()
    {
        return $this->j++;
    }
}

This will generate two warnings for you because the variables are never accessed

Hailstone answered 19/1, 2012 at 16:24 Comment(3)
If there's some way of catching errors like the one described in the question I'd like to know what it is though.Ashelyashen
@vascowhite: Doesn't seem to (Netbeans 7.0)Ashelyashen
@Ashelyashen I've expanded the question a little showing a tool that can help you finding those issuesHailstone
K
4

I just want to point out one thing. Let's forget about private fields for a moment and focus on what client of your class cares about. Your class exposes a contract, in this case - ability to alter and retrieve name (via getter and setter). Expected functionality is simple:

  • when I set name with setName to "Gerald", I expect to get "Gerald" when I call getName

That's all. Client won't (well, shouldn't!) care about internal implementation. Whether you used private field name, hashset or called web service via dynamically generated code - doesn't matter for client. The bug you are currently experiencing, from user point of view - is not a bug at all.

Whether PHPUnit allows you to test private variables - I don't know. But from unit-testing perspective, you shouldn't do that.

Edit (in response to comment):

I understand your concerns about possible exposure of internal state, however I don't think unit testing is the right tool to deal with that. You can come up with a lot of possible scenarios how something might do something else which wasn't planned. Unit tests are by no means cure for all and shouldn't be used as such.

Kaylenekayley answered 19/1, 2012 at 16:6 Comment(1)
You're right that in the case of honour the contract the class works as advertised. Whether or not the class fulfils the promises it makes is something that a black box PHPUnit test can usually spot with ease. The problem here is that whilst the API contract is fulfilled, a potential serious issue (unintentional exposure of internal state) has occured. This is something that requires "glass box" or "white-box" tests to catch (and that's how I tagged the question)Ashelyashen
A
2

I agree with the others that in general you want to avoid accessing privates in your tests, but for the cases where you need to, you can use reflection to read and write the property.

Ayesha answered 19/1, 2012 at 19:12 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.