bug #68 Better compatibility with Behat itself (pamil, alanpoulain)

This PR was merged into the 2.0 branch.

Discussion
----------

Fixes #56.

If passed context identifier is a Symfony service, it's handled by our custom logic. If it's not, it's handled by default Behat logic.

Commits
-------

cd792704fa Add support for class resolvers
9e9529c320 Add tests for context initializers
d74cd251d5 Add more sanity checks
c54c581e74 Refactor our environment handler to decorate the original one
8a972b5b48 Apply suggestions from code review

composer.json

@@ -21,6 +21,7 @@
         "behat/mink": "^1.7",
         "behat/mink-browserkit-driver": "^1.3",
         "behat/mink-extension": "^2.2",
+        "friends-of-behat/service-container-extension": "^1.0",
         "phpstan/phpstan-shim": "^0.11",
         "sylius-labs/coding-standard": "^3.0",
         "symfony/framework-bundle": "^3.4|^4.1",

features/mink_integration/mink_integration.feature

@@ -1,4 +1,4 @@
-Feature: Mink integration with dependency injection
+Feature: Mink integration
 
     Background:
         Given a working Symfony application with SymfonyExtension configured

features/mink_integration/mink_integration_with_context_initializer.feature

@@ -1,78 +0,0 @@
-Feature: Mink integration with context initializer
-
-    Scenario: Passing Mink instance and parameters through context initializer
-        Given a working Symfony application with SymfonyExtension configured
-        And a Behat configuration containing:
-        """
-        default:
-            extensions:
-                Behat\MinkExtension:
-                    base_url: "http://localhost:8080/"
-                    default_session: symfony
-                    sessions:
-                        symfony:
-                            symfony: ~
-            suites:
-                default:
-                    contexts:
-                        - App\Tests\SomeContext
-        """
-        And a feature file containing:
-        """
-        Feature:
-            Scenario:
-                When I visit the page "/hello-world"
-                Then I should see "Hello world!" on the page
-                And the base url from Mink parameters should be "http://localhost:8080/"
-
-            # Doubling the scenario to account for some weird error connected to Mink's session
-            Scenario:
-                When I visit the page "/hello-world"
-                Then I should see "Hello world!" on the page
-                And the base url from Mink parameters should be "http://localhost:8080/"
-        """
-        And a context file "tests/SomeContext.php" containing:
-        """
-        <?php
-
-        namespace App\Tests;
-
-        use Behat\Mink\Mink;
-        use Behat\MinkExtension\Context\MinkAwareContext;
-
-        final class SomeContext implements MinkAwareContext {
-            private $mink;
-            private $parameters;
-
-            public function setMink(Mink $mink): void
-            {
-                $this->mink = $mink;
-            }
-
-            public function setMinkParameters(array $minkParameters): void
-            {
-                $this->parameters = $minkParameters;
-            }
-
-            /** @When I visit the page :page */
-            public function visitPage(string $page): void
-            {
-                $this->mink->getSession()->visit($page);
-            }
-
-            /** @Then I should see :content on the page */
-            public function shouldSeeContentOnPage(string $content): void
-            {
-                assert(false !== strpos($this->mink->getSession()->getPage()->getContent(), $content));
-            }
-
-            /** @Then the base url from Mink parameters should be :expected */
-            public function baseUrlShouldBe(string $expected): void
-            {
-                assert(isset($this->parameters['base_url']));
-                assert($this->parameters['base_url'] === $expected);
-            }
-        }
-        """
-        When I run Behat
-        Then it should pass

features/sanity_checks/class_resolvers_compatibility.feature

@@ -0,0 +1,65 @@
+Feature: Class resolvers compatibility
+
+    Scenario: Using class resolvers while handling context environment
+        Given a working Symfony application with SymfonyExtension configured
+        And a Behat configuration containing:
+        """
+        default:
+            extensions:
+                FriendsOfBehat\ServiceContainerExtension:
+                    imports:
+                        - "tests/class_resolver.yml"
+
+            suites:
+                default:
+                    contexts:
+                        - class:resolved:context
+        """
+        And a Behat services definition file "tests/class_resolver.yml" containing:
+        """
+        services:
+            App\Tests\CustomClassResolver:
+                tags: ["context.class_resolver"]
+        """
+        And a Behat service implementation file "tests/CustomClassResolver.php" containing:
+        """
+        <?php
+
+        namespace App\Tests;
+
+        use Behat\Behat\Context\ContextClass\ClassResolver;
+
+        final class CustomClassResolver implements ClassResolver
+        {
+            public function supportsClass($contextClass): bool
+            {
+                return $contextClass === 'class:resolved:context';
+            }
+
+            public function resolveClass($contextClass): string
+            {
+                return 'App\Tests\SomeContext';
+            }
+        }
+        """
+        And a feature file containing:
+        """
+        Feature:
+            Scenario:
+                Then it should pass
+        """
+        And a context file "tests/SomeContext.php" containing:
+        """
+        <?php
+
+        namespace App\Tests;
+
+        use Behat\Behat\Context\Context;
+
+        final class SomeContext implements Context {
+            /** @Then it should pass */
+            public function itShouldPass(): void {}
+        }
+        """
+        When I run Behat
+        Then it should pass

features/sanity_checks/context_constructor_dependency_injection_compatibility.feature

@@ -0,0 +1,56 @@
+Feature: Context constructor dependency injection compatibility
+
+    Scenario: Using context constructor dependency injection
+        Given a working Symfony application with SymfonyExtension configured
+        And a Behat configuration containing:
+        """
+        default:
+            suites:
+                default:
+                    contexts:
+                        - App\Tests\SomeContext:
+                            - "@App\\Foo"
+
+                    services:
+                        App\Foo: ~
+        """
+        And a class file "src/Foo.php" containing:
+        """
+        <?php
+
+        namespace App;
+
+        final class Foo
+        {
+        }
+        """
+        And a feature file containing:
+        """
+        Feature:
+            Scenario:
+                Then it should pass
+        """
+        And a context file "tests/SomeContext.php" containing:
+        """
+        <?php
+
+        namespace App\Tests;
+
+        use App\Foo;
+        use Behat\Behat\Context\Context;
+
+        final class SomeContext implements Context {
+            public function __construct(Foo $foo)
+            {
+                $this->foo = $foo;
+            }
+
+            /** @Then it should pass */
+            public function itShouldPass(): void
+            {
+                assert($this->foo instanceof Foo);
+            }
+        }
+        """
+        When I run Behat
+        Then it should pass

features/sanity_checks/context_initializer_compatibility.feature

@@ -0,0 +1,71 @@
+Feature: Context initializer compatibility
+
+    Scenario: Using class resolvers while handling context environment
+        Given a working Symfony application with SymfonyExtension configured
+        And a Behat configuration containing:
+        """
+        default:
+            extensions:
+                FriendsOfBehat\ServiceContainerExtension:
+                    imports:
+                        - "tests/context_initializer.yml"
+
+            suites:
+                default:
+                    contexts:
+                        - App\Tests\SomeContext
+        """
+        And a Behat services definition file "tests/context_initializer.yml" containing:
+        """
+        services:
+            App\Tests\CustomContextInitializer:
+                tags: ["context.initializer"]
+        """
+        And a Behat service implementation file "tests/CustomContextInitializer.php" containing:
+        """
+        <?php
+
+        namespace App\Tests;
+
+        use Behat\Behat\Context\Context;
+        use Behat\Behat\Context\Initializer\ContextInitializer;
+
+        final class CustomContextInitializer implements ContextInitializer
+        {
+            public function initializeContext(Context $context): void
+            {
+                $context->makeItPass(true);
+            }
+        }
+        """
+        And a feature file containing:
+        """
+        Feature:
+            Scenario:
+                Then it should pass
+        """
+        And a context file "tests/SomeContext.php" containing:
+        """
+        <?php
+
+        namespace App\Tests;
+
+        use Behat\Behat\Context\Context;
+
+        final class SomeContext implements Context {
+            private $shouldPass = false;
+
+            public function makeItPass(bool $shouldPass)
+            {
+                $this->shouldPass = $shouldPass;
+            }
+
+            /** @Then it should pass */
+            public function itShouldPass(): void
+            {
+                assert($this->shouldPass === true);
+            }
+        }
+        """
+        When I run Behat
+        Then it should pass

phpstan.neon

@@ -5,5 +5,5 @@ parameters:
         - '/Cannot access offset 0 on callable/'
         - '/Cannot access offset 1 on callable/'
         - '/Cannot call method [a-zA-Z0-9]+\(\) on Symfony\\Component\\Config\\Definition\\Builder\\NodeParentInterface|null\./'
