Skip to content

Added a note about redirections to absolute URLs in tests #7521

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Added a note about redirections to absolute URLs in tests #7521

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
149b82d
Added a note about redirections to absolute URLs in tests
javiereguiluz Feb 22, 2017
0b78def
Fixed a typo
javiereguiluz Feb 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions testing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,9 @@ document::

// Assert that the response is a redirect to /demo/contact
$this->assertTrue(
$client->getResponse()->isRedirect('/demo/contact'),
'response is a redirect to /demo/contact'
$client->getResponse()->isRedirect('/demo/contact')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ease debugging, I learned to favor:

$this->assertSame(Response::HTTP_FOUND, $client->getResponse()->getStatusCode());
$this->assertSame('/demo/contact', $client->getResponse()->headers->get('location'));

And since it's very verbose and repetitive I've got a ControllerTestTrait which provides assertions like:

public function assertResponseStatusCode(Client $client, $code)
{
    $this->assertSame($code, $client->getResponse()->getStatusCode());
}

public function assertClientIsRedirectedTo(Client $client, $url, $permanent = false)
{
    $this->assertResponseStatusCode($client, $permanent ? Response::HTTP_MOVED_PERMANENTLY : Response::HTTP_FOUND);
    $this->assertSame($url, $client->getResponse()->headers->get('location'));
}

As said in another PR, we need to avoid failure messages like "true is not false" as much as possible to debug faster. Comparing values gives insight immediately.

Maybe it is worth to update the example to advise it, what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that's too much for this PR. Even if I agree with your idea (although I use a different implementation) I'd say that change would need a discussion first. Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Javier. Let's keep it as is here.

// if the redirection URL was generated as an absolute URL
// $client->getResponse()->isRedirect('http://localhost/demo/contact')
);
// ...or simply check that the response is a redirect to any URL
$this->assertTrue($client->getResponse()->isRedirect());
Expand Down