Add unit tests for Collection home action. - #5661
Conversation
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @3pleX-dev! See below for a few suggestions.
| * @return HomeAction | ||
| */ | ||
| protected function buildAction( | ||
| array $config, |
There was a problem hiding this comment.
Is it worth defaulting this to empty array so you don't have to provide it, e.g. in line 138 below?
| array $config, | |
| array $config = [], |
| ->withAttribute('route-match', $routeMatch); | ||
| $this->setProperty($action, 'request', $request); | ||
|
|
||
| return $this->callMethod($action, 'action', [$request, new Response()]); |
There was a problem hiding this comment.
Might this be a case where you could actually use the __invoke method and not have to use reflection to trigger the logic?
I don't mind using reflection if it saves significant complexity in the tests, but it's good to use the public interface where possible to reduce the changes of internal refactoring breaking tests.
I think if invoke works here, then you can also skip the setProperty call for 'request' above.
|
|
||
| $driver = $this->createMock(RecordDriver::class); | ||
| $driver->method('tryMethod')->with('isCollection')->willReturn(true); | ||
| $this->setProperty($action, 'driver', $driver); |
There was a problem hiding this comment.
Is it worth considering injecting a mock record loader to return the driver, rather than forcibly inserting it like this? As noted above, if this really greatly simplifies matters, I can live with it, but using the record loader might exercise a little bit more code.
| * | ||
| * @return void | ||
| */ | ||
| public function testActionRedirectsToCollectionRouteForCollectionRecord(): void |
There was a problem hiding this comment.
This and the following test might be good candidates for simplification into a single test with a data provider that sends in the active sid (or null) and the expected query parameters.
2 participants
No description provided.