-        - '/Method FriendsOfBehat\\SymfonyExtension\\Context\\Environment\\InitialisedContextServiceEnvironment::bindCallee\(\) should return callable/'
+        - '/Method FriendsOfBehat\\SymfonyExtension\\Context\\Environment\\InitializedSymfonyExtensionEnvironment::bindCallee\(\) should return callable/'
         - '/Strict comparison using === between 0\|1 and 2 will always evaluate to false\./'

src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php

@@ -14,15 +14,17 @@ declare(strict_types=1);
 namespace FriendsOfBehat\SymfonyExtension\Context\Environment\Handler;
 
 use Behat\Behat\Context\Context;
-use Behat\Behat\Context\Initializer\ContextInitializer;
+use Behat\Behat\Context\Environment\ContextEnvironment;
+use Behat\Behat\Context\Environment\InitializedContextEnvironment;
 use Behat\Testwork\Environment\Environment;
 use Behat\Testwork\Environment\Exception\EnvironmentIsolationException;
 use Behat\Testwork\Environment\Handler\EnvironmentHandler;
 use Behat\Testwork\Suite\Exception\SuiteConfigurationException;
+use Behat\Testwork\Suite\GenericSuite;
 use Behat\Testwork\Suite\Suite;
 use FriendsOfBehat\SymfonyExtension\Bundle\FriendsOfBehatSymfonyExtensionBundle;
-use FriendsOfBehat\SymfonyExtension\Context\Environment\InitialisedContextServiceEnvironment;
-use FriendsOfBehat\SymfonyExtension\Context\Environment\UninitialisedContextServiceEnvironment;
+use FriendsOfBehat\SymfonyExtension\Context\Environment\InitializedSymfonyExtensionEnvironment;
+use FriendsOfBehat\SymfonyExtension\Context\Environment\UninitializedSymfonyExtensionEnvironment;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpKernel\KernelInterface;
 
@@ -31,12 +33,13 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler
     /** @var KernelInterface */
     private $symfonyKernel;
 
-    /** @var ContextInitializer[] */
-    private $contextInitializers = [];
+    /** @var EnvironmentHandler */
+    private $decoratedEnvironmentHandler;
 
-    public function __construct(KernelInterface $symfonyKernel)
+    public function __construct(KernelInterface $symfonyKernel, EnvironmentHandler $decoratedEnvironmentHandler)
     {
         $this->symfonyKernel = $symfonyKernel;
+        $this->decoratedEnvironmentHandler = $decoratedEnvironmentHandler;
     }
 
     public function supportsSuite(Suite $suite): bool
