-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Fix deprecations on 4.3 #33000
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
Fix deprecations on 4.3 #33000
Conversation
ab1d931 to
39a4b52
Compare
src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
Outdated
Show resolved
Hide resolved
4d6765b to
d485174
Compare
| public function testProcessNotExistingActionParam() | ||
| { | ||
| $this->expectException('Symfony\Component\DependencyInjection\Exception\AutowiringFailedException'); | ||
| $this->expectExceptionMessage('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\CompilerEslaAction": argument "$notExisting" of method "Symfony\Component\DependencyInjection\Tests\Compiler\ElsaAction::__construct()" has type "Symfony\Component\DependencyInjection\Tests\Compiler\NotExisting" but this class was not found.'); |
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.
This assertion triggers an exception in PHPUnit Because AutowiringFailedException::message is not a string but a class with a __toString method
src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
Outdated
Show resolved
Hide resolved
| (new ResolveClassPass())->process($container); | ||
| (new AutowirePass())->process($container); | ||
|
|
||
| $this->assertCount(1, $container->getDefinition('c')->getArguments()); |
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.
container->getDefinition('c')->getArguments() equals to 0.
This assert were never reached because call to process triggers an exception and cautch by the former $this->expectException('Symfony\Component\DependencyInjection\Exception\RuntimeException');
| (new ResolveClassPass())->process($container); | ||
| (new AutowirePass())->process($container); | ||
|
|
||
| $this->assertCount(3, $container->getDefinition('g')->getArguments()); |
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.
same here
fd6568d to
046e45e
Compare
68d1426 to
4edb17c
Compare
| $this->assertStringEqualsFile(__DIR__.'/Fixtures/proxy-factory.php', $factory); | ||
|
|
||
| require_once __DIR__.'/Fixtures/proxy-implem.php'; | ||
| eval(preg_replace('/^<\?php/', '', $implem)); |
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.
since ProxyManager 2.2 the suffix in properties is predictable, BUT it vary with version of dependencies installed. Which make a simple assertEquals unusable. Now, proxy-implem.php contains a pattern to match, and $implem contains the true code to evaluate.
|
Thank you @jderusse. |
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 | NA Fix deprecations in branch 4.3 note: remaining deprecation `assertStringContainsString` will be fixed in #32977 * [ ] fix tests in branch 3.4 in #32981 Commits ------- 8fd16a6 Fix deprecation on 4.3
Fix deprecations in branch 4.3
note: remaining deprecation
assertStringContainsStringwill be fixed in #32977