Unwrap objects which implement \Traversable automatically #179

Open
rommsen opened this Issue Aug 29, 2013 · 10 comments

Comments

Projects
None yet
7 participants
@rommsen
Contributor

rommsen commented Aug 29, 2013

As a follow up to phpspec/prophecy#26 by @mkilmanas-inviqa we patch our version of https://github.com/phpspec/phpspec/blob/master/src/PhpSpec/Wrapper/Unwrapper.php with the following code

if(is_object($argument) && $argument instanceof \Traversable) {
    foreach($argument as $key => $value) {
        $argument[$key] = $this->unwrapOne($value);
    }
    return $argument;
}

with this patch we are able to spec all kinds of structures which are are referred to in phpspec/prophecy#26. Does anyone sees a problem in this approach? If this is not the case I would prepare a pull request with the change.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Aug 29, 2013

Member

this is assuming that the object implements both Traversable and ArrayAccess, not only Traversable (and that the ArrayAccess is accessing the same elements than the loop, but this one could be a sane expectation).

there is lots of cases where such code cannot work at all.

Member

stof commented Aug 29, 2013

this is assuming that the object implements both Traversable and ArrayAccess, not only Traversable (and that the ArrayAccess is accessing the same elements than the loop, but this one could be a sane expectation).

there is lots of cases where such code cannot work at all.

@rommsen

This comment has been minimized.

Show comment
Hide comment
@rommsen

rommsen Aug 29, 2013

Contributor

Ok, I see. Then we will keep patching this in our own version. I still think that this is a pity because this problem prevents e.g. the specking of Doctrine entities with XtoMany associations.

Contributor

rommsen commented Aug 29, 2013

Ok, I see. Then we will keep patching this in our own version. I still think that this is a pity because this problem prevents e.g. the specking of Doctrine entities with XtoMany associations.

@MarcelloDuarte

This comment has been minimized.

Show comment
Hide comment
@MarcelloDuarte

MarcelloDuarte Sep 8, 2013

Member

@rommsen can you paste an example of a spec in which this patch has been useful for you

Member

MarcelloDuarte commented Sep 8, 2013

@rommsen can you paste an example of a spec in which this patch has been useful for you

@peterjmit

This comment has been minimized.

Show comment
Hide comment
@peterjmit

peterjmit Sep 8, 2013

Contributor

@rommsen I spec entities with XtoMany associations no problem?

Contributor

peterjmit commented Sep 8, 2013

@rommsen I spec entities with XtoMany associations no problem?

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Sep 12, 2013

Member

@rommsen why would it prevent it ?

Member

stof commented Sep 12, 2013

@rommsen why would it prevent it ?

@rommsen

This comment has been minimized.

Show comment
Hide comment
@rommsen

rommsen Sep 27, 2013

Contributor

Hey guys,

sorry for the late reply. I have to take back the statement that this prevents Doctrine entities with XtoMany associations from being specked but if I am not totally wrong it makes it much harder.

If you look at this gist: https://gist.github.com/rommsen/6730438
The problems arises in CollectionWorkerSpec

function it_should_not_work($entry1, $entry2) {
  $entry1->getImportantValue()->willReturn(1);
  $entry1->setImportantValue(2)->shouldBeCalled();
  $entry2->getImportantValue()->willReturn(2);
  $entry2->setImportantValue(3)->shouldBeCalled();

  $this->doSomethingWithCollection(new ArrayCollection([$entry1, $entry2]));
}

With the patch everything works fine but without the patch it does not. If I had used an array it would work without the patch but doSomethingWithCollection uses a typehint for its parameter, so i can not do this either.

If I want to make it work without the patch I have to create a double of ArrayCollection so that it can be used e.g. in a foreach loop.

To quote from the original issue:
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.

Am I missing something fundamental here?

Regards,

Ro

Contributor

rommsen commented Sep 27, 2013

Hey guys,

sorry for the late reply. I have to take back the statement that this prevents Doctrine entities with XtoMany associations from being specked but if I am not totally wrong it makes it much harder.

If you look at this gist: https://gist.github.com/rommsen/6730438
The problems arises in CollectionWorkerSpec

function it_should_not_work($entry1, $entry2) {
  $entry1->getImportantValue()->willReturn(1);
  $entry1->setImportantValue(2)->shouldBeCalled();
  $entry2->getImportantValue()->willReturn(2);
  $entry2->setImportantValue(3)->shouldBeCalled();

  $this->doSomethingWithCollection(new ArrayCollection([$entry1, $entry2]));
}

With the patch everything works fine but without the patch it does not. If I had used an array it would work without the patch but doSomethingWithCollection uses a typehint for its parameter, so i can not do this either.

If I want to make it work without the patch I have to create a double of ArrayCollection so that it can be used e.g. in a foreach loop.

To quote from the original issue:
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.

Am I missing something fundamental here?

Regards,

Ro

@rommsen

This comment has been minimized.

Show comment
Hide comment
@rommsen

rommsen Jan 2, 2014

Contributor

Hey guys,

sorry to bumb this, but am I really the only one running into problems with this?

Contributor

rommsen commented Jan 2, 2014

Hey guys,

sorry to bumb this, but am I really the only one running into problems with this?

@andytson

This comment has been minimized.

Show comment
Hide comment
@andytson

andytson Jan 18, 2014

I'm finding I want to use generators in methods as a simplification (and memory friendly), so having this problem. It'd be nice to be able to compare a iterator return from a method call to an array.

I'm finding I want to use generators in methods as a simplification (and memory friendly), so having this problem. It'd be nice to be able to compare a iterator return from a method call to an array.

@ciaranmcnulty

This comment has been minimized.

Show comment
Hide comment
@ciaranmcnulty

ciaranmcnulty Feb 7, 2014

Member

@andy can you not call iterator_to_array and compare? Does this issue prevent that behaviour?

Member

ciaranmcnulty commented Feb 7, 2014

@andy can you not call iterator_to_array and compare? Does this issue prevent that behaviour?

@shanethehat

This comment has been minimized.

Show comment
Hide comment
@shanethehat

shanethehat Feb 12, 2014

Contributor

I have also experienced this issue while working with Magento collections. See https://gist.github.com/shanethehat/8951398. The provided patch works from me, although I have added a test for $argument instanceof \ArrayAccess.

You can see it in situ here: https://github.com/shanethehat/phpspec/blob/feature/support-traversable-unwrapping/src/PhpSpec/Wrapper/Unwrapper.php#L62.

Contributor

shanethehat commented Feb 12, 2014

I have also experienced this issue while working with Magento collections. See https://gist.github.com/shanethehat/8951398. The provided patch works from me, although I have added a test for $argument instanceof \ArrayAccess.

You can see it in situ here: https://github.com/shanethehat/phpspec/blob/feature/support-traversable-unwrapping/src/PhpSpec/Wrapper/Unwrapper.php#L62.

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