From a338157776ce39b343f42386953ac8faacb59dbd Mon Sep 17 00:00:00 2001 From: Michel Roca Date: Mon, 19 Jan 2026 15:57:41 +0100 Subject: [PATCH] chore: fix all phpcs and phpstan issues --- .github/workflows/php.yml | 32 ++++++------------ README.md | 9 ++++- composer.json | 29 ++++++++++------ phpmd.xml.dist | 33 ------------------- src/Cli/ParallelController.php | 7 ++-- src/Event/EventDispatcherDecorator.php | 5 ++- src/Extension.php | 23 ++----------- src/Task/SpecificationsFinder.php | 13 ++++---- src/Task/TestworkSpecificationsFinder.php | 2 +- src/Worker/TaskWorker.php | 29 +++++----------- src/Worker/Worker.php | 6 ++-- tests/Behat/Context/EnvironmentContext.php | 13 +++----- tests/Behat/Context/ParallelBehatContext.php | 10 +----- .../Util/ReadWriteDataToFileWithLocking.php | 4 +++ tests/Cli/SigintControllerTest.php | 2 +- tests/Event/EventDispatcherDecoratorTest.php | 12 ++++--- tests/Event/ParallelTestCompletedTest.php | 8 +++-- tests/Event/ParallelTestsAbortedTest.php | 8 +++-- tests/ExtensionTest.php | 1 - 19 files changed, 95 insertions(+), 151 deletions(-) delete mode 100644 phpmd.xml.dist diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 40cb4d8..254c91f 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -8,13 +8,13 @@ on: jobs: quality: - runs-on: 'ubuntu-20.04' + runs-on: 'ubuntu-22.04' steps: - uses: actions/checkout@v3 - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: '7.4' + php-version: '8.4' - name: Cache Composer packages id: composer-cache uses: actions/cache@v3 @@ -23,41 +23,29 @@ jobs: key: ${{ runner.os }}-quality-php-${{ hashFiles('**/composer.lock') }} restore-keys: | ${{ runner.os }}-quality-php - - name: Require symfony quality tools - run: | - composer require --dev --no-update \ - phpstan/phpstan \ - squizlabs/php_codesniffer \ - phpstan/phpstan-phpunit \ - doctrine/coding-standard:^8.0 \ - phpstan/phpstan-symfony - name: Install dependencies run: composer install --prefer-dist --no-progress --no-suggest - name: Run phpcs - run: ./vendor/bin/phpcs + run: composer phpcs - name: Run phpstan - run: ./vendor/bin/phpstan analyse + run: composer phpstan test: strategy: matrix: - os: ['ubuntu-20.04'] - version: [ '7.4', '8.0', '8.1', '8.2'] + os: ['ubuntu-22.04'] + version: [ '7.4', '8.0', '8.1', '8.2', '8.3', '8.4'] include: - - os: 'ubuntu-20.04' - version: '7.2' - - os: 'ubuntu-20.04' - version: '7.3' - - os: 'ubuntu-20.04' + - os: 'ubuntu-22.04' version: '7.4' symfony: '^4.0' - - os: 'ubuntu-20.04' + - os: 'ubuntu-22.04' version: '7.4' symfony: '^5.0' - - os: 'ubuntu-20.04' + - os: 'ubuntu-22.04' version: '8.1' symfony: '^6.0' - - os: 'ubuntu-20.04' + - os: 'ubuntu-22.04' version: '8.2' symfony: '^7.0' runs-on: ${{ matrix.os }} diff --git a/README.md b/README.md index 11358e0..373baa2 100644 --- a/README.md +++ b/README.md @@ -173,6 +173,13 @@ Use `--parallel` or `-l` option for start in parallel scenario mode. 3/3 [============================] 100% 12 secs/12 secs ``` +## Test with the oldest supported PHP version + +```bash +composer update --prefer-lowest --prefer-stable +docker run --rm -ti -v $(pwd):/app -w /app -u $(id -u):$(id -g) php:7.4-cli bash -c "vendor/bin/phpunit; vendor/bin/behat" +``` + ## Tools and Coding standards The extension uses the following coding standards and quality tools: @@ -215,4 +222,4 @@ classes for behat tests in directory [tests/Behat](tests/Behat). [PSR-12]: [Behat]: [SymfonyExtension]: - [MinkExtension]: \ No newline at end of file + [MinkExtension]: diff --git a/composer.json b/composer.json index ffe47c6..26248f8 100644 --- a/composer.json +++ b/composer.json @@ -12,20 +12,24 @@ } ], "require": { - "php": ">=7.2", - "behat/behat": "^3.6", - "symfony/process": "^2.7.52 || ^3.0 || ^4.0 || ^5.0 || ^6.0 || ^7.0", - "symfony/console": "^2.7.52 || ^2.8.33 || ^3.3.15 || ^3.4.3 || ^4.0 || ^5.0 || ^6.0 || ^7.0", - "symfony/serializer": "^2.7.52 || ^3.0 || ^4.0 || ^5.0 || ^6.0 || ^7.0", - "symfony/dependency-injection": "^2.7.52 || ^3.0 || ^4.0 || ^4.1.12 || ^5.0 || ^6.0 || ^7.0", - "symfony/event-dispatcher": "^2.7.52 || ^3.0 || ^4.0 || ^5.0 || ^6.0 || ^7.0", - "symfony/yaml": "^2.7.52 || ^3.0 || ^4.0 || ^5.0 || ^6.0 || ^7.0", - "symfony/expression-language": "^2.7.52 || ^3.0 || ^4.0 || ^5.0 || ^6.0 || ^7.0" + "php": ">=7.4", + "behat/behat": "^3.9", + "symfony/process": "^4.0 || ^5.0 || ^6.0 || ^7.0", + "symfony/console": "^4.0 || ^5.0 || ^6.0 || ^7.0", + "symfony/serializer": "^4.0 || ^5.0 || ^6.0 || ^7.0", + "symfony/dependency-injection": "^4.0 || ^4.1.12 || ^5.0 || ^6.0 || ^7.0", + "symfony/event-dispatcher": "^4.0 || ^5.0 || ^6.0 || ^7.0", + "symfony/yaml": "^4.0 || ^5.0 || ^6.0 || ^7.0", + "symfony/expression-language": "^4.0 || ^5.0 || ^6.0 || ^7.0" }, "require-dev": { "phpunit/phpunit": "^8.5 || ^9.0", "ext-json": "*", - "dg/bypass-finals": "^1.1 || ^1.2" + "dg/bypass-finals": "^1.1 || ^1.2", + "phpstan/phpstan": "^2.1", + "phpstan/phpstan-phpunit": "^2.0", + "squizlabs/php_codesniffer": "^3.13", + "doctrine/coding-standard": "^9.0" }, "autoload": { "psr-4": { @@ -63,5 +67,10 @@ "@phpunit", "@behat" ] + }, + "config": { + "allow-plugins": { + "dealerdirect/phpcodesniffer-composer-installer": true + } } } diff --git a/phpmd.xml.dist b/phpmd.xml.dist deleted file mode 100644 index 7d06f41..0000000 --- a/phpmd.xml.dist +++ /dev/null @@ -1,33 +0,0 @@ - - - - App ruleset - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/src/Cli/ParallelController.php b/src/Cli/ParallelController.php index 368922c..8d45672 100644 --- a/src/Cli/ParallelController.php +++ b/src/Cli/ParallelController.php @@ -148,9 +148,6 @@ private function addEventListeners(): void $this->eventDispatcher->addListener(ParallelTestsAborted::ABORTED, [$this, 'parallelTestsAborted']); } - /** - * @SuppressWarnings(PHPMD.StaticAccess) - */ private function setupTasksWithProgressBar(): void { $tasks = $this->createTasks(); @@ -195,11 +192,11 @@ private function createTasks() { $paths = $this->input->hasArgument('paths') ? $this->input->getArgument('paths') : null; - if (is_null($paths) || is_string($paths)) { + if ($paths === null || is_string($paths)) { return $this->taskFactory->createTasks($this->input, $paths); } - if (!is_array($paths)) { + if (! is_array($paths)) { throw new UnexpectedValue('Expected array, string or null'); } diff --git a/src/Event/EventDispatcherDecorator.php b/src/Event/EventDispatcherDecorator.php index ffe05dc..ae4f23b 100644 --- a/src/Event/EventDispatcherDecorator.php +++ b/src/Event/EventDispatcherDecorator.php @@ -26,7 +26,10 @@ public function __construct(TestworkEventDispatcher $eventDispatcher) */ public function dispatch($event, $eventName = null) { - if (TestworkEventDispatcher::DISPATCHER_VERSION === 2) { + $dispatcherVersion = defined(TestworkEventDispatcher::class . '::DISPATCHER_VERSION') + ? constant(TestworkEventDispatcher::class . '::DISPATCHER_VERSION') + : 1; + if ((int) $dispatcherVersion === 2) { return $this->eventDispatcher->dispatch($event, $eventName); } diff --git a/src/Extension.php b/src/Extension.php index 133483c..ed35c7c 100644 --- a/src/Extension.php +++ b/src/Extension.php @@ -8,7 +8,6 @@ use DMarynicz\BehatParallelExtension\Exception\UnexpectedValue; use DMarynicz\BehatParallelExtension\Worker\Poll; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; -use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -47,10 +46,7 @@ public function load(ContainerBuilder $container, array $config): void $loader->load('services.yaml'); $pollDefinition = $container->findDefinition(Poll::class); - $pollDefinition->setArgument( - '$environments', - $config['environments'] - ); + $pollDefinition->setArgument('$environments', $config['environments']); } public function process(ContainerBuilder $container): void @@ -64,7 +60,6 @@ private function addEventsNode() { $node = $this->getNewArrayNode('events'); - // @phpstan-ignore-next-line $node ->prototype('array') ->children() @@ -95,7 +90,7 @@ private function addEventsNode() } /** - * @return ArrayNodeDefinition|NodeDefinition + * @return ArrayNodeDefinition */ private function addEnvironmentsNode() { @@ -145,22 +140,10 @@ private function addEnvironmentsNode() */ private function getNewArrayNode($name): ArrayNodeDefinition { - if (method_exists(TreeBuilder::class, 'root')) { - // @phpstan-ignore-next-line - $treeBuilder = new TreeBuilder(); - - // @phpstan-ignore-next-line - $node = $treeBuilder->root($name); - if (! $node instanceof ArrayNodeDefinition) { - throw new UnexpectedValue('expected ArrayNodeDefinition'); - } - - return $node; - } - $treeBuilder = new TreeBuilder($name); $node = $treeBuilder->getRootNode(); + // @phpstan-ignore-next-line if (! $node instanceof ArrayNodeDefinition) { throw new UnexpectedValue('expected ArrayNodeDefinition'); } diff --git a/src/Task/SpecificationsFinder.php b/src/Task/SpecificationsFinder.php index b7e18e1..a415022 100644 --- a/src/Task/SpecificationsFinder.php +++ b/src/Task/SpecificationsFinder.php @@ -13,9 +13,12 @@ class SpecificationsFinder implements TestworkSpecificationsFinder /** @var SuiteRepository */ private $suiteRepository; - /** @var TestworkSpecificationFinder */ + /** @var TestworkSpecificationFinder */ private $specificationFinder; + /** + * @param TestworkSpecificationFinder $specificationFinder + */ public function __construct( SuiteRepository $suiteRepository, TestworkSpecificationFinder $specificationFinder @@ -27,9 +30,7 @@ public function __construct( /** * @param string|null $path * - * @return GroupedSpecificationIterator[] - * - * @SuppressWarnings(PHPMD.StaticAccess) + * @return array> */ public function findGroupedSpecifications($path) { @@ -43,7 +44,7 @@ public function findGroupedSpecifications($path) * * @param string|null $path * - * @return SpecificationIterator[] + * @return list> */ private function findSpecifications($path) { @@ -56,7 +57,7 @@ private function findSpecifications($path) * @param Suite[] $suites * @param string|null $locator * - * @return SpecificationIterator[] + * @return list> */ private function findSuitesSpecifications($suites, $locator) { diff --git a/src/Task/TestworkSpecificationsFinder.php b/src/Task/TestworkSpecificationsFinder.php index 2df61cc..eb0a0ec 100644 --- a/src/Task/TestworkSpecificationsFinder.php +++ b/src/Task/TestworkSpecificationsFinder.php @@ -9,7 +9,7 @@ interface TestworkSpecificationsFinder /** * @param string|null $path * - * @return GroupedSpecificationIterator[] + * @return array> */ public function findGroupedSpecifications($path); } diff --git a/src/Worker/TaskWorker.php b/src/Worker/TaskWorker.php index 12bef50..7a49584 100644 --- a/src/Worker/TaskWorker.php +++ b/src/Worker/TaskWorker.php @@ -8,7 +8,6 @@ use DMarynicz\BehatParallelExtension\Event\WorkerCreated; use DMarynicz\BehatParallelExtension\Event\WorkerDestroyed; use DMarynicz\BehatParallelExtension\Exception\Runtime; -use DMarynicz\BehatParallelExtension\Exception\UnexpectedValue; use DMarynicz\BehatParallelExtension\Task\Queue; use DMarynicz\BehatParallelExtension\Task\TaskEntity; use DMarynicz\BehatParallelExtension\Util\ProcessFactory; @@ -17,7 +16,7 @@ class TaskWorker implements Worker { - /** @var string[] */ + /** @var array */ private $environment; /** @var Queue */ @@ -42,8 +41,8 @@ class TaskWorker implements Worker private $processFactory; /** - * @param string[] $environment - * @param int $workerId + * @param array $environment + * @param int $workerId */ public function __construct( Queue $queue, @@ -55,11 +54,7 @@ public function __construct( $this->environment = $environment; $this->queue = $queue; $this->eventDispatcher = $eventDispatcher; - if (! is_int($workerId)) { - throw new UnexpectedValue('Expected int'); - } - - $this->workerId = $workerId; + $this->workerId = $workerId; $this->eventDispatcher->dispatch(new WorkerCreated($this), WorkerCreated::WORKER_CREATED); if (! $processFactory instanceof ProcessFactory) { $processFactory = new SymfonyProcessFactory(); @@ -120,27 +115,19 @@ public function stop(): void } /** - * @return string[] + * @return array */ - public function getEnvironment() + public function getEnvironment(): array { return $this->environment; } /** - * @param string[] $env - * - * @return Worker + * @param array $env */ - public function setEnvironment($env) + public function setEnvironment(array $env): void { - if (! is_array($env)) { - throw new UnexpectedValue('Expected array'); - } - $this->environment = $env; - - return $this; } public function getWorkerId(): int diff --git a/src/Worker/Worker.php b/src/Worker/Worker.php index cc83e34..d1f30ac 100644 --- a/src/Worker/Worker.php +++ b/src/Worker/Worker.php @@ -31,16 +31,14 @@ public function stop(): void; * * @return string[] */ - public function getEnvironment(); + public function getEnvironment(): array; /** * Sets worker environment * * @param string[] $env - * - * @return Worker */ - public function setEnvironment($env); + public function setEnvironment(array $env): void; /** * Return's worker id diff --git a/tests/Behat/Context/EnvironmentContext.php b/tests/Behat/Context/EnvironmentContext.php index d753fea..dc9a5ec 100644 --- a/tests/Behat/Context/EnvironmentContext.php +++ b/tests/Behat/Context/EnvironmentContext.php @@ -46,16 +46,13 @@ public function iAppendEnvironmentVariableToJson($name, $filename): void $data = $handle->read(); - $array = json_decode($data); + $array = json_decode($data, true); if (! is_array($array)) { throw new Logic('Expected array'); } $array[] = getenv($name); $data = json_encode($array); - if (! is_array($array)) { - throw new Logic('Expected array'); - } if (! is_string($data)) { throw new Logic('Expected string'); @@ -75,15 +72,15 @@ public function theOrderedUniqueDataOfTheFileShouldMatch($filename, TableNode $t $path = $this->getRealPath($filename); $handle = new ReadWriteDataToFileWithLocking($path); $data = $handle->read(); - $array = json_decode($data); + $array = json_decode($data, true); + if (! is_array($array)) { + throw new Logic('Expected array'); + } $array = array_unique($array); sort($array); $actualCount = count($array); - if (! is_int($actualCount)) { - throw new Logic('Expected int'); - } Assert::assertCount($actualCount, $tableNode->getRows()); diff --git a/tests/Behat/Context/ParallelBehatContext.php b/tests/Behat/Context/ParallelBehatContext.php index c9114c3..a567337 100644 --- a/tests/Behat/Context/ParallelBehatContext.php +++ b/tests/Behat/Context/ParallelBehatContext.php @@ -170,15 +170,7 @@ public function thenPrintLastOutput(): void */ private function processFromShellCommandline($cmd): void { - if (method_exists('\\Symfony\\Component\\Process\\Process', 'fromShellCommandline')) { - $this->process = Process::fromShellCommandline($cmd); - } else { - // BC layer for symfony/process 4.1 and older - // @phpstan-ignore-next-line - $this->process = new Process(null); - // @phpstan-ignore-next-line - $this->process->setCommandLine($cmd); - } + $this->process = Process::fromShellCommandline($cmd); } private function getExitCode(): ?int diff --git a/tests/Behat/Util/ReadWriteDataToFileWithLocking.php b/tests/Behat/Util/ReadWriteDataToFileWithLocking.php index b25d234..2c8bed2 100644 --- a/tests/Behat/Util/ReadWriteDataToFileWithLocking.php +++ b/tests/Behat/Util/ReadWriteDataToFileWithLocking.php @@ -37,6 +37,10 @@ public function read(): string throw new Logic("Can't get file size"); } + if ($fileSize === 0) { + return ''; + } + $data = fread($this->handle, $fileSize); if (! is_string($data)) { throw new Logic("Can't read data"); diff --git a/tests/Cli/SigintControllerTest.php b/tests/Cli/SigintControllerTest.php index 5e12b15..bf2178c 100644 --- a/tests/Cli/SigintControllerTest.php +++ b/tests/Cli/SigintControllerTest.php @@ -24,8 +24,8 @@ protected function setUp(): void public function testConfigure(): void { + $this->command->expects($this->never())->method('addOption'); $this->controller->configure($this->command); - $this->assertTrue(true); } /** diff --git a/tests/Event/EventDispatcherDecoratorTest.php b/tests/Event/EventDispatcherDecoratorTest.php index 05b40ba..a004178 100644 --- a/tests/Event/EventDispatcherDecoratorTest.php +++ b/tests/Event/EventDispatcherDecoratorTest.php @@ -19,18 +19,22 @@ public function testDispatch(): void { $dispatcher = new TestworkEventDispatcher(); $decorator = new EventDispatcherDecorator($dispatcher); - $decorator->dispatch($this->createMock(Event::class), 'some-name'); + $event = $this->createMock(Event::class); + $returned = $decorator->dispatch($event, 'some-name'); - $this->assertTrue(true); + $this->assertSame($event, $returned); } public function testAddListener(): void { $dispatcher = new TestworkEventDispatcher(); $decorator = new EventDispatcherDecorator($dispatcher); - $decorator->addListener('some-name', static function () { + $called = false; + $decorator->addListener('some-name', static function () use (&$called) { + $called = true; }, 123); + $decorator->dispatch($this->createMock(Event::class), 'some-name'); - $this->assertTrue(true); + $this->assertTrue($called); } } diff --git a/tests/Event/ParallelTestCompletedTest.php b/tests/Event/ParallelTestCompletedTest.php index ef07607..24b9065 100644 --- a/tests/Event/ParallelTestCompletedTest.php +++ b/tests/Event/ParallelTestCompletedTest.php @@ -4,12 +4,16 @@ use DMarynicz\BehatParallelExtension\Event\ParallelTestsCompleted; use PHPUnit\Framework\TestCase; +use ReflectionClass; class ParallelTestCompletedTest extends TestCase { public function testParallelTestCompleted(): void { - new ParallelTestsCompleted(); - $this->assertTrue(true); + $event = new ParallelTestsCompleted(); + $reflection = new ReflectionClass($event); + + $this->assertTrue($reflection->hasConstant('COMPLETED')); + $this->assertSame('parallel_extension.parallel_tests_completed', $reflection->getConstant('COMPLETED')); } } diff --git a/tests/Event/ParallelTestsAbortedTest.php b/tests/Event/ParallelTestsAbortedTest.php index e1e077d..bc9af75 100644 --- a/tests/Event/ParallelTestsAbortedTest.php +++ b/tests/Event/ParallelTestsAbortedTest.php @@ -4,12 +4,16 @@ use DMarynicz\BehatParallelExtension\Event\ParallelTestsAborted; use PHPUnit\Framework\TestCase; +use ReflectionClass; class ParallelTestsAbortedTest extends TestCase { public function test(): void { - new ParallelTestsAborted(); - $this->assertTrue(true); + $event = new ParallelTestsAborted(); + $reflection = new ReflectionClass($event); + + $this->assertTrue($reflection->hasConstant('ABORTED')); + $this->assertSame('parallel_extension.parallel_tests_aborted', $reflection->getConstant('ABORTED')); } } diff --git a/tests/ExtensionTest.php b/tests/ExtensionTest.php index 93bae53..8d00587 100644 --- a/tests/ExtensionTest.php +++ b/tests/ExtensionTest.php @@ -7,7 +7,6 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\NodeBuilder; -use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition;