Add support for php 5.5 generators #379

Closed
veewee opened this Issue Jul 2, 2014 · 18 comments

Comments

Projects
None yet
4 participants
@veewee
Contributor

veewee commented Jul 2, 2014

At the moment, it is not possible to spec php 5.5 generators:
http://www.php.net/manual/en/language.generators.php

Here is a simplified sample spec that I cannot get working:

_The subject_

namespace Sample;

final class Generator
{

    public function generate()
    {
        foreach(range(1,3) as $index => $value) {
            yield $index => $value;
        }
    }

}

_The Spec_

namespace spec\Sample;

use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

final class GeneratorSpec extends ObjectBehavior
{
    public function it_should_loop_over_generator_values_attempt1()
    {
        $generated = $this->generate();
        $generated->shouldBeAnInstanceOf('Generator');
        foreach ($generated as $index => $value) {
            die('not getting here');
        }
    }

    public function it_should_loop_over_generated_values_attempt2()
    {
        foreach ($this->generate() as $index => $value) {
            die('not getting here');
        }

    }

    public function it_should_loop_over_generated_values_attempt3()
    {
        foreach ($this->generate()->getWrappedObject() as $index => $value) {
           // This works but $index and $value are just integers, so nothing can be tested in a phpspec way. 
        }

    }   

    public function it_should_loop_over_generated_values_attempt4()
    {
        $generated = $this->generate();
        $next = $generated->next();
        $next->shouldBe(1);
    }
}
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 21, 2014

Having similar difficulties. I'm going to try to find a workaround.

I've tried not using collaborators but using instances of actual objects and then comparing them using the Expect module. Still cannot get it working, this is a shame as I have the intention of making extensive use of generators in this project.

ghost commented Jul 21, 2014

Having similar difficulties. I'm going to try to find a workaround.

I've tried not using collaborators but using instances of actual objects and then comparing them using the Expect module. Still cannot get it working, this is a shame as I have the intention of making extensive use of generators in this project.

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

This comment has been minimized.

Show comment
Hide comment
Contributor

docteurklein commented Jul 21, 2014

see also #179 and phpspec/prophecy#26

@veewee

This comment has been minimized.

Show comment
Hide comment
@veewee

veewee Jul 21, 2014

Contributor

I have tried some of these patches but didnt get it working. The problem is that these generators don't work as normal Traversable objects. When a value gets 'yielded', this value is instantly passed to the code that is processing this value. I think this asynchronous behavior makes it impossible to fetch the results in the current implementation. Not sure how all these phpspec and prophecy classes work internally though.

Contributor

veewee commented Jul 21, 2014

I have tried some of these patches but didnt get it working. The problem is that these generators don't work as normal Traversable objects. When a value gets 'yielded', this value is instantly passed to the code that is processing this value. I think this asynchronous behavior makes it impossible to fetch the results in the current implementation. Not sure how all these phpspec and prophecy classes work internally though.

@docteurklein docteurklein referenced this issue in phpspec/prophecy Jul 21, 2014

Closed

[RFC] support for generators #115

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 21, 2014

Contributor

@veewee can you have a look to phpspec/prophecy#115 ?

Contributor

docteurklein commented Jul 21, 2014

@veewee can you have a look to phpspec/prophecy#115 ?

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 21, 2014

Contributor

it will only resolve attempts 1 and 2, though.

Contributor

docteurklein commented Jul 21, 2014

it will only resolve attempts 1 and 2, though.

@veewee

This comment has been minimized.

Show comment
Hide comment
@veewee

veewee Jul 23, 2014

Contributor

I can not get this to work.
Maybe a bit deeper into my simplified problem:

final class ExcelReader
{

    /**
     * @param \PHPExcel_Worksheet $sheet
     *
     * @return \Generator
     */
    public function getResources($sheet)
    {
        foreach($sheet->getRowIterator(0) as $row) {
            $index = $row->getRowIndex();
            $resource = $this->mapper->createFromRow($row);
            yield $index => new $resource;
        }
    }

}
class ExcelReaderSpec extends ObjectBehavior
{
 /**
     * @param \PHPExcel_Worksheet $sheet
     * @param \PHPExcel_Worksheet_Row $row
     * @param MyMapper $mapper
     * @param MyResource $resource
     */
    public function it_should_convert_excel_rows_to_resources($sheet, $row,  $mapper, $resource)
    {
        $sheet->getRowIterator(0)->willReturn($row);
        $mapper->createFromRow($row)->willReturn($resource);
        $this->getResources($sheet)->shouldIterateLike([$resource]);
    }


    public function getMatchers()
    {
        return [
            'iterateLike' => function ($subject, $expect) {
                return $expect == iterator_to_array($subject);
            },
        ];
    }
}

When I var_dump the $subject in the iterateLike matcher, I get an empty array.
I have added the GeneratorPatch and also registered it to Prophet.
The supports method doesn't ever return true.
Maybe I am doing something wrong?

For this case the iterateLike matcher seems enough. But maybe in some cases it is to limited. A (set of) Default Matcher(s) could always be handy.

Contributor

veewee commented Jul 23, 2014

I can not get this to work.
Maybe a bit deeper into my simplified problem:

final class ExcelReader
{

    /**
     * @param \PHPExcel_Worksheet $sheet
     *
     * @return \Generator
     */
    public function getResources($sheet)
    {
        foreach($sheet->getRowIterator(0) as $row) {
            $index = $row->getRowIndex();
            $resource = $this->mapper->createFromRow($row);
            yield $index => new $resource;
        }
    }

}
class ExcelReaderSpec extends ObjectBehavior
{
 /**
     * @param \PHPExcel_Worksheet $sheet
     * @param \PHPExcel_Worksheet_Row $row
     * @param MyMapper $mapper
     * @param MyResource $resource
     */
    public function it_should_convert_excel_rows_to_resources($sheet, $row,  $mapper, $resource)
    {
        $sheet->getRowIterator(0)->willReturn($row);
        $mapper->createFromRow($row)->willReturn($resource);
        $this->getResources($sheet)->shouldIterateLike([$resource]);
    }


    public function getMatchers()
    {
        return [
            'iterateLike' => function ($subject, $expect) {
                return $expect == iterator_to_array($subject);
            },
        ];
    }
}

When I var_dump the $subject in the iterateLike matcher, I get an empty array.
I have added the GeneratorPatch and also registered it to Prophet.
The supports method doesn't ever return true.
Maybe I am doing something wrong?

For this case the iterateLike matcher seems enough. But maybe in some cases it is to limited. A (set of) Default Matcher(s) could always be handy.

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 23, 2014

Contributor

well, maybe my class patch is totally useless, because it is only useful if you spec a class that is a generator. Which means my PR is useless, just the inline matchers are actually useful.

Contributor

docteurklein commented Jul 23, 2014

well, maybe my class patch is totally useless, because it is only useful if you spec a class that is a generator. Which means my PR is useless, just the inline matchers are actually useful.

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 23, 2014

Contributor

i'm a bit confused right now :) I need to investigate further.

Contributor

docteurklein commented Jul 23, 2014

i'm a bit confused right now :) I need to investigate further.

@veewee

This comment has been minimized.

Show comment
Hide comment
@veewee

veewee Jul 23, 2014

Contributor

Posted to early. The inline matcher _does_ work!
Made a mistake by not returning an array:

$sheet->getRowIterator(0)->willReturn( [$row] );
Contributor

veewee commented Jul 23, 2014

Posted to early. The inline matcher _does_ work!
Made a mistake by not returning an array:

$sheet->getRowIterator(0)->willReturn( [$row] );
@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 23, 2014

Contributor

but what if you don't register the patch ? I think this patch is useless currently.

Contributor

docteurklein commented Jul 23, 2014

but what if you don't register the patch ? I think this patch is useless currently.

@veewee

This comment has been minimized.

Show comment
Hide comment
@veewee

veewee Jul 23, 2014

Contributor

When I remove the patch, it also works as suspected.
The $subject is not a double but the actual generator.

Contributor

veewee commented Jul 23, 2014

When I remove the patch, it also works as suspected.
The $subject is not a double but the actual generator.

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 23, 2014

Contributor

it's a generator with or without the patch right? it would be logical

Contributor

docteurklein commented Jul 23, 2014

it's a generator with or without the patch right? it would be logical

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 23, 2014

Contributor

the patch should do something slightly more complicated.
It should proxy every method call, checking if the proxied return value is actually a generator. if so, it could try to wrap it.

Don't know if it's a good idea or not.

Contributor

docteurklein commented Jul 23, 2014

the patch should do something slightly more complicated.
It should proxy every method call, checking if the proxied return value is actually a generator. if so, it could try to wrap it.

Don't know if it's a good idea or not.

@veewee

This comment has been minimized.

Show comment
Hide comment
@veewee

veewee Jul 23, 2014

Contributor

Yes it is a Generator in both cases.
Maybe it should just be accessible like with arrays, so that you can do:

$resources = $this->getResources($sheet);
$resources[1]->shouldBe($resource);

So when the result is a generator class, the iterator_to_array gets called and this result just works like normal arrays? Not sure if this is possible

Contributor

veewee commented Jul 23, 2014

Yes it is a Generator in both cases.
Maybe it should just be accessible like with arrays, so that you can do:

$resources = $this->getResources($sheet);
$resources[1]->shouldBe($resource);

So when the result is a generator class, the iterator_to_array gets called and this result just works like normal arrays? Not sure if this is possible

@docteurklein

This comment has been minimized.

Show comment
Hide comment
@docteurklein

docteurklein Jul 23, 2014

Contributor

it is possible if we modifiy the existing array* matchers in phpspec.

Contributor

docteurklein commented Jul 23, 2014

it is possible if we modifiy the existing array* matchers in phpspec.

@nightlinus

This comment has been minimized.

Show comment
Hide comment
@nightlinus

nightlinus Oct 16, 2015

Contributor

Does this inline matcher solve the issue?

   public function getMatchers()
   {
        return [
            'generate' => function ($subject, $value) {
                if (!$subject instanceof \Traversable) {
                    throw new FailureException('Return value should be instance of \Traversable');
                }
                $array = iterator_to_array($subject);

                return $array === $value;
            }
        ];
   }

   function it_should_generate_numbers()
   {
        $this->generateNumbers(4)->shouldGenerate([ 1, 2, 3, 4]);
   }
Contributor

nightlinus commented Oct 16, 2015

Does this inline matcher solve the issue?

   public function getMatchers()
   {
        return [
            'generate' => function ($subject, $value) {
                if (!$subject instanceof \Traversable) {
                    throw new FailureException('Return value should be instance of \Traversable');
                }
                $array = iterator_to_array($subject);

                return $array === $value;
            }
        ];
   }

   function it_should_generate_numbers()
   {
        $this->generateNumbers(4)->shouldGenerate([ 1, 2, 3, 4]);
   }
@veewee

This comment has been minimized.

Show comment
Hide comment
@veewee

veewee Oct 16, 2015

Contributor

Yes it does.

Contributor

veewee commented Oct 16, 2015

Yes it does.

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