Skip to content

[DI] Fix FactoryReturnTypePassTest on PHP 5 #29949

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[DI] Fix FactoryReturnTypePassTest on PHP 5 #29949

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* @author Guilhem N. <egetick@gmail.com>
*
* @group legacy
* @requires function ReflectionMethod::getReturnType
*/
class FactoryReturnTypePassTest extends TestCase
{
Expand All @@ -44,13 +45,8 @@ public function testProcess()
$pass = new FactoryReturnTypePass();
$pass->process($container);

if (method_exists(\ReflectionMethod::class, 'getReturnType')) {
Copy link
Member

Choose a reason for hiding this comment

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

Seeing this change it looks like the test was expected to pass on older PHP versions too. And since the test doesn't fail always we should probably rather investigate why they sometimes fail and why they pass under other circumstances.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes there is an if/else clause in the 3.4 branch, however a few lines above that, there is reference to FactoryDummy fixture, that causes the syntax (parse) error, because it does not have a valid PHP 5 syntax:

symfony/src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php

Line 34 in aca3d2c

$factory->setFactory([FactoryDummy::class, 'createFactory']);

AppVeyor CI test also points out the same issue:
https://ci.appveyor.com/project/fabpot/symfony/builds/21760919?fullLog=true#L2048
Anyway, what can I do to help solve this issue? If there is an issue, to begin with ...

$this->assertEquals(FactoryDummy::class, $factory->getClass());
$this->assertEquals(\stdClass::class, $foo->getClass());
} else {
$this->assertNull($factory->getClass());
$this->assertNull($foo->getClass());
}
$this->assertEquals(FactoryDummy::class, $factory->getClass());
$this->assertEquals(\stdClass::class, $foo->getClass());
$this->assertEquals(__CLASS__, $bar->getClass());
}

Expand All @@ -71,11 +67,7 @@ public function testReturnTypes($factory, $returnType, $hhvmSupport = true)
$pass = new FactoryReturnTypePass();
$pass->process($container);

if (method_exists(\ReflectionMethod::class, 'getReturnType')) {
$this->assertEquals($returnType, $service->getClass());
} else {
$this->assertNull($service->getClass());
}
$this->assertEquals($returnType, $service->getClass());
}

public function returnTypesProvider()
Expand Down Expand Up @@ -107,7 +99,6 @@ public function testCircularReference()
}

/**
* @requires function ReflectionMethod::getReturnType
* @expectedDeprecation Relying on its factory's return-type to define the class of service "factory" is deprecated since Symfony 3.3 and won't work in 4.0. Set the "class" attribute to "Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy" on the service definition instead.
*/
public function testCompile()
Expand Down