-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process] getIncrementalOutput should work without calling getOutput #18023
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
Conversation
romainneutron
commented
Mar 5, 2016
Q | A |
---|---|
Branch | 2.7 |
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #17937 |
License | MIT |
* @dataProvider provideVariousIncrementals | ||
*/ | ||
public function testIncrementalOutputDoesNotRequiresAnotherCall($stream, $method) { | ||
$process = new Process("php -r '\$n = 0; while (\$n < 3) { fwrite($stream, \$n, 1); \$n++; usleep(1000); }'", null, null, null, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use self::$phpBin
instead of php
direclty as done elsewhere to enhance compatibility of tests with more runtimes
38e4739
to
4bcfbc2
Compare
PR updated, comments addressed |
Arg, it fails on windows... |
bd6aedb
to
260aeb2
Compare
@@ -1166,6 +1166,31 @@ public function pipesCodeProvider() | |||
} | |||
|
|||
/** | |||
* @dataProvider provideVariousIncrementals | |||
*/ | |||
public function testIncrementalOutputDoesNotRequiresAnotherCall($stream, $method) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo DoesNotRequire
260aeb2
to
31aca23
Compare
31aca23
to
37d8695
Compare
👍 |
👍 |
Thank you @romainneutron. |
… getOutput (romainneutron) This PR was merged into the 2.7 branch. Discussion ---------- [Process] getIncrementalOutput should work without calling getOutput | Q | A | ------------- | --- | Branch | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17937 | License | MIT Commits ------- 37d8695 [Process] getIncrementalOutput should work without calling getOutput
* | ||
* @param $caller The name of the method that needs fresh outputs | ||
* | ||
* @throw LogicException in case output has been disabled or process is not started |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo. Should be @throws
This PR was merged into the 2.7 branch. Discussion ---------- [Process] fix docblock syntax | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18023 (comment) | License | MIT | Doc PR | Commits ------- f909fca [Process] fix docblock syntax