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

Impossible to get the HttpFoundation objects from the Client #17

Closed
Pierstoval opened this issue Apr 15, 2018 · 15 comments
Closed

Impossible to get the HttpFoundation objects from the Client #17

Pierstoval opened this issue Apr 15, 2018 · 15 comments

Comments

@Pierstoval
Copy link
Contributor

@Pierstoval Pierstoval commented Apr 15, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? yes

When using PanthereTestCase, and when making a request, we can't get access to the Symfony\Component\HttpFoundation\Response object that should normally be sent to the Kernel. Instead, we only get access to the BrowserKit one.

Steps to reproduce:

<?php

use Panthere\PanthereTestCase;

class MyPanthereTest extends PanthereTestCase
{
    public function testMyApp()
    {
        $client = static::createPanthereClient();
        $crawler = $client->request('GET', static::$baseUri.'/any-page');

        static::assertSame(200, $client->getResponse()->getStatusCode());
    }
}

PHPUnit's output:

Error : Call to undefined method Symfony\Component\BrowserKit\Response::getStatusCode()

I consider this as a BC break, because the $client->request() method SHOULD return an instance of the Symfony\Component\BrowserKit\Client class, for which getResponse() returns an instance of Symfony\Component\HttpFoundation\Response.

This is the same for the Request via $client->getRequest() by the way.

The issue comes from these lines:
https://github.com/dunglas/panthere/blob/b3f0601e105010d365360f238ca1b572a4e5ec82/src/Client.php#L238-L241

IMO, the Request and Response object should be either blocked from the PanthereClient via an exception (easy pick), or created & adapted based on the WebDriver results (harder).

WDYT?

@dunglas
Copy link
Member

@dunglas dunglas commented Apr 16, 2018

I’m for blocking them, because in some cases we cannot access to the exact HTTP request issued by the browser.

@Pierstoval
Copy link
Contributor Author

@Pierstoval Pierstoval commented Apr 16, 2018

I’m for blocking them, because in some cases we cannot access to the exact HTTP request issued by the browser.

Even though we could still create a Response object based on the response headers retrieved from the client?
Isn't there an API to communicate with the browser and retrieve request & response data? This would be great help in creating BC Request and Response objects..

@dunglas
Copy link
Member

@dunglas dunglas commented Apr 16, 2018

@Pierstoval
Copy link
Contributor Author

@Pierstoval Pierstoval commented Apr 16, 2018

Too bad, then let's go for a blocker 😕

@stof
Copy link
Member

@stof stof commented Jun 21, 2018

the BrowserKit component does not require at all that you can get the HttpFoundation objects from it. Client::getResponse actually has a return type differing based on the implementation (due to a mistake in the HttpKernel component implementation early on, which was identified only years later and so could not be broken). So you have no guarantee on the available methods (return type is object|null).
Use Client::getInternalResponse to always get a BrowserKit Response object in all implementations.

then, the fact that the status code will not be accurate (always 200 due to webdriver not exposing it) is a separate issue.

@dunglas
Copy link
Member

@dunglas dunglas commented Jun 29, 2018

Can we close this issue?

@Pierstoval
Copy link
Contributor Author

@Pierstoval Pierstoval commented Jun 29, 2018

I think we should add a blocker in the API that sends an exception to remind that the request is not available because of the driver 🤔

@stof
Copy link
Member

@stof stof commented Jul 2, 2018

@Pierstoval in which API ? The method typed as @return object is returning a different object for which the method does not exist

@Pierstoval
Copy link
Contributor Author

@Pierstoval Pierstoval commented Jul 2, 2018

In Panthere's one, check #42, could you give feedback to know whether it's a correct way to do it or not?

@dunglas dunglas closed this in #42 Sep 25, 2018
@9ae8sdf76
Copy link

@9ae8sdf76 9ae8sdf76 commented Jan 9, 2020

Hi, I know this is closed, but why wasn't the decision made to proxy Client::getInternalResponse() through Client::getReponse()? Throwing an exception means that it breaks compatibility with the BrowserKitAssertionsTrait via the WebTestAssertionsTrait; i.e., calling self::assertResponseIsSuccessful() will throw an exception because it relies on Client::getResponse().

I'm new to using Panther, and I'm trying to understand the decision process so I can write better tests. Thank you!

@gcollombet
Copy link

@gcollombet gcollombet commented Sep 1, 2021

Hi, I know this is closed, but why wasn't the decision made to proxy Client::getInternalResponse() through Client::getReponse()? Throwing an exception means that it breaks compatibility with the BrowserKitAssertionsTrait via the WebTestAssertionsTrait; i.e., calling self::assertResponseIsSuccessful() will throw an exception because it relies on Client::getResponse().

I'm new to using Panther, and I'm trying to understand the decision process so I can write better tests. Thank you!

Same problem for me

@stof
Copy link
Member

@stof stof commented Sep 1, 2021

@9ae8sdf76 the issue is that Webdriver does not handle a HttpFoundation response at all, so it cannot return it.

regarding WebTestAssertionsTrait, the right fix would be to change the trait to use Client::getInternalResponse() whenever it can, which is guaranteed to return a browserkit response for all implementations.

@Pierstoval
Copy link
Contributor Author

@Pierstoval Pierstoval commented Sep 1, 2021

If you want an HttpFoundation Response object, the only way (for now) is to create it yourself with everything that's available through the browserkit response and/or what the Webdriver returns

@dunglas
Copy link
Member

@dunglas dunglas commented Sep 1, 2021

@Pierstoval this will be almost useless because - by design - WebDriver doesn't expose the HTTP response and there is no way to access to the headers or to the status code.

@Pierstoval
Copy link
Contributor Author

@Pierstoval Pierstoval commented Sep 1, 2021

Panther is for E2E tests anyway, so if one needs to check the returned HTTP response instead of the DOM page, they'd better use a simpler WebTestCase-based setup, it's more performant, and does what's needed for that 😅

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

Successfully merging a pull request may close this issue.

None yet
5 participants