@@ -46,21 +49,31 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler
 
     public function buildEnvironment(Suite $suite): Environment
     {
-        $environment = new UninitialisedContextServiceEnvironment($suite);
-        foreach ($this->getSuiteContextsServices($suite) as $contextId) {
-            $environment->registerContextService($contextId, $this->getContextClass($contextId));
+        $symfonyContexts = [];
+
+        foreach ($this->getSuiteContextsServices($suite) as $serviceId) {
+            if (!$this->getContainer()->has($serviceId)) {
+                continue;
             }
 
-        return $environment;
+            $symfonyContexts[$serviceId] = get_class($this->getContainer()->get($serviceId));
+        }
+
+        $delegatedSuite = $this->cloneSuiteWithoutContexts($suite, array_keys($symfonyContexts));
+
+        /** @var ContextEnvironment $delegatedEnvironment */
+        $delegatedEnvironment = $this->decoratedEnvironmentHandler->buildEnvironment($delegatedSuite);
+
+        return new UninitializedSymfonyExtensionEnvironment($suite, $symfonyContexts, $delegatedEnvironment);
     }
 
     public function supportsEnvironmentAndSubject(Environment $environment, $testSubject = null): bool
     {
-        return $environment instanceof UninitialisedContextServiceEnvironment;
+        return $environment instanceof UninitializedSymfonyExtensionEnvironment;
     }
 
     /**
-     * @param UninitialisedContextServiceEnvironment $uninitializedEnvironment
+     * @param UninitializedSymfonyExtensionEnvironment $uninitializedEnvironment
      *
      * @throws EnvironmentIsolationException
      */
@@ -68,22 +81,25 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler
     {
         $this->assertEnvironmentCanBeIsolated($uninitializedEnvironment, $testSubject);
 
-        $environment = new InitialisedContextServiceEnvironment($uninitializedEnvironment->getSuite());
-        foreach ($uninitializedEnvironment->getContextServices() as $contextId) {
+        $environment = new InitializedSymfonyExtensionEnvironment($uninitializedEnvironment->getSuite());
+
+        foreach ($uninitializedEnvironment->getServices() as $serviceId) {
             /** @var Context $context */
-            $context = $this->getContext($contextId);
-            $this->initializeInstance($context);
+            $context = $this->getContainer()->get($serviceId);
+
+            $environment->registerContext($context);
+        }
+
+        /** @var InitializedContextEnvironment $delegatedEnvironment */
+        $delegatedEnvironment = $this->decoratedEnvironmentHandler->isolateEnvironment($uninitializedEnvironment->getDelegatedEnvironment());
+
+        foreach ($delegatedEnvironment->getContexts() as $context) {
             $environment->registerContext($context);
         }
 
         return $environment;
     }
 
-    public function registerContextInitializer(ContextInitializer $initializer): void
-    {
-        $this->contextInitializers[] = $initializer;
-    }
-
     /**
      * @return string[]
      *
@@ -91,17 +107,49 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler
      */
     private function getSuiteContextsServices(Suite $suite): array
     {
-        $contextsServices = $suite->getSetting('contexts');
+        $contexts = $suite->getSetting('contexts');
 
-        if (!is_array($contextsServices)) {
+        if (!is_array($contexts)) {
             throw new SuiteConfigurationException(sprintf(
                 '"contexts" setting of the "%s" suite is expected to be an array, %s given.',
                 $suite->getName(),
-                gettype($contextsServices)
+                gettype($contexts)
             ), $suite->getName());
         }
 
-        return $contextsServices;
+        return array_map([$this, 'normalizeContext'], $contexts);
+    }
+
+    private function cloneSuiteWithoutContexts(Suite $suite, array $contextsToRemove): Suite
+    {
+        $contexts = $suite->getSetting('contexts');
+
+        if (!is_array($contexts)) {
+            throw new SuiteConfigurationException(sprintf(
+                '"contexts" setting of the "%s" suite is expected to be an array, %s given.',
+                $suite->getName(),
+                gettype($contexts)
+            ), $suite->getName());
+        }
+
+        $contexts = array_filter($contexts, function ($context) use ($contextsToRemove): bool {
+            return !in_array($this->normalizeContext($context), $contextsToRemove, true);
+        });
+
+        return new GenericSuite($suite->getName(), array_merge($suite->getSettings(), ['contexts' => $contexts]));
+    }
+
+    private function normalizeContext($context): string
+    {
+        if (is_array($context)) {
+            return current(array_keys($context));
+        }
+
+        if (is_string($context)) {
+            return $context;
+        }
+
+        throw new \Exception();
     }
 
     /**
@@ -118,52 +166,6 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler
         }
     }
 
-    private function initializeInstance(Context $context): void
-    {
-        foreach ($this->contextInitializers as $initializer) {
-            $initializer->initializeContext($context);
-        }
-    }
-
-    private function getContextClass(string $contextId): string
-    {
-        if ($this->getContainer()->has($contextId)) {
-            return get_class($this->getContainer()->get($contextId));
-        }
-
-        $class = '\\' . ltrim($contextId, '\\');
-
-        if (class_exists($class)) {
-            return $class;
-        }
-
-        throw new \DomainException(sprintf('There is no service or class "%s".', $contextId));
-    }
-
-    private function getContext(string $contextId): Context
-    {
-        $class = '\\' . ltrim($contextId, '\\');
-
-        if ($this->getContainer()->has($contextId)) {
-            $context = $this->getContainer()->get($contextId);
-        } elseif (class_exists($class)) {
-            $context = new $class();
-        } else {
-            throw new \DomainException(sprintf('There is no service or class "%s".', $contextId));
-        }
-
-        if (!$context instanceof Context) {
-            throw new \DomainException(sprintf(
-                'Context "%s" referenced as "%s" needs to implement "%s".',
-                get_class($context),
-                $contextId,
-                Context::class
-            ));
-        }
-
-        return $context;
-    }
-
     private function getContainer(): ContainerInterface
     {
         try {

src/Context/Environment/InitializedSymfonyExtensionEnvironment.php

@@ -22,7 +22,7 @@ use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEn
 /**
  * @see ContextServiceEnvironmentHandler
  */
-final class InitialisedContextServiceEnvironment implements ContextServiceEnvironment
+final class InitializedSymfonyExtensionEnvironment implements SymfonyExtensionEnvironment
 {
     /** @var Suite */
     private $suite;

src/Context/Environment/SymfonyExtensionEnvironment.php

@@ -19,6 +19,6 @@ use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEn
 /**
  * @see ContextServiceEnvironmentHandler
  */
-interface ContextServiceEnvironment extends ContextEnvironment
+interface SymfonyExtensionEnvironment extends ContextEnvironment
 {
 }

src/Context/Environment/UninitialisedContextServiceEnvironment.php

@@ -1,51 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-/*
- * This file is part of the SymfonyExtension package.
- *
- * (c) Kamil Kokot <kamil@kokot.me>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace FriendsOfBehat\SymfonyExtension\Context\Environment;
-
-use Behat\Testwork\Environment\StaticEnvironment;
-use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEnvironmentHandler;
-
-/**
- * @see ContextServiceEnvironmentHandler
- */
-final class UninitialisedContextServiceEnvironment extends StaticEnvironment implements ContextServiceEnvironment
-{
-    /** @var string[] */
-    private $contextServices = [];
-
-    public function registerContextService(string $serviceId, string $serviceClass): void
-    {
-        $this->contextServices[$serviceId] = $serviceClass;
-    }
-
-    public function getContextServices(): array
-    {
-        return array_keys($this->contextServices);
-    }
-
-    public function hasContexts(): bool
-    {
-        return count($this->contextServices) > 0;
-    }
-
-    public function getContextClasses(): array
-    {
-        return array_values($this->contextServices);
-    }
-
-    public function hasContextClass($class): bool
-    {
-        return in_array($class, $this->contextServices, true);
-    }
-}

src/Context/Environment/UninitializedSymfonyExtensionEnvironment.php

@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the SymfonyExtension package.
+ *
+ * (c) Kamil Kokot <kamil@kokot.me>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FriendsOfBehat\SymfonyExtension\Context\Environment;
+
+use Behat\Behat\Context\Environment\ContextEnvironment;
+use Behat\Testwork\Environment\StaticEnvironment;
+use Behat\Testwork\Suite\Suite;
+use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEnvironmentHandler;
+
+/**
+ * @see ContextServiceEnvironmentHandler
+ */
+final class UninitializedSymfonyExtensionEnvironment extends StaticEnvironment implements SymfonyExtensionEnvironment
+{
+    /** @var string[] */
+    private $contexts;
+
+    /** @var ContextEnvironment|null */
+    private $delegatedEnvironment;
+
+    public function __construct(Suite $suite, array $contexts, ContextEnvironment $delegatedEnvironment)
+    {
+        parent::__construct($suite);
+
+        $this->contexts = $contexts;
+        $this->delegatedEnvironment = $delegatedEnvironment;
+    }
+
+    public function getServices(): array
+    {
+        return array_keys($this->contexts);
+    }
+
+    public function hasContexts(): bool
+    {
+        return count($this->contexts) > 0 || $this->delegatedEnvironment->hasContexts();
+    }
+
+    public function getContextClasses(): array
+    {
+        return array_merge(array_values($this->contexts), $this->delegatedEnvironment->getContextClasses());
+    }
+
+    public function hasContextClass($class): bool
+    {
+        return in_array($class, $this->contexts, true) || $this->delegatedEnvironment->hasContextClass($class);
+    }
+
+    public function getDelegatedEnvironment(): ContextEnvironment
+    {
+        return $this->delegatedEnvironment;
+    }
+}

src/ServiceContainer/SymfonyExtension.php

@@ -88,7 +88,6 @@ final class SymfonyExtension implements Extension
 
     public function process(ContainerBuilder $container): void
     {
-        $this->processEnvironmentHandler($container);
     }
 
     private function registerMinkDriver(ExtensionManager $extensionManager): void
@@ -137,6 +136,7 @@ final class SymfonyExtension implements Extension
     {
         $definition = new Definition(ContextServiceEnvironmentHandler::class, [
             new Reference(self::KERNEL_ID),
+            new Reference('environment.handler.context'),
         ]);
         $definition->addTag(EnvironmentExtension::HANDLER_TAG, ['priority' => 128]);
 
@@ -245,13 +245,4 @@ final class SymfonyExtension implements Extension
 
         return is_string($bootstrap) ? $bootstrap : null;
     }
-
-    private function processEnvironmentHandler(ContainerBuilder $container): void
-    {
-        $definition = $container->findDefinition('fob_symfony.environment_handler.context_service');
-
-        foreach ($container->findTaggedServiceIds(ContextExtension::INITIALIZER_TAG) as $serviceId => $tags) {
-            $definition->addMethodCall('registerContextInitializer', [$container->getDefinition($serviceId)]);
-        }
-    }
 }