Skip to content
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

Automated test responses don't include relationships #31

Closed
imjohnbon opened this issue Oct 31, 2016 · 9 comments
Closed

Automated test responses don't include relationships #31

imjohnbon opened this issue Oct 31, 2016 · 9 comments

Comments

@imjohnbon
Copy link

imjohnbon commented Oct 31, 2016

In my application using this package (latest), I have an automated test that uses Laravel's (5.3 latest) built-in helpers to test a URL like this:

$this->json('GET', "api/users/{$user->id}?include=posts");

It is supposed to give me back a list of users with their posts. When I test the URL in postman or chrome, it works just fine. However, when I run the test on the command line (by running "phpunit") and dump-and-die the output, it just includes the user's information without the "posts" include.

This is happening across the board for me in my automated tests for any endpoint that has a relationship. Based on what I've found by digging into Fractal and dump-and-dying all over the place, it looks like for some reason the "parseIncludes" function doesn't get set correctly when automated tests are run. But like I said, it's fine in a Postman test.

To be honest, I can't tell for sure if this is Laravel's problem, Fractal's problem, or this packages problem. I am continuing to dig into it.

@imjohnbon
Copy link
Author

I temporarily switched to https://github.com/spatie/laravel-fractal to see if the issue would still persist, and it did. Which causes me to believe this may not be an issue with this package, and rather with Laravel or Fractal instead.

This is completely perplexing me at this point. I can't imagine why this would work in practice but fail during automated tests. Very odd. Would love to hear from the creator of this package to see if he ever successfully application tested relationships.

@maximebeaudoin
Copy link
Member

maximebeaudoin commented Oct 31, 2016

I'll try to take a look at it. Thank you for the report

@maximebeaudoin
Copy link
Member

@imjohnbon I found the problem, but not the solution for now.

The problem is simple, it's because the parsing is triggered on the boot method of the service provider like this : $manager->parseIncludes(explode(',', $this->app['Illuminate\Http\Request']->get('include'))); So at this point, when the http request occur, the content of the request is populate with the include value.

BUT, in the testing process, we are not doing the normal http request. Laravel doing a internal call via $this->call and the value of includes was already loaded and will NOT be refresh.

For now, the only way i see to resolve this is to replace the method in Illuminate\Foundation\Testing\Concerns\MakesHttpRequests::call by this code

    /**
     * Call the given URI and return the Response.
     *
     * @param  string  $method
     * @param  string  $uri
     * @param  array   $parameters
     * @param  array   $cookies
     * @param  array   $files
     * @param  array   $server
     * @param  string  $content
     * @return \Illuminate\Http\Response
     */
    public function call($method, $uri, $parameters = [], $cookies = [], $files = [], $server = [], $content = null)
    {
        $kernel = $this->app->make('Illuminate\Contracts\Http\Kernel');

        $this->currentUri = $this->prepareUrlForRequest($uri);

        $this->resetPageContext();

        $symfonyRequest = SymfonyRequest::create(
            $this->currentUri, $method, $parameters,
            $cookies, $this->filterFiles($files), array_replace($this->serverVariables, $server), $content
        );

        $request = Request::createFromBase($symfonyRequest);

        $response = app(\EllipseSynergie\ApiResponse\Contracts\Response::class);

        $response
            ->getManager()
            ->parseIncludes(explode(',', $request->get('include')));

        $response = $kernel->handle($request);

        $kernel->terminate($request, $response);

        return $this->response = $response;
    }

This code will intercept the new Request object and parse the include value.

Someday i may probably create some Trait to help people resolve that issue.

@imjohnbon
Copy link
Author

Thanks for the update! I figured it was something like that. Unfortunately editing core Laravel files isn't an option for me or most people I would assume. Hopefully one day this can be solved within the package.

@maximebeaudoin
Copy link
Member

@imjohnbon You just have to create your own call method in the TestCase class for now if you want a quick fix.

@imjohnbon
Copy link
Author

Ah of course, thanks for the tip!

maximebeaudoin added a commit that referenced this issue May 4, 2017
Add AddTestingSupportForInclude to resolve issue #31
@mehrdad-shokri
Copy link

@maximebeaudoin can provide a code snippet on hwo to create m own call method in TestCase

@maximebeaudoin
Copy link
Member

maximebeaudoin commented Aug 17, 2017

@mehrdad-shokri
Copy link

@maximebeaudoin thanks, I had a suggestion for this trait, Could we convert call to a private function and create 3 or 4 protected functions like get, post etc to keep Laravel signature untouched?
One problem I can think of it that it would create confusion for the trait client.
Any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants