-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix save_to for Mock #1020
Fix save_to for Mock #1020
Conversation
Allow using “save_to” in mocked requests. Now you can write tests for code that uses “save_to” easily
$save_to = $event->getRequest()->getConfig()->get('save_to'); | ||
|
||
if ($save_to) { | ||
file_put_contents($save_to, $item->getBody()); |
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.
save_to can be of various types: http://docs.guzzlephp.org/en/latest/clients.html#save-to. I think this method would need to account for that.
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.
Yeah, sorry about that, totally forgot. here's the updated PR with all the 3 use cases
@@ -60,6 +61,20 @@ public function onBefore(BeforeEvent $event) | |||
} | |||
} | |||
|
|||
$save_to = $event->getRequest()->getConfig()->get('save_to'); |
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.
Shouldn't the variable be called $saveTo
?
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.
I think scrutinizer could be configured to spot (and create patches for) these kinds of style violations automatically. Anyway fixed.
} elseif (is_string($saveTo)) { | ||
file_put_contents($saveTo, $body); | ||
} elseif ($saveTo instanceof StreamInterface) { | ||
$saveTo->write($body); |
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.
Can you fix the indentation here to match the rest of the library (4 spaces)?
Looks good! Just need a minor tweak to fix the spacing, but other than that, LGTM. |
Great project, just a small fix for an issue I ran into while using your library - one cannot use "save_to" and write proper tests for it at the moment.
This allows using “save_to” in mocked requests. Now you can write tests for code that uses “save_to” easily.
Test included.