[FrameworkBundle][HttpFoundation] reduce response constraints verbosity #49184

Nicals · Feb 1, 2023

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #42948
License	MIT
Doc PR	symfony/symfony-docs#17848

Adds a verbose argument to HttpFoundation response test constraints.
If set to false, only the HTTP response command and headers will be displayed in case of test failure.

This argument is true by default to keep backward compatibility, but I think it would make more sense to be false.

Before

$this->assertResponseIsSuccessful();

-- PHPUnit output:
Testing App\Controller\AppControllerTest
F                                                                   1 / 1 (100%)

Time: 00:00.299, Memory: 42.50 MB

There was 1 failure:

1) App\Controller\AppControllerTest::testSuccessfulResponse
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:                   Fri, 03 Feb 2023 10:54:05 GMT
Expires:                Fri, 03 Feb 2023 10:54:05 GMT
X-Debug-Exception:
X-Debug-Exception-File: ...
X-Robots-Tag:           noindex

<!--  (404 Not Found) -->
<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8" />
        <meta name="robots" content="noindex,nofollow" />
    <!-- undred of lines later -->
        Sfjs.createToggles();
        Sfjs.createFilters();
    });
}
/*]]>*/        </script>
    </body>
</html>
<!--  (404 Not Found) -->

/home/user/project/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:142
/home/user/project/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:33
/home/user/project/tests/Controller/AppControllerTest.php:35

After

$this->assertResponseIsSuccessful(verbose: false);

-- PHPUnit output:
Testing App\Controller\AppControllerTest
F                                                                   1 / 1 (100%)

Time: 00:00.300, Memory: 42.50 MB

There was 1 failure:

1) App\Controller\AppControllerTest::testSuccessfulResponse
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:                   Fri, 03 Feb 2023 10:53:03 GMT
Expires:                Fri, 03 Feb 2023 10:53:03 GMT
X-Debug-Exception:
X-Debug-Exception-File: ...
X-Robots-Tag:           noindex

/home/user/project/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:142
/home/user/project/vendor/symfony/framework-bundle/Test/BrowserKitAssertionsTrait.php:33
/home/user/project/tests/Controller/AppControllerTest.php:35

I added a method Symfony\Component\HttpFoundation\Response::getCommandString() that builds the first line of an HTTP response to avoid code duplication in constraints classes.
I thought of other solutions:

Build the HTTP command line in each Constraint. This seems that too much code duplication.
Add an AbstractResponseConstraint with a utility method to build a less verbose response string. I wasn't sure we needed to add an extend layer here.
Add a ResponseConstraintTrait with an exposed method to build the less verbose response string. This did not feel right.

Overall, I think the Response class is the more appropriate to build this string.

I also had some issue with psalm complaining about the documented $response argument type of additionalFailureDescription:

