Skip to content
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

Merged
merged 1 commit into from Aug 8, 2019
Merged

Fix deprecations on 4.3 #33000

merged 1 commit into from Aug 8, 2019

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented Aug 6, 2019

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

.travis.yml Outdated Show resolved Hide resolved
@jderusse jderusse force-pushed the deprec-43 branch 2 times, most recently from ab1d931 to 39a4b52 Compare August 7, 2019 07:49
@jderusse jderusse force-pushed the deprec-43 branch 3 times, most recently from 4d6765b to d485174 Compare August 7, 2019 09:38
@@ -54,15 +54,19 @@ public function testProcess()
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.');
Copy link
Member Author

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

$container = new ContainerBuilder();

$container->register(B::class);
$cDefinition = $container->register('c', __NAMESPACE__.'\C');
$cDefinition->setAutowired(true);

(new ResolveClassPass())->process($container);
(new AutowirePass())->process($container);

$this->assertCount(1, $container->getDefinition('c')->getArguments());
Copy link
Member Author

@jderusse jderusse Aug 7, 2019

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');

$container = new ContainerBuilder();

$container->register(F::class);
$gDefinition = $container->register('g', __NAMESPACE__.'\G');
$gDefinition->setAutowired(true);

(new ResolveClassPass())->process($container);
(new AutowirePass())->process($container);

$this->assertCount(3, $container->getDefinition('g')->getArguments());
Copy link
Member Author

Choose a reason for hiding this comment

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

same here

@jderusse jderusse force-pushed the deprec-43 branch 2 times, most recently from fd6568d to 046e45e Compare August 7, 2019 10:18
@jderusse jderusse changed the title Fix deprecations on 4.3 WIP - Fix deprecations on 4.3 Aug 7, 2019
@nicolas-grekas nicolas-grekas mentioned this pull request Aug 7, 2019
23 tasks
@jderusse jderusse force-pushed the deprec-43 branch 5 times, most recently from 68d1426 to 4edb17c Compare August 7, 2019 22:16
$this->assertStringEqualsFile(__DIR__.'/Fixtures/proxy-factory.php', $factory);

require_once __DIR__.'/Fixtures/proxy-implem.php';
eval(preg_replace('/^<\?php/', '', $implem));
Copy link
Member Author

@jderusse jderusse Aug 7, 2019

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.

@jderusse jderusse changed the title WIP - Fix deprecations on 4.3 Fix deprecations on 4.3 Aug 8, 2019
@nicolas-grekas
Copy link
Member

Thank you @jderusse.

@nicolas-grekas nicolas-grekas merged commit 8fd16a6 into symfony:4.3 Aug 8, 2019
nicolas-grekas added a commit that referenced this pull request Aug 8, 2019
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
@jderusse jderusse deleted the deprec-43 branch March 5, 2020 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants