Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
fabpot commentedApr 1, 2019
Green and ready! |
javiereguiluz commentedApr 1, 2019
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. |
fabpot commentedApr 1, 2019
@javiereguiluz If you have a closer look, you will see that PHPUnit defines a Not alternative foralmost all their assertions. |
d6d8a72 tob7537e3Comparefabpot commentedApr 1, 2019
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/aWhile 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<?phpnamespace 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<?phpnamespace 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 components2f8040e 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 🤔
vudaltsov commentedApr 3, 2019
@fabpot , why it is not suffixed with |
fabpot commentedApr 3, 2019
@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? |
vudaltsov commentedApr 3, 2019
…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 assertionsFixes#11266 , as ofsymfony/symfony#30813Commits-------3b1a1a1 Add docs of new WebTestCase assertions
TomasVotruba commentedApr 13, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 testsHi, 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
… 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 testsHi, this PR is to fix the way of calling the assertions static methods of PHPunit in the generated unit and functional tests.[First commit](symfony/maker-bundle@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](symfony/maker-bundle@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
Uh oh!
There was an error while loading.Please reload this page.
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