-
-
Notifications
You must be signed in to change notification settings - Fork 892
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
Add files to test client #3486
Add files to test client #3486
Conversation
CoalaJoe
commented
Apr 9, 2020
Q | A |
---|---|
Bug fix? | yes |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tickets | #3299 |
License | MIT |
Doc PR | api-platform/docs#... |
@@ -84,7 +84,7 @@ public function setDefaultOptions(array $defaultOptions): void | |||
* | |||
* @return Response | |||
*/ | |||
public function request(string $method, string $url, array $options = []): ResponseInterface | |||
public function request(string $method, string $url, array $options = [], array $files = []): ResponseInterface |
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.
We can't change the method signature here as it should respect this interface: https://github.com/symfony/contracts/blob/master/HttpClient/HttpClientInterface.php#L84. Files aren't in the option list, we could maybe support this via the extra
key?
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.
Updated to your suggestion
@@ -121,7 +122,8 @@ public function request(string $method, string $url, array $options = []): Respo | |||
'url' => $resolvedUrl, | |||
'primary_port' => 'http:' === $url['scheme'] ? 80 : 443, | |||
]; | |||
$this->kernelBrowser->request($method, $resolvedUrl, [], [], $server, $options['body'] ?? null); | |||
$extra = $options['extra'] ?? []; | |||
$this->kernelBrowser->request($method, $resolvedUrl, [], $extra['files'] ?? [], $server, $options['body'] ?? 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.
you can use $options['extra']['files'] ?? []
directly here
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've updated it based on your suggestion
Thanks @CoalaJoe we could maybe add some documentation to that in api-platform/docs! |
As reported on Slack, this should be backported in 2.5 as it's a bug fix. |
* Add files to test client * Respect method signature in favor of interface * Add missing default * Add request parameters to client. Co-authored-by: Ashura <[email protected]>