From 4fbad41a1a6eeed7a6676d5e0c70bcefdc43bc4f Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 3 Jul 2020 13:14:22 +0200 Subject: [PATCH 1/5] Add regression test --- phpstan-baseline.neon | 5 - src/DependencyInjection/Configuration.php | 2 - .../App/Controller/MessengerController.php | 25 +++++ test/End2End/App/Kernel.php | 5 + test/End2End/App/Messenger/FooMessage.php | 19 ++++ .../App/Messenger/FooMessageHandler.php | 19 ++++ .../App/Messenger/StaticInMemoryTransport.php | 93 +++++++++++++++++++ .../StaticInMemoryTransportFactory.php | 26 ++++++ test/End2End/App/messenger.yml | 28 ++++++ test/End2End/App/routing.yml | 4 + test/End2End/End2EndTest.php | 51 ++++++++++ 11 files changed, 270 insertions(+), 7 deletions(-) create mode 100644 test/End2End/App/Controller/MessengerController.php create mode 100644 test/End2End/App/Messenger/FooMessage.php create mode 100644 test/End2End/App/Messenger/FooMessageHandler.php create mode 100644 test/End2End/App/Messenger/StaticInMemoryTransport.php create mode 100644 test/End2End/App/Messenger/StaticInMemoryTransportFactory.php create mode 100644 test/End2End/App/messenger.yml diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 00067d1e..ffabadbe 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -225,11 +225,6 @@ parameters: count: 1 path: test/DependencyInjection/SentryExtensionTest.php - - - message: "#^Method Sentry\\\\SentryBundle\\\\Test\\\\DependencyInjection\\\\ClientMock\\:\\:captureEvent\\(\\) has parameter \\$payload with no value type specified in iterable type array\\.$#" - count: 1 - path: test/DependencyInjection/SentryExtensionTest.php - - message: "#^Method Sentry\\\\SentryBundle\\\\Test\\\\End2End\\\\App\\\\Kernel\\:\\:build\\(\\) has no return typehint specified\\.$#" count: 1 diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index df6b65e0..10ffe610 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -2,9 +2,7 @@ namespace Sentry\SentryBundle\DependencyInjection; -use Composer\InstalledVersions; use Jean85\PrettyVersions; -use PackageVersions\Versions; use Sentry\Options; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; diff --git a/test/End2End/App/Controller/MessengerController.php b/test/End2End/App/Controller/MessengerController.php new file mode 100644 index 00000000..3db49743 --- /dev/null +++ b/test/End2End/App/Controller/MessengerController.php @@ -0,0 +1,25 @@ +messenger = $messenger; + } + + public function dispatchMessage(): Response + { + $this->messenger->dispatch(new FooMessage()); + + return new Response('Success'); + } +} diff --git a/test/End2End/App/Kernel.php b/test/End2End/App/Kernel.php index b66bab8e..0328e52a 100644 --- a/test/End2End/App/Kernel.php +++ b/test/End2End/App/Kernel.php @@ -6,6 +6,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\BundleInterface; use Symfony\Component\HttpKernel\Kernel as SymfonyKernel; +use Symfony\Component\Messenger\MessageBusInterface; class Kernel extends SymfonyKernel { @@ -26,6 +27,10 @@ public function registerBundles() public function registerContainerConfiguration(LoaderInterface $loader): void { $loader->load(__DIR__ . '/config.yml'); + + if (interface_exists(MessageBusInterface::class)) { + $loader->load(__DIR__ . '/messenger.yml'); + } } protected function build(ContainerBuilder $container) diff --git a/test/End2End/App/Messenger/FooMessage.php b/test/End2End/App/Messenger/FooMessage.php new file mode 100644 index 00000000..d340c992 --- /dev/null +++ b/test/End2End/App/Messenger/FooMessage.php @@ -0,0 +1,19 @@ +shouldRetry = $shouldRetry; + } + + public function shouldRetry(): bool + { + return $this->shouldRetry; + } +} diff --git a/test/End2End/App/Messenger/FooMessageHandler.php b/test/End2End/App/Messenger/FooMessageHandler.php new file mode 100644 index 00000000..9ff6bbdf --- /dev/null +++ b/test/End2End/App/Messenger/FooMessageHandler.php @@ -0,0 +1,19 @@ +shouldRetry()) { + throw new class() extends \Exception implements UnrecoverableExceptionInterface { + }; + } + + throw new \Exception('This is an intentional failure while handling a message of class ' . get_class($message)); + } +} diff --git a/test/End2End/App/Messenger/StaticInMemoryTransport.php b/test/End2End/App/Messenger/StaticInMemoryTransport.php new file mode 100644 index 00000000..8c9a4a6c --- /dev/null +++ b/test/End2End/App/Messenger/StaticInMemoryTransport.php @@ -0,0 +1,93 @@ +getMessage()); + unset(self::$queue[$id]); + } + + /** + * {@inheritdoc} + */ + public function reject(Envelope $envelope): void + { + self::$rejected[] = $envelope; + $id = spl_object_hash($envelope->getMessage()); + unset(self::$queue[$id]); + } + + /** + * {@inheritdoc} + */ + public function send(Envelope $envelope): Envelope + { + self::$sent[] = $envelope; + $id = spl_object_hash($envelope->getMessage()); + self::$queue[$id] = $envelope; + + return $envelope; + } + + public static function reset(): void + { + self::$sent = self::$queue = self::$rejected = self::$acknowledged = []; + } + + /** + * @return Envelope[] + */ + public function getAcknowledged(): array + { + return self::$acknowledged; + } + + /** + * @return Envelope[] + */ + public function getRejected(): array + { + return self::$rejected; + } + + /** + * @return Envelope[] + */ + public function getSent(): array + { + return self::$sent; + } +} diff --git a/test/End2End/App/Messenger/StaticInMemoryTransportFactory.php b/test/End2End/App/Messenger/StaticInMemoryTransportFactory.php new file mode 100644 index 00000000..49a55bf2 --- /dev/null +++ b/test/End2End/App/Messenger/StaticInMemoryTransportFactory.php @@ -0,0 +1,26 @@ +assertLastEventIdIsNotNull($client); } + public function testMessengerCaptureSoftFailCanBeDisabled(): void + { + if (! interface_exists(MessageBusInterface::class)) { + $this->markTestSkipped('Messenger missing'); + } + + $client = static::createClient(); + + $client->request('GET', '/dispatch-message'); + + $response = $client->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame(200, $response->getStatusCode()); + + $this->consumeOneMessage($client->getKernel()); + + $this->assertLastEventIdIsNull($client); + } + + private function consumeOneMessage(KernelInterface $kernel): void + { + $application = new Application($kernel); + + $command = $application->find('messenger:consume'); + $commandTester = new CommandTester($command); + $commandTester->execute([ + 'receivers' => ['async'], + '--limit' => 1, + '--time-limit' => 1, + '-vvv' => true, + ]); + + $this->assertSame(0, $commandTester->getStatusCode()); + } + private function assertLastEventIdIsNotNull(KernelBrowser $client): void { $container = $client->getContainer(); @@ -123,4 +163,15 @@ private function assertLastEventIdIsNotNull(KernelBrowser $client): void $this->assertNotNull($hub->getLastEventId(), 'Last error not captured'); } + + private function assertLastEventIdIsNull(KernelBrowser $client): void + { + $container = $client->getContainer(); + $this->assertNotNull($container); + + $hub = $container->get('test.hub'); + $this->assertInstanceOf(HubInterface::class, $hub); + + $this->assertNull($hub->getLastEventId(), 'Some error was captured'); + } } From 91a9362f61878d29df091694e5bdf7289dcaae05 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 3 Jul 2020 13:28:33 +0200 Subject: [PATCH 2/5] Apply fix by lowering the MessengerListener priority --- src/DependencyInjection/Configuration.php | 2 +- test/DependencyInjection/ConfigurationTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 10ffe610..e2945b27 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -143,7 +143,7 @@ public function getConfigTreeBuilder(): TreeBuilder $listenerPriorities->scalarNode('console_error') ->defaultValue(128); $listenerPriorities->scalarNode('worker_error') - ->defaultValue(128); + ->defaultValue(99); // Monolog handler configuration $monologConfiguration = $rootNode->children() diff --git a/test/DependencyInjection/ConfigurationTest.php b/test/DependencyInjection/ConfigurationTest.php index 2d5a3d5e..2f3027ee 100644 --- a/test/DependencyInjection/ConfigurationTest.php +++ b/test/DependencyInjection/ConfigurationTest.php @@ -51,7 +51,7 @@ public function testConfigurationDefaults(): void 'console' => 1, 'request_error' => 128, 'console_error' => 128, - 'worker_error' => 128, + 'worker_error' => 99, ], 'options' => [ 'class_serializers' => [], From 82c7021027b1fb169c30c9e08787bb4b89e8a5a3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 3 Jul 2020 13:31:09 +0200 Subject: [PATCH 3/5] Add test for direct capturing from the Messenger --- .../App/Controller/MessengerController.php | 7 +++++++ test/End2End/App/routing.yml | 4 ++++ test/End2End/End2EndTest.php | 20 +++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/test/End2End/App/Controller/MessengerController.php b/test/End2End/App/Controller/MessengerController.php index 3db49743..b7fb69b9 100644 --- a/test/End2End/App/Controller/MessengerController.php +++ b/test/End2End/App/Controller/MessengerController.php @@ -22,4 +22,11 @@ public function dispatchMessage(): Response return new Response('Success'); } + + public function dispatchUnrecoverableMessage(): Response + { + $this->messenger->dispatch(new FooMessage(false)); + + return new Response('Success'); + } } diff --git a/test/End2End/App/routing.yml b/test/End2End/App/routing.yml index ef74b2fc..3202227b 100644 --- a/test/End2End/App/routing.yml +++ b/test/End2End/App/routing.yml @@ -17,3 +17,7 @@ subrequest: dispatch: path: /dispatch-message defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MessengerController::dispatchMessage' } + +dispatch_unrecoverable: + path: /dispatch-unrecoverable-message + defaults: { _controller: 'Sentry\SentryBundle\Test\End2End\App\Controller\MessengerController::dispatchUnrecoverableMessage' } diff --git a/test/End2End/End2EndTest.php b/test/End2End/End2EndTest.php index 0ff8bc35..97f0239f 100644 --- a/test/End2End/End2EndTest.php +++ b/test/End2End/End2EndTest.php @@ -117,6 +117,26 @@ public function testGet500(): void $this->assertLastEventIdIsNotNull($client); } + public function testMessengerCaptureHardFailure(): void + { + if (! interface_exists(MessageBusInterface::class)) { + $this->markTestSkipped('Messenger missing'); + } + + $client = static::createClient(); + + $client->request('GET', '/dispatch-unrecoverable-message'); + + $response = $client->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame(200, $response->getStatusCode()); + + $this->consumeOneMessage($client->getKernel()); + + $this->assertLastEventIdIsNotNull($client); + } + public function testMessengerCaptureSoftFailCanBeDisabled(): void { if (! interface_exists(MessageBusInterface::class)) { From 918565183d2362b54552025ceb54a7fd54c860ce Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Sat, 4 Jul 2020 22:37:02 +0200 Subject: [PATCH 4/5] Fix prefer-lowest in CI --- phpstan-baseline.neon | 2 +- test/End2End/App/Kernel.php | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ffabadbe..ef4b9ca3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -226,7 +226,7 @@ parameters: path: test/DependencyInjection/SentryExtensionTest.php - - message: "#^Method Sentry\\\\SentryBundle\\\\Test\\\\End2End\\\\App\\\\Kernel\\:\\:build\\(\\) has no return typehint specified\\.$#" + message: "#^Comparison operation \"\\>\\=\" between 50102 and 40300 is always true\\.$#" count: 1 path: test/End2End/App/Kernel.php diff --git a/test/End2End/App/Kernel.php b/test/End2End/App/Kernel.php index 0328e52a..11a5785f 100644 --- a/test/End2End/App/Kernel.php +++ b/test/End2End/App/Kernel.php @@ -13,29 +13,28 @@ class Kernel extends SymfonyKernel /** * @return BundleInterface[] */ - public function registerBundles() + public function registerBundles(): array { - $bundles = [ + return [ new \Symfony\Bundle\FrameworkBundle\FrameworkBundle(), new \Symfony\Bundle\MonologBundle\MonologBundle(), new \Sentry\SentryBundle\SentryBundle(), ]; - - return $bundles; } public function registerContainerConfiguration(LoaderInterface $loader): void { $loader->load(__DIR__ . '/config.yml'); - if (interface_exists(MessageBusInterface::class)) { + if (interface_exists(MessageBusInterface::class) && self::VERSION_ID >= 40300) { $loader->load(__DIR__ . '/messenger.yml'); } } - protected function build(ContainerBuilder $container) + protected function build(ContainerBuilder $container): void { $container->setParameter('routing_config_dir', __DIR__); + parent::build($container); } } From f18b44020a12aa7bd81d8fb496fc050d62176985 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Sat, 4 Jul 2020 22:46:48 +0200 Subject: [PATCH 5/5] Skip test properly in prefer-lowest --- phpstan-baseline.neon | 5 +++++ test/End2End/End2EndTest.php | 15 +++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ef4b9ca3..de8cfaf9 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -240,6 +240,11 @@ parameters: count: 1 path: test/End2End/End2EndTest.php + - + message: "#^Comparison operation \"\\<\" between 50102 and 40300 is always false\\.$#" + count: 1 + path: test/End2End/End2EndTest.php + - message: "#^Method Sentry\\\\SentryBundle\\\\Test\\\\ErrorTypesParserTest\\:\\:testParse\\(\\) has parameter \\$value with no typehint specified\\.$#" count: 1 diff --git a/test/End2End/End2EndTest.php b/test/End2End/End2EndTest.php index 97f0239f..fd504130 100644 --- a/test/End2End/End2EndTest.php +++ b/test/End2End/End2EndTest.php @@ -119,9 +119,7 @@ public function testGet500(): void public function testMessengerCaptureHardFailure(): void { - if (! interface_exists(MessageBusInterface::class)) { - $this->markTestSkipped('Messenger missing'); - } + $this->skipIfMessengerIsMissing(); $client = static::createClient(); @@ -139,9 +137,7 @@ public function testMessengerCaptureHardFailure(): void public function testMessengerCaptureSoftFailCanBeDisabled(): void { - if (! interface_exists(MessageBusInterface::class)) { - $this->markTestSkipped('Messenger missing'); - } + $this->skipIfMessengerIsMissing(); $client = static::createClient(); @@ -194,4 +190,11 @@ private function assertLastEventIdIsNull(KernelBrowser $client): void $this->assertNull($hub->getLastEventId(), 'Some error was captured'); } + + private function skipIfMessengerIsMissing(): void + { + if (! interface_exists(MessageBusInterface::class) || Kernel::VERSION_ID < 40300) { + $this->markTestSkipped('Messenger missing'); + } + } }