diff --git a/.ddev/commands/web/phpunit b/.ddev/commands/web/phpunit new file mode 100755 index 00000000..28b3c5b6 --- /dev/null +++ b/.ddev/commands/web/phpunit @@ -0,0 +1,18 @@ +#!/usr/bin/env bash + +set -euo pipefail + +## Description: Run PHPUnit tests for the MageForge module +## Usage: phpunit [options] +## Example: ddev phpunit +## Example: ddev phpunit --testdox +## Example: ddev phpunit tests/Unit/Service/Hyva/ + +cd /var/www/html + +if [[ ! -x vendor/bin/phpunit ]]; then + echo "phpunit not found. Installing module dev dependencies..." + composer install --no-interaction +fi + +vendor/bin/phpunit "$@" diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml new file mode 100644 index 00000000..baf97f7a --- /dev/null +++ b/.github/workflows/phpunit.yml @@ -0,0 +1,37 @@ +name: PHPUnit + +on: [pull_request] + +permissions: + contents: read + +env: + PHP_VERSION: "8.4" + +jobs: + phpunit: + name: Unit Tests + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + + - name: Set up PHP + uses: shivammathur/setup-php@accd6127cb78bee3e8082180cb391013d204ef9f # v2 + with: + php-version: ${{ env.PHP_VERSION }} + tools: composer:v2 + + - name: Cache Composer packages + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.composer/cache/files + key: ${{ runner.os }}-composer-phpunit-${{ hashFiles('composer.json') }} + restore-keys: ${{ runner.os }}-composer-phpunit + + - name: Install dev dependencies + run: composer install --no-interaction --no-progress + + - name: Run PHPUnit + run: vendor/bin/phpunit diff --git a/.gitignore b/.gitignore index 732980ce..1a5f69b8 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,9 @@ /vendor/ /composer.lock +# PHPUnit +/.phpunit.cache/ + .vscode /magento/ /magento-temp/ diff --git a/composer.json b/composer.json index 4523a4ce..7a200a8b 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,13 @@ }, "require-dev": { "carthage-software/mago": "^1.30", - "magento/magento-coding-standard": "^40" + "magento/magento-coding-standard": "^40", + "phpunit/phpunit": "^11.0" + }, + "autoload-dev": { + "psr-4": { + "OpenForgeProject\\MageForge\\Test\\": "tests/" + } }, "repositories": [ { diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 00000000..e067aaea --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,17 @@ + + + + + tests/Unit + + + + + src + + + diff --git a/src/Console/Command/Hyva/CompatibilityCheckCommand.php b/src/Console/Command/Hyva/CompatibilityCheckCommand.php index fa6436f4..0fc385b7 100644 --- a/src/Console/Command/Hyva/CompatibilityCheckCommand.php +++ b/src/Console/Command/Hyva/CompatibilityCheckCommand.php @@ -176,7 +176,7 @@ private function runInteractiveMode(InputInterface $input, OutputInterface $outp $this->io->newLine(); // Run scan with selected options - return $this->runScan($showAll, $thirdPartyOnly, $includeVendor, $detailed, $incompatibleOnly, $output); + return $this->runScan($showAll, $thirdPartyOnly, $includeVendor, $detailed, $incompatibleOnly); } catch (\Throwable $e) { $this->io->error('Interactive mode failed: ' . $e->getMessage()); $this->io->info('Falling back to default scan (third-party modules only)...'); @@ -204,7 +204,7 @@ private function runDirectMode(InputInterface $input, OutputInterface $output): $this->io->title('Hyvä Theme Compatibility Check'); - return $this->runScan($showAll, $thirdPartyOnly, $includeVendor, $detailed, false, $output); + return $this->runScan($showAll, $thirdPartyOnly, $includeVendor, $detailed, false); } /** @@ -215,7 +215,6 @@ private function runDirectMode(InputInterface $input, OutputInterface $output): * @param bool $includeVendor * @param bool $detailed * @param bool $incompatibleOnly - * @param OutputInterface $output * @return int */ private function runScan( @@ -224,7 +223,6 @@ private function runScan( bool $includeVendor, bool $detailed, bool $incompatibleOnly, - OutputInterface $output, ): int { // Determine filter logic: // - thirdPartyOnly: Only scan non-Magento_* modules (default behavior) @@ -234,13 +232,7 @@ private function runScan( $excludeVendor = false; // Run the compatibility check - $results = $this->compatibilityChecker->check( - $this->io, - $output, - $showAll, - $scanThirdPartyOnly, - $excludeVendor, - ); + $results = $this->compatibilityChecker->check($this->io, $showAll, $scanThirdPartyOnly, $excludeVendor); // Determine display mode: // showAll = show all modules including compatible ones @@ -310,7 +302,7 @@ private function displayDetailedIssues(array $results): void $this->io->text(sprintf('%s', $moduleName)); - $detailedIssues = $this->compatibilityChecker->getDetailedIssues($moduleName, $moduleData); + $detailedIssues = $this->compatibilityChecker->getDetailedIssues($moduleData); foreach ($detailedIssues as $fileData) { $this->io->text(sprintf(' %s', $fileData['file'])); diff --git a/src/Service/Hyva/CompatibilityChecker.php b/src/Service/Hyva/CompatibilityChecker.php index 55f979f3..5abf1e54 100644 --- a/src/Service/Hyva/CompatibilityChecker.php +++ b/src/Service/Hyva/CompatibilityChecker.php @@ -6,7 +6,6 @@ use Magento\Framework\Component\ComponentRegistrar; use Magento\Framework\Component\ComponentRegistrarInterface; -use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; /** @@ -54,7 +53,6 @@ public function __construct( * Check all modules for Hyvä compatibility * * @param SymfonyStyle $io Symfony Style IO for output - * @param OutputInterface $output Console output interface * @param bool $showAll Whether to show all modules (including compatible ones) * @param bool $thirdPartyOnly Whether to scan only third-party modules (excludes Magento_* modules) * @param bool $excludeVendor Whether to exclude modules from the vendor/ directory @@ -64,7 +62,6 @@ public function __construct( */ public function check( SymfonyStyle $io, - OutputInterface $output, bool $showAll = false, bool $thirdPartyOnly = false, bool $excludeVendor = true, @@ -116,13 +113,18 @@ public function check( ]; // Update summary - if ($isCompatible && !$hasWarnings) { + if ($isCompatible) { $results['summary']['compatible']++; } else { $results['summary']['incompatible']++; $results['hasIncompatibilities'] = true; } + // Warnings alone still trigger the detail/recommendation display + if ($hasWarnings) { + $results['hasIncompatibilities'] = true; + } + if ($moduleInfo['isHyvaAware']) { $results['summary']['hyvaAware']++; } @@ -195,19 +197,17 @@ public function formatResultsForDisplay(array $results, bool $showAll = false): */ private function getStatusDisplay(array $moduleData): string { - if ($moduleData['moduleInfo']['isHyvaAware']) { - return '✓ Hyvä-Aware'; - } + $hyvaTag = $moduleData['moduleInfo']['isHyvaAware'] ? ' (Hyvä-Aware)' : ''; if ($moduleData['compatible'] && !$moduleData['hasWarnings']) { - return '✓ Compatible'; + return $moduleData['moduleInfo']['isHyvaAware'] ? '✓ Hyvä-Aware' : '✓ Compatible'; } if ($moduleData['compatible']) { - return '⚠ Warnings'; + return sprintf('⚠ Warnings%s', $hyvaTag); } - return '✗ Incompatible'; + return sprintf('✗ Incompatible%s', $hyvaTag); } /** @@ -243,12 +243,11 @@ private function getIssuesDisplay(array $moduleData): string /** * Get detailed file issues for a module * - * @param string $moduleName * @param array $moduleData * @phpstan-param ModuleEntry $moduleData * @return array}> */ - public function getDetailedIssues(string $moduleName, array $moduleData): array + public function getDetailedIssues(array $moduleData): array { $scanResult = $moduleData['scanResult']; $files = $scanResult['files']; diff --git a/src/Service/Hyva/IncompatibilityDetector.php b/src/Service/Hyva/IncompatibilityDetector.php index c3be3133..6c3e6922 100644 --- a/src/Service/Hyva/IncompatibilityDetector.php +++ b/src/Service/Hyva/IncompatibilityDetector.php @@ -35,7 +35,12 @@ class IncompatibilityDetector 'severity' => self::SEVERITY_CRITICAL, ], [ - 'pattern' => '/ko\.observable|ko\.observableArray|ko\.computed/', + 'pattern' => '/\bko\.(observable|observableArray|computed|pureComputed)\b/', + 'description' => 'Knockout.js usage', + 'severity' => self::SEVERITY_CRITICAL, + ], + [ + 'pattern' => '/\bko\.(applyBindings|components|bindingHandlers)\b/', 'description' => 'Knockout.js usage', 'severity' => self::SEVERITY_CRITICAL, ], @@ -78,6 +83,11 @@ class IncompatibilityDetector 'description' => 'data-mage-init JavaScript initialization', 'severity' => self::SEVERITY_CRITICAL, ], + [ + 'pattern' => '/data-bind\b\s*=/', + 'description' => 'Knockout.js data-bind attribute', + 'severity' => self::SEVERITY_CRITICAL, + ], [ 'pattern' => '/x-magento-init/', 'description' => 'x-magento-init JavaScript initialization', @@ -93,6 +103,11 @@ class IncompatibilityDetector 'description' => 'RequireJS in template', 'severity' => self::SEVERITY_CRITICAL, ], + [ + 'pattern' => '/", + 'Knockout.js virtual element binding', + 'critical', + ], + 'ko virtual element no space' => [ + '', + 'Knockout.js virtual element binding', + 'critical', + ], + ]; + } + + public function testDoesNotFlagCleanHyvaPhtml(): void + { + $cleanPhtml = <<<'PHTML' + +
+ + +
+ + PHTML; + + $this->fileMock->method('fileGetContents')->willReturn($cleanPhtml); + $issues = $this->detector->detectInFile('product-view.phtml'); + $this->assertEmpty($issues, 'Clean Hyvä/Alpine.js template must not trigger any issues'); + } + + // ------------------------------------------------------------------------- + // HTML template patterns (Knockout component templates) + // ------------------------------------------------------------------------- + + #[DataProvider('incompatibleHtmlProvider')] + public function testDetectsIncompatibleHtmlPattern( + string $content, + string $expectedDescription, + ): void { + $this->fileMock->method('fileGetContents')->willReturn($content); + $issues = $this->detector->detectInFile('view/frontend/web/template/listing.html'); + $this->assertIssueFound($issues, $expectedDescription, 'critical'); + } + + /** + * @return array + */ + public static function incompatibleHtmlProvider(): array + { + return [ + 'data-bind in html template' => [ + '
', + 'Knockout.js data-bind attribute', + ], + 'ko virtual element in html' => [ + '

Hello

', + 'Knockout.js virtual element binding', + ], + 'x-magento-init in html' => [ + '', + 'x-magento-init JavaScript initialization', + ], + ]; + } + + // ------------------------------------------------------------------------- + // Edge cases + // ------------------------------------------------------------------------- + + public function testReturnsEmptyArrayWhenFileNotExists(): void + { + $this->fileMock->method('isExists')->willReturn(false); + $issues = $this->detector->detectInFile('nonexistent.js'); + $this->assertEmpty($issues); + } + + public function testReturnsEmptyArrayForUnknownExtension(): void + { + $this->fileMock->method('fileGetContents')->willReturn("define(['jquery'], function() {});"); + $issues = $this->detector->detectInFile('script.coffee'); + $this->assertEmpty($issues, 'Unknown file extensions must be ignored'); + } + + public function testReturnsEmptyArrayOnFileReadError(): void + { + $this->fileMock->method('fileGetContents')->willThrowException(new \RuntimeException('Read error')); + $issues = $this->detector->detectInFile('test.js'); + $this->assertEmpty($issues, 'File read errors must be handled gracefully'); + } + + public function testReportsCorrectLineNumbers(): void + { + $content = "const x = 1;\nconst y = 2;\nko.applyBindings(viewModel);\nconst z = 3;"; + $this->fileMock->method('fileGetContents')->willReturn($content); + + $issues = $this->detector->detectInFile('test.js'); + + $this->assertNotEmpty($issues); + $this->assertSame(3, $issues[0]['line'], 'Line number must be 1-based'); + } + + // ------------------------------------------------------------------------- + // Helper + // ------------------------------------------------------------------------- + + /** + * @param array> $issues + */ + private function assertIssueFound(array $issues, string $description, string $severity): void + { + $this->assertNotEmpty( + $issues, + sprintf('Expected issue "%s" but no issues were detected at all', $description), + ); + + $found = array_filter( + $issues, + static fn(array $issue): bool => $issue['description'] === $description, + ); + + $this->assertNotEmpty( + $found, + sprintf( + 'Expected issue "%s" not found. Detected: %s', + $description, + implode(', ', array_map('strval', array_column($issues, 'description'))), + ), + ); + + $issue = array_values($found)[0]; + $this->assertSame( + $severity, + (string) $issue['severity'], + sprintf('Issue "%s" expected severity "%s" but got "%s"', $description, $severity, $issue['severity']), + ); + } +}