Support Traversable interfaces in mock objects #604

Closed
webmozart opened this Issue Jul 13, 2012 · 10 comments

Comments

4 participants
@webmozart
Contributor

webmozart commented Jul 13, 2012

Currently, PHPUnit fails when trying to mock an interface that implements \Traversable because PHP expects implementations of that interface to also either implement \Iterator or \IteratorAggregate.

<?php

interface MyInterface extends \Traversable {}

// doesn't work
$this->getMock('MyInterface');

The fix currently is to extend the interface for testing purposes and to add one of the two required interfaces (Note: The order of the interfaces is important here!)

<?php

interface TestMyInterface extends \Iterator, MyInterface {}

// works
$this->getMock('TestMyInterface');

IMO PHPUnit should be able to add that interface automatically in such situations, i.e.:

  • when the mock is created for an interface or an abstract class, and
  • that interface/abstract class implements \Traversable, but not \Iterator nor \IteratorAggregate
@edorian

This comment has been minimized.

Show comment
Hide comment
@edorian

edorian Jul 27, 2012

Contributor

He,

thanks for the detailed bug report. I'm currently failing to understand which use case requires one to extend the traversable interface.

It should work to throw that into the mock building but I'd like to state a use case in the tests for that.

Contributor

edorian commented Jul 27, 2012

He,

thanks for the detailed bug report. I'm currently failing to understand which use case requires one to extend the traversable interface.

It should work to throw that into the mock building but I'd like to state a use case in the tests for that.

@webmozart

This comment has been minimized.

Show comment
Hide comment
@webmozart

webmozart Jul 27, 2012

Contributor

I'm currently failing to understand which use case requires one to extend the traversable interface.

Any interface whose implementation is supposed to be traversable needs to extend it. A very simple example:

<?php
interface ArrayInterface extends \ArrayAccess, \Countable, \Traversable {}

// The implementation can chose to implement \Iterator...
class Array1 implements \Iterator, ArrayInterface
{
    ...
}

// or \IteratorAggregate
class Array2 implements \IteratorAggregate, ArrayInterface
{
    ...
}

Interfaces usually specify a contract, without specifying how that contract should be implemented. These core interfaces are special in that \Traversable specifies the contract ("This object should be iterable") while \Iterator and \IteratorAggregate specify how that contract is implemented. Consequently, no userland interface should them, but always \Traversable IMHO.

Contributor

webmozart commented Jul 27, 2012

I'm currently failing to understand which use case requires one to extend the traversable interface.

Any interface whose implementation is supposed to be traversable needs to extend it. A very simple example:

<?php
interface ArrayInterface extends \ArrayAccess, \Countable, \Traversable {}

// The implementation can chose to implement \Iterator...
class Array1 implements \Iterator, ArrayInterface
{
    ...
}

// or \IteratorAggregate
class Array2 implements \IteratorAggregate, ArrayInterface
{
    ...
}

Interfaces usually specify a contract, without specifying how that contract should be implemented. These core interfaces are special in that \Traversable specifies the contract ("This object should be iterable") while \Iterator and \IteratorAggregate specify how that contract is implemented. Consequently, no userland interface should them, but always \Traversable IMHO.

@edorian

This comment has been minimized.

Show comment
Hide comment
@edorian

edorian Jul 27, 2012

Contributor

So what the interface is saying there is "I can foreach over things that implement this but i don't care how the implementing class makes that happen (Iterator or IteratorAggregate).

Makes sense to me. Thanks for explaining :)

Contributor

edorian commented Jul 27, 2012

So what the interface is saying there is "I can foreach over things that implement this but i don't care how the implementing class makes that happen (Iterator or IteratorAggregate).

Makes sense to me. Thanks for explaining :)

@webmozart

This comment has been minimized.

Show comment
Hide comment
@webmozart

webmozart Jul 27, 2012

Contributor

Exactly :)

Contributor

webmozart commented Jul 27, 2012

Exactly :)

@sebastianbergmann

This comment has been minimized.

Show comment
Hide comment
@sebastianbergmann

sebastianbergmann Oct 7, 2012

Owner

The PHP Manual states "This is an internal engine interface which cannot be implemented in PHP scripts. Either IteratorAggregate or Iterator must be used instead." While the documentation continues with "When implementing an interface which extends Traversable, make sure to list IteratorAggregate or Iterator before its name in the implements clause.", I do not think that Traversable should be used in userland at all and therefore am against adding support for this edge-case to PHPUnit.

Owner

sebastianbergmann commented Oct 7, 2012

The PHP Manual states "This is an internal engine interface which cannot be implemented in PHP scripts. Either IteratorAggregate or Iterator must be used instead." While the documentation continues with "When implementing an interface which extends Traversable, make sure to list IteratorAggregate or Iterator before its name in the implements clause.", I do not think that Traversable should be used in userland at all and therefore am against adding support for this edge-case to PHPUnit.

@webmozart

This comment has been minimized.

Show comment
Hide comment
@webmozart

webmozart Oct 7, 2012

Contributor

@sebastianbergmann IMHO you are misquoting the manual. The manual talks about implementing Traversable, not extending it.

As I have shown above, there is a very clear and valid use case for extending Traversable in other interfaces (without contradicting the manual you quoted).

Contributor

webmozart commented Oct 7, 2012

@sebastianbergmann IMHO you are misquoting the manual. The manual talks about implementing Traversable, not extending it.

As I have shown above, there is a very clear and valid use case for extending Traversable in other interfaces (without contradicting the manual you quoted).

@webmozart

This comment has been minimized.

Show comment
Hide comment
@beberlei

This comment has been minimized.

Show comment
Hide comment
@beberlei

beberlei Oct 7, 2012

Contributor

If the traversable interface was internal to the engine, it would not be exported to userland.

Contributor

beberlei commented Oct 7, 2012

If the traversable interface was internal to the engine, it would not be exported to userland.

@sebastianbergmann

This comment has been minimized.

Show comment
Hide comment
@sebastianbergmann

sebastianbergmann Oct 7, 2012

Owner

I would accept a pull request for phpunit-mock-objects that implements the fix described in the original ticket.

Owner

sebastianbergmann commented Oct 7, 2012

I would accept a pull request for phpunit-mock-objects that implements the fix described in the original ticket.

@webmozart webmozart referenced this issue in sebastianbergmann/phpunit-mock-objects Oct 7, 2012

Closed

Support Traversable interfaces in mock objects #103

@webmozart

This comment has been minimized.

Show comment
Hide comment
@webmozart

webmozart Oct 7, 2012

Contributor

Ok. I opened a corresponding ticket in that repository.

Contributor

webmozart commented Oct 7, 2012

Ok. I opened a corresponding ticket in that repository.

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