google2fa icon indicating copy to clipboard operation
google2fa copied to clipboard

Bug in testing due to PHPUnit's interpretation

Open alexw23 opened this issue 3 years ago • 1 comments

As was building some of my own tests I noticed that where you assert verifyKeyNewer should be returning a timestamp, it actually returns a boolean.

Take the following as an example of this (Source):

$this->assertEquals(
    26213400,
    $this->google2fa->verifyKeyNewer(
        Constants::SECRET,
        '758576',
        null,
        2,
        26213400
    )
); // 26213400

It turns out that if the "timestamp" is empty verifyKeyNewer returns true not a timestamp.

And PHPunit for some reason considers these both to be equal, for some reason I have no idea.

The following example also works:

$this->assertEquals(
    26213400,
    true
); // 26213400

Just thought I would report this in an effort to have any expectation of how it should be working clarified.

Edit: looks like you need to be using assertSame.

alexw23 avatar Jun 14 '21 03:06 alexw23

And PHPunit for some reason considers these both to be equal, for some reason I have no idea.

I know it's been a long time since this issue was open. But just to clarify that PHPUnit's interpretation is not wrong here.

It's interpreting the assertion correctly, 26213400 is a truthy value in PHP. Therefore assetEquals will return true since a truthy value is equal to true in PHP. Using assertSame will fail however since the expectation and the actual value aren't of the same type, they are both truthy but one is an int and the other is a boolean.

Boiled down, PHPUnit's assertEquals is a non-strict equality check in PHP (==), assertSame would be a strict equality check (===).

This can be seen if you try and write it out the checks yourself in plain PHP, as demonstrated here.

var_dump(26213400 == true);  // true
var_dump(26213400 === true); // false

thinkverse avatar Oct 20 '21 06:10 thinkverse