Skip to content

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Apr 14, 2020

expectPropertyDoesNotExistException

$pair = new \Ds\Pair('a', 1); $pair->x;

PHP Notice: Undefined property: Ds\Pair::$x

@simPod simPod changed the title Fix tests Fix expectPropertyDoesNotExistException tests Apr 14, 2020
@simPod simPod changed the title Fix expectPropertyDoesNotExistException tests Fix expectPropertyDoesNotExistException test Apr 14, 2020
@rtheunissen
Copy link
Member

I don't think the expected exception should be a PHPUnit exception to catch that PHP notice. We should take a look at why the OutOfBounds is not being thrown.

@simPod
Copy link
Contributor Author

simPod commented Apr 14, 2020

@rtheunissen I thought that throwing OutOfBounds is wrong in the first place. I always understood OutOfBounds to be valid in a situation such this $a = new Map(['a'=>1]); $a->get('b');

Here we're just accessing a property on an object that does not exist so that Notice seemed valid. I may be wrong tho.

@rtheunissen
Copy link
Member

Accessing by property is a shortcut for $a->get('x') so I think we should keep those consistent.

@simPod
Copy link
Contributor Author

simPod commented Apr 14, 2020

@rtheunissen even on a Pair? I'd rather expect it on a Map. Feel free to close this.

Maybe this should not fully extend CollectionTest https://github.com/php-ds/tests/blob/master/tests/PairTest.php 🤔

@rtheunissen
Copy link
Member

Good call, I think you're right. I'll merge this and come back to it to verify what's happening.

Thank you

@rtheunissen rtheunissen merged commit 6ad2e4c into php-ds:master Apr 14, 2020
@simPod simPod deleted the fix-tests branch April 14, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants