Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2] Allow objects to be passed as pipes #13024

Merged
merged 1 commit into from
Apr 8, 2016
Merged

Conversation

rkgrep
Copy link

@rkgrep rkgrep commented Apr 5, 2016

Allows following code to be executed, very useful in case pipes require some complex constructing or already exist.

(new Pipeline($app))->send($subject)->through([new A, new B])->via('pipe_method')->then(...);

@GrahamCampbell
Copy link
Member

@rkgrep rkgrep force-pushed the patch-3 branch 2 times, most recently from 61885a8 to 1aafae2 Compare April 5, 2016 18:49
@taylorotwell
Copy link
Member

Can you add a unit test?

Allows following code to be executed, very useful in case pipes require some complex constructing or already exist.
```php
(new Pipeline($app))->send($subject)->through([new A, new B])->via('pipe_method')->then(...);
```
@rkgrep
Copy link
Author

rkgrep commented Apr 7, 2016

@taylorotwell Done!

$this->assertEquals('foo', $result);
$this->assertEquals('foo', $_SERVER['__test.pipe.one']);

unset($_SERVER['__test.pipe.one']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to call this in a try, finally, otherwise if the assertion fails, it'll screw up the other tests

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamCampbell there is no exception catching in most of unit tests, moreover the previous one (line 17) will throw uncaught ReflectionException in case class does not exist, so it will be screwed up even without my test 😃

Time: 1.93 seconds, Memory: 64.00Mb

There were 3 errors:

1) PipelineTest::testPipelineBasicUsage
ReflectionException: Class PipelineTestPipeOne does not exist

/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Container/Container.php:738
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Container/Container.php:633
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Pipeline/Pipeline.php:122
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Pipeline/Pipeline.php:102
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/tests/Pipeline/PipelineTest.php:20

2) PipelineTest::testPipelineUsageWithObjects
Error: Class 'PipelineTestPipeOne' not found

/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/tests/Pipeline/PipelineTest.php:34

3) PipelineTest::testPipelineViaChangesTheMethodBeingCalledOnThePipes
ReflectionException: Class PipelineTestPipeOne does not exist

/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Container/Container.php:738
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Container/Container.php:633
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Pipeline/Pipeline.php:122
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/src/Illuminate/Pipeline/Pipeline.php:102
/home/likewise-open/FRAMGIA/roman.kinyakin/Code/framework/tests/Pipeline/PipelineTest.php:70

--

@taylorotwell taylorotwell merged commit 229c3fa into laravel:5.2 Apr 8, 2016
@rkgrep rkgrep deleted the patch-3 branch July 17, 2016 14:56
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.

3 participants