From cd792704faaa05533a8375495b396128abe9e7cf Mon Sep 17 00:00:00 2001 From: Kamil Kokot Date: Tue, 12 Feb 2019 22:51:59 +0100 Subject: [PATCH 1/5] Add support for class resolvers --- composer.json | 1 + .../class_resolvers_compatibility.feature | 65 +++++++++++++++++++ .../ContextServiceEnvironmentHandler.php | 28 +++++++- src/ServiceContainer/SymfonyExtension.php | 4 ++ 4 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 features/sanity_checks/class_resolvers_compatibility.feature diff --git a/composer.json b/composer.json index 4157b20..8883805 100644 --- a/composer.json +++ b/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", diff --git a/features/sanity_checks/class_resolvers_compatibility.feature b/features/sanity_checks/class_resolvers_compatibility.feature new file mode 100644 index 0000000..761c8b8 --- /dev/null +++ b/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: + """ + symfonyKernel = $symfonyKernel; @@ -79,9 +83,14 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler return $environment; } - public function registerContextInitializer(ContextInitializer $initializer): void + public function registerContextInitializer(ContextInitializer $contextInitializer): void { - $this->contextInitializers[] = $initializer; + $this->contextInitializers[] = $contextInitializer; + } + + public function registerClassResolver(ClassResolver $classResolver): void + { + $this->classResolvers[] = $classResolver; } /** @@ -125,8 +134,21 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler } } + private function resolveContextId(string $contextId): string + { + foreach ($this->classResolvers as $resolver) { + if ($resolver->supportsClass($contextId)) { + return $resolver->resolveClass($contextId); + } + } + + return $contextId; + } + private function getContextClass(string $contextId): string { + $contextId = $this->resolveContextId($contextId); + if ($this->getContainer()->has($contextId)) { return get_class($this->getContainer()->get($contextId)); } @@ -142,6 +164,8 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler private function getContext(string $contextId): Context { + $contextId = $this->resolveContextId($contextId); + $class = '\\' . ltrim($contextId, '\\'); if ($this->getContainer()->has($contextId)) { diff --git a/src/ServiceContainer/SymfonyExtension.php b/src/ServiceContainer/SymfonyExtension.php index fe20b2e..7810a0c 100644 --- a/src/ServiceContainer/SymfonyExtension.php +++ b/src/ServiceContainer/SymfonyExtension.php @@ -253,5 +253,9 @@ final class SymfonyExtension implements Extension foreach ($container->findTaggedServiceIds(ContextExtension::INITIALIZER_TAG) as $serviceId => $tags) { $definition->addMethodCall('registerContextInitializer', [$container->getDefinition($serviceId)]); } + + foreach ($container->findTaggedServiceIds(ContextExtension::CLASS_RESOLVER_TAG) as $serviceId => $tags) { + $definition->addMethodCall('registerClassResolver', [$container->getDefinition($serviceId)]); + } } } From 9e9529c320bb9eec74ab5f91f367b948bab25e38 Mon Sep 17 00:00:00 2001 From: Kamil Kokot Date: Tue, 12 Feb 2019 22:56:16 +0100 Subject: [PATCH 2/5] Add tests for context initializers --- ...ction.feature => mink_integration.feature} | 2 +- ...tegration_with_context_initializer.feature | 78 ------------------- .../context_initializer_compatibility.feature | 71 +++++++++++++++++ 3 files changed, 72 insertions(+), 79 deletions(-) rename features/mink_integration/{mink_integration_with_dependency_injection.feature => mink_integration.feature} (98%) delete mode 100644 features/mink_integration/mink_integration_with_context_initializer.feature create mode 100644 features/sanity_checks/context_initializer_compatibility.feature diff --git a/features/mink_integration/mink_integration_with_dependency_injection.feature b/features/mink_integration/mink_integration.feature similarity index 98% rename from features/mink_integration/mink_integration_with_dependency_injection.feature rename to features/mink_integration/mink_integration.feature index 3318d94..fbe9290 100644 --- a/features/mink_integration/mink_integration_with_dependency_injection.feature +++ b/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 diff --git a/features/mink_integration/mink_integration_with_context_initializer.feature b/features/mink_integration/mink_integration_with_context_initializer.feature deleted file mode 100644 index e2264c6..0000000 --- a/features/mink_integration/mink_integration_with_context_initializer.feature +++ /dev/null @@ -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: - """ - 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 diff --git a/features/sanity_checks/context_initializer_compatibility.feature b/features/sanity_checks/context_initializer_compatibility.feature new file mode 100644 index 0000000..df545f2 --- /dev/null +++ b/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: + """ + makeItPass(true); + } + } + """ + And a feature file containing: + """ + Feature: + Scenario: + Then it should pass + """ + And a context file "tests/SomeContext.php" containing: + """ + shouldPass = $shouldPass; + } + + /** @Then it should pass */ + public function itShouldPass(): void + { + assert($this->shouldPass === true); + } + } + """ + When I run Behat + Then it should pass From d74cd251d504b500f1b3e2755f0265cf8326c705 Mon Sep 17 00:00:00 2001 From: Kamil Kokot Date: Tue, 12 Feb 2019 23:42:41 +0100 Subject: [PATCH 3/5] Add more sanity checks --- ...dependency_injection_compatibility.feature | 56 +++++++++++++++ .../ContextServiceEnvironmentHandler.php | 72 ++++++++++++------- ...UninitialisedContextServiceEnvironment.php | 20 ++++-- src/ServiceContainer/SymfonyExtension.php | 6 +- 4 files changed, 120 insertions(+), 34 deletions(-) create mode 100644 features/sanity_checks/context_constructor_dependency_injection_compatibility.feature diff --git a/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature b/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature new file mode 100644 index 0000000..8b6c005 --- /dev/null +++ b/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature @@ -0,0 +1,56 @@ +Feature: Context constructor dependency injection compatibility + + Scenario: Using context consturctor 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: + """ + foo = $foo; + } + + /** @Then it should pass */ + public function itShouldPass(): void + { + assert($this->foo instanceof Foo); + } + } + """ + When I run Behat + Then it should pass diff --git a/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php b/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php index 88130f9..d64c1f6 100644 --- a/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php +++ b/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php @@ -13,9 +13,13 @@ declare(strict_types=1); namespace FriendsOfBehat\SymfonyExtension\Context\Environment\Handler; +use Behat\Behat\Context\Argument\ArgumentResolverFactory; +use Behat\Behat\Context\Argument\NullFactory; +use Behat\Behat\Context\Argument\SuiteScopedResolverFactory; +use Behat\Behat\Context\Argument\SuiteScopedResolverFactoryAdapter; use Behat\Behat\Context\Context; use Behat\Behat\Context\ContextClass\ClassResolver; -use Behat\Behat\Context\Initializer\ContextInitializer; +use Behat\Behat\Context\ContextFactory; use Behat\Testwork\Environment\Environment; use Behat\Testwork\Environment\Exception\EnvironmentIsolationException; use Behat\Testwork\Environment\Handler\EnvironmentHandler; @@ -32,15 +36,29 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler /** @var KernelInterface */ private $symfonyKernel; - /** @var ContextInitializer[] */ - private $contextInitializers = []; - /** @var ClassResolver[] */ private $classResolvers = []; - public function __construct(KernelInterface $symfonyKernel) + /** @var ContextFactory */ + private $contextFactory; + + /** @var ArgumentResolverFactory */ + private $resolverFactory; + + /** + * @param ArgumentResolverFactory|SuiteScopedResolverFactory $resolverFactory + */ + public function __construct(KernelInterface $symfonyKernel, ContextFactory $factory, $resolverFactory = null) { $this->symfonyKernel = $symfonyKernel; + + $this->contextFactory = $factory; + + if ($resolverFactory && !$resolverFactory instanceof ArgumentResolverFactory) { + $resolverFactory = new SuiteScopedResolverFactoryAdapter($resolverFactory); + } + + $this->resolverFactory = $resolverFactory ?: new NullFactory(); } public function supportsSuite(Suite $suite): bool @@ -51,8 +69,8 @@ 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)); + foreach ($this->getSuiteContextsServices($suite) as [$contextId, $contextArguments]) { + $environment->registerContextService($contextId, $this->getContextClass($contextId), $contextArguments); } return $environment; @@ -73,28 +91,24 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler $this->assertEnvironmentCanBeIsolated($uninitializedEnvironment, $testSubject); $environment = new InitialisedContextServiceEnvironment($uninitializedEnvironment->getSuite()); - foreach ($uninitializedEnvironment->getContextServices() as $contextId) { + $resolvers = $this->resolverFactory->createArgumentResolvers($environment); + + foreach ($uninitializedEnvironment->getContextServicesWithArguments() as $contextId => $arguments) { /** @var Context $context */ - $context = $this->getContext($contextId); - $this->initializeInstance($context); + $context = $this->getContext($contextId, $arguments, $resolvers); $environment->registerContext($context); } return $environment; } - public function registerContextInitializer(ContextInitializer $contextInitializer): void - { - $this->contextInitializers[] = $contextInitializer; - } - public function registerClassResolver(ClassResolver $classResolver): void { $this->classResolvers[] = $classResolver; } /** - * @return string[] + * @return array[] * * @throws SuiteConfigurationException If "contexts" setting is not an array */ @@ -110,7 +124,20 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler ), $suite->getName()); } - return $contextsServices; + return array_map( + function ($context): array { + $class = $context; + $arguments = []; + + if (is_array($context)) { + $class = current(array_keys($context)); + $arguments = $context[$class]; + } + + return [$class, $arguments]; + }, + $contextsServices + ); } /** @@ -127,13 +154,6 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler } } - private function initializeInstance(Context $context): void - { - foreach ($this->contextInitializers as $initializer) { - $initializer->initializeContext($context); - } - } - private function resolveContextId(string $contextId): string { foreach ($this->classResolvers as $resolver) { @@ -162,7 +182,7 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler throw new \DomainException(sprintf('There is no service or class "%s".', $contextId)); } - private function getContext(string $contextId): Context + private function getContext(string $contextId, array $arguments = [], array $resolvers = []): Context { $contextId = $this->resolveContextId($contextId); @@ -171,7 +191,7 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler if ($this->getContainer()->has($contextId)) { $context = $this->getContainer()->get($contextId); } elseif (class_exists($class)) { - $context = new $class(); + $context = $this->contextFactory->createContext($class, $arguments, $resolvers); } else { throw new \DomainException(sprintf('There is no service or class "%s".', $contextId)); } diff --git a/src/Context/Environment/UninitialisedContextServiceEnvironment.php b/src/Context/Environment/UninitialisedContextServiceEnvironment.php index bbd6211..abc8a4a 100644 --- a/src/Context/Environment/UninitialisedContextServiceEnvironment.php +++ b/src/Context/Environment/UninitialisedContextServiceEnvironment.php @@ -24,9 +24,12 @@ final class UninitialisedContextServiceEnvironment extends StaticEnvironment imp /** @var string[] */ private $contextServices = []; - public function registerContextService(string $serviceId, string $serviceClass): void + public function registerContextService(string $serviceId, string $serviceClass, array $arguments = []): void { - $this->contextServices[$serviceId] = $serviceClass; + $this->contextServices[$serviceId] = [ + 'class' => $serviceClass, + 'arguments' => $arguments, + ]; } public function getContextServices(): array @@ -41,11 +44,20 @@ final class UninitialisedContextServiceEnvironment extends StaticEnvironment imp public function getContextClasses(): array { - return array_values($this->contextServices); + return array_map(function (array $contextDetails): string { + return $contextDetails['class']; + }, $this->contextServices); } public function hasContextClass($class): bool { - return in_array($class, $this->contextServices, true); + return in_array($class, $this->getContextClasses(), true); + } + + public function getContextServicesWithArguments(): iterable + { + foreach ($this->contextServices as $contextDetails) { + yield $contextDetails['class'] => $contextDetails['arguments']; + } } } diff --git a/src/ServiceContainer/SymfonyExtension.php b/src/ServiceContainer/SymfonyExtension.php index 7810a0c..bf99aa7 100644 --- a/src/ServiceContainer/SymfonyExtension.php +++ b/src/ServiceContainer/SymfonyExtension.php @@ -137,6 +137,8 @@ final class SymfonyExtension implements Extension { $definition = new Definition(ContextServiceEnvironmentHandler::class, [ new Reference(self::KERNEL_ID), + new Reference(ContextExtension::FACTORY_ID), + new Reference(ContextExtension::AGGREGATE_RESOLVER_FACTORY_ID) ]); $definition->addTag(EnvironmentExtension::HANDLER_TAG, ['priority' => 128]); @@ -250,10 +252,6 @@ final class SymfonyExtension implements Extension { $definition = $container->findDefinition('fob_symfony.environment_handler.context_service'); - foreach ($container->findTaggedServiceIds(ContextExtension::INITIALIZER_TAG) as $serviceId => $tags) { - $definition->addMethodCall('registerContextInitializer', [$container->getDefinition($serviceId)]); - } - foreach ($container->findTaggedServiceIds(ContextExtension::CLASS_RESOLVER_TAG) as $serviceId => $tags) { $definition->addMethodCall('registerClassResolver', [$container->getDefinition($serviceId)]); } From c54c581e74355b24e06134a15e360cce69112658 Mon Sep 17 00:00:00 2001 From: Kamil Kokot Date: Wed, 13 Feb 2019 00:38:07 +0100 Subject: [PATCH 4/5] Refactor our environment handler to decorate the original one --- phpstan.neon | 2 +- .../ContextServiceEnvironmentHandler.php | 190 +++++++----------- ...nitialisedSymfonyExtensionEnvironment.php} | 2 +- ...nt.php => SymfonyExtensionEnvironment.php} | 2 +- ...UninitialisedContextServiceEnvironment.php | 63 ------ ...initialisedSymfonyExtensionEnvironment.php | 64 ++++++ src/ServiceContainer/SymfonyExtension.php | 13 +- 7 files changed, 145 insertions(+), 191 deletions(-) rename src/Context/Environment/{InitialisedContextServiceEnvironment.php => InitialisedSymfonyExtensionEnvironment.php} (95%) rename src/Context/Environment/{ContextServiceEnvironment.php => SymfonyExtensionEnvironment.php} (89%) delete mode 100644 src/Context/Environment/UninitialisedContextServiceEnvironment.php create mode 100644 src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php diff --git a/phpstan.neon b/phpstan.neon index 5894523..e1d4afb 100644 --- a/phpstan.neon +++ b/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\\InitialisedSymfonyExtensionEnvironment::bindCallee\(\) should return callable/' - '/Strict comparison using === between 0\|1 and 2 will always evaluate to false\./' diff --git a/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php b/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php index d64c1f6..887d264 100644 --- a/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php +++ b/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php @@ -13,21 +13,18 @@ declare(strict_types=1); namespace FriendsOfBehat\SymfonyExtension\Context\Environment\Handler; -use Behat\Behat\Context\Argument\ArgumentResolverFactory; -use Behat\Behat\Context\Argument\NullFactory; -use Behat\Behat\Context\Argument\SuiteScopedResolverFactory; -use Behat\Behat\Context\Argument\SuiteScopedResolverFactoryAdapter; use Behat\Behat\Context\Context; -use Behat\Behat\Context\ContextClass\ClassResolver; -use Behat\Behat\Context\ContextFactory; +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\InitialisedSymfonyExtensionEnvironment; +use FriendsOfBehat\SymfonyExtension\Context\Environment\UninitialisedSymfonyExtensionEnvironment; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpKernel\KernelInterface; @@ -36,29 +33,13 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler /** @var KernelInterface */ private $symfonyKernel; - /** @var ClassResolver[] */ - private $classResolvers = []; + /** @var EnvironmentHandler */ + private $decoratedEnvironmentHandler; - /** @var ContextFactory */ - private $contextFactory; - - /** @var ArgumentResolverFactory */ - private $resolverFactory; - - /** - * @param ArgumentResolverFactory|SuiteScopedResolverFactory $resolverFactory - */ - public function __construct(KernelInterface $symfonyKernel, ContextFactory $factory, $resolverFactory = null) + public function __construct(KernelInterface $symfonyKernel, EnvironmentHandler $decoratedEnvironmentHandler) { $this->symfonyKernel = $symfonyKernel; - - $this->contextFactory = $factory; - - if ($resolverFactory && !$resolverFactory instanceof ArgumentResolverFactory) { - $resolverFactory = new SuiteScopedResolverFactoryAdapter($resolverFactory); - } - - $this->resolverFactory = $resolverFactory ?: new NullFactory(); + $this->decoratedEnvironmentHandler = $decoratedEnvironmentHandler; } public function supportsSuite(Suite $suite): bool @@ -68,21 +49,34 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler public function buildEnvironment(Suite $suite): Environment { - $environment = new UninitialisedContextServiceEnvironment($suite); - foreach ($this->getSuiteContextsServices($suite) as [$contextId, $contextArguments]) { - $environment->registerContextService($contextId, $this->getContextClass($contextId), $contextArguments); + $symfonyContexts = []; + + foreach ($this->getSuiteContextsServices($suite) as $serviceId) { + if (!$this->getContainer()->has($serviceId)) { + continue; + } + + $symfonyContexts[$serviceId] = get_class($this->getContainer()->get($serviceId)); } - return $environment; + $delegatedSuite = $this->cloneSuiteWithoutContexts($suite, array_keys($symfonyContexts)); + + $delegatedEnvironment = $this->decoratedEnvironmentHandler->buildEnvironment($delegatedSuite); + + if (!$delegatedEnvironment instanceof ContextEnvironment) { + throw new \Exception(); + } + + return new UninitialisedSymfonyExtensionEnvironment($suite, $symfonyContexts, $delegatedEnvironment); } public function supportsEnvironmentAndSubject(Environment $environment, $testSubject = null): bool { - return $environment instanceof UninitialisedContextServiceEnvironment; + return $environment instanceof UninitialisedSymfonyExtensionEnvironment; } /** - * @param UninitialisedContextServiceEnvironment $uninitializedEnvironment + * @param UninitialisedSymfonyExtensionEnvironment $uninitializedEnvironment * * @throws EnvironmentIsolationException */ @@ -90,54 +84,78 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler { $this->assertEnvironmentCanBeIsolated($uninitializedEnvironment, $testSubject); - $environment = new InitialisedContextServiceEnvironment($uninitializedEnvironment->getSuite()); - $resolvers = $this->resolverFactory->createArgumentResolvers($environment); + $environment = new InitialisedSymfonyExtensionEnvironment($uninitializedEnvironment->getSuite()); - foreach ($uninitializedEnvironment->getContextServicesWithArguments() as $contextId => $arguments) { + foreach ($uninitializedEnvironment->getServices() as $serviceId) { /** @var Context $context */ - $context = $this->getContext($contextId, $arguments, $resolvers); + $context = $this->getContainer()->get($serviceId); + + $environment->registerContext($context); + } + + $delegatedEnvironment = $this->decoratedEnvironmentHandler->isolateEnvironment($uninitializedEnvironment->getDelegatedEnvironment()); + + if (!$delegatedEnvironment instanceof InitializedContextEnvironment) { + throw new \Exception(); + } + + foreach ($delegatedEnvironment->getContexts() as $context) { $environment->registerContext($context); } return $environment; } - public function registerClassResolver(ClassResolver $classResolver): void - { - $this->classResolvers[] = $classResolver; - } - /** - * @return array[] + * @return string[] * * @throws SuiteConfigurationException If "contexts" setting is not an array */ 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 array_map( - function ($context): array { - $class = $context; - $arguments = []; + return array_map([$this, 'normaliseContext'], $contexts); + } - if (is_array($context)) { - $class = current(array_keys($context)); - $arguments = $context[$class]; - } + private function cloneSuiteWithoutContexts(Suite $suite, array $contextsToRemove): Suite + { + $contexts = $suite->getSetting('contexts'); - return [$class, $arguments]; - }, - $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($contexts) + ), $suite->getName()); + } + + $contexts = array_filter($contexts, function ($context) use ($contextsToRemove): bool { + return !in_array($this->normaliseContext($context), $contextsToRemove, true); + }); + + return new GenericSuite($suite->getName(), array_merge($suite->getSettings(), ['contexts' => $contexts])); + } + + private function normaliseContext($context): string + { + if (is_array($context)) { + return current(array_keys($context)); + } + + if (is_string($context)) { + return $context; + } + + throw new \Exception(); } /** @@ -154,60 +172,6 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler } } - private function resolveContextId(string $contextId): string - { - foreach ($this->classResolvers as $resolver) { - if ($resolver->supportsClass($contextId)) { - return $resolver->resolveClass($contextId); - } - } - - return $contextId; - } - - private function getContextClass(string $contextId): string - { - $contextId = $this->resolveContextId($contextId); - - 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, array $arguments = [], array $resolvers = []): Context - { - $contextId = $this->resolveContextId($contextId); - - $class = '\\' . ltrim($contextId, '\\'); - - if ($this->getContainer()->has($contextId)) { - $context = $this->getContainer()->get($contextId); - } elseif (class_exists($class)) { - $context = $this->contextFactory->createContext($class, $arguments, $resolvers); - } 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 { diff --git a/src/Context/Environment/InitialisedContextServiceEnvironment.php b/src/Context/Environment/InitialisedSymfonyExtensionEnvironment.php similarity index 95% rename from src/Context/Environment/InitialisedContextServiceEnvironment.php rename to src/Context/Environment/InitialisedSymfonyExtensionEnvironment.php index d8c1904..609bbe4 100644 --- a/src/Context/Environment/InitialisedContextServiceEnvironment.php +++ b/src/Context/Environment/InitialisedSymfonyExtensionEnvironment.php @@ -22,7 +22,7 @@ use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEn /** * @see ContextServiceEnvironmentHandler */ -final class InitialisedContextServiceEnvironment implements ContextServiceEnvironment +final class InitialisedSymfonyExtensionEnvironment implements SymfonyExtensionEnvironment { /** @var Suite */ private $suite; diff --git a/src/Context/Environment/ContextServiceEnvironment.php b/src/Context/Environment/SymfonyExtensionEnvironment.php similarity index 89% rename from src/Context/Environment/ContextServiceEnvironment.php rename to src/Context/Environment/SymfonyExtensionEnvironment.php index ebf7b95..b75b232 100644 --- a/src/Context/Environment/ContextServiceEnvironment.php +++ b/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 { } diff --git a/src/Context/Environment/UninitialisedContextServiceEnvironment.php b/src/Context/Environment/UninitialisedContextServiceEnvironment.php deleted file mode 100644 index abc8a4a..0000000 --- a/src/Context/Environment/UninitialisedContextServiceEnvironment.php +++ /dev/null @@ -1,63 +0,0 @@ - - * - * 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, array $arguments = []): void - { - $this->contextServices[$serviceId] = [ - 'class' => $serviceClass, - 'arguments' => $arguments, - ]; - } - - public function getContextServices(): array - { - return array_keys($this->contextServices); - } - - public function hasContexts(): bool - { - return count($this->contextServices) > 0; - } - - public function getContextClasses(): array - { - return array_map(function (array $contextDetails): string { - return $contextDetails['class']; - }, $this->contextServices); - } - - public function hasContextClass($class): bool - { - return in_array($class, $this->getContextClasses(), true); - } - - public function getContextServicesWithArguments(): iterable - { - foreach ($this->contextServices as $contextDetails) { - yield $contextDetails['class'] => $contextDetails['arguments']; - } - } -} diff --git a/src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php b/src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php new file mode 100644 index 0000000..dbdebb5 --- /dev/null +++ b/src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php @@ -0,0 +1,64 @@ + + * + * 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 UninitialisedSymfonyExtensionEnvironment 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; + } +} diff --git a/src/ServiceContainer/SymfonyExtension.php b/src/ServiceContainer/SymfonyExtension.php index bf99aa7..fdcafd5 100644 --- a/src/ServiceContainer/SymfonyExtension.php +++ b/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,8 +136,7 @@ final class SymfonyExtension implements Extension { $definition = new Definition(ContextServiceEnvironmentHandler::class, [ new Reference(self::KERNEL_ID), - new Reference(ContextExtension::FACTORY_ID), - new Reference(ContextExtension::AGGREGATE_RESOLVER_FACTORY_ID) + new Reference('environment.handler.context'), ]); $definition->addTag(EnvironmentExtension::HANDLER_TAG, ['priority' => 128]); @@ -247,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::CLASS_RESOLVER_TAG) as $serviceId => $tags) { - $definition->addMethodCall('registerClassResolver', [$container->getDefinition($serviceId)]); - } - } } From 8a972b5b48accd6a9a1580d874f5c2f3e573160f Mon Sep 17 00:00:00 2001 From: Alan Poulain Date: Wed, 13 Feb 2019 11:35:22 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-Authored-By: pamil --- ...dependency_injection_compatibility.feature | 2 +- phpstan.neon | 2 +- .../ContextServiceEnvironmentHandler.php | 28 ++++++++----------- ...nitializedSymfonyExtensionEnvironment.php} | 2 +- ...nitializedSymfonyExtensionEnvironment.php} | 2 +- 5 files changed, 15 insertions(+), 21 deletions(-) rename src/Context/Environment/{InitialisedSymfonyExtensionEnvironment.php => InitializedSymfonyExtensionEnvironment.php} (96%) rename src/Context/Environment/{UninitialisedSymfonyExtensionEnvironment.php => UninitializedSymfonyExtensionEnvironment.php} (96%) diff --git a/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature b/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature index 8b6c005..6069c68 100644 --- a/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature +++ b/features/sanity_checks/context_constructor_dependency_injection_compatibility.feature @@ -1,6 +1,6 @@ Feature: Context constructor dependency injection compatibility - Scenario: Using context consturctor dependency injection + Scenario: Using context constructor dependency injection Given a working Symfony application with SymfonyExtension configured And a Behat configuration containing: """ diff --git a/phpstan.neon b/phpstan.neon index e1d4afb..64c7cba 100644 --- a/phpstan.neon +++ b/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\\InitialisedSymfonyExtensionEnvironment::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\./' diff --git a/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php b/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php index 887d264..dca3f34 100644 --- a/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php +++ b/src/Context/Environment/Handler/ContextServiceEnvironmentHandler.php @@ -23,8 +23,8 @@ 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\InitialisedSymfonyExtensionEnvironment; -use FriendsOfBehat\SymfonyExtension\Context\Environment\UninitialisedSymfonyExtensionEnvironment; +use FriendsOfBehat\SymfonyExtension\Context\Environment\InitializedSymfonyExtensionEnvironment; +use FriendsOfBehat\SymfonyExtension\Context\Environment\UninitializedSymfonyExtensionEnvironment; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpKernel\KernelInterface; @@ -61,22 +61,19 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler $delegatedSuite = $this->cloneSuiteWithoutContexts($suite, array_keys($symfonyContexts)); + /** @var ContextEnvironment $delegatedEnvironment */ $delegatedEnvironment = $this->decoratedEnvironmentHandler->buildEnvironment($delegatedSuite); - if (!$delegatedEnvironment instanceof ContextEnvironment) { - throw new \Exception(); - } - - return new UninitialisedSymfonyExtensionEnvironment($suite, $symfonyContexts, $delegatedEnvironment); + return new UninitializedSymfonyExtensionEnvironment($suite, $symfonyContexts, $delegatedEnvironment); } public function supportsEnvironmentAndSubject(Environment $environment, $testSubject = null): bool { - return $environment instanceof UninitialisedSymfonyExtensionEnvironment; + return $environment instanceof UninitializedSymfonyExtensionEnvironment; } /** - * @param UninitialisedSymfonyExtensionEnvironment $uninitializedEnvironment + * @param UninitializedSymfonyExtensionEnvironment $uninitializedEnvironment * * @throws EnvironmentIsolationException */ @@ -84,7 +81,7 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler { $this->assertEnvironmentCanBeIsolated($uninitializedEnvironment, $testSubject); - $environment = new InitialisedSymfonyExtensionEnvironment($uninitializedEnvironment->getSuite()); + $environment = new InitializedSymfonyExtensionEnvironment($uninitializedEnvironment->getSuite()); foreach ($uninitializedEnvironment->getServices() as $serviceId) { /** @var Context $context */ @@ -93,12 +90,9 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler $environment->registerContext($context); } + /** @var InitializedContextEnvironment $delegatedEnvironment */ $delegatedEnvironment = $this->decoratedEnvironmentHandler->isolateEnvironment($uninitializedEnvironment->getDelegatedEnvironment()); - if (!$delegatedEnvironment instanceof InitializedContextEnvironment) { - throw new \Exception(); - } - foreach ($delegatedEnvironment->getContexts() as $context) { $environment->registerContext($context); } @@ -123,7 +117,7 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler ), $suite->getName()); } - return array_map([$this, 'normaliseContext'], $contexts); + return array_map([$this, 'normalizeContext'], $contexts); } private function cloneSuiteWithoutContexts(Suite $suite, array $contextsToRemove): Suite @@ -139,13 +133,13 @@ final class ContextServiceEnvironmentHandler implements EnvironmentHandler } $contexts = array_filter($contexts, function ($context) use ($contextsToRemove): bool { - return !in_array($this->normaliseContext($context), $contextsToRemove, true); + return !in_array($this->normalizeContext($context), $contextsToRemove, true); }); return new GenericSuite($suite->getName(), array_merge($suite->getSettings(), ['contexts' => $contexts])); } - private function normaliseContext($context): string + private function normalizeContext($context): string { if (is_array($context)) { return current(array_keys($context)); diff --git a/src/Context/Environment/InitialisedSymfonyExtensionEnvironment.php b/src/Context/Environment/InitializedSymfonyExtensionEnvironment.php similarity index 96% rename from src/Context/Environment/InitialisedSymfonyExtensionEnvironment.php rename to src/Context/Environment/InitializedSymfonyExtensionEnvironment.php index 609bbe4..b07da6e 100644 --- a/src/Context/Environment/InitialisedSymfonyExtensionEnvironment.php +++ b/src/Context/Environment/InitializedSymfonyExtensionEnvironment.php @@ -22,7 +22,7 @@ use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEn /** * @see ContextServiceEnvironmentHandler */ -final class InitialisedSymfonyExtensionEnvironment implements SymfonyExtensionEnvironment +final class InitializedSymfonyExtensionEnvironment implements SymfonyExtensionEnvironment { /** @var Suite */ private $suite; diff --git a/src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php b/src/Context/Environment/UninitializedSymfonyExtensionEnvironment.php similarity index 96% rename from src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php rename to src/Context/Environment/UninitializedSymfonyExtensionEnvironment.php index dbdebb5..f3d0626 100644 --- a/src/Context/Environment/UninitialisedSymfonyExtensionEnvironment.php +++ b/src/Context/Environment/UninitializedSymfonyExtensionEnvironment.php @@ -21,7 +21,7 @@ use FriendsOfBehat\SymfonyExtension\Context\Environment\Handler\ContextServiceEn /** * @see ContextServiceEnvironmentHandler */ -final class UninitialisedSymfonyExtensionEnvironment extends StaticEnvironment implements SymfonyExtensionEnvironment +final class UninitializedSymfonyExtensionEnvironment extends StaticEnvironment implements SymfonyExtensionEnvironment { /** @var string[] */ private $contexts;