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

Enable Symfony DI autowiring #128

Closed
vincentchalamon opened this issue Nov 13, 2017 · 44 comments
Closed

Enable Symfony DI autowiring #128

vincentchalamon opened this issue Nov 13, 2017 · 44 comments

Comments

@vincentchalamon
Copy link

As a bridge with Symfony2, it should be interesting to autowire contexts arguments using Symfony DI autowiring, if they're detected as Symfony services. For instance:

# behat.yml
default:
    suites:
        default:
            contexts:
                - FeatureContext: ~
    extensions:
        Behat\Symfony2Extension:
            kernel:
                bootstrap: 'vendor/autoload.php'
                path: 'src/Kernel.php'
                class: 'App\Kernel'
class FeatureContext implements Context
{
    private $clientManager;
    private $tokenManager;

    public function __construct(ClientManagerInterface $clientManager, TokenManagerInterface $tokenManager)
    {
        $this->clientManager = $clientManager;
        $this->tokenManager = $tokenManager;
    }

    // ...
}
@sroze
Copy link
Contributor

sroze commented Nov 13, 2017

Definitely agree with you @vincentchalamon. Actually, we might just rewrite a Behat extension for Symfony 4+...

@vincentchalamon
Copy link
Author

vincentchalamon commented Nov 13, 2017

A different extension for Symfony2 vs Symfony4? Why not just updating this existing one?

@sroze
Copy link
Contributor

sroze commented Nov 13, 2017

Something like that :)

@vincentchalamon
Copy link
Author

@sroze As I've seen, this extension loads the container after kernel boot (which is normal). At this point, I cannot access any ContainerBuilder to look for a service by its class name. Do you know another way to do it?

@tsantos84
Copy link

Same question as @vincentchalamon here. If I understood correctly the process of context creation and argument resolving, Symfony2Extension should be able to share its container definitions with kernel container and create the Context classes as a service. By doing this Behat will create the contexts from service container and its arguments will be resolved directly by the container auto-wiring mechanism.

@sroze
Copy link
Contributor

sroze commented Dec 15, 2017

Erm, that's a good idea @tsantos84

@tsantos84
Copy link

A workaround to inject your private services to contexts may be create a CompilerPass to make all service definitions public and register it only on testenvironments. Didn't test this here!

@sroze
Copy link
Contributor

sroze commented Dec 15, 2017

I had a quick look and.. even if you idea is very good @tsantos84... @vincentchalamon is right, we do not have the container builder when locating the context (actually's it's behat's job, not from the extension..). Looking at it still 🤔

@vincentchalamon
Copy link
Author

@tsantos84 To be clear, your idea is to connect the Behat contexts in the Symfony application container through Symfony2Extension? If so, it could unblock lot of features to extend Behat!

Otherwise, as the Behat container is different from the application container, it's still possible to guess the context arguments in the ServiceArgumentResolver::resolveArguments method. But we don't have access to the application container builder to guess the service based on the typehint of the context arguments.

@vincentchalamon
Copy link
Author

Or did you mean to inject the application services in the Behat container? So we could have an access to a container builder on Behat build, and it could allow to guess a service if by its typehint

@tsantos84
Copy link

tsantos84 commented Dec 15, 2017

Exactly @vincentchalamon, ContainerBuilder has a method to merge another container to it. So, if we merge behat container with application container may we could register the context classes as a service and use the container to instantiate them.

@stof
Copy link
Member

stof commented Dec 15, 2017

@tsantos84 this would merge service definitions, but you would still have 2 separate containers: the one used by Behat, and the one used by the kernel. so you would not reuse the same service instances.

@vincentchalamon
Copy link
Author

@stof 👍
But I think this approach could still help to guess the service id by it's typehint from the context arguments (in the container builder process in Behat). Then we could get the right service from the application container.
Also, merging all services in the Behat container may be not required as we won't use thoses services after build.

@tsantos84
Copy link

@stof do we need the kernel container definitions once it was merged to behat's container?

@tsantos84
Copy link

@vincentchalamon we still have problem with auto-wiring private services, right?

@sroze
Copy link
Contributor

sroze commented Dec 15, 2017

we still have problem with auto-wiring private services, right?

Correct, but you'd have Symfony DI's error message that is quite good. i.e. something like "the service is not accessible because private, make it public, blah blah"

@tsantos84
Copy link

I agree that this approach is a way to follow, but we would loose the performance improvement added recently to DI component by making services public just to fit the Behat architecture.

@tsantos84
Copy link

tsantos84 commented Dec 15, 2017

@stof I understood your point and you're right. We still have two redundant containers with redundant services. That said, what do you think about creating a Behat application with an existing ContainerBuilder?

@vincentchalamon
Copy link
Author

@sroze Did you mean something like that? 😄
capture d ecran 2017-12-20 a 09 05 59

@sroze
Copy link
Contributor

sroze commented Dec 20, 2017 via email

@vincentchalamon
Copy link
Author

When you said:

Correct, but you'd have Symfony DI's error message that is quite good. i.e. something like "the service is not accessible because private, make it public, blah blah"

@tsantos84
Copy link

Any news about this issue?

@sroze
Copy link
Contributor

sroze commented Apr 4, 2018

@tsantos84 are you willing to work on a PR? :)

