Create new PHPUnit assertions for the WebTestCase #29990
Conversation
|
Thank you ! so tired of always recoding the same bits of tests... This will greatly increase testing :) |
|
Thanks! Friendly update to the reviewers: I added a slight summary of the assertions in the PR description so we can directly see what's added, for easier review. Waiting for more reviews 👍 Edit: And it's rebased & squashed 🌮 |
I guess because the |
|
Friendly ping to @dunglas and @stof and @nicolas-grekas I fixed the compatibility with PHPUnit8 by using the same technique as https://github.com/symfony/symfony/pull/30474/files However, the aforementioned PR introduces an issue IMO: the |
|
The PR is ready, still waiting for reviews 😀 |
|
Thanks @fabpot for the review, I fixed the cosmetic things. fabbot.io's error seems like a false positive to me, since the phpdoc is necessary to sort of "warn" about the fact that the return type is void with PHPUnit 8. I squashed everything, so the PR seems finished to me, unless anyone has another comment or idea I could work on 😄 |
| * @return void | ||
| */ | ||
| private function doSetUp() | ||
| private function doSetUp(): void |
There was a problem hiding this comment.
Why this change and the one on the Validator component? They don't seem related to your PR.
There was a problem hiding this comment.
I squashed the commit with all the others, but this is intended to fix the fact that PHPUnit 8 needs the type-hint, and not PHPUnit 7.
As Symfony needs PHP 7.1, the void return type can be added with no harm, and since these traits are new as of the upcoming v4.3, IMO they can benefit from this return type right from the start.
I may separate the fix into another commit or into another PR if needed, but since I also need to add such trait in the new WebTestAssertions, I thought it would be also necessary to bring the fix to other traits at the same place.
WYDT? Keep it as-is? New commit/PR?
There was a problem hiding this comment.
For 4.2 and should be applied to all similar traits if there are more similar cases.
There was a problem hiding this comment.
So should I split the change and put it in another PR targeted on 4.2?
There was a problem hiding this comment.
So, let's remove this change from here and let's create another PR for 4.2.
|
@Pierstoval I think we also need some tests for these new assertions. |
|
@fabpot I'd be interested to know how I could test this, maybe I need a bit of coaching on this. I can create a single test and you could tell me if the direction is good or not 🙂 |
|
@Pierstoval I've taken over this PR in #30813 where I've refactored the code and added tests. |
|
closing in favor of #30813 |
…, fabpot) This PR was merged into the 4.3-dev branch. Discussion ---------- New PHPUnit assertions for the WebTestCase | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | replaces #29990 | License | MIT | Doc PR | n/a While reviewing #29990, and working on some tests, I realized that we could do better by adding PHPUnit constraint classes in various components that are then used in WebTextCase. **Before** ```php <?php namespace App\Tests; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; class DefaultControllerTest extends WebTestCase { public function testSomething() { $client = static::createClient(); $crawler = $client->request('GET', '/test'); $this->assertSame(200, $client->getResponse()->getStatusCode()); $this->assertContains('Hello World', $crawler->filter('h1')->text()); } } ``` **After** ```php <?php namespace App\Tests; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; class DefaultControllerTest extends WebTestCase { public function testSomething() { $client = static::createClient(); $client->request('GET', '/test'); $this->assertResponseIsSuccessful(); $this->assertSelectorTextContains('h1', 'Hello World'); } } ``` Commits ------- 4f91020 added PHPUnit assertions in various components 2f8040e Create new PHPUnit assertions for the WebTestCase
I got inspirations from Laravel Dusk's assertions to bring new assertion capabilities to the
WebTestCasein order to ease functional testing.WDYT?
Edit: Here's the list of the added assertions, as a summary of this PR:
assertResponseIsSuccessfulassertHttpCodeEqualsassertResponseHasHeaderassertResponseNotHasHeaderassertResponseHeaderEqualsassertResponseHeaderNotEqualsassertResponseRedirectsassertPageTitleEqualsassertPageTitleContainsassertClientHasCookieassertClientNotHasCookieassertClientCookieValueEqualsassertClientRawCookieValueEqualsassertResponseHasCookieassertResponseNotHasCookieassertResponseCookieValueEqualsassertResponseCookieValueNotEqualsassertSelectorExistsassertSelectorNotExistsassertSelectorContainsTextassertSelectorNotContainsTextassertInputValueEqualsassertInputValueNotEqualsassertRouteEqualsassertRequestAttributeValueEquals