ERROR: MoreSpecificImplementedParamType - src/Symfony/Component/HttpFoundation/Test/Constraint/ResponseStatusCodeSame.php:50:53 - Argument 1 of Symfony\Component\HttpFoundation\Test\Constraint\ResponseStatusCodeSame::additionalFailureDescription has the more specific type 'Symfony\Component\HttpFoundation\Response', expecting 'mixed' as defined by PHPUnit\Framework\Constraint\Constraint::additionalFailureDescription (see https://psalm.dev/140)
    protected function additionalFailureDescription($other): string

I needed to delete the phpdoc comment to comply with this. I only changed the lines were Psalm was giving me errors, but there are still some mismatch between HttpFoundation constraints signatures and PHPUnit ones.

carsonbot · Mar 2, 2023

Hey!

I think @alli83 has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

GromNaN · Mar 16, 2023

Hi Nicolas! Thank you for submitting this pull request and for all the hard work you put into it. I appreciate your effort and dedication to improving this project.

However, after reviewing your changes, I'm not entirely sure if your solution is necessary or if using $client->catchExceptions(false); might already achieve the same result.

Nicals · Apr 17, 2023

Catching exception effectively reduces the verbosity.

This PR intends to reduces the verbosity of test logs in all cases.

For example, a failed assertion about response status code will dump the entire response, including body.
When debugging such assertion I want to see the actual response status code, but this information is in the response header, hundreds of lines above the tail of the unit test logs.
Exception catching is not the issue in this case.

src/Symfony/Component/HttpFoundation/Response.php

fabpot · Feb 3, 2024

@Nicals Are you still interested in finishing this PR?

Nicals · Feb 4, 2024

@fabpot I totally forgot about this MR. I don't work with php/symfony anymore so I did not keep track of it.

I quickly rebased this branch on up to date 7.1 and built the response headers manually.

fabpot · Mar 17, 2024

Thank you @Nicals.

…priou) This PR was submitted for the 6.3 branch but it was merged into the 7.1 branch instead. Discussion ---------- 42948 reduce response contraints verbosity Link the issue [#49184](symfony/symfony#49184), and [PR](symfony/symfony#49184). Update signature of HTTP response constraints. Commits ------- d159a4a [HttpFoundation] Update http response test constraint signature

carsonbot added Status: Needs Review Feature labels Feb 1, 2023

carsonbot added this to the 6.3 milestone Feb 1, 2023

Nicals mentioned this pull request Feb 1, 2023

42948 reduce response contraints verbosity symfony/symfony-docs#17848

Merged

Nicals force-pushed the 42948_reduce_response_constraints_verbosity branch from 78772a9 to ab0eea3 Compare February 2, 2023 07:46

xabbuh added HttpFoundation FrameworkBundle labels Feb 2, 2023

carsonbot changed the title ~~42948 reduce response constraints verbosity~~ [FrameworkBundle][HttpFoundation] 42948 reduce response constraints verbosity Feb 2, 2023

Nicals force-pushed the 42948_reduce_response_constraints_verbosity branch from ab0eea3 to 54c1bbb Compare February 3, 2023 10:12

nicolas-grekas changed the title ~~[FrameworkBundle][HttpFoundation] 42948 reduce response constraints verbosity~~ [FrameworkBundle][HttpFoundation] reduce response constraints verbosity Mar 17, 2023

nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023

fabpot requested changes Aug 1, 2023

View reviewed changes

src/Symfony/Component/HttpFoundation/Response.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels Aug 1, 2023

nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023

Nicals force-pushed the 42948_reduce_response_constraints_verbosity branch from c9f5ae0 to 7d0265c Compare February 4, 2024 11:53

fabpot approved these changes Mar 17, 2024

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Mar 17, 2024

[FrameworkBundle][HttpFoundation] reduce response constraints verbosity

cabb2dc

fabpot force-pushed the 42948_reduce_response_constraints_verbosity branch from 7d0265c to cabb2dc Compare March 17, 2024 16:31

fabpot merged commit e491616 into symfony:7.1 Mar 17, 2024

fabpot mentioned this pull request May 2, 2024

Release v7.1.0-BETA1 #54820

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle][HttpFoundation] reduce response constraints verbosity #49184

[FrameworkBundle][HttpFoundation] reduce response constraints verbosity #49184

Nicals commented Feb 1, 2023 •

edited

Loading

carsonbot commented Mar 2, 2023

GromNaN commented Mar 16, 2023 •

edited

Loading

Nicals commented Apr 17, 2023

fabpot commented Feb 3, 2024 •

edited

Loading

Nicals commented Feb 4, 2024

fabpot commented Mar 17, 2024

Search code, repositories, users, issues, pull requests...

[FrameworkBundle][HttpFoundation] reduce response constraints verbosity #49184

[FrameworkBundle][HttpFoundation] reduce response constraints verbosity #49184

Conversation

Nicals commented Feb 1, 2023 • edited Loading

carsonbot commented Mar 2, 2023

GromNaN commented Mar 16, 2023 • edited Loading

Nicals commented Apr 17, 2023

fabpot commented Feb 3, 2024 • edited Loading

Nicals commented Feb 4, 2024

fabpot commented Mar 17, 2024

Nicals commented Feb 1, 2023 •

edited

Loading

GromNaN commented Mar 16, 2023 •

edited

Loading

fabpot commented Feb 3, 2024 •

edited

Loading