Conversation

Wirone

@ondrejmirtes I've added support for ServiceProviderInterface and checked it against our codebase - after my changes and implementing this interface in our locators (mentioned here) I don't have errors, so rule is working properly.

But I had tough time with this mostly because of test scenario provided in #151 which IMHO does not work as expected. I started by adding ExampleController::privateServiceFromServiceProvider() but wanted to make it fail and I couldn't. After several tries I just checked what will happen if I remove $isServiceLocator from ContainerInterfacePrivateServiceRule and... tests were still green. So I've just removed those services from tests/Rules/Symfony/container.xml completely and I would like to provide proper test cases for service locator/provider but I don't know how 😅 I've tried to create some example files, loaded in new test method within ContainerInterfacePrivateServiceRuleTest, but I did not manage to achieve failing scenario, which could be fixed by my changes. Could you help me with this (or @lookyman)?

It does not check what it should, even after removing `ServiceLocator` condition in `ContainerInterfacePrivateServiceRule` tests are green.
Accessing services from `ServiceProviderInterface` (which extends `ContainerInterface`) should not trigger "Service is private".
@Wirone

@ondrejmirtes @lookyman any news on this?

@Wirone

@ondrejmirtes @lookyman friendly ping 😉

Choose a reason for hiding this comment

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

I don't understand why this is tested by removing code from tests. Typically you should test things by adding more code?

@Wirone

@ondrejmirtes I described it in the PR's description 🤷‍♂️ I really would like to provide proper test cases, but I had hard time with that and just asked for help here.

Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.