-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New PHPUnit assertions for the WebTestCase #30813
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
Conversation
Green and ready! |
Minor comment: checking PHPUnit's assertions (https://phpunit.de/manual/6.5/en/appendixes.assertions.html) I saw that they don't define "negative asserts" (NotHas..., NotContains..., etc.) but here we're defining asserts in symmetric pairs: (assertResponseHasCookie() + assertResponseNotHasCookie(), assertInputValueSame() + assertInputValueNotSame(), etc.) Just in case you want to double check if we really want this. Thanks. |
@javiereguiluz If you have a closer look, you will see that PHPUnit defines a Not alternative for almost all their assertions. |
d6d8a72
to
b7537e3
Compare
Thank you @Pierstoval. |
…, 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
private function getResponseTester(Response $response): WebTestCase | ||
{ | ||
$client = $this->createMock(KernelBrowser::class); | ||
$client->expects($this->any())->method('getResponse')->will($this->returnValue($response)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
->expects($this->any())
is useless because it doesn't do anything, I'm not sure we should keep encouraging this method when using mocks 🤔
@fabpot , why it is not suffixed with |
@vudaltsov Good catch! That's because I got the code from another PR and didn't think about that. Can you submit a PR to rename it properly? |
…ertionsTrait (vudaltsov) This PR was merged into the 4.3-dev branch. Discussion ---------- [FrameworkBundle] Rename WebTestAssertions -> WebTestAssertionsTrait | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30813 (comment) | License | MIT | Doc PR | Renamed according to the Symfony coding standards. Commits ------- 2ae30a7 Rename WebTestAssertions -> WebTestAssertionsTrait
…rstoval) This PR was merged into the master branch. Discussion ---------- [Testing] Add docs for new WebTestCase assertions Fixes #11266 , as of symfony/symfony#30813 Commits ------- 3b1a1a1 Add docs of new WebTestCase assertions
Just wondering, the Symfony blog post mentions many assertsions that are not there. E.g. When you search for this string in whole Symfony code (here), you see it was in previous PR, but never merged Blog post should be updated with this PR, not previous one. It's very confusing now |
… in the generated functional tests (adrienlucas) This PR was merged into the 1.0-dev branch. Discussion ---------- [make:*-test] Use the new WebTestAssertionsTrait methods in the generated functional tests Hi, this PR is to fix the way of calling the assertions static methods of PHPunit in the generated unit and functional tests. [First commit](8f4af14) : As pointed out in [this article](https://arkadiuszkondas.com/the-right-way-to-call-assertion-in-phpunit/), we should not be calling the assertions using `$this->assert*` but instead with `static::assert*`. [Second commit](cfb0715) : To go further, we can use [the new assertion API](symfony/symfony#30813) provided by the `WebTestCase`. I don't kown if this could be considered a BC break... but i hope not. Commits ------- 5982f3c Use the new api provided by the WebTestAssertionsTrait
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
After