From 18a6146bc56a28fcf15c1d6a6ceb9729acf5b13a Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:08:35 +0000 Subject: [PATCH 1/8] Drop support for PHP <= v8.0 --- .github/workflows/code-checks.yml | 5 ----- CHANGELOG.md | 3 +++ composer.json | 4 ++-- src/Helper.php | 6 +++--- src/Middleware.php | 2 +- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/.github/workflows/code-checks.yml b/.github/workflows/code-checks.yml index 7e31214..d57ddf2 100644 --- a/.github/workflows/code-checks.yml +++ b/.github/workflows/code-checks.yml @@ -11,11 +11,6 @@ jobs: strategy: matrix: php: - - '7.1' - - '7.2' - - '7.3' - - '7.4' - - '8.0' - '8.1' - '8.2' - '8.3' diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e4f3e5..682b47a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Removed +- **BC break**: Removed support for PHP versions <= v8.0 as they are no longer + [actively supported](https://php.net/supported-versions.php) by the PHP project. ## [5.1.1] - 2021-08-10 ### Fixed diff --git a/composer.json b/composer.json index 185cbd9..1e622ae 100644 --- a/composer.json +++ b/composer.json @@ -16,12 +16,12 @@ }, "require": { - "php" : "^7.1|^8", + "php": "^8.1", "ext-fileinfo": "*", "ext-json" : "*", "ext-zip": "*", - "psr/log" : "^1 || ^2 || ^3", + "psr/log": "^2 || ^3", "guzzlehttp/guzzle": "^6 || ^7" }, diff --git a/src/Helper.php b/src/Helper.php index 5c940ba..f64c166 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -185,7 +185,7 @@ public function downloadFile(string $type, $primaryId, array $options = []): str // Add extension to filename, optionally extract zip switch (true) { - case (strpos($mime, 'zip')): + case str_contains($mime, 'zip'): if (!isset($options['extract']) || $options['extract'] === true) { // Maxemail only compresses CSV files, and only contains one file in a zip $zip = new \ZipArchive(); @@ -204,12 +204,12 @@ public function downloadFile(string $type, $primaryId, array $options = []): str break; - case (strpos($mime, 'pdf')): + case str_contains($mime, 'pdf'): rename($filename, $filename . '.pdf'); $filename = $filename . '.pdf'; break; - case (strpos($mime, 'csv')): + case str_contains($mime, 'csv'): case ($mime === 'text/plain'): rename($filename, $filename . '.csv'); $filename = $filename . '.csv'; diff --git a/src/Middleware.php b/src/Middleware.php index 5f25c8b..0c1166c 100644 --- a/src/Middleware.php +++ b/src/Middleware.php @@ -89,7 +89,7 @@ public static function addMaxemailErrorParser(HandlerStack $stack): void if ($error instanceof \stdClass && isset($error->msg)) { throw new Exception\ClientException($error->msg, $response->getStatusCode()); } - } catch (\UnexpectedValueException $e) { + } catch (\UnexpectedValueException) { // Failed to decode as Maxemail error, leave Guzzle to handle it } return $response; From ea74127693a3829541632f575b9656738f355aee Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:24:00 +0000 Subject: [PATCH 2/8] Update docblock shape notation Use Psalm-style syntax, which doesn't trigger ECS error. --- src/Client.php | 24 ++++++++++++------------ src/Helper.php | 8 ++++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Client.php b/src/Client.php index b041f32..d65a4cd 100644 --- a/src/Client.php +++ b/src/Client.php @@ -101,14 +101,14 @@ class Client implements \Psr\Log\LoggerAwareInterface private $debugLoggingEnabled = false; /** - * @param array $config { - * @var string $username Required - * @var string $password Required - * @var string $uri Optional. Default https://mxm.xtremepush.com/ - * @var string $user @deprecated See username - * @var string $pass @deprecated See password - * @var bool $debugLogging Optional. Enable logging of request/response. Default false - * } + * @param array{ + * username: string, // Required + * password: string, // Required + * uri: string, // Optional. Default https://mxm.xtremepush.com/ + * user: string, // @deprecated See username + * pass: string, // @deprecated See password + * debugLogging: bool, // Optional. Enable logging of request/response. Default false + * } $config */ public function __construct(array $config) { @@ -187,10 +187,10 @@ private function getClient(): GuzzleClient /** * Get API connection config * - * @return array { - * @var string $uri - * @var string $username - * @var string $password + * @return array{ + * uri: string, + * username: string, + * password: string, * } */ public function getConfig(): array diff --git a/src/Helper.php b/src/Helper.php index f64c166..b45e2c0 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -109,10 +109,10 @@ public function uploadFile(string $path): string * Download file by type * * @param string|int $primaryId - * @param array $options { - * @var bool $extract Whether to extract a compressed download, default true - * @var string $dir Directory to use for downloaded file(s), default sys_temp_dir - * } + * @param array{ + * extract: bool, // Whether to extract a compressed download, default true + * dir: string, // Directory to use for downloaded file(s), default sys_temp_dir + * } $options * @return string filename */ public function downloadFile(string $type, $primaryId, array $options = []): string From b304a6e533aae6bc46c628e8281762bb9b084927 Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:10:47 +0000 Subject: [PATCH 3/8] Upgrade ECS to v12 --- composer.json | 12 ++++--- ecs.php | 70 +++++++++++++++++++++++++++++++--------- src/Helper.php | 4 +-- src/Middleware.php | 4 +-- tests/HelperTest.php | 20 ++++++------ tests/MiddlewareTest.php | 26 +++++++-------- tests/ServiceTest.php | 6 ++-- 7 files changed, 93 insertions(+), 49 deletions(-) diff --git a/composer.json b/composer.json index 1e622ae..18852ca 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "name": "maxemail/api-php", "description": "Maxemail API Client", - "license" : "LGPL-3.0", + "license": "LGPL-3.0", "authors": [ { "name": "Maxemail Team & Contributors", @@ -18,7 +18,7 @@ "require": { "php": "^8.1", "ext-fileinfo": "*", - "ext-json" : "*", + "ext-json": "*", "ext-zip": "*", "psr/log": "^2 || ^3", @@ -26,7 +26,7 @@ }, "suggest": { - "psr/log-implementation" : "A logger can be used to provide debug information" + "psr/log-implementation": "A logger can be used to provide debug information" }, "autoload-dev": { @@ -38,6 +38,10 @@ "require-dev": { "phpunit/phpunit" : "^7|^8|^9", "php-mock/php-mock-phpunit": "^2.0", - "symplify/easy-coding-standard": "^9" + "symplify/easy-coding-standard": "^12.1" + }, + "scripts": { + "check-cs": "vendor/bin/ecs check --ansi", + "fix-cs": "vendor/bin/ecs check --fix --ansi" } } diff --git a/ecs.php b/ecs.php index 81dc286..86501dc 100644 --- a/ecs.php +++ b/ecs.php @@ -2,25 +2,65 @@ declare(strict_types=1); -use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; -use Symplify\EasyCodingStandard\ValueObject\Option; +use Symplify\EasyCodingStandard\Config\ECSConfig; use Symplify\EasyCodingStandard\ValueObject\Set\SetList; -return static function (ContainerConfigurator $containerConfigurator): void { - $parameters = $containerConfigurator->parameters(); - $parameters->set(Option::PATHS, [ +return ECSConfig::configure() + ->withPaths([ __DIR__ . '/src', __DIR__ . '/tests', - ]); + ]) + ->withRootFiles() - $services = $containerConfigurator->services(); + ->withSets([ + SetList::COMMON, + SetList::PSR_12, + SetList::STRICT, + ]) - $containerConfigurator->import(SetList::COMMON); - // Remove sniff, from common/control-structures - $services->remove(\PhpCsFixer\Fixer\ClassNotation\OrderedClassElementsFixer::class); - // Remove sniff, from common/spaces - $services->remove(\PhpCsFixer\Fixer\Operator\NotOperatorWithSuccessorSpaceFixer::class); - $services->remove(\PhpCsFixer\Fixer\CastNotation\CastSpacesFixer::class); + ->withSkip([ + // Remove sniff, from common/control-structures + \PhpCsFixer\Fixer\ClassNotation\OrderedClassElementsFixer::class, - $containerConfigurator->import(SetList::PSR_12); -}; + // Remove sniff, from common/spaces + \PhpCsFixer\Fixer\Operator\NotOperatorWithSuccessorSpaceFixer::class, + ]) + + // Rule from common/spaces. No space after cast. + ->withConfiguredRule( + \PhpCsFixer\Fixer\CastNotation\CastSpacesFixer::class, + [ + 'space' => 'none', + ], + ) + + /* + * Rule missing from PSR12. PER Coding Style 3: + * "... each of the blocks [of import statements] MUST be separated by a single blank line ..." + */ + ->withRules([ + \PhpCsFixer\Fixer\Whitespace\BlankLineBetweenImportGroupsFixer::class, + ]) + + // Rule from PSR12. PER Coding Style 7.1: "The `fn` keyword MUST NOT be succeeded by a space." + ->withConfiguredRule( + \PhpCsFixer\Fixer\FunctionNotation\FunctionDeclarationFixer::class, + [ + 'closure_fn_spacing' => 'none', + ], + ) + + /* + * Rule from PER Coding Style 2.6: + * "If the list is split across multiple lines, then the last item MUST have a trailing comma." + */ + ->withConfiguredRule( + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::class, + [ + 'elements' => [ + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::ELEMENTS_ARGUMENTS, + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::ELEMENTS_ARRAYS, + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::ELEMENTS_PARAMETERS, + ], + ], + ); diff --git a/src/Helper.php b/src/Helper.php index b45e2c0..fbed029 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -130,7 +130,7 @@ public function downloadFile(string $type, $primaryId, array $options = []): str // Create target file $filename = tempnam( ($options['dir'] ?? sys_get_temp_dir()), - "mxm-{$type}-{$primaryId}-" + "mxm-{$type}-{$primaryId}-", ); $local = @fopen($filename, 'w'); if ($local === false) { @@ -210,7 +210,7 @@ public function downloadFile(string $type, $primaryId, array $options = []): str break; case str_contains($mime, 'csv'): - case ($mime === 'text/plain'): + case $mime === 'text/plain': rename($filename, $filename . '.csv'); $filename = $filename . '.csv'; break; diff --git a/src/Middleware.php b/src/Middleware.php index 0c1166c..3c23282 100644 --- a/src/Middleware.php +++ b/src/Middleware.php @@ -38,10 +38,10 @@ public static function addLogging(HandlerStack $stack, LoggerInterface $logger): $middleware = GuzzleMiddleware::log( $logger, new MessageFormatter($messageFormat), - LogLevel::DEBUG + LogLevel::DEBUG, ); $stack->push($middleware, 'log' . $idx); - }; + } } /** diff --git a/tests/HelperTest.php b/tests/HelperTest.php index 7a5fef4..ed47cc4 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -53,7 +53,7 @@ protected function setUp(): void $stack = HandlerStack::create($this->mockHandler); $stack->push( - GuzzleMiddleware::history($this->clientHistory) + GuzzleMiddleware::history($this->clientHistory), ); $httpClient = new GuzzleClient([ @@ -93,7 +93,7 @@ public function testUpload() function (Request $request) use (&$requestBody) { $requestBody = (string)$request->getBody(); return new Response(200, [], ''); - } + }, ); $actual = $this->helper->uploadFile($sampleFile); @@ -146,7 +146,7 @@ public function testDownloadCsv() $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('file', $key); @@ -170,7 +170,7 @@ public function testDownloadZip() $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($responseFile, 'r')) + new Response(200, [], fopen($responseFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('file', $key); @@ -193,7 +193,7 @@ public function testDownloadZipNoExtract() $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('file', $key, [ @@ -218,7 +218,7 @@ public function testDownloadPdf() $key = 'abc123def456'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFilename = $this->helper->downloadFile('file', $key); @@ -241,7 +241,7 @@ public function testDownloadListExport() $key = 123; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('listexport', $key); @@ -264,7 +264,7 @@ public function testDownloadDataExport() $key = 123; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $downloadedFile = $this->helper->downloadFile('dataexport', $key); @@ -341,7 +341,7 @@ public function testDownloadTmpFileDeletedRequestError() $this->expectExceptionMessage('400 Bad Request'); $this->mockHandler->append( - new Response(400, [], 'Error') + new Response(400, [], 'Error'), ); $directory = __DIR__ . '/__files'; @@ -383,7 +383,7 @@ public function testDownloadTmpFileDeletedWriteError() $sampleFile = __DIR__ . '/__files/sample-file.csv'; $this->mockHandler->append( - new Response(200, [], fopen($sampleFile, 'r')) + new Response(200, [], fopen($sampleFile, 'r')), ); $directory = __DIR__ . '/__files'; diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index d64c72b..7f04bfa 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -79,8 +79,8 @@ public function testWarningLoggerCreatesLog() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); @@ -104,8 +104,8 @@ public function testWarningLoggerSkipsNoAgent() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); @@ -129,8 +129,8 @@ public function testWarningLoggerSkipsWrongAgent() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); @@ -154,8 +154,8 @@ public function testWarningLoggerSkipsWrongCode() [ 'Warning' => $warning, ], - json_encode('OK') - ) + json_encode('OK'), + ), ); $service = new Service('dummy_service', $this->httpClient); @@ -171,7 +171,7 @@ public function testWarningLoggerSkipsNoWarning() Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( - new Response(200, [], json_encode('OK')) + new Response(200, [], json_encode('OK')), ); $service = new Service('dummy_service', $this->httpClient); @@ -182,7 +182,7 @@ public function testErrorHandlerSkips200() { Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( - new Response(200, [], json_encode('OK')) + new Response(200, [], json_encode('OK')), ); $service = new Service('dummy_service', $this->httpClient); @@ -197,7 +197,7 @@ public function testErrorHandlerSkips500() Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( - new Response(500, [], 'Internal server error') + new Response(500, [], 'Internal server error'), ); $service = new Service('dummy_service', $this->httpClient); @@ -210,7 +210,7 @@ public function testErrorHandlerHandles400Standard() Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( - new Response(400, [], 'Error') + new Response(400, [], 'Error'), ); $service = new Service('dummy_service', $this->httpClient); @@ -230,7 +230,7 @@ public function testErrorHandlerHandles400Maxemail() 'msg' => $errorMsg, ]; $this->mockHandler->append( - new Response(400, [], json_encode($mxmError)) + new Response(400, [], json_encode($mxmError)), ); $service = new Service('dummy_service', $this->httpClient); diff --git a/tests/ServiceTest.php b/tests/ServiceTest.php index 321f529..89ba627 100644 --- a/tests/ServiceTest.php +++ b/tests/ServiceTest.php @@ -40,7 +40,7 @@ protected function setUp(): void $stack = HandlerStack::create($this->mockHandler); Middleware::addMaxemailErrorParser($stack); $stack->push( - GuzzleMiddleware::history($this->clientHistory) + GuzzleMiddleware::history($this->clientHistory), ); $this->httpClient = new GuzzleClient([ @@ -52,7 +52,7 @@ protected function setUp(): void public function testMagicCallSendsRequest() { $this->mockHandler->append( - new Response(200, [], json_encode('OK')) + new Response(200, [], json_encode('OK')), ); $service = new Service('dummy_service', $this->httpClient); @@ -68,7 +68,7 @@ public function testMagicCallSendsRequest() 'foo' => [ 'bar' => 'bob', ], - ] + ], ); $expectedParams = [ From a607a5d7787786f95977b0f1768caac1c7db0f20 Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:55:02 +0000 Subject: [PATCH 4/8] Add type declarations --- CHANGELOG.md | 2 ++ src/Client.php | 42 ++++++++--------------------- src/Helper.php | 23 ++++++---------- src/Middleware.php | 4 +-- src/Service.php | 10 ++----- tests/ApiTest.php | 20 +++++++------- tests/FunctionalTest.php | 15 +++++------ tests/HelperTest.php | 57 +++++++++++++++++----------------------- tests/MiddlewareTest.php | 33 +++++++++-------------- tests/ServiceTest.php | 14 +++------- 10 files changed, 81 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 682b47a..5411480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Added +- Add PHP 8 type declarations. ### Removed - **BC break**: Removed support for PHP versions <= v8.0 as they are no longer [actively supported](https://php.net/supported-versions.php) by the PHP project. diff --git a/src/Client.php b/src/Client.php index d65a4cd..14faf8f 100644 --- a/src/Client.php +++ b/src/Client.php @@ -6,6 +6,7 @@ use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\HandlerStack; +use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; /** @@ -56,49 +57,28 @@ * @property mixed transactional https://docs.maxemail.xtremepush.com/mxm-dev/api/services/transactional * @property mixed data_export_quick_transactional https://docs.maxemail.xtremepush.com/mxm-dev/api/services/data-export-quick-transactional */ -class Client implements \Psr\Log\LoggerAwareInterface +class Client implements LoggerAwareInterface { public const VERSION = '5.1'; - /** - * @var string - */ - private $uri = 'https://mxm.xtremepush.com/'; + private string $uri = 'https://mxm.xtremepush.com/'; - /** - * @var string - */ - private $username; + private readonly string $username; - /** - * @var string - */ - private $password; + private readonly string $password; /** * @var Service[] */ - private $services = []; + private array $services = []; - /** - * @var Helper - */ - private $helper; + private Helper $helper; - /** - * @var LoggerInterface - */ - private $logger; + private LoggerInterface $logger; - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var bool - */ - private $debugLoggingEnabled = false; + private bool $debugLoggingEnabled = false; /** * @param array{ @@ -159,7 +139,7 @@ private function getInstance(string $serviceName): Service private function getClient(): GuzzleClient { - if ($this->httpClient === null) { + if (!isset($this->httpClient)) { $stack = HandlerStack::create(); Middleware::addMaxemailErrorParser($stack); Middleware::addWarningLogging($stack, $this->getLogger()); diff --git a/src/Helper.php b/src/Helper.php index fbed029..5c26b97 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -16,20 +16,11 @@ */ class Helper { - /** - * @var Client - */ - private $api; + private Client $api; - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var string - */ - private $logLevel = LogLevel::DEBUG; + private string $logLevel = LogLevel::DEBUG; public function __construct(Client $api, GuzzleClient $httpClient) { @@ -108,15 +99,17 @@ public function uploadFile(string $path): string /** * Download file by type * - * @param string|int $primaryId * @param array{ * extract: bool, // Whether to extract a compressed download, default true * dir: string, // Directory to use for downloaded file(s), default sys_temp_dir * } $options * @return string filename */ - public function downloadFile(string $type, $primaryId, array $options = []): string - { + public function downloadFile( + string $type, + string|int $primaryId, + array $options = [], + ): string { $typePrimary = [ 'file' => 'key', 'listexport' => 'id', diff --git a/src/Middleware.php b/src/Middleware.php index 3c23282..5380959 100644 --- a/src/Middleware.php +++ b/src/Middleware.php @@ -50,7 +50,7 @@ public static function addLogging(HandlerStack $stack, LoggerInterface $logger): */ public static function addWarningLogging(HandlerStack $stack, LoggerInterface $logger): void { - $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response) use ($logger) { + $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response) use ($logger): ResponseInterface { if ($response->hasHeader('Warning')) { foreach ($response->getHeader('Warning') as $message) { // Code, agent, message, [date] @@ -76,7 +76,7 @@ public static function addWarningLogging(HandlerStack $stack, LoggerInterface $l */ public static function addMaxemailErrorParser(HandlerStack $stack): void { - $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response) { + $middleware = GuzzleMiddleware::mapResponse(function (ResponseInterface $response): ResponseInterface { $code = $response->getStatusCode(); if ($code < 400 || $code >= 500) { // Allow success response to continue, and 500-level errors to be handled by Guzzle diff --git a/src/Service.php b/src/Service.php index e56beeb..66d1e60 100644 --- a/src/Service.php +++ b/src/Service.php @@ -17,15 +17,9 @@ class Service { use JsonTrait; - /** - * @var string - */ - private $service; + private string $service; - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; public function __construct(string $service, GuzzleClient $httpClient) { diff --git a/tests/ApiTest.php b/tests/ApiTest.php index 7aed6ae..a2e5457 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -16,20 +16,20 @@ */ class ApiTest extends TestCase { - private $testConfig = [ + private array $testConfig = [ 'uri' => 'https://maxemail.example.com/', 'username' => 'api@user.com', 'password' => 'apipass', ]; - public function testConfigValid() + public function testConfigValid(): void { $api = new Client($this->testConfig); $this->assertSame($this->testConfig, $api->getConfig()); } - public function testConfigSupportDeprecatedUserPass() + public function testConfigSupportDeprecatedUserPass(): void { $config = [ 'user' => 'api@user.com', @@ -42,7 +42,7 @@ public function testConfigSupportDeprecatedUserPass() $this->assertSame($config['pass'], $api->getConfig()['password']); } - public function testConfigDefaultHost() + public function testConfigDefaultHost(): void { $config = [ 'username' => 'api@user.com', @@ -54,7 +54,7 @@ public function testConfigDefaultHost() $this->assertSame('https://mxm.xtremepush.com/', $api->getConfig()['uri']); } - public function testConfigStripsUriPath() + public function testConfigStripsUriPath(): void { $config = [ 'uri' => 'http://maxemail.example.com/some/extra/path', @@ -67,7 +67,7 @@ public function testConfigStripsUriPath() $this->assertSame('http://maxemail.example.com/', $api->getConfig()['uri']); } - public function testConfigInvalidUri() + public function testConfigInvalidUri(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('URI malformed'); @@ -81,7 +81,7 @@ public function testConfigInvalidUri() new Client($config); } - public function testConfigMissingUriProtocol() + public function testConfigMissingUriProtocol(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('URI must contain protocol scheme and host'); @@ -95,7 +95,7 @@ public function testConfigMissingUriProtocol() new Client($config); } - public function testSetGetLogger() + public function testSetGetLogger(): void { /** @var \Psr\Log\LoggerInterface|MockObject $logger */ $logger = $this->createMock(\Psr\Log\LoggerInterface::class); @@ -107,7 +107,7 @@ public function testSetGetLogger() $this->assertSame($logger, $api->getLogger()); } - public function testGetHelper() + public function testGetHelper(): void { $api = new Client($this->testConfig); @@ -119,7 +119,7 @@ public function testGetHelper() /** * Test getInstance() returns same Service instance for same name, different for different name */ - public function testGetInstance() + public function testGetInstance(): void { $api = new Client($this->testConfig); diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php index 255b5a1..410a4f9 100644 --- a/tests/FunctionalTest.php +++ b/tests/FunctionalTest.php @@ -17,10 +17,7 @@ */ class FunctionalTest extends TestCase { - /** - * @var Client - */ - private $client; + private Client $client; protected function setUp(): void { @@ -39,7 +36,7 @@ protected function setUp(): void /** * The most basic of tests */ - public function testUserAuth() + public function testUserAuth(): void { $user = $this->client->user->isLoggedIn(); $this->assertTrue($user); @@ -48,7 +45,7 @@ public function testUserAuth() /** * Test non-scalar result using Email tree which will always exist */ - public function testFetchTree() + public function testFetchTree(): void { $tree = $this->client->tree->fetchRoot('email', []); $root = $tree[0]; @@ -61,7 +58,7 @@ public function testFetchTree() * This test isn't about the message per-se, * but it checks that we're getting the properly decoded Maxemail error */ - public function testFetchTreeError() + public function testFetchTreeError(): void { $this->expectException(Exception\ClientException::class); $this->expectExceptionMessage('Invalid Node Class'); @@ -69,7 +66,7 @@ public function testFetchTreeError() $this->client->tree->fetchRoot('notATree', []); } - public function testDeprecatedMethod() + public function testDeprecatedMethod(): void { // @todo phpunit > v7, change to `expectDeprecation()` etc. $this->expectException(\PHPUnit\Framework\Error\Deprecated::class); @@ -81,7 +78,7 @@ public function testDeprecatedMethod() /** * The file uploaded should be identical to the file then downloaded */ - public function testHelperUploadDownload() + public function testHelperUploadDownload(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = $this->client->getHelper()->uploadFile($sampleFile); diff --git a/tests/HelperTest.php b/tests/HelperTest.php index ed47cc4..3b418b1 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -28,22 +28,13 @@ class HelperTest extends TestCase { use PHPMock; - /** - * @var Client|MockObject - */ - private $apiClientMock; + private Client&MockObject $apiClientMock; - /** - * @var MockHandler - */ - private $mockHandler; + private MockHandler $mockHandler; - private $clientHistory = []; + private array $clientHistory = []; - /** - * @var Helper - */ - private $helper; + private Helper $helper; protected function setUp(): void { @@ -64,7 +55,7 @@ protected function setUp(): void $this->helper = new Helper($this->apiClientMock, $httpClient); } - public function testUpload() + public function testUpload(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 'abc123def456'; @@ -90,7 +81,7 @@ public function testUpload() // as the file handle will be closed by then $requestBody = ''; $this->mockHandler->append( - function (Request $request) use (&$requestBody) { + function (Request $request) use (&$requestBody): Response { $requestBody = (string)$request->getBody(); return new Response(200, [], ''); }, @@ -115,7 +106,7 @@ function (Request $request) use (&$requestBody) { $this->assertSame($key, $actual); } - public function testUploadUnreadable() + public function testUploadUnreadable(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('File path is not readable'); @@ -125,7 +116,7 @@ public function testUploadUnreadable() $this->helper->uploadFile($sampleFile); } - public function testUploadUnableToOpen() + public function testUploadUnableToOpen(): void { $this->expectException(Exception\RuntimeException::class); $this->expectExceptionMessage('Unable to open local file'); @@ -140,7 +131,7 @@ public function testUploadUnableToOpen() $this->helper->uploadFile($sampleFile); } - public function testDownloadCsv() + public function testDownloadCsv(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 'abc123def456'; @@ -163,7 +154,7 @@ public function testDownloadCsv() $this->assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadZip() + public function testDownloadZip(): void { $responseFile = __DIR__ . '/__files/sample-file.csv.zip'; $expectedFile = __DIR__ . '/__files/sample-file.csv'; @@ -187,7 +178,7 @@ public function testDownloadZip() $this->assertFileEquals($expectedFile, $downloadedFile); } - public function testDownloadZipNoExtract() + public function testDownloadZipNoExtract(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv.zip'; $key = 'abc123def456'; @@ -212,7 +203,7 @@ public function testDownloadZipNoExtract() $this->assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadPdf() + public function testDownloadPdf(): void { $sampleFile = __DIR__ . '/__files/sample-file.pdf'; $key = 'abc123def456'; @@ -235,7 +226,7 @@ public function testDownloadPdf() $this->assertFileEquals($sampleFile, $downloadedFilename); } - public function testDownloadListExport() + public function testDownloadListExport(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 123; @@ -258,7 +249,7 @@ public function testDownloadListExport() $this->assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadDataExport() + public function testDownloadDataExport(): void { $sampleFile = __DIR__ . '/__files/sample-file.csv'; $key = 123; @@ -281,7 +272,7 @@ public function testDownloadDataExport() $this->assertFileEquals($sampleFile, $downloadedFile); } - public function testDownloadUnknownType() + public function testDownloadUnknownType(): void { $this->expectException(Exception\InvalidArgumentException::class); $this->expectExceptionMessage('Invalid download type specified'); @@ -292,7 +283,7 @@ public function testDownloadUnknownType() /** * Use exception to stop method early */ - public function testDownloadTmpFileLocation() + public function testDownloadTmpFileLocation(): void { $this->expectException(\Exception::class); @@ -307,7 +298,7 @@ public function testDownloadTmpFileLocation() /** * Use exception to stop method early */ - public function testDownloadTmpFileLocationCustom() + public function testDownloadTmpFileLocationCustom(): void { $this->expectException(\Exception::class); @@ -323,7 +314,7 @@ public function testDownloadTmpFileLocationCustom() ]); } - public function testDownloadTmpFileUnableToOpen() + public function testDownloadTmpFileUnableToOpen(): void { $this->expectException(Exception\RuntimeException::class); $this->expectExceptionMessage('Unable to open local file'); @@ -335,7 +326,7 @@ public function testDownloadTmpFileUnableToOpen() $this->helper->downloadFile('file', 123); } - public function testDownloadTmpFileDeletedRequestError() + public function testDownloadTmpFileDeletedRequestError(): void { $this->expectException(ClientException::class); $this->expectExceptionMessage('400 Bad Request'); @@ -359,14 +350,14 @@ public function testDownloadTmpFileDeletedRequestError() $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); $fcloseMock->expects($this->once()) - ->willReturnCallback(function ($resource) use (&$tmpResource) { + ->willReturnCallback(function ($resource) use (&$tmpResource): bool { $this->assertSame($tmpResource, $resource); return \fclose($tmpResource); }); $unlinkMock = $this->getFunctionMock(__NAMESPACE__, 'unlink'); $unlinkMock->expects($this->once()) - ->willReturnCallback(function ($filename) use (&$tmpFile) { + ->willReturnCallback(function ($filename) use (&$tmpFile): bool { $this->assertSame($tmpFile, $filename); return \unlink($tmpFile); }); @@ -376,7 +367,7 @@ public function testDownloadTmpFileDeletedRequestError() ]); } - public function testDownloadTmpFileDeletedWriteError() + public function testDownloadTmpFileDeletedWriteError(): void { $this->expectException(Exception\RuntimeException::class); $this->expectExceptionMessage('Unable to write to local file'); @@ -405,14 +396,14 @@ public function testDownloadTmpFileDeletedWriteError() $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); $fcloseMock->expects($this->once()) - ->willReturnCallback(function ($resource) use (&$tmpResource) { + ->willReturnCallback(function ($resource) use (&$tmpResource): bool { $this->assertSame($tmpResource, $resource); return \fclose($tmpResource); }); $unlinkMock = $this->getFunctionMock(__NAMESPACE__, 'unlink'); $unlinkMock->expects($this->once()) - ->willReturnCallback(function ($filename) use (&$tmpFile) { + ->willReturnCallback(function ($filename) use (&$tmpFile): bool { $this->assertSame($tmpFile, $filename); return \unlink($tmpFile); }); diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 7f04bfa..8ca8e56 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -24,20 +24,11 @@ */ class MiddlewareTest extends TestCase { - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var MockHandler - */ - private $mockHandler; + private MockHandler $mockHandler; - /** - * @var HandlerStack - */ - private $handlerStack; + private HandlerStack $handlerStack; protected function setUp(): void { @@ -50,7 +41,7 @@ protected function setUp(): void ]); } - public function testWarningLoggerCreatesLog() + public function testWarningLoggerCreatesLog(): void { $warningMsg = 'dummyMethod Deprecated: some example description'; $warning = "299 MxmApi/v100 \"{$warningMsg}\""; @@ -87,7 +78,7 @@ public function testWarningLoggerCreatesLog() $service->dummyMethod(); } - public function testWarningLoggerSkipsNoAgent() + public function testWarningLoggerSkipsNoAgent(): void { $warningMsg = 'some warning with no agent'; $warning = "299 - \"{$warningMsg}\""; @@ -112,7 +103,7 @@ public function testWarningLoggerSkipsNoAgent() $service->dummyMethod(); } - public function testWarningLoggerSkipsWrongAgent() + public function testWarningLoggerSkipsWrongAgent(): void { $warningMsg = 'some other system'; $warning = "299 other/1.2.3 \"{$warningMsg}\""; @@ -137,7 +128,7 @@ public function testWarningLoggerSkipsWrongAgent() $service->dummyMethod(); } - public function testWarningLoggerSkipsWrongCode() + public function testWarningLoggerSkipsWrongCode(): void { $warningMsg = 'something which looks like Maxemail'; $warning = "199 MxmApi/v100 \"{$warningMsg}\""; @@ -162,7 +153,7 @@ public function testWarningLoggerSkipsWrongCode() $service->dummyMethod(); } - public function testWarningLoggerSkipsNoWarning() + public function testWarningLoggerSkipsNoWarning(): void { /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); @@ -178,7 +169,7 @@ public function testWarningLoggerSkipsNoWarning() $service->dummyMethod(); } - public function testErrorHandlerSkips200() + public function testErrorHandlerSkips200(): void { Middleware::addMaxemailErrorParser($this->handlerStack); $this->mockHandler->append( @@ -191,7 +182,7 @@ public function testErrorHandlerSkips200() $this->assertSame('OK', $result); } - public function testErrorHandlerSkips500() + public function testErrorHandlerSkips500(): void { $this->expectException(ServerException::class); @@ -204,7 +195,7 @@ public function testErrorHandlerSkips500() $service->dummyMethod(); } - public function testErrorHandlerHandles400Standard() + public function testErrorHandlerHandles400Standard(): void { $this->expectException(ClientException::class); @@ -217,7 +208,7 @@ public function testErrorHandlerHandles400Standard() $service->dummyMethod(); } - public function testErrorHandlerHandles400Maxemail() + public function testErrorHandlerHandles400Maxemail(): void { $errorMsg = 'Maxemail API error message'; diff --git a/tests/ServiceTest.php b/tests/ServiceTest.php index 89ba627..e9819c4 100644 --- a/tests/ServiceTest.php +++ b/tests/ServiceTest.php @@ -21,17 +21,11 @@ */ class ServiceTest extends TestCase { - /** - * @var GuzzleClient - */ - private $httpClient; + private GuzzleClient $httpClient; - /** - * @var MockHandler - */ - private $mockHandler; + private MockHandler $mockHandler; - private $clientHistory = []; + private array $clientHistory = []; protected function setUp(): void { @@ -49,7 +43,7 @@ protected function setUp(): void ]); } - public function testMagicCallSendsRequest() + public function testMagicCallSendsRequest(): void { $this->mockHandler->append( new Response(200, [], json_encode('OK')), From a7e92fee03e3f32ac6c2ee00b95a55457345c68e Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:57:02 +0000 Subject: [PATCH 5/8] Use constructor property promotion --- src/Helper.php | 12 ++++-------- src/Service.php | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/Helper.php b/src/Helper.php index 5c26b97..73fb208 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -16,16 +16,12 @@ */ class Helper { - private Client $api; - - private GuzzleClient $httpClient; - private string $logLevel = LogLevel::DEBUG; - public function __construct(Client $api, GuzzleClient $httpClient) - { - $this->api = $api; - $this->httpClient = $httpClient; + public function __construct( + private readonly Client $api, + private readonly GuzzleClient $httpClient, + ) { } public function setLogLevel(string $level): self diff --git a/src/Service.php b/src/Service.php index 66d1e60..c3fc856 100644 --- a/src/Service.php +++ b/src/Service.php @@ -17,14 +17,10 @@ class Service { use JsonTrait; - private string $service; - - private GuzzleClient $httpClient; - - public function __construct(string $service, GuzzleClient $httpClient) - { - $this->service = $service; - $this->httpClient = $httpClient; + public function __construct( + private readonly string $service, + private readonly GuzzleClient $httpClient, + ) { } /** From 0ea5d0769af6d31c521face192816a562ed78418 Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 16:00:33 +0000 Subject: [PATCH 6/8] Update calls to static PHPUnit methods --- tests/ApiTest.php | 18 +++---- tests/FunctionalTest.php | 10 ++-- tests/HelperTest.php | 112 +++++++++++++++++++-------------------- tests/MiddlewareTest.php | 20 +++---- tests/ServiceTest.php | 8 +-- 5 files changed, 84 insertions(+), 84 deletions(-) diff --git a/tests/ApiTest.php b/tests/ApiTest.php index a2e5457..d9103f0 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -26,7 +26,7 @@ public function testConfigValid(): void { $api = new Client($this->testConfig); - $this->assertSame($this->testConfig, $api->getConfig()); + static::assertSame($this->testConfig, $api->getConfig()); } public function testConfigSupportDeprecatedUserPass(): void @@ -38,8 +38,8 @@ public function testConfigSupportDeprecatedUserPass(): void $api = new Client($config); - $this->assertSame($config['user'], $api->getConfig()['username']); - $this->assertSame($config['pass'], $api->getConfig()['password']); + static::assertSame($config['user'], $api->getConfig()['username']); + static::assertSame($config['pass'], $api->getConfig()['password']); } public function testConfigDefaultHost(): void @@ -51,7 +51,7 @@ public function testConfigDefaultHost(): void $api = new Client($config); - $this->assertSame('https://mxm.xtremepush.com/', $api->getConfig()['uri']); + static::assertSame('https://mxm.xtremepush.com/', $api->getConfig()['uri']); } public function testConfigStripsUriPath(): void @@ -64,7 +64,7 @@ public function testConfigStripsUriPath(): void $api = new Client($config); - $this->assertSame('http://maxemail.example.com/', $api->getConfig()['uri']); + static::assertSame('http://maxemail.example.com/', $api->getConfig()['uri']); } public function testConfigInvalidUri(): void @@ -104,7 +104,7 @@ public function testSetGetLogger(): void $api->setLogger($logger); - $this->assertSame($logger, $api->getLogger()); + static::assertSame($logger, $api->getLogger()); } public function testGetHelper(): void @@ -113,7 +113,7 @@ public function testGetHelper(): void $helper = $api->getHelper(); - $this->assertInstanceOf(Helper::class, $helper); + static::assertInstanceOf(Helper::class, $helper); } /** @@ -127,7 +127,7 @@ public function testGetInstance(): void $sameService = $api->service; $differentService = $api->different; - $this->assertSame($originalService, $sameService); - $this->assertNotSame($originalService, $differentService); + static::assertSame($originalService, $sameService); + static::assertNotSame($originalService, $differentService); } } diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php index 410a4f9..f19955d 100644 --- a/tests/FunctionalTest.php +++ b/tests/FunctionalTest.php @@ -22,7 +22,7 @@ class FunctionalTest extends TestCase protected function setUp(): void { if (!getenv('FUNC_ENABLED')) { - $this->markTestSkipped('Functional tests are disabled'); + static::markTestSkipped('Functional tests are disabled'); } $config = [ @@ -39,7 +39,7 @@ protected function setUp(): void public function testUserAuth(): void { $user = $this->client->user->isLoggedIn(); - $this->assertTrue($user); + static::assertTrue($user); } /** @@ -50,8 +50,8 @@ public function testFetchTree(): void $tree = $this->client->tree->fetchRoot('email', []); $root = $tree[0]; - $this->assertSame('email', $root->text); - $this->assertTrue($root->rootNode); + static::assertSame('email', $root->text); + static::assertTrue($root->rootNode); } /** @@ -85,7 +85,7 @@ public function testHelperUploadDownload(): void $downloadFile = $this->client->getHelper()->downloadFile('file', $key); - $this->assertFileEquals($sampleFile, $downloadFile); + static::assertFileEquals($sampleFile, $downloadFile); unlink($downloadFile); } } diff --git a/tests/HelperTest.php b/tests/HelperTest.php index 3b418b1..98f58f3 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -64,12 +64,12 @@ public function testUpload(): void $fileUploadService = $this->createMock(Service::class); // The API client will be used to initialise the upload - $this->apiClientMock->expects($this->once()) + $this->apiClientMock->expects(static::once()) ->method('__get') ->with('file_upload') ->willReturn($fileUploadService); - $fileUploadService->expects($this->once()) + $fileUploadService->expects(static::once()) ->method('__call') ->with('initialise', []) ->willReturn((object)[ @@ -89,21 +89,21 @@ function (Request $request) use (&$requestBody): Response { $actual = $this->helper->uploadFile($sampleFile); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; // Check request is multipart and body contains expected parameters - $this->assertTrue($request->hasHeader('Content-Type')); - $this->assertStringStartsWith('multipart/form-data', $request->getHeader('Content-Type')[0]); + static::assertTrue($request->hasHeader('Content-Type')); + static::assertStringStartsWith('multipart/form-data', $request->getHeader('Content-Type')[0]); // @todo phpunit > v8, change to `assertMatchesRegularExpression()` - $this->assertRegExp("/name=\"method\".*\r\n\r\nhandle\r\n/sU", $requestBody); - $this->assertRegExp("/name=\"key\".*\r\n\r\n{$key}\r\n/sU", $requestBody); + static::assertRegExp("/name=\"method\".*\r\n\r\nhandle\r\n/sU", $requestBody); + static::assertRegExp("/name=\"key\".*\r\n\r\n{$key}\r\n/sU", $requestBody); $fileContents = file_get_contents($sampleFile); - $this->assertRegExp("/name=\"file\".*\r\n\r\n{$fileContents}\r\n/sU", $requestBody); + static::assertRegExp("/name=\"file\".*\r\n\r\n{$fileContents}\r\n/sU", $requestBody); - $this->assertSame($key, $actual); + static::assertSame($key, $actual); } public function testUploadUnreadable(): void @@ -124,7 +124,7 @@ public function testUploadUnableToOpen(): void $sampleFile = __DIR__ . '/__files/sample-file.csv'; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->with($sampleFile, 'r') ->willReturn(false); @@ -142,16 +142,16 @@ public function testDownloadCsv(): void $downloadedFile = $this->helper->downloadFile('file', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } public function testDownloadZip(): void @@ -166,16 +166,16 @@ public function testDownloadZip(): void $downloadedFile = $this->helper->downloadFile('file', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($expectedFile, $downloadedFile); + static::assertFileEquals($expectedFile, $downloadedFile); } public function testDownloadZipNoExtract(): void @@ -191,16 +191,16 @@ public function testDownloadZipNoExtract(): void 'extract' => false, ]); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } public function testDownloadPdf(): void @@ -214,16 +214,16 @@ public function testDownloadPdf(): void $downloadedFilename = $this->helper->downloadFile('file', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/file/key/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFilename); + static::assertFileEquals($sampleFile, $downloadedFilename); } public function testDownloadListExport(): void @@ -237,16 +237,16 @@ public function testDownloadListExport(): void $downloadedFile = $this->helper->downloadFile('listexport', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/listexport/id/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/listexport/id/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } public function testDownloadDataExport(): void @@ -260,16 +260,16 @@ public function testDownloadDataExport(): void $downloadedFile = $this->helper->downloadFile('dataexport', $key); - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('GET', $request->getMethod()); - $this->assertSame('/download/dataexport/id/' . $key, $request->getUri()->getPath()); - $this->assertSame(['*'], $request->getHeader('Accept')); + static::assertSame('GET', $request->getMethod()); + static::assertSame('/download/dataexport/id/' . $key, $request->getUri()->getPath()); + static::assertSame(['*'], $request->getHeader('Accept')); - $this->assertFileEquals($sampleFile, $downloadedFile); + static::assertFileEquals($sampleFile, $downloadedFile); } public function testDownloadUnknownType(): void @@ -288,8 +288,8 @@ public function testDownloadTmpFileLocation(): void $this->expectException(\Exception::class); $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) - ->with($this->stringStartsWith(realpath(sys_get_temp_dir())), 'w') + $fopenMock->expects(static::once()) + ->with(static::stringStartsWith(realpath(sys_get_temp_dir())), 'w') ->willReturn(false); $this->helper->downloadFile('file', 123); @@ -305,8 +305,8 @@ public function testDownloadTmpFileLocationCustom(): void $directory = __DIR__ . '/__files'; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) - ->with($this->stringStartsWith($directory), 'w') + $fopenMock->expects(static::once()) + ->with(static::stringStartsWith($directory), 'w') ->willReturn(false); $this->helper->downloadFile('file', 123, [ @@ -320,7 +320,7 @@ public function testDownloadTmpFileUnableToOpen(): void $this->expectExceptionMessage('Unable to open local file'); $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->willReturn(false); $this->helper->downloadFile('file', 123); @@ -341,7 +341,7 @@ public function testDownloadTmpFileDeletedRequestError(): void $tmpResource = null; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->willReturnCallback(function ($filename, $mode) use (&$tmpFile, &$tmpResource) { $tmpFile = $filename; $tmpResource = \fopen($filename, $mode); @@ -349,16 +349,16 @@ public function testDownloadTmpFileDeletedRequestError(): void }); $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); - $fcloseMock->expects($this->once()) + $fcloseMock->expects(static::once()) ->willReturnCallback(function ($resource) use (&$tmpResource): bool { - $this->assertSame($tmpResource, $resource); + static::assertSame($tmpResource, $resource); return \fclose($tmpResource); }); $unlinkMock = $this->getFunctionMock(__NAMESPACE__, 'unlink'); - $unlinkMock->expects($this->once()) + $unlinkMock->expects(static::once()) ->willReturnCallback(function ($filename) use (&$tmpFile): bool { - $this->assertSame($tmpFile, $filename); + static::assertSame($tmpFile, $filename); return \unlink($tmpFile); }); @@ -383,7 +383,7 @@ public function testDownloadTmpFileDeletedWriteError(): void $tmpResource = null; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); - $fopenMock->expects($this->once()) + $fopenMock->expects(static::once()) ->willReturnCallback(function ($filename, $mode) use (&$tmpFile, &$tmpResource) { $tmpFile = $filename; $tmpResource = \fopen($filename, $mode); @@ -391,20 +391,20 @@ public function testDownloadTmpFileDeletedWriteError(): void }); $fwriteMock = $this->getFunctionMock(__NAMESPACE__, 'fwrite'); - $fwriteMock->expects($this->once()) + $fwriteMock->expects(static::once()) ->willReturn(false); $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); - $fcloseMock->expects($this->once()) + $fcloseMock->expects(static::once()) ->willReturnCallback(function ($resource) use (&$tmpResource): bool { - $this->assertSame($tmpResource, $resource); + static::assertSame($tmpResource, $resource); return \fclose($tmpResource); }); $unlinkMock = $this->getFunctionMock(__NAMESPACE__, 'unlink'); - $unlinkMock->expects($this->once()) + $unlinkMock->expects(static::once()) ->willReturnCallback(function ($filename) use (&$tmpFile): bool { - $this->assertSame($tmpFile, $filename); + static::assertSame($tmpFile, $filename); return \unlink($tmpFile); }); diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 8ca8e56..897d0c3 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -59,7 +59,7 @@ public function testWarningLoggerCreatesLog(): void /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->once()) + $logger->expects(static::once()) ->method('warning') ->with($warningMsg); @@ -85,8 +85,8 @@ public function testWarningLoggerSkipsNoAgent(): void /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -110,8 +110,8 @@ public function testWarningLoggerSkipsWrongAgent(): void /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -135,8 +135,8 @@ public function testWarningLoggerSkipsWrongCode(): void /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -157,8 +157,8 @@ public function testWarningLoggerSkipsNoWarning(): void { /** @var LoggerInterface|MockObject $logger */ $logger = $this->createMock(LoggerInterface::class); - $logger->expects($this->never()) - ->method($this->anything()); + $logger->expects(static::never()) + ->method(static::anything()); Middleware::addWarningLogging($this->handlerStack, $logger); $this->mockHandler->append( @@ -179,7 +179,7 @@ public function testErrorHandlerSkips200(): void $service = new Service('dummy_service', $this->httpClient); $result = $service->dummyMethod(); - $this->assertSame('OK', $result); + static::assertSame('OK', $result); } public function testErrorHandlerSkips500(): void diff --git a/tests/ServiceTest.php b/tests/ServiceTest.php index e9819c4..fc6de35 100644 --- a/tests/ServiceTest.php +++ b/tests/ServiceTest.php @@ -77,13 +77,13 @@ public function testMagicCallSendsRequest(): void ]), ]; - $this->assertCount(1, $this->clientHistory); + static::assertCount(1, $this->clientHistory); /** @var Request $request */ $request = $this->clientHistory[0]['request']; - $this->assertSame('POST', $request->getMethod()); - $this->assertSame('/api/json/dummy_service', $request->getUri()->getPath()); - $this->assertSame(http_build_query($expectedParams), (string)$request->getBody()); + static::assertSame('POST', $request->getMethod()); + static::assertSame('/api/json/dummy_service', $request->getUri()->getPath()); + static::assertSame(http_build_query($expectedParams), (string)$request->getBody()); } } From 74aa8140c3e230fd671c7e9139e759fb2dd4c8fe Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 15:15:17 +0000 Subject: [PATCH 7/8] Upgrade PHPUnit to v10 --- .gitignore | 2 +- .runConfigurations/All tests.run.xml | 2 +- .runConfigurations/All unit tests.run.xml | 2 +- .runConfigurations/Functional tests.run.xml | 2 +- composer.json | 4 ++-- phpunit.xml.dist | 17 +++++---------- tests/FunctionalTest.php | 21 +++++++++++++----- tests/HelperTest.php | 11 +++++----- tests/MiddlewareTest.php | 24 ++++++++++++--------- 9 files changed, 47 insertions(+), 38 deletions(-) diff --git a/.gitignore b/.gitignore index 8557bf4..369d4a1 100644 --- a/.gitignore +++ b/.gitignore @@ -5,5 +5,5 @@ /composer.lock ### Tests -/.phpunit.result.cache +/.phpunit.cache/ /phpunit.xml diff --git a/.runConfigurations/All tests.run.xml b/.runConfigurations/All tests.run.xml index 65f948c..a6489a4 100644 --- a/.runConfigurations/All tests.run.xml +++ b/.runConfigurations/All tests.run.xml @@ -5,7 +5,7 @@ - + \ No newline at end of file diff --git a/.runConfigurations/All unit tests.run.xml b/.runConfigurations/All unit tests.run.xml index 2b8b16b..fd854d2 100644 --- a/.runConfigurations/All unit tests.run.xml +++ b/.runConfigurations/All unit tests.run.xml @@ -1,6 +1,6 @@ - + \ No newline at end of file diff --git a/.runConfigurations/Functional tests.run.xml b/.runConfigurations/Functional tests.run.xml index 44647a7..078ef31 100644 --- a/.runConfigurations/Functional tests.run.xml +++ b/.runConfigurations/Functional tests.run.xml @@ -5,7 +5,7 @@ - + \ No newline at end of file diff --git a/composer.json b/composer.json index 18852ca..2c00314 100644 --- a/composer.json +++ b/composer.json @@ -36,8 +36,8 @@ }, "require-dev": { - "phpunit/phpunit" : "^7|^8|^9", - "php-mock/php-mock-phpunit": "^2.0", + "phpunit/phpunit" : "^10", + "php-mock/php-mock-phpunit": "^2", "symplify/easy-coding-standard": "^12.1" }, "scripts": { diff --git a/phpunit.xml.dist b/phpunit.xml.dist index e6230c1..07e8dd3 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,10 +1,10 @@ @@ -17,20 +17,13 @@ - + tests - - - - src/ - - - - + src - + diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php index f19955d..84b174b 100644 --- a/tests/FunctionalTest.php +++ b/tests/FunctionalTest.php @@ -4,6 +4,7 @@ namespace Maxemail\Api; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; /** @@ -12,9 +13,8 @@ * @package Maxemail\Api * @copyright 2007-2019 Emailcenter UK Ltd. (https://maxemail.xtremepush.com) * @license LGPL-3.0 - * - * @group functional */ +#[Group('functional')] class FunctionalTest extends TestCase { private Client $client; @@ -68,11 +68,22 @@ public function testFetchTreeError(): void public function testDeprecatedMethod(): void { - // @todo phpunit > v7, change to `expectDeprecation()` etc. - $this->expectException(\PHPUnit\Framework\Error\Deprecated::class); + // Capture deprecation error + $originalErrorLevel = error_reporting(); + error_reporting(E_ALL); + set_error_handler(static function (int $errno, string $errstr): never { + throw new \Exception($errstr, $errno); + }, E_USER_DEPRECATED); + + $this->expectException(\Exception::class); $this->expectExceptionMessage('searchLists Deprecated'); - $this->client->recipient->searchLists('test@example.com'); + try { + $this->client->recipient->searchLists('test@example.com'); + } finally { + error_reporting($originalErrorLevel); + restore_error_handler(); + } } /** diff --git a/tests/HelperTest.php b/tests/HelperTest.php index 98f58f3..ffb13e3 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -12,6 +12,7 @@ use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use phpmock\phpunit\PHPMock; +use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -22,8 +23,9 @@ * @copyright 2007-2019 Emailcenter UK Ltd. (https://maxemail.xtremepush.com) * @license LGPL-3.0 * - * @runTestsInSeparateProcesses Used so global functions can be mocked after already accessed in other tests + * RunTestsInSeparateProcesses: Allow global functions to be mocked after already accessed in other tests */ +#[RunTestsInSeparateProcesses] class HelperTest extends TestCase { use PHPMock; @@ -97,11 +99,10 @@ function (Request $request) use (&$requestBody): Response { // Check request is multipart and body contains expected parameters static::assertTrue($request->hasHeader('Content-Type')); static::assertStringStartsWith('multipart/form-data', $request->getHeader('Content-Type')[0]); - // @todo phpunit > v8, change to `assertMatchesRegularExpression()` - static::assertRegExp("/name=\"method\".*\r\n\r\nhandle\r\n/sU", $requestBody); - static::assertRegExp("/name=\"key\".*\r\n\r\n{$key}\r\n/sU", $requestBody); + static::assertMatchesRegularExpression("/name=\"method\".*\r\n\r\nhandle\r\n/sU", $requestBody); + static::assertMatchesRegularExpression("/name=\"key\".*\r\n\r\n{$key}\r\n/sU", $requestBody); $fileContents = file_get_contents($sampleFile); - static::assertRegExp("/name=\"file\".*\r\n\r\n{$fileContents}\r\n/sU", $requestBody); + static::assertMatchesRegularExpression("/name=\"file\".*\r\n\r\n{$fileContents}\r\n/sU", $requestBody); static::assertSame($key, $actual); } diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 897d0c3..ecb4152 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -46,15 +46,14 @@ public function testWarningLoggerCreatesLog(): void $warningMsg = 'dummyMethod Deprecated: some example description'; $warning = "299 MxmApi/v100 \"{$warningMsg}\""; - // @todo phpunit > v7, change to `expectDeprecation()` etc. - // Requires convertDeprecationsToExceptions='true' in PHPUnit config - if (version_compare(\PHPUnit\Runner\Version::id(), '8.0.0') < 0) { - // PHPUnit v7 - $this->expectException(\PHPUnit\Framework\Error\Deprecated::class); - } else { - // PHPUnit v8+ - $this->expectDeprecation(); - } + // Capture deprecation error + $originalErrorLevel = error_reporting(); + error_reporting(E_ALL); + set_error_handler(static function (int $errno, string $errstr): never { + throw new \Exception($errstr, $errno); + }, E_USER_DEPRECATED); + + $this->expectException(\Exception::class); $this->expectExceptionMessage($warningMsg); /** @var LoggerInterface|MockObject $logger */ @@ -75,7 +74,12 @@ public function testWarningLoggerCreatesLog(): void ); $service = new Service('dummy_service', $this->httpClient); - $service->dummyMethod(); + try { + $service->dummyMethod(); + } finally { + error_reporting($originalErrorLevel); + restore_error_handler(); + } } public function testWarningLoggerSkipsNoAgent(): void From f1c57afba67928bc21a24f2df26a5253c772b76d Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Thu, 13 Feb 2025 21:52:02 +0000 Subject: [PATCH 8/8] Update HelperTest to mock file error messages Avoids warning from array access on null error. --- tests/HelperTest.php | 70 +++++++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/tests/HelperTest.php b/tests/HelperTest.php index ffb13e3..4b0f5d4 100644 --- a/tests/HelperTest.php +++ b/tests/HelperTest.php @@ -119,8 +119,10 @@ public function testUploadUnreadable(): void public function testUploadUnableToOpen(): void { + $reasonMessage = sha1(uniqid('error')); + $this->expectException(Exception\RuntimeException::class); - $this->expectExceptionMessage('Unable to open local file'); + $this->expectExceptionMessage('Unable to open local file: ' . $reasonMessage); $sampleFile = __DIR__ . '/__files/sample-file.csv'; @@ -129,6 +131,12 @@ public function testUploadUnableToOpen(): void ->with($sampleFile, 'r') ->willReturn(false); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => $reasonMessage, + ]); + $this->helper->uploadFile($sampleFile); } @@ -281,49 +289,69 @@ public function testDownloadUnknownType(): void $this->helper->downloadFile('unknown', 123); } - /** - * Use exception to stop method early - */ public function testDownloadTmpFileLocation(): void { - $this->expectException(\Exception::class); - $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); $fopenMock->expects(static::once()) ->with(static::stringStartsWith(realpath(sys_get_temp_dir())), 'w') + // Return false to exit early ->willReturn(false); - $this->helper->downloadFile('file', 123); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => 'exit early', + ]); + + try { + $this->helper->downloadFile('file', 123); + } catch (Exception\RuntimeException) { + // Ignore exception from exiting early + } } - /** - * Use exception to stop method early - */ public function testDownloadTmpFileLocationCustom(): void { - $this->expectException(\Exception::class); - $directory = __DIR__ . '/__files'; $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); $fopenMock->expects(static::once()) ->with(static::stringStartsWith($directory), 'w') + // Return false to exit early ->willReturn(false); - $this->helper->downloadFile('file', 123, [ - 'dir' => $directory, - ]); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => 'exit early', + ]); + + try { + $this->helper->downloadFile('file', 123, [ + 'dir' => $directory, + ]); + } catch (Exception\RuntimeException) { + // Ignore exception from exiting early + } } public function testDownloadTmpFileUnableToOpen(): void { + $reasonMessage = sha1(uniqid('error')); + $this->expectException(Exception\RuntimeException::class); - $this->expectExceptionMessage('Unable to open local file'); + $this->expectExceptionMessage('Unable to open local file: ' . $reasonMessage); $fopenMock = $this->getFunctionMock(__NAMESPACE__, 'fopen'); $fopenMock->expects(static::once()) ->willReturn(false); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => $reasonMessage, + ]); + $this->helper->downloadFile('file', 123); } @@ -370,8 +398,10 @@ public function testDownloadTmpFileDeletedRequestError(): void public function testDownloadTmpFileDeletedWriteError(): void { + $reasonMessage = sha1(uniqid('error')); + $this->expectException(Exception\RuntimeException::class); - $this->expectExceptionMessage('Unable to write to local file'); + $this->expectExceptionMessage('Unable to write to local file: ' . $reasonMessage); $sampleFile = __DIR__ . '/__files/sample-file.csv'; $this->mockHandler->append( @@ -395,6 +425,12 @@ public function testDownloadTmpFileDeletedWriteError(): void $fwriteMock->expects(static::once()) ->willReturn(false); + $errorMock = $this->getFunctionMock(__NAMESPACE__, 'error_get_last'); + $errorMock->expects(static::once()) + ->willReturn([ + 'message' => $reasonMessage, + ]); + $fcloseMock = $this->getFunctionMock(__NAMESPACE__, 'fclose'); $fcloseMock->expects(static::once()) ->willReturnCallback(function ($resource) use (&$tmpResource): bool {