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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants