Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Remove deprecated assertContains#32977
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
Remove deprecated assertContains#32977
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedAug 6, 2019
Thanks, we just merged#32971 which does the same on 3.4 |
7b09a9e to622c638CompareMarioBlazek commentedAug 6, 2019
@nicolas-grekas PR rebased against 4.3 |
nicolas-grekas commentedAug 6, 2019
All the cases that fail with "mb_strpos() expects parameter 1 to be string, array given" should use assertContainsEquals instead |
nicolas-grekas commentedAug 6, 2019
Wait, assertContains isnot deprecated.Some uses of it are. This should be more selective about those. |
| $this->assertContains('BaseBundle:controller:custom.format.engine',$templates); | ||
| $this->assertContains('::this.is.a.template.format.engine',$templates); | ||
| $this->assertContains('::resource.format.engine',$templates); | ||
| $this->assertStringContainsString('BaseBundle::base.format.engine',$templates); |
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.
templates is an array, you should revert and keepassertContains.
The deprecation is about usingassertContains when the second argument is a string (ie.assertContains('foo', 'foobarbaz') )
Same comment for many changes bellow.
An easy way to spot theme, is to run test and revert reverting the failing tests ;-)
MarioBlazek commentedAug 7, 2019
Thank you guys, I will update PR accordingly asap. |
MarioBlazek commentedAug 7, 2019
@nicolas-grekas@jderusse tests now pass, but build for some strange reason is hanging too long. Did you experienced something similar before? |
nicolas-grekas commentedAug 7, 2019
@MarioBlazek tests should be back to OK with#33005 |
MarioBlazek commentedAug 7, 2019
Tests are now OK, thanks@nicolas-grekas |
7b9c145 to43acda6Comparenicolas-grekas commentedAug 7, 2019
Thank you@MarioBlazek. |
This PR was squashed before being merged into the 4.3 branch (closes#32977).Discussion----------Remove deprecated assertContains| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#32844| License | MIT| Doc PR | symfonyThis PR replaces `assertContains()` with `assertStringContainsString()`.Commits-------43acda6 Remove deprecated assertContains
This PR was merged into the 4.3 branch.Discussion----------Fix deprecations on 4.3| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#32844| License | MIT| Doc PR | NAFix deprecations in branch 4.3note: remaining deprecation `assertStringContainsString` will be fixed in#32977* [ ] fix tests in branch 3.4 in#32981Commits-------8fd16a6 Fix deprecation on 4.3
Uh oh!
There was an error while loading.Please reload this page.
This PR replaces
assertContains()withassertStringContainsString().