Skip to content

[PhpUnitBridge] Replace ErrorAssert by @expectedDeprecation #20255

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

Merged
merged 1 commit into from
Oct 21, 2016
Merged

[PhpUnitBridge] Replace ErrorAssert by @expectedDeprecation #20255

Show file tree
Hide file tree
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
44 changes: 42 additions & 2 deletions src/Symfony/Bridge/PhpUnit/SymfonyTestsListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class SymfonyTestsListener extends \PHPUnit_Framework_BaseTestListener
private $skippedFile = false;
private $wasSkipped = array();
private $isSkipped = array();
private $expectedDeprecations;
private $gatheredDeprecations;
private $previousErrorHandler;

/**
* @param array $mockedNamespaces List of namespaces, indexed by mocked features (time-sensitive or dns-sensitive)
Expand Down Expand Up @@ -142,7 +145,7 @@ public function addSkippedTest(\PHPUnit_Framework_Test $test, \Exception $e, $ti
public function startTest(\PHPUnit_Framework_Test $test)
{
if (-2 < $this->state && $test instanceof \PHPUnit_Framework_TestCase) {
$groups = \PHPUnit_Util_Test::getGroups(get_class($test), $test->getName());
$groups = \PHPUnit_Util_Test::getGroups(get_class($test), $test->getName(false));

if (in_array('time-sensitive', $groups, true)) {
ClockMock::register(get_class($test));
Expand All @@ -151,13 +154,37 @@ public function startTest(\PHPUnit_Framework_Test $test)
if (in_array('dns-sensitive', $groups, true)) {
DnsMock::register(get_class($test));
}

$annotations = \PHPUnit_Util_Test::parseTestMethodAnnotations(get_class($test), $test->getName(false));

if (isset($annotations['class']['expectedDeprecation'])) {
$test->getTestResultObject()->addError($test, new \PHPUnit_Framework_AssertionFailedError('`@expectedDeprecation` annotations are not allowed at the class level.'), 0);
}
if (isset($annotations['method']['expectedDeprecation'])) {
if (!in_array('legacy', $groups, true)) {
$test->getTestResultObject()->addError($test, new \PHPUnit_Framework_AssertionFailedError('Only tests with the `@group legacy` annotation can have `@expectedDeprecation`.'), 0);
}
$this->expectedDeprecations = $annotations['method']['expectedDeprecation'];
$this->previousErrorHandler = set_error_handler(array($this, 'handleError'));
}
}
}

public function endTest(\PHPUnit_Framework_Test $test, $time)
{
if ($this->expectedDeprecations) {
restore_error_handler();
try {
$prefix = "@expectedDeprecation:\n ";
$test->assertStringMatchesFormat($prefix.implode("\n ", $this->expectedDeprecations), $prefix.implode("\n ", $this->gatheredDeprecations));
Copy link
Member

Choose a reason for hiding this comment

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

This now differs in one detail from the current implementation: Previously, the order in which you declared multiple expected deprecations did not matter. Now you need to know in which order they will be triggered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fine with me :)

} catch (\PHPUnit_Framework_AssertionFailedError $e) {
$test->getTestResultObject()->addFailure($test, $e, $time);
}

$this->expectedDeprecations = $this->gatheredDeprecations = $this->previousErrorHandler = null;
}
if (-2 < $this->state && $test instanceof \PHPUnit_Framework_TestCase) {
$groups = \PHPUnit_Util_Test::getGroups(get_class($test), $test->getName());
$groups = \PHPUnit_Util_Test::getGroups(get_class($test), $test->getName(false));

if (in_array('time-sensitive', $groups, true)) {
ClockMock::withClockMock(false);
Expand All @@ -167,4 +194,17 @@ public function endTest(\PHPUnit_Framework_Test $test, $time)
}
}
}

public function handleError($type, $msg, $file, $line, $context)
{
if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
$h = $this->previousErrorHandler;

return $h ? $h($type, $msg, $file, $line, $context) : false;
}
if (error_reporting()) {
Copy link
Member

Choose a reason for hiding this comment

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

0 !== error_reporting() as we are dealing with integers here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I disagree here: this practice makes the code more fragile, because it needs one more assumption to be correct. When I read this kind of code, I have to verify how null, false and array() are handled. Resorting to behavioral statements saying "there is no pb here because whatever", is the hint that this is bad practice.

$msg = 'Unsilenced deprecation: '.$msg;
}
$this->gatheredDeprecations[] = $msg;
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/PhpUnit/bin/simple-phpunit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

// Please update when phpunit needs to be reinstalled with fresh deps:
// Cache-Id-Version: 2016-09-12 09:00 UTC
// Cache-Id-Version: 2016-10-20 14:00 UTC

error_reporting(-1);

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Console/Formatter/OutputFormatter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ private function createStyleFromString($string)
try {
$style->setOption($option);
} catch (\InvalidArgumentException $e) {
trigger_error(sprintf('Unknown style options are deprecated since version 3.2 and will be removed in 4.0. Exception "%s".', $e->getMessage()), E_USER_DEPRECATED);
@trigger_error(sprintf('Unknown style options are deprecated since version 3.2 and will be removed in 4.0. Exception "%s".', $e->getMessage()), E_USER_DEPRECATED);

return false;
}
Expand Down
12 changes: 3 additions & 9 deletions src/Symfony/Component/Console/Tests/Formatter/OutputFormatterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\Console\Tests\Formatter;

use Symfony\Bridge\PhpUnit\ErrorAssert;
use Symfony\Component\Console\Formatter\OutputFormatter;
use Symfony\Component\Console\Formatter\OutputFormatterStyle;

Expand Down Expand Up @@ -196,17 +195,12 @@ public function provideInlineStyleOptionsCases()
/**
* @group legacy
* @dataProvider provideInlineStyleTagsWithUnknownOptions
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Unknown style options are deprecated since version 3.2 and will be removed in 4.0. Exception "Invalid option specified: "%s". Expected one of (bold, underscore, blink, reverse, conceal)".
*/
public function testInlineStyleOptionsUnknownAreDeprecated($tag, $option)
{
ErrorAssert::assertDeprecationsAreTriggered(
array(sprintf('Unknown style options are deprecated since version 3.2 and will be removed in 4.0. Exception "Invalid option specified: "%s". Expected one of (bold, underscore, blink, reverse, conceal)".', $option)),
function () use ($tag) {
$formatter = new OutputFormatter(true);
$formatter->format($tag);
}
);
$formatter = new OutputFormatter(true);
$formatter->format($tag);
}

