From 35eded9117f5c008d83e6e042992bdcde59cec30 Mon Sep 17 00:00:00 2001 From: Markus Hofmann Date: Mon, 2 Jun 2025 15:52:39 +0200 Subject: [PATCH] [WIP][TASK] Refactor tests The tests used a php mocking system, which did not respect the TYPO3 configurations for HTTP and used the functionality of cURL requests instead of the TYPO3 RequestFactory, which respects all settings. This has bad impacts on testing, in special for proxy and building correct requests against the DeepL API. Therefore, the AbstractDeepLTestCase is refactored using proper TYPO3 configuration. --- Classes/AbstractClient.php | 22 +++++++ Classes/Client.php | 2 + Tests/Functional/AbstractDeepLTestCase.php | 60 +++---------------- .../ContentElementsInContainerTest.php | 1 - .../Configuration/Services.php | 21 ------- .../test_services_override/composer.json | 15 ----- .../test_services_override/ext_emconf.php | 30 ---------- ...lationWithModifiedTcaConfigurationTest.php | 1 - .../LocalizationInlineRegressionTest.php | 9 --- .../Functional/Services/UsageServiceTest.php | 44 ++++++++------ 10 files changed, 58 insertions(+), 147 deletions(-) delete mode 100644 Tests/Functional/Fixtures/Extensions/test_services_override/Configuration/Services.php delete mode 100644 Tests/Functional/Fixtures/Extensions/test_services_override/composer.json delete mode 100644 Tests/Functional/Fixtures/Extensions/test_services_override/ext_emconf.php diff --git a/Classes/AbstractClient.php b/Classes/AbstractClient.php index 366de9ef..66aec7f3 100644 --- a/Classes/AbstractClient.php +++ b/Classes/AbstractClient.php @@ -22,6 +22,23 @@ abstract class AbstractClient implements ClientInterface protected LoggerInterface $logger; + /** + * Copied from TranslatorOptions + * for iterating over Client Configuration + * @see TranslatorOptions::OPTIONS_KEYS + */ + private const OPTIONS_KEYS = [ + TranslatorOptions::SERVER_URL, + TranslatorOptions::HEADERS, + TranslatorOptions::TIMEOUT, + TranslatorOptions::MAX_RETRIES, + TranslatorOptions::PROXY, + TranslatorOptions::LOGGER, + TranslatorOptions::HTTP_CLIENT, + TranslatorOptions::SEND_PLATFORM_INFO, + TranslatorOptions::APP_INFO, + ]; + public function __construct(ConfigurationInterface $configuration) { $this->configuration = $configuration; @@ -46,6 +63,11 @@ protected function getTranslator(): Translator throw new ApiKeyNotSetException('The api key ist not set', 1708081233823); } $options[TranslatorOptions::HTTP_CLIENT] = GeneralUtility::makeInstance(GuzzleClientFactory::class)->getClient(); + foreach (self::OPTIONS_KEYS as $option) { + if ($options[TranslatorOptions::HTTP_CLIENT]->getConfig($option) !== null) { + $options[$option] = $options[TranslatorOptions::HTTP_CLIENT]->getConfig($option); + } + } $this->translator = new Translator($this->configuration->getApiKey(), $options); return $this->translator; } diff --git a/Classes/Client.php b/Classes/Client.php index ffe2fe6e..30538470 100644 --- a/Classes/Client.php +++ b/Classes/Client.php @@ -12,11 +12,13 @@ use DeepL\TextResult; use DeepL\TranslateTextOptions; use DeepL\Usage; +use Symfony\Component\DependencyInjection\Attribute\AsAlias; use WebVision\Deepltranslate\Core\Exception\ApiKeyNotSetException; /** * @internal No public usage */ +#[AsAlias(id: ClientInterface::class, public: true)] final class Client extends AbstractClient { /** diff --git a/Tests/Functional/AbstractDeepLTestCase.php b/Tests/Functional/AbstractDeepLTestCase.php index 24c4acaf..ca51e3f7 100644 --- a/Tests/Functional/AbstractDeepLTestCase.php +++ b/Tests/Functional/AbstractDeepLTestCase.php @@ -4,25 +4,17 @@ namespace WebVision\Deepltranslate\Core\Tests\Functional; -use Closure; -use DeepL\Translator; use DeepL\TranslatorOptions; use Exception; -use phpmock\phpunit\PHPMock; -use Psr\Log\NullLogger; use Ramsey\Uuid\Uuid; use RuntimeException; use SBUERK\TYPO3\Testing\TestCase\FunctionalTestCase; -use Symfony\Component\DependencyInjection\Container; use TYPO3\CMS\Core\Information\Typo3Version; +use TYPO3\CMS\Core\Utility\ArrayUtility; use TYPO3\CMS\Core\Utility\StringUtility; -use WebVision\Deepltranslate\Core\Client; -use WebVision\Deepltranslate\Core\ClientInterface; -use WebVision\Deepltranslate\Core\ConfigurationInterface; abstract class AbstractDeepLTestCase extends FunctionalTestCase { - use PHPMock; /** * @var string @@ -112,7 +104,6 @@ abstract class AbstractDeepLTestCase extends FunctionalTestCase protected array $testExtensionsToLoad = [ 'web-vision/deepl-base', 'web-vision/deepltranslate-core', - __DIR__ . '/Fixtures/Extensions/test_services_override', ]; protected const EXAMPLE_DOCUMENT_INPUT = AbstractDeepLTestCase::EXAMPLE_TEXT['en']; @@ -152,13 +143,13 @@ protected function setUp(): void } $this->authKey = getenv('DEEPL_AUTH_KEY'); } - parent::setUp(); $this->instantiateMockServerClient(); + parent::setUp(); } private function makeSessionName(): string { - return sprintf('%s/%s', self::getInstanceIdentifier(), StringUtility::getUniqueId()); + return sprintf('%s/%s', self::getInstanceIdentifier(), StringUtility::getUniqueId('deepl-mock-')); } /** @@ -210,29 +201,12 @@ protected function instantiateMockServerClient(array $options = []): void if ($this->serverUrl !== false) { $mergedOptions[TranslatorOptions::SERVER_URL] = $this->serverUrl; } - $mockConfiguration = $this - ->getMockBuilder(ConfigurationInterface::class) - ->getMock(); - $mockConfiguration - ->method('getApiKey') - ->willReturn(self::getInstanceIdentifier()); - - $client = new Client($mockConfiguration); - $client->setLogger(new NullLogger()); - - // use closure to set private option for translation - $translator = new Translator(self::getInstanceIdentifier(), $mergedOptions); - Closure::bind( - function (Translator $translator) { - $this->translator = $translator; - }, - $client, - Client::class - )->call($client, $translator); - - /** @var Container $container */ - $container = $this->getContainer(); - $container->set(ClientInterface::class, $client); + ArrayUtility::mergeRecursiveWithOverrule( + $this->configurationToUseInTestInstance, + [ + 'HTTP' => $mergedOptions, + ], + ); } public static function readFile(string $filepath): string @@ -301,20 +275,4 @@ public function assertExceptionClass(string $class, callable $function): Excepti } static::fail("Expected exception of class '$class' but nothing was thrown"); } - - /** - * This is necessary due to https://github.com/php-mock/php-mock-phpunit#restrictions - * In short, as these methods can be called by other tests before UserAgentTest and other - * tests that use their mocks are executed, we need to call `defineFunctionMock` before - * calling the unmocked function, or the mock will not work. - * Otherwise the tests will fail with: - * Expectation failed for method name is "delegate" when invoked 1 time(s). - * Method was expected to be called 1 times, actually called 0 times. - */ - public static function setUpBeforeClass(): void - { - self::defineFunctionMock(__NAMESPACE__, 'curl_exec'); - self::defineFunctionMock(__NAMESPACE__, 'curl_getinfo'); - self::defineFunctionMock(__NAMESPACE__, 'curl_setopt_array'); - } } diff --git a/Tests/Functional/Container/ContentElementsInContainerTest.php b/Tests/Functional/Container/ContentElementsInContainerTest.php index 640908b2..35f65fa8 100644 --- a/Tests/Functional/Container/ContentElementsInContainerTest.php +++ b/Tests/Functional/Container/ContentElementsInContainerTest.php @@ -23,7 +23,6 @@ final class ContentElementsInContainerTest extends AbstractDeepLTestCase 'b13/container', 'web-vision/deepl-base', 'web-vision/deepltranslate-core', - __DIR__ . '/../Fixtures/Extensions/test_services_override', __DIR__ . '/Fixtures/Extensions/test_container', ]; diff --git a/Tests/Functional/Fixtures/Extensions/test_services_override/Configuration/Services.php b/Tests/Functional/Fixtures/Extensions/test_services_override/Configuration/Services.php deleted file mode 100644 index 7cf0d4b7..00000000 --- a/Tests/Functional/Fixtures/Extensions/test_services_override/Configuration/Services.php +++ /dev/null @@ -1,21 +0,0 @@ -services(); - $services - ->defaults() - ->autowire() - ->autoconfigure(); - - // Set the ClientInterface to public for testing purpose only. This is needed, that - // functional testcases can set a special configured or mocked service - // instance for the alias. No need to have it public in general. - $services - ->set(ClientInterface::class) - ->public(); -}; diff --git a/Tests/Functional/Fixtures/Extensions/test_services_override/composer.json b/Tests/Functional/Fixtures/Extensions/test_services_override/composer.json deleted file mode 100644 index 47962c4f..00000000 --- a/Tests/Functional/Fixtures/Extensions/test_services_override/composer.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "name": "web-vision/test-services-override", - "type": "typo3-cms-extension", - "description": "Change service registrations for testing purposes.", - "license": ["GPL-2.0-or-later"], - "extra": { - "typo3/cms": { - "extension-key": "test_services_override" - } - }, - "require": { - "typo3/cms-core": "*", - "web-vision/deepltranslate-core": "*" - } -} diff --git a/Tests/Functional/Fixtures/Extensions/test_services_override/ext_emconf.php b/Tests/Functional/Fixtures/Extensions/test_services_override/ext_emconf.php deleted file mode 100644 index 3d611e8c..00000000 --- a/Tests/Functional/Fixtures/Extensions/test_services_override/ext_emconf.php +++ /dev/null @@ -1,30 +0,0 @@ - 'DeepL Translate - Test Overrides', - 'description' => 'Change service registrations for testing purposes.', - 'category' => 'backend', - 'author' => 'web-vision GmbH Team', - 'author_company' => 'web-vision GmbH', - 'author_email' => 'hello@web-vision.de', - 'state' => 'stable', - 'version' => '1.0.0.', - 'constraints' => [ - 'depends' => [ - 'typo3' => '12.4.0-12.4.99', - 'deepltranslate_core' => '*', - ], - 'conflicts' => [], - 'suggests' => [], - ], -]; diff --git a/Tests/Functional/Hooks/TranslationWithModifiedTcaConfigurationTest.php b/Tests/Functional/Hooks/TranslationWithModifiedTcaConfigurationTest.php index cb8524a0..d7a3500c 100644 --- a/Tests/Functional/Hooks/TranslationWithModifiedTcaConfigurationTest.php +++ b/Tests/Functional/Hooks/TranslationWithModifiedTcaConfigurationTest.php @@ -68,7 +68,6 @@ final class TranslationWithModifiedTcaConfigurationTest extends AbstractDeepLTes protected array $testExtensionsToLoad = [ 'web-vision/deepl-base', 'web-vision/deepltranslate-core', - __DIR__ . '/../Fixtures/Extensions/test_services_override', __DIR__ . '/Fixtures/Extensions/test_tca_override', ]; diff --git a/Tests/Functional/Regression/LocalizationInlineRegressionTest.php b/Tests/Functional/Regression/LocalizationInlineRegressionTest.php index a8aafb96..efa4a246 100644 --- a/Tests/Functional/Regression/LocalizationInlineRegressionTest.php +++ b/Tests/Functional/Regression/LocalizationInlineRegressionTest.php @@ -14,15 +14,6 @@ final class LocalizationInlineRegressionTest extends AbstractDeepLTestCase { use SiteBasedTestTrait; - /** - * @var non-empty-string[] - */ - protected array $testExtensionsToLoad = [ - 'web-vision/deepl-base', - 'web-vision/deepltranslate-core', - __DIR__ . '/../Fixtures/Extensions/test_services_override', - ]; - protected const LANGUAGE_PRESETS = [ 'EN' => [ 'id' => 0, diff --git a/Tests/Functional/Services/UsageServiceTest.php b/Tests/Functional/Services/UsageServiceTest.php index db040d91..f05ecc7a 100644 --- a/Tests/Functional/Services/UsageServiceTest.php +++ b/Tests/Functional/Services/UsageServiceTest.php @@ -6,7 +6,9 @@ use DeepL\Usage; use DeepL\UsageDetail; +use PHPUnit\Framework\Attributes\RunClassInSeparateProcess; use PHPUnit\Framework\Attributes\Test; +use WebVision\Deepltranslate\Core\Domain\Dto\TranslateContext; use WebVision\Deepltranslate\Core\Service\DeeplService; use WebVision\Deepltranslate\Core\Service\ProcessingInstruction; use WebVision\Deepltranslate\Core\Service\UsageService; @@ -35,7 +37,7 @@ public function classLoadable(): void { $usageService = $this->get(UsageService::class); - static::assertInstanceOf(UsageService::class, $usageService); + self::assertInstanceOf(UsageService::class, $usageService); } #[Test] @@ -46,7 +48,7 @@ public function usageReturnsValue(): void $usage = $usageService->getCurrentUsage(); - static::assertInstanceOf(Usage::class, $usage); + self::assertInstanceOf(Usage::class, $usage); } #[Test] @@ -55,13 +57,13 @@ public function limitExceedReturnsFalse(): void /** @var UsageService $usageService */ $usageService = $this->get(UsageService::class); - static::assertFalse($usageService->checkTranslateLimitWillBeExceeded('')); + self::assertFalse($usageService->checkTranslateLimitWillBeExceeded('')); } #[Test] public function limitExceedReturnsTrueIfLimitIsReached(): void { - $translateContent = 'proton beam'; + $translateContent = self::EXAMPLE_TEXT['en']; /** @var UsageService $usageService */ $usageService = $this->get(UsageService::class); @@ -70,20 +72,20 @@ public function limitExceedReturnsTrueIfLimitIsReached(): void $deeplService = $this->get(DeeplService::class); // Execute translation to check translation limit - $responseObject = $deeplService->translateRequest( - $translateContent, - 'DE', - 'EN' - ); + $translateContext = new TranslateContext($translateContent); + $translateContext->setSourceLanguageCode('EN'); + $translateContext->setTargetLanguageCode('DE'); + $translatedContent = $deeplService->translateContent($translateContext); + self::assertEquals(self::EXAMPLE_TEXT['de'], $translatedContent); $isLimitExceeded = $usageService->checkTranslateLimitWillBeExceeded($translateContent); - static::assertTrue($isLimitExceeded); + self::assertTrue($isLimitExceeded); } #[Test] public function checkHTMLMarkupsIsNotPartOfLimit(): void { - $translateContent = 'proton beam'; + $translateContent = self::EXAMPLE_TEXT['en']; /** @var UsageService $usageService */ $usageService = $this->get(UsageService::class); @@ -91,17 +93,21 @@ public function checkHTMLMarkupsIsNotPartOfLimit(): void /** @var DeeplService $deeplService */ $deeplService = $this->get(DeeplService::class); + $translateContext = new TranslateContext('

' . $translateContent . '

'); + $translateContext->setSourceLanguageCode('EN'); + $translateContext->setTargetLanguageCode('DE'); // Execute translation to check translation limit - $responseObject = $deeplService->translateRequest( - '

' . $translateContent . '

', - 'DE', - 'EN' - ); + // @todo at the moment the mock server returns an empty result, when the + // translation string is given with HTML tags, but increases character + // usage. I have no idea, why this is happening, but with this behaviour + // there is no possibility checking the response onto valid value. + $response = $deeplService->translateContent($translateContext); + $usage = $usageService->getCurrentUsage(); - static::assertInstanceOf(Usage::class, $usage); + self::assertInstanceOf(Usage::class, $usage); $character = $usage->character; - static::assertInstanceOf(UsageDetail::class, $character); - static::assertEquals(strlen($translateContent), $character->count); + self::assertInstanceOf(UsageDetail::class, $character); + self::assertEquals(strlen($translateContent), $character->count); } }