Add replace argument to IntegrationTestTrait::configRequest
cnizzardini opened this issue · 9 comments
Description
If I have a common configured request in my setUp method such as
public function setUp(): void
{
parent::setUp();
$this->requestAsJson();
$this->configRequest([
'headers' => [
'Authorization' => 'Bearer ' . (new JwtHelper())->withSubscriptions([SubscriptionPlanEnum::STARTER]),
]
]);And need to replace it only in a few instances such as:
public function test_index_with_growth_plan(): void
{
$this->configRequest([
'headers' => [
'Authorization' => 'Bearer ' . (new JwtHelper())->withSubscriptions([SubscriptionPlanEnum::GROWTH]),
]
]);This adds two bearer tokens because configRequest() uses array_merge_recursive(). Adding a $replace bool argument that defaults to false but uses array_replace_recursive() if true would be a nice option here rather than having to manually clear the $this->_request property in these cases.
Unless there is another way to accomplish this... and yes I understand this is next level laziness, but this appears to be how it worked in CakePHP 4.
CakePHP Version
5.0
This adds two bearer tokens because configRequest() uses array_merge_recursive(). Adding a $replace bool argument that defaults to false but uses array_replace_recursive() if true would be a nice option here rather than having to manually clear the $this->_request property in these cases.This adds two bearer tokens because configRequest() uses array_merge_recursive(). Adding a $replace bool argument that defaults to false but uses array_replace_recursive() if true would be a nice option here rather than having to manually clear the $this->_request property in these cases.
What if we had a $this->resetRequest(); method to reset the request? I think that would be clearer to read, and not much harder to write especially with autocomplete.
This amounts to a wrapper around $this->_request = []. It is cleaner than working directly with an underscored property, but still requires rebuilding the entire request. I don't want to harp on this too much as this is a minor gripe. I leave this call to you.
It is cleaner than working directly with an underscored property, but still requires rebuilding the entire request.
That's fair. Would having a method that does the replacement work instead? Perhaps replaceRequest? 🤷
replaceRequest is probably the cleanest approach of all rather than mangling an existing method.
Would you say this method should clear the request and then add? Or should it do an array_replace_recursive?
Would you say this method should clear the request and then add? Or should it do an array_replace_recursive?
A recursive replace is likely the most ergonomic solution. It emulates 'merging' without the tedious duplicate values that merging arrays can create.
Is there a specific reason this is targeted as a 5.1 minor feature. Does Cake strictly put features into minors and fixes into revisions? Only curious.
Also I briefly looked at doing this, the solution is obvious, the question is what kind of tests would be involved here. There doesn't seem to be tests specifically for configRequest, despite it clearly being covered via its use within tests.