public function provideInlineStyleTagsWithUnknownOptions()
Expand Down
129 changes: 43 additions & 86 deletions src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\DependencyInjection\Tests;

use Symfony\Bridge\PhpUnit\ErrorAssert;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
Expand Down Expand Up @@ -131,20 +130,14 @@ public function testGetServiceIds()

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
*/
public function testGetLegacyServiceIds()
{
$sc = new LegacyProjectServiceContainer();
$sc->set('foo', $obj = new \stdClass());

$deprecations = array(
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () use ($sc) {
$this->assertEquals(array('internal', 'bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'service_container', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by getXXXService() methods, followed by service ids defined by set()');
});
$this->assertEquals(array('internal', 'bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'service_container', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by getXXXService() methods, followed by service ids defined by set()');
}

public function testSet()
Expand Down Expand Up @@ -193,39 +186,33 @@ public function testGet()

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
*/
public function testLegacyGet()
{
$sc = new LegacyProjectServiceContainer();
$sc->set('foo', $foo = new \stdClass());

$deprecations = array(
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () use ($sc, $foo) {
$this->assertEquals($foo, $sc->get('foo'), '->get() returns the service for the given id');
$this->assertEquals($foo, $sc->get('Foo'), '->get() returns the service for the given id, and converts id to lowercase');
$this->assertEquals($sc->__bar, $sc->get('bar'), '->get() returns the service for the given id');
$this->assertEquals($sc->__foo_bar, $sc->get('foo_bar'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo.baz'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo\\baz'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($foo, $sc->get('foo'), '->get() returns the service for the given id');
$this->assertEquals($foo, $sc->get('Foo'), '->get() returns the service for the given id, and converts id to lowercase');
$this->assertEquals($sc->__bar, $sc->get('bar'), '->get() returns the service for the given id');
$this->assertEquals($sc->__foo_bar, $sc->get('foo_bar'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo.baz'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo\\baz'), '->get() returns the service if a get*Method() is defined');

$sc->set('bar', $bar = new \stdClass());
$this->assertEquals($bar, $sc->get('bar'), '->get() prefers to return a service defined with set() than one defined with a getXXXMethod()');
$sc->set('bar', $bar = new \stdClass());
$this->assertEquals($bar, $sc->get('bar'), '->get() prefers to return a service defined with set() than one defined with a getXXXMethod()');

try {
$sc->get('');
$this->fail('->get() throws a \InvalidArgumentException exception if the service is empty');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException', $e, '->get() throws a ServiceNotFoundException exception if the service is empty');
}
$this->assertNull($sc->get('', ContainerInterface::NULL_ON_INVALID_REFERENCE), '->get() returns null if the service is empty');
});
try {
$sc->get('');
$this->fail('->get() throws a \InvalidArgumentException exception if the service is empty');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException', $e, '->get() throws a ServiceNotFoundException exception if the service is empty');
}
$this->assertNull($sc->get('', ContainerInterface::NULL_ON_INVALID_REFERENCE), '->get() returns null if the service is empty');
}

public function testGetThrowServiceNotFoundException()
Expand Down Expand Up @@ -288,28 +275,22 @@ public function testHas()

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
* @expectedDeprecation Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.
*/
public function testLegacyHas()
{
$sc = new LegacyProjectServiceContainer();
$sc->set('foo', new \stdClass());

$deprecations = array(
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
'Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () use ($sc) {
$this->assertFalse($sc->has('foo1'), '->has() returns false if the service does not exist');
$this->assertTrue($sc->has('foo'), '->has() returns true if the service exists');
$this->assertTrue($sc->has('bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo_bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo.baz'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo\\baz'), '->has() returns true if a get*Method() is defined');
});
$this->assertFalse($sc->has('foo1'), '->has() returns false if the service does not exist');
$this->assertTrue($sc->has('foo'), '->has() returns true if the service exists');
$this->assertTrue($sc->has('bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo_bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo.baz'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo\\baz'), '->has() returns true if a get*Method() is defined');
}

public function testInitialized()
Expand Down Expand Up @@ -400,66 +381,42 @@ public function testThatCloningIsNotSupported()

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Unsetting the "internal" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
*/
public function testUnsetInternalPrivateServiceIsDeprecated()
{
$deprecations = array(
'Unsetting the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->set('internal', null);
});
$c = new ProjectServiceContainer();
$c->set('internal', null);
}

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Setting the "internal" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0. A new public service will be created instead.
*/
public function testChangeInternalPrivateServiceIsDeprecated()
{
$deprecations = array(
'Setting the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0. A new public service will be created instead.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->set('internal', new \stdClass());
});
$c = new ProjectServiceContainer();
$c->set('internal', new \stdClass());
}

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Checking for the existence of the "internal" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
*/
public function testCheckExistenceOfAnInternalPrivateServiceIsDeprecated()
{
$deprecations = array(
'Checking for the existence of the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->has('internal');
});
$c = new ProjectServiceContainer();
$c->has('internal');
}

/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
* @expectedDeprecation Requesting the "internal" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
*/
public function testRequestAnInternalSharedPrivateServiceIsDeprecated()
{
$deprecations = array(
'Requesting the "internal" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.',
);

ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () {
$c = new ProjectServiceContainer();
$c->get('internal');
});
$c = new ProjectServiceContainer();
$c->get('internal');
}
}

Expand Down
Loading