Iterator unwrapping should come out-of-the-box #26

Closed
ghost opened this Issue Jun 14, 2013 · 3 comments

Comments

Projects
None yet
3 participants
@ghost

ghost commented Jun 14, 2013

Iterator objects cannot be used as a wrapping object or intermediate layer for doubles at the moment. The workaround could be to have a double of iterator itself, which doesn't make sense because one would mock it with the exact mimic of it's actual implementation.

Particular situation from phpSpec/MageSpec.
Spec code snippet:

function it_builds_<...>_format(Category_Collection $collection, Category $category1, Category $category2)
{
    $category1->getParentId()->willReturn(4);
    $category2->getParentId()->willReturn(5);

    $collection->getIterator()->willReturn(new \ArrayIterator(array($category1, $category2)));
    <...>
    $this->doSomething()->shouldReturn($expectedResult);
}

Production code snippet:

foreach ($categories as $category) {
    if ($category->getParentId() == $category_root_id) {
       <...>
    }
}

Expected result: $category->getParentId() should return the mocked value (integer)

Actual result:

 notice: Object of class Prophecy\Prophecy\MethodProphecy could not be converted to int in <...>

The unmocked ArrayIterator here breaks the unwrapping chain and Prophecy object is returned instead of it's unwrapped value. But the issue is more general to Iterators and possibly other areas of SPL (e.g. Datastructures)

@rommsen

This comment has been minimized.

Show comment
Hide comment
@rommsen

rommsen Jul 15, 2013

Is there any feasible solution for this? In my opinion this prevents testing of most of (Doctrine) Entities that use Collections for their relationships. Our current "solution" is to add the following snippet to PhpSpec\Wrapper\Unwrapper#unwrapOne:

if($argument instanceof \Doctrine\Common\Collections\Collection) {
    return $argument->map(function($value){
        return $this->unwrapOne($value);
    });
}

This works for us at the moment but as @mkilmanas-inviqa said, pretty much all object based datastructures are affected.

rommsen commented Jul 15, 2013

Is there any feasible solution for this? In my opinion this prevents testing of most of (Doctrine) Entities that use Collections for their relationships. Our current "solution" is to add the following snippet to PhpSpec\Wrapper\Unwrapper#unwrapOne:

if($argument instanceof \Doctrine\Common\Collections\Collection) {
    return $argument->map(function($value){
        return $this->unwrapOne($value);
    });
}

This works for us at the moment but as @mkilmanas-inviqa said, pretty much all object based datastructures are affected.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Jul 15, 2013

Member

I don't see any solution for it. Prophecy does not hook into the object instances to unwrap their return value if they return prophecy. It cannot work (it would require proxying the method calls of the instance, which cannot be done for an instance you provide). Currently, iterators are working exactly the same way than any other objects: as soon as a normal object is found, the revealer stops its work as the value is not a prophecy anymore.

On a side note, the code @rommsen modified is not part of Prophecy

Member

stof commented Jul 15, 2013

I don't see any solution for it. Prophecy does not hook into the object instances to unwrap their return value if they return prophecy. It cannot work (it would require proxying the method calls of the instance, which cannot be done for an instance you provide). Currently, iterators are working exactly the same way than any other objects: as soon as a normal object is found, the revealer stops its work as the value is not a prophecy anymore.

On a side note, the code @rommsen modified is not part of Prophecy

@everzet

This comment has been minimized.

Show comment
Hide comment
@everzet

everzet Nov 29, 2014

Member

Solution is to stop mocking collections. Just use them as is.

Member

everzet commented Nov 29, 2014

Solution is to stop mocking collections. Just use them as is.

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