@tsantos84
Copy link

@sroze sure. Did we concluded any final solution in this issue?

@warslett
Copy link

warslett commented Apr 23, 2018

This improvement would be a massive help. Can I make a further suggestion: using service tags to automatically load all contexts tagged with something like behat.context.

# services_test.yml
services:
    App\Contexts\MyContext:
        autowire: true
        tags: [behat.context]

Or even better (not sure whether this works out the box or not):

# services_test.yml
services:
    App\Contexts\:
        resource: '../features/contexts/*'
        autowire: true
        tags: [behat.context]

If not, the same thing could be achieved with autoconfigure:

// src/Kernel.php
class Kernel extends Kernel
{
    // ...

    protected function build(ContainerBuilder $container)
    {
        $container->registerForAutoconfiguration(\Behat\Behat\Context\Context::class)
            ->addTag('behat.context')
        ;
    }
}

@jcornide
Copy link

jcornide commented Jun 7, 2018

hi,

is there any way right now to inject a Symfony 4 service to a behat context without having to touch the compiler pass or similar?

@warslett
Copy link

warslett commented Jun 7, 2018

@jcornide basically no unless you declare everything as public which means you lose the speed enhancements of the new container. The way I'm doing it at the moment is that I have a few basic services that Behat Contexts interact with directly and they are declared public. So rather than injecting the entity manager into my Behat context I have services, for example TestFixtureService which is public and the entity manager is injected into that with autoloading. I only declare those services in my services_test.yml so they don't get loaded at all in production. It's actually quite a nice design that works really well but it's not necessarily what I would have done if I'd just been able to inject my services as normal.

@stof
Copy link
Member

stof commented Jun 7, 2018

So rather than injecting the entity manager into my Behat context I have services, for example TestFixtureService which is public and the entity manager is injected into that with autoloading

@warslett that's a bad example, as the Doctrine entity_manager is already a public service 😄

@vincentchalamon
Copy link
Author

vincentchalamon commented Jun 7, 2018

@jcornide For now, I override some private services in my test Symfony configuration (app/config/config_test.yml or config/services_test.yaml) and inject those public services in my behat configuration:

# config/services_test.yaml
# Hack for Behat: allow to inject some private services
# Waiting for Behat/Symfony2Extension to support autowiring (https://goo.gl/z8BPpG)
services:
    test.property_info:
        parent: property_info
        public: true

# behat.yml.dist
default:
    # ...
    suites:
        default:
            contexts:
                - FeatureContext:
                    propertyInfo: '@test.property_info'

Be aware it's a temporary fix waiting for current issue to be fixed.

@warslett
Copy link

warslett commented Jun 7, 2018

@stof this is interesting, if I inject it with the service name @doctrine.orm.entity_manager it works with no problems but I've been using the new style @Doctrine\ORM\EntityManagerInterface which results in the error below. I can't have been the first person to make that mistake. Would have saved me alot of faff this time a few months ago.

In Container.php line 260:
                                                                                                                                                                                                                                        
The "Doctrine\ORM\EntityManagerInterface" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.

@DonCallisto
Copy link
Contributor

Are you guys aware of this? We should only provide access to symfony container in order to leverage on autowire that's now implemented in behat.

@ciaranmcnulty
Copy link
Contributor

@DonCallisto yes and I'm using that directly in one codebase by giving it the Symfony container - however it doesn't support parameters.

@DonCallisto
Copy link
Contributor

@ciaranmcnulty how do you managed it to work if this extension does not provide that facility?

@vincentchalamon
Copy link
Author

@DonCallisto but what about private services in the Symfony container?

@DonCallisto
Copy link
Contributor

Private ones can be (should be) fetched from a particular container reserved for tests. It's already enabled since symfony 4.1 IIRC. For services never injected, you should alias them with public: false

@DonCallisto
Copy link
Contributor

@vincentchalamon Take a look https://symfony.com/blog/new-in-symfony-4-1-simpler-service-testing

@vincentchalamon
Copy link
Author

Didn't know about that, it helps me so much in my Behat extension. Thanks for sharing ❤️

@tyx
Copy link

tyx commented Apr 18, 2019

Hi guys,

anyone start something about that ? even a gist ?

@tyx
Copy link

tyx commented Apr 18, 2019

Ok I guess everyone moved to https://github.com/FriendsOfBehat/SymfonyExtension and nobody tells me 😭

@DonCallisto
Copy link
Contributor

I was just suggesting this and then you found out it by yourself 😄
Right now we're not still moved to FOB/SymfonyExtension but we have planned to do this ASAP.

@sroze
Copy link
Contributor

sroze commented Apr 22, 2019

How did they solve the issue in FOB/SymfonyExtension?

@tyx
Copy link

tyx commented Apr 23, 2019

They inversed the principle. You load context directly from your Symfony app. So your contexts files are basically just classic services from Symfony POV with autowire

@sroze
Copy link
Contributor

sroze commented Apr 23, 2019 via email

@vincentchalamon
Copy link
Author

Closing in favor of https://github.com/FriendsOfBehat/SymfonyExtension

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

No branches or pull requests

9 participants