Skip to content

Commit

Permalink
bug #27312 Supress deprecation notices thrown when getting private se…
Browse files Browse the repository at this point in the history
…rvies from container in tests (arderyp)

This PR was submitted for the master branch but it was squashed and merged into the 4.1 branch instead (closes #27312).

Discussion
----------

Supress deprecation notices thrown when getting private servies from container in tests

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27037
| License       | MIT
| Doc PR        | na

This is my first attempt at fixing #27037.

This approach works for me, and I believe it will work globally, but I am not entirely sure if it is the "right" way to do this.  Its not that I think it's the "wrong" way to do it, its just that the `phpunit-bridge` code structure is not the type of code structure that I'm used to, and I'm still new to this project, so I don't know all of the ins and outs, or if there might be a more ideal place to inject the logic we want.

Also, I think my code may be a bit redundant, and I was hoping to get feedback on that.  @nicolas-grekas  (or anyone else), do you think it is safe to simply check if the deprecation message contains the substring ` service is private, getting it from the container...`? if so, then I think we can simply get rid of my `$isNoticeForContainerGetUsage` bit altogether.  If I am understanding things properly, `$isNoticeForContainerGetUsage` will always be true if `$isPrivateServiceNotice` is true, thus we can safely remove the former (as the later is more intuitive, IMO).

The last thing I wanted to confirm was my interpretation of the stack trace, as detailed in my code comment. If the bit after `NOTE` is not correct, this code might not be a proper solution.  I tested various contexts where the error should be suppressed (from within tests) and also where it should not (from within controllers, etc), which is what I based my `NOTE` comment on.

Commits
-------

00603bf Supress deprecation notices thrown when getting private servies from container in tests
  • Loading branch information
nicolas-grekas committed May 21, 2018
2 parents 7497ad4 + 00603bf commit 70c70e2
Showing 1 changed file with 24 additions and 0 deletions.
24 changes: 24 additions & 0 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php
Expand Up @@ -109,6 +109,30 @@ public static function register($mode = 0)
}

$trace = debug_backtrace(true);

// Silence deprecation warnings about private service accessed
// from the service container if done so from a Test class.
// As of Symfony 4.1, there is a new TestContainer that allows
// fetching of private services within tests, so we no longer
// need to warn about this behavior.
//
// NOTE: the event at the top of the stack $trace (index 0) should
// always be the PhpUnitBridge's DeprecationErrorHandler; the
// second event (index 1) should be the trigger_error() event;
// the third event (index 2) should be the actual source of the
// triggered deprecation notice; and the fourth event (index 3)
// represents the action that called the deprecated code. In the
// scenario that we want to suppress, the 4th event will be an
// object instance of \PHPUnit\Framework\TestCase.
if (isset($trace[3]['object'])) {
$isPrivateServiceNotice = false !== strpos($msg, ' service is private, ');
$isNoticeForContainerGetHasUsage = 'Symfony\Component\DependencyInjection\Container' === $trace[2]['class'] && in_array($trace[2]['function'], array('get', 'has'));
$noticeWasTriggeredByPhpUnitTest = $trace[3]['object'] instanceof \PHPUnit\Framework\TestCase;
if ($isPrivateServiceNotice && $isNoticeForContainerGetHasUsage && $noticeWasTriggeredByPhpUnitTest) {
return false;
}
}

$group = 'other';
$isVendor = DeprecationErrorHandler::MODE_WEAK_VENDORS === $mode && $inVendors($file);

Expand Down

0 comments on commit 70c70e2

Please sign in to comment.