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

[HttpFoundation] Make test constraints less verbose #42948

Closed
janklan opened this issue Sep 9, 2021 · 13 comments · Fixed by #49184
Closed

[HttpFoundation] Make test constraints less verbose #42948

janklan opened this issue Sep 9, 2021 · 13 comments · Fixed by #49184
Labels
Help wanted Issues and PRs which are looking for volunteers to complete them. HttpFoundation

Comments

@janklan
Copy link

janklan commented Sep 9, 2021

Description

I have been wondering about a behaviour present when running a PHPUnit functional test in a command line.

Current behaviour: when using the off the shelf test assertions, such as assertResponseIsSuccessful(), each constraint includes the entire response body via additionalFailureDescription() method.

This makes the PHPUnit output quite verbose, thanks to the beautiful error page generated by Symfony, which is great via the browser, but when it comes out as a plaintext, it's not so handy.

There is a good chance I'm doing something wrong - maybe I'm trying to solve a wrong problem, maybe there is a way to turn the error page off when accessing the page with the testing client. If so, please point me somewhere I can learn about it and read no more, and I apologise for wasting everyone's time.

If I'm right about the behaviour, then I'd like to propose changing it, because a single failed assertion generates so much clutter that it is difficult to navigate/read the test results.

I'm happy to create a PR for this, but before I jump into it, I'd like to collect some feedback.

My proposal:

  1. Let the developer say whether or not the constraint is verbose. Sometimes we might want to how the response exactly looks like, but most of the time the exception message might be just enough to give us a hint.
  2. Add a boolean argument to constraints living in Symfony\Component\HttpFoundation\Test\Constraint, say $verbose = true
  3. Support for this argument be added to all methods in \Symfony\Bundle\FrameworkBundle\Test\BrowserKitAssertionsTrait
  4. Optionally, the BrowserKitAssertionsTrait could define a default $verbose value, which could be overridden by individual test classes using the trait.

All this can be done without affecting any existing functionality, providing the tests would be verbose by default.

What do you think?

Thanks for reading.


Example: Failed test in the current state, with the complete response body attached:

PHPUnit 9.5.9 by Sebastian Bergmann and contributors.

Testing ControllerSmokeTest
F                                                                  1 / 1 (100%)

Time: 00:03.920, Memory: 68.50 MB

There was 1 failure:

1) App\Tests\ControllerSmokeTest::testNoSmoke with data set "test" (Closure Object (...), array('test'))
Failed asserting that the Response is successful.
HTTP/1.1 404 Not Found
Cache-Control:             max-age=0, must-revalidate, private
Content-Type:              text/html; charset=UTF-8
Date:                      Thu, 09 Sep 2021 01:08:53 GMT
Expires:                   Thu, 09 Sep 2021 01:08:53 GMT
Referrer-Policy:           no-referrer, strict-origin-when-cross-origin
Set-Cookie:                MOCKSESSID=375c8950a650cc3698c4603766660b93b590317cf18bc3b2ba3eed6b911b4709; path=/; secure; httponly; samesite=lax
X-Content-Type-Options:    nosniff
X-Debug-Exception:         No%20route%20found%20for%20%22GET%20http%3A%2F%2Flocalhost%2Ftest
X-Debug-Exception-File:    %2Fapp%2Fvendor%2Fsymfony%2Fhttp-kernel%2FEventListener%2FRouterListener.php:136
X-Frame-Options:           DENY
X-Robots-Tag:              noindex
X-Xss-Protection:          1; mode=block

<!-- No route found for &quot;GET http://localhost/test&quot; (404 Not Found) -->
<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8" />
        <meta name="robots" content="noindex,nofollow" />
        <meta name="viewport" content="width=device-width,initial-scale=1" />
        <title>No route found for &quot;GET http://localhost/test&quot; (404 Not Found)</title>
        <style>/* This file is based on WebProfilerBundle/Resources/views/Profiler/profiler.css.twig.
   If you make any change in this file, verify the same change is needed in the other file. */
