Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

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

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Create new PHPUnit assertions for the WebTestCase#29990

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

Closed
Pierstoval wants to merge1 commit intosymfony:masterfromPierstoval:assertions
Closed

Create new PHPUnit assertions for the WebTestCase#29990

Pierstoval wants to merge1 commit intosymfony:masterfromPierstoval:assertions

Conversation

@Pierstoval
Copy link
Contributor

@PierstovalPierstoval commentedJan 25, 2019•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT
Doc PR~

I got inspirations from Laravel Dusk's assertions to bring new assertion capabilities to theWebTestCase in order to ease functional testing.

WDYT?

Edit: Here's the list of the added assertions, as a summary of this PR:

  • assertResponseIsSuccessful
  • assertHttpCodeEquals
  • assertResponseHasHeader
  • assertResponseNotHasHeader
  • assertResponseHeaderEquals
  • assertResponseHeaderNotEquals
  • assertResponseRedirects
  • assertPageTitleEquals
  • assertPageTitleContains
  • assertClientHasCookie
  • assertClientNotHasCookie
  • assertClientCookieValueEquals
  • assertClientRawCookieValueEquals
  • assertResponseHasCookie
  • assertResponseNotHasCookie
  • assertResponseCookieValueEquals
  • assertResponseCookieValueNotEquals
  • assertSelectorExists
  • assertSelectorNotExists
  • assertSelectorContainsText
  • assertSelectorNotContainsText
  • assertInputValueEquals
  • assertInputValueNotEquals
  • assertRouteEquals
  • assertRequestAttributeValueEquals

Koc, daFish, radamou, rvanlaak, XuruDragon, ro0NL, mhujer, and sstok reacted with thumbs up emojiOskarStark reacted with heart emoji
@nicolas-grekasnicolas-grekas added this to thenext milestoneJan 27, 2019
Copy link
Member

@dunglasdunglas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Cool!

rvanlaak and Pierstoval reacted with hooray emoji
@XuruDragon
Copy link

Thank you ! so tired of always recoding the same bits of tests... This will greatly increase testing :)

Pierstoval reacted with thumbs up emoji

@Pierstoval
Copy link
ContributorAuthor

Pierstoval commentedFeb 18, 2019•
edited
Loading

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 🌮

XuruDragon reacted with thumbs up emoji

Copy link
Contributor

@localheinzlocalheinz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Why are there no tests?

@Pierstoval
Copy link
ContributorAuthor

@localheinz:

Why are there no tests?

I guess because theKernelTestCase is not tested?

@Pierstoval
Copy link
ContributorAuthor

Pierstoval commentedMar 17, 2019•
edited
Loading

Friendly ping to@dunglas and@stof and@nicolas-grekas

I fixed the compatibility with PHPUnit8 by using the same technique ashttps://github.com/symfony/symfony/pull/30474/files

However, the aforementioned PR introduces an issue IMO: thedoSetUp() anddoTearDown() don't include the: void return type in the "PHPUnit <8 compatible" version of the class. Since Symfony 4 is compatible with PHP 7.1 only, I added the return type in all versions of theTestCaseSetUpTearDownTrait to be sure to not trigger the same issues than the one we have with PHPUnit8's BC break.

@Pierstoval
Copy link
ContributorAuthor

The PR is ready, still waiting for reviews 😀

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks interesting. I have not reviewed all assertions but I've made some cosmetic recommendations.

@Pierstoval
Copy link
ContributorAuthor

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
*/
privatefunctiondoSetUp()
privatefunctiondoSetUp():void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Why this change and the one on the Validator component? They don't seem related to your PR.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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 newWebTestAssertions, 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?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

For 4.2 and should be applied to all similar traits if there are more similar cases.

Copy link
ContributorAuthor

@PierstovalPierstovalMar 31, 2019•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

So should I split the change and put it in another PR targeted on 4.2?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

So, let's remove this change from here and let's create another PR for 4.2.

@fabpot
Copy link
Member

@Pierstoval I think we also need some tests for these new assertions.

@Pierstoval
Copy link
ContributorAuthor

@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 🙂

@fabpot
Copy link
Member

@Pierstoval I've taken over this PR in#30813 where I've refactored the code and added tests.

@fabpot
Copy link
Member

closing in favor of#30813

@fabpotfabpot closed thisApr 1, 2019
fabpot added a commit that referenced this pull requestApr 1, 2019
…, 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
@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot requested changes

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

@dunglasdunglasdunglas approved these changes

+6 more reviewers

@renanbrrenanbrrenanbr left review comments

@mhujermhujermhujer left review comments

@sstoksstoksstok left review comments

@ro0NLro0NLro0NL left review comments

@XuruDragonXuruDragonXuruDragon left review comments

@localheinzlocalheinzlocalheinz requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

12 participants

@Pierstoval@XuruDragon@fabpot@dunglas@nicolas-grekas@renanbr@mhujer@stof@localheinz@sstok@ro0NL@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp