From 1e9094f2b0ca26c7ab6afb883af3ceda5c407d68 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 21 Feb 2023 11:33:08 +0100 Subject: [PATCH] minor #49431 [Mailer][Translation] Remove some `static` occurrences that may cause unstable tests (alexandre-daubois) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR was merged into the 6.3 branch. Discussion ---------- [Mailer][Translation] Remove some `static` occurrences that may cause unstable tests | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | _NA_ | License | MIT | Doc PR | _NA_ I had a little tchat with `@nicolas`-grekas who warned me that a few of my late edits on static data providers are a bit dangerous. Indeed, some helper methods were using static properties, which could lead to some leaks between test cases, and/or unstable tests. Helper classes doing so, found in `Translation` and `Mailer`, have been reverted to non-static ones. Data-providers are of course still statics and have been adapted to not use those methods. The targeted branch is 6.3 and this is intended, as requested by Nicolas. If you need any help during the backport of these edits, I'll be happy to help again! ℹ️ A lot of notifier bridges has been introduced in 6.3 and their data providers weren't updated. I also bundled this change in the PR, which should fix 6.3 pipeline as well. Finally, I updated `SmsapiTransportFactoryTest::missingRequiredOptionProvider`. As the `from` option has been made optional, the only dataset provided failed. Commits ------- 2ca9cf8988 [Mailer][Translation][Notifier] Remove some `static` occurrences that may cause unstable tests --- Test/ProviderTestCase.php | 40 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/Test/ProviderTestCase.php b/Test/ProviderTestCase.php index 692ca984..13dd9ba6 100644 --- a/Test/ProviderTestCase.php +++ b/Test/ProviderTestCase.php @@ -13,12 +13,11 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\Translation\Dumper\XliffFileDumper; use Symfony\Component\Translation\Loader\LoaderInterface; -use Symfony\Component\Translation\MessageCatalogue; use Symfony\Component\Translation\Provider\ProviderInterface; +use Symfony\Component\Translation\TranslatorBagInterface; use Symfony\Contracts\HttpClient\HttpClientInterface; /** @@ -30,13 +29,13 @@ */ abstract class ProviderTestCase extends TestCase { - protected static $client; - protected static $logger; - protected static $defaultLocale; - protected static $loader; - protected static $xliffFileDumper; + protected $client; + protected $logger; + protected $defaultLocale; + protected $loader; + protected $xliffFileDumper; - abstract public static function createProvider(HttpClientInterface $client, LoaderInterface $loader, LoggerInterface $logger, string $defaultLocale, string $endpoint): ProviderInterface; + abstract public static function createProvider(HttpClientInterface $client, LoaderInterface $loader, LoggerInterface $logger, string $defaultLocale, string $endpoint, TranslatorBagInterface $translatorBag = null): ProviderInterface; /** * @return iterable @@ -51,33 +50,28 @@ public function testToString(ProviderInterface $provider, string $expected) $this->assertSame($expected, (string) $provider); } - protected static function getClient(): MockHttpClient + protected function getClient(): MockHttpClient { - return static::$client ?? static::$client = new MockHttpClient(); + return $this->client ?? $this->client = new MockHttpClient(); } - protected static function getLoader(): LoaderInterface + protected function getLoader(): LoaderInterface { - return static::$loader ?? static::$loader = new class() implements LoaderInterface { - public function load($resource, string $locale, string $domain = 'messages'): MessageCatalogue - { - return new MessageCatalogue($locale); - } - }; + return $this->loader ?? $this->loader = $this->createMock(LoaderInterface::class); } - protected static function getLogger(): LoggerInterface + protected function getLogger(): LoggerInterface { - return static::$logger ?? static::$logger = new NullLogger(); + return $this->logger ?? $this->logger = $this->createMock(LoggerInterface::class); } - protected static function getDefaultLocale(): string + protected function getDefaultLocale(): string { - return static::$defaultLocale ?? static::$defaultLocale = 'en'; + return $this->defaultLocale ?? $this->defaultLocale = 'en'; } - protected static function getXliffFileDumper(): XliffFileDumper + protected function getXliffFileDumper(): XliffFileDumper { - return static::$xliffFileDumper ?? static::$xliffFileDumper = new XliffFileDumper(); + return $this->xliffFileDumper ?? $this->xliffFileDumper = $this->createMock(XliffFileDumper::class); } }