From f1971fde57ee5b21dc8dbf24a121340c49622bda Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 24 Nov 2022 12:22:48 +0000 Subject: [PATCH] * Reset also `fob_symfony.driver_kernel` between scenarios * Make sure reset() for the Mink driver implementation creates new KernelBrowser instances, to achieve consistent reboots of the `fob_symfony.driver_kernel` when making more than one request within a single scenario --- .../resetting_service_container_state.feature | 198 ++++++++++++++++++ src/Driver/SymfonyDriver.php | 59 +++++- src/Listener/KernelOrchestrator.php | 23 +- src/ServiceContainer/SymfonyExtension.php | 2 +- tests/Behat/Context/TestContext.php | 2 +- 5 files changed, 270 insertions(+), 14 deletions(-) create mode 100644 features/mink_integration/resetting_service_container_state.feature diff --git a/features/mink_integration/resetting_service_container_state.feature b/features/mink_integration/resetting_service_container_state.feature new file mode 100644 index 0000000..fb2473c --- /dev/null +++ b/features/mink_integration/resetting_service_container_state.feature @@ -0,0 +1,198 @@ +Feature: Resetting the driver's service container in the right places + + Background: + 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 YAML services file containing: + """ + services: + _defaults: + autowire: true + autoconfigure: true + + App\Tests\SomeContext: ~ + """ + And a context file "tests/SomeContext.php" containing: + """ + mink = $mink; + $this->driverContainer = $driverContainer; + } + + /** @Given the counter service is zeroed */ + public function counterServiceIsZeroed(): void + { + assert(0 === $this->getCounterService()->get()); + } + + /** @When I visit the page :page */ + public function visitPage(string $page): void + { + $this->mink->getSession()->visit($page); + } + + /** @When I increment the counter */ + public function incrementCounter(): void + { + $this->getCounterService()->increase(); + } + + /** @Then the counter service should return :number */ + public function counterServiceShouldReturn(int $number): void + { + assert($number === $this->getCounterService()->get()); + } + + /** @Then I should see :content on the page */ + public function shouldSeeContentOnPage(string $content): void + { + assert(false !== strpos($this->mink->getSession()->getPage()->getContent(), $content)); + } + + private function getCounterService(): Counter + { + return $this->driverContainer->get('App\Counter'); + } + } + """ + + Scenario: Driver's service container is reset between scenarios + # Regression testing https://github.com/FriendsOfBehat/SymfonyExtension/issues/149 + Given a feature file containing: + """ + Feature: + Scenario: First pass + Given the counter service is zeroed + When I increment the counter + Then the counter service should return 1 + Scenario: Second pass + Given the counter service is zeroed + """ + When I run Behat + Then it should pass + + Scenario: Driver's service container is reset between requests + Given a feature file containing: + """ + Feature: + Scenario: + Given the counter service is zeroed + When I visit the page "/hello-world" + Then I should see "The counter value is 1" on the page + When I visit the page "/hello-world" + Then I should see "The counter value is 1" on the page + """ + When I run Behat + Then it should pass + + Scenario: Driver's service container can be prepared before a request is made + Given a feature file containing: + """ + Feature: + Scenario: + Given the counter service is zeroed + And I increment the counter + When I visit the page "/hello-world" + Then I should see "The counter value is 2" on the page + """ + When I run Behat + Then it should pass + + Scenario: Driver's service container is not reset before a request is made, even when another scenario made a request before + # Regression testing https://github.com/FriendsOfBehat/SymfonyExtension/issues/149 + Given a feature file containing: + """ + Feature: + Scenario: + Given the counter service is zeroed + And I increment the counter + When I visit the page "/hello-world" + Then I should see "The counter value is 2" on the page + Scenario: + Given the counter service is zeroed + And I increment the counter + When I visit the page "/hello-world" + Then I should see "The counter value is 2" on the page + """ + When I run Behat + Then it should pass + + Scenario: Driver's service container can be inspected after a request has been made + Given a feature file containing: + """ + Feature: + Scenario: + Given the counter service is zeroed + When I visit the page "/hello-world" + Then the counter service should return 1 + """ + When I run Behat + Then it should pass + + Scenario: When multiple requests are made, the driver's service container is reset, but we can inspect "in between" states + Given a feature file containing: + """ + Feature: + Scenario: + Given the counter service is zeroed + When I visit the page "/hello-world" + Then the counter service should return 1 + # This will reset the driver's container, so we will see "1" again + When I visit the page "/hello-world" + Then the counter service should return 1 + """ + When I run Behat + Then it should pass + + Scenario: Driver's service container can not reasonably be modified between requests + # This is not really a feature, but rather documenting current behavior (judge yourself). + # One way around it might be to (how?) disable the reboot feature on the KernelBrowser and + # take responsibility for resetting the driver's container yourself. + Given a feature file containing: + """ + Feature: + Scenario: + Given the counter service is zeroed + And I increment the counter + When I visit the page "/hello-world" + Then I should see "The counter value is 2" on the page + And the counter service should return 2 + # Now a second request is made, which will reset the container, but leaves us + # with no easy way of pre-setting the container once again: + When I increment the counter + # ... you might expect "3" + And I visit the page "/hello-world" + # ... now you might expect "4". But, in fact, the reset happened just before the request. + Then I should see "The counter value is 1" on the page + And the counter service should return 1 + """ + When I run Behat + Then it should pass diff --git a/src/Driver/SymfonyDriver.php b/src/Driver/SymfonyDriver.php index 2621c10..77adad9 100644 --- a/src/Driver/SymfonyDriver.php +++ b/src/Driver/SymfonyDriver.php @@ -6,35 +6,72 @@ namespace FriendsOfBehat\SymfonyExtension\Driver; use Behat\Mink\Driver\BrowserKitDriver; use Symfony\Component\BrowserKit\AbstractBrowser; -use Symfony\Component\BrowserKit\Client; use Symfony\Component\HttpKernel\KernelInterface; final class SymfonyDriver extends BrowserKitDriver { + /** @var KernelInterface */ + private $kernel; + + /** @var string|null */ + private $baseUrl; + public function __construct(KernelInterface $kernel, ?string $baseUrl) { - if (!$kernel->getContainer()->has('test.client')) { + $this->kernel = $kernel; + $this->baseUrl = $baseUrl; + + if (!$this->kernel->getContainer()->has('test.client')) { throw new \RuntimeException(sprintf( 'Kernel "%s" used by Behat with "%s" environment and debug %s does not have "test.client" service. ' . "\n" . 'Please make sure the kernel is using "test" environment or have "framework.test" configuration option enabled.', - get_class($kernel), - $kernel->getEnvironment(), - $kernel->isDebug() ? 'enabled' : 'disabled', + get_class($this->kernel), + $this->kernel->getEnvironment(), + $this->kernel->isDebug() ? 'enabled' : 'disabled', )); } - /** @var object $testClient */ - $testClient = $kernel->getContainer()->get('test.client'); + parent::__construct($this->createBrowser(), $this->baseUrl); + } - if (!$testClient instanceof Client && !$testClient instanceof AbstractBrowser) { + public function reset() + { + parent::reset(); + + /* + * When \Behat\Mink\Driver\DriverInterface::visit() is called on this driver here, + * we ultimately end up in \Symfony\Bundle\FrameworkBundle\KernelBrowser::doRequest(). + * That method tracks state across multiple requests to detect whether it is necessary + * to reboot the targeted-at kernel before performing the next request. + * + * We do not want this state to leak between Behat scenarios, and so this method here + * seems to be a good place to reset driver state as well. + * + * Since there is no other way to reset the KernelBrowser, we create a new instance. + * + * This also makes sense for another reason: The $kernel instance is rebooted by the + * KernelOrchestrator between Behat scenarios. So, every time we reset the driver + * (which happens at least for the first request during a scenario) we want to make + * sure we are using a KernelBrowser instance created in the currently active + * kernel "state" ("epoch"? "generation"?) + */ + + parent::__construct($this->createBrowser(), $this->baseUrl); + } + + private function createBrowser(): AbstractBrowser + { + /** @var object $testClient */ + $testClient = $this->kernel->getContainer()->get('test.client'); + + if (!$testClient instanceof AbstractBrowser) { throw new \RuntimeException(sprintf( - 'Service "test.client" should be an instance of "%s" or "%s", "%s" given.', - Client::class, + 'Service "test.client" should be an instance of "%s", "%s" given.', AbstractBrowser::class, get_class($testClient), )); } - parent::__construct($testClient, $baseUrl); + return $testClient; } } diff --git a/src/Listener/KernelOrchestrator.php b/src/Listener/KernelOrchestrator.php index 2fad7d7..6daaaa5 100644 --- a/src/Listener/KernelOrchestrator.php +++ b/src/Listener/KernelOrchestrator.php @@ -15,12 +15,16 @@ final class KernelOrchestrator implements EventSubscriberInterface /** @var KernelInterface */ private $symfonyKernel; + /** @var KernelInterface */ + private $driverKernel; + /** @var ContainerInterface */ private $behatContainer; - public function __construct(KernelInterface $symfonyKernel, ContainerInterface $behatContainer) + public function __construct(KernelInterface $symfonyKernel, KernelInterface $driverKernel, ContainerInterface $behatContainer) { $this->symfonyKernel = $symfonyKernel; + $this->driverKernel = $driverKernel; $this->behatContainer = $behatContainer; } @@ -42,8 +46,25 @@ final class KernelOrchestrator implements EventSubscriberInterface public function tearDown(): void { + $this->driverKernel->shutdown(); + + /* + * Reset both Kernel instances after a scenario has been run: The Kernel (and thus Container) + * used in Behat to configure Contexts; and the Kernel used by the SymfonyDriver to which + * requests are dispatched (through Mink). + * + * Since the "symfony" container is needed in a few other places (where and why exactly?) and + * has to be in a booted/usable state most of the time, we do not shut it down here in tearDown() + * and boot it in setUp(). + * + * Instead, the definitions in \FriendsOfBehat\SymfonyExtension\ServiceContainer\SymfonyExtension + * make sure both kernels are booted immediately after being created, and we also initiate the + * re-boot() here right away. + */ $this->symfonyKernel->getContainer()->set('behat.service_container', null); $this->symfonyKernel->shutdown(); $this->symfonyKernel->boot(); + + $this->driverKernel->boot(); } } diff --git a/src/ServiceContainer/SymfonyExtension.php b/src/ServiceContainer/SymfonyExtension.php index 2000800..5b8258e 100644 --- a/src/ServiceContainer/SymfonyExtension.php +++ b/src/ServiceContainer/SymfonyExtension.php @@ -134,7 +134,7 @@ final class SymfonyExtension implements Extension private function loadKernelRebooter(ContainerBuilder $container): void { - $definition = new Definition(KernelOrchestrator::class, [new Reference(self::KERNEL_ID), $container]); + $definition = new Definition(KernelOrchestrator::class, [new Reference(self::KERNEL_ID), new Reference(self::DRIVER_KERNEL_ID), $container]); $definition->addTag(EventDispatcherExtension::SUBSCRIBER_TAG); $container->setDefinition('fob_symfony.kernel_orchestrator', $definition); diff --git a/tests/Behat/Context/TestContext.php b/tests/Behat/Context/TestContext.php index 4f72975..1a1234d 100644 --- a/tests/Behat/Context/TestContext.php +++ b/tests/Behat/Context/TestContext.php @@ -167,7 +167,7 @@ final class Controller { $this->counter->increase(); - return new Response('Hello world!'); + return new Response('Hello world! The counter value is ' . $this->counter->get()); } } CON