:root {
    --font-sans-serif: Helvetica, Arial, sans-serif;
    --page-background: #f9f9f9;
    --color-text: #222;
    /* when updating any of these colors, do the same in toolbar.css.twig */
    --color-success: #4f805d;
    --color-warning: #a46a1f;
    --color-error: #b0413e;
    --color-muted: #999;
    --tab-background: #fff;
    --tab-color: #444;







[... thousands of lines of HTML and CSS - you get the point ...]







app/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:137
app/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:33
app/tests/BaseTestCase.php:125
app/tests/BaseTestCase.php:66
app/tests/ControllerSmokeTest.php:31

Caused by
ErrorException: No route found for "GET http://localhost/path" in app/vendor/symfony/http-kernel/EventListener/RouterListener.php:136
Stack trace:
#0 app/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php(33): Symfony\Bundle\FrameworkBundle\Test\WebTestCase::assertThatForResponse(Object(Symfony\Component\HttpFoundation\Test\Constraint\ResponseIsSuccessful), '')
#1 app/tests/BaseTestCase.php(125): Symfony\Bundle\FrameworkBundle\Test\WebTestCase::assertResponseIsSuccessful()

[... stack trace not important...]

#13 app/bin/phpunit(11): PHPUnit\TextUI\Command::main()
#14 {main}
FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

Example: without the response body, the error is still perfectly legible

PHPUnit 9.5.9 by Sebastian Bergmann and contributors.

Testing ControllerSmokeTest
F                                                                  1 / 1 (100%)

Time: 00:03.920, Memory: 68.50 MB

There was 1 failure:

1) App\Tests\ControllerSmokeTest::testNoSmoke with data set "test" (Closure Object (...), array('test'))
Failed asserting that the Response is successful.

app/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:137
app/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:33
app/tests/BaseTestCase.php:125
app/tests/BaseTestCase.php:66
app/tests/ControllerSmokeTest.php:31

Caused by
ErrorException: No route found for "GET http://localhost/path" in app/vendor/symfony/http-kernel/EventListener/RouterListener.php:136
Stack trace:
#0 app/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php(33): Symfony\Bundle\FrameworkBundle\Test\WebTestCase::assertThatForResponse(Object(Symfony\Component\HttpFoundation\Test\Constraint\ResponseIsSuccessful), '')
#1 app/tests/BaseTestCase.php(125): Symfony\Bundle\FrameworkBundle\Test\WebTestCase::assertResponseIsSuccessful()

[... stack trace not important...]

#13 app/bin/phpunit(11): PHPUnit\TextUI\Command::main()
#14 {main}
FAILURES!
Tests: 1, Assertions: 1, Failures: 1.
@JB-oclock
Copy link

The way I've come to not have this verbose output is to use/override PHPUnit onNotSuccessfulTest method
But i'm not satisfied with this, and consider it more like a hack

/**
* Override PHPUnit fail method
* to catch "assertResponse" exceptions
* 
* @link https://devdocs.io/phpunit~9/fixtures
*/
protected function onNotSuccessfulTest(\Throwable $t): void
{
    // If "assertResponse" is found in the trace, custom message
    if (strpos($t->getTraceAsString(), 'assertResponse') > 0) {
        $arrayMessage = explode("\n", $t->getMessage());
        $message = $arrayMessage[0] . "\n" . $arrayMessage[1];
        $this->fail($message);
    }

    // Other Exceptions
    throw $t;
}```

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@janklan
Copy link
Author

janklan commented Jul 14, 2022

Nope

@carsonbot carsonbot removed the Stalled label Jul 14, 2022
@chalasr
Copy link
Member

chalasr commented Jul 14, 2022

👍 PR welcome

@chalasr chalasr added the Help wanted Issues and PRs which are looking for volunteers to complete them. label Jul 14, 2022
Nicals pushed a commit to Nicals/symfony that referenced this issue Feb 1, 2023
Nicals pushed a commit to Nicals/symfony that referenced this issue Feb 3, 2023
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Could I get a reply or should I close this?

@janklan
Copy link
Author

janklan commented Mar 16, 2023

I still plan to get this done. An aspirational goal os to make it part of our "upgrade to phpunit 10" initiative.

@carsonbot carsonbot removed the Stalled label Mar 16, 2023
@GromNaN
Copy link
Member

GromNaN commented Mar 16, 2023

There is an option to disable the HTML error page in WebTestCase. Does it solves this issue?

$client = self::createClient();
$client->catchExceptions(false);
$client->request('GET', $uri);
$this->assertResponseIsSuccessful();

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Could I get a reply or should I close this?

@janklan
Copy link
Author

janklan commented Oct 2, 2023

Keep open.

@carsonbot carsonbot removed the Stalled label Oct 2, 2023
@janklan
Copy link
Author

janklan commented Nov 9, 2023

@GromNaN FYI using $client->catchExceptions(false); has an unintended side effect: if the exception is AccessDeniedException, the request is not automatically redirected to the authentication entry point (login page).

@fsa
Copy link

fsa commented Nov 28, 2023

With a simple error 500 or 404, I get about 40 kilobytes of text. This is enough for any console buffer to run out. In addition, PHPUnit begins to complain about lack of memory.
$client->catchExceptions(false); partially solves the problem.

Nicals pushed a commit to Nicals/symfony that referenced this issue Feb 4, 2024
@fabpot fabpot closed this as completed in e491616 Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted Issues and PRs which are looking for volunteers to complete them. HttpFoundation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants