From b4fc41e8bbf0598c5747b293f5312394a8197137 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:32:53 +0200 Subject: [PATCH 1/6] fix(pages): make waitForPageLoaded() actually wait, stop __call swallowing typos waitForPageLoaded() called $this->waitForNavigation(), which CommonPage does not define. __call only forwards to the chrome-php Page, which does not have it either (it lives on PageNavigation), and it dropped anything it could not forward. So the method returned immediately and every caller believed the page was loaded. goToProduct() and the "[Debug] This page has moved" fallback in FrontOfficePage::goToPage() made the same dead call. waitForPageLoaded() now polls document.readyState through waitForJsCondition() until the DOM is parsed and has a body, takes an optional timeout (30s by default) and throws TimeoutException when it runs out. The dead calls now go through it. __call now throws BadMethodCallException for a name neither the page object nor the chrome-php Page defines, so the next such mistake fails the step instead of passing silently. It also returns what the proxied method returned rather than null. With no browser page yet, a genuine chrome-php method is still a no-op, as before. Co-Authored-By: Claude Opus 5.5 --- src/Pages/CommonPage.php | 39 +++++++- src/Pages/FrontOfficePage.php | 4 +- src/Pages/v9/FrontOffice/Product/Page.php | 2 +- tests/Unit/Pages/WaitForPageLoadedTest.php | 107 +++++++++++++++++++++ 4 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/Pages/WaitForPageLoadedTest.php diff --git a/src/Pages/CommonPage.php b/src/Pages/CommonPage.php index d6207c9..781160c 100644 --- a/src/Pages/CommonPage.php +++ b/src/Pages/CommonPage.php @@ -125,8 +125,23 @@ public function __call($name, $arguments) $page = $this->getPage(); if (!is_null($page) && method_exists($page, $name)) { - call_user_func_array([$page, $name], $arguments); + return call_user_func_array([$page, $name], $arguments); } + + // No browser page yet: keep the historical no-op for real chrome-php + // methods, but never for a name nobody defines. + if (is_null($page) && method_exists(DomPage::class, $name)) { + return null; + } + + // A typo or a method that lives elsewhere (e.g. waitForNavigation(), which + // belongs to PageNavigation, not Page) used to be swallowed silently, so + // the step "passed" without doing anything. + throw new \BadMethodCallException(sprintf( + 'Call to undefined method %s::%s() (not defined on the page object nor on the chrome-php page).', + static::class, + $name + )); } public function setGlobals($globals) @@ -546,9 +561,27 @@ public function goToUrl(string $url) $this->getPage()->navigate($url)->waitForNavigation(DomPage::DOM_CONTENT_LOADED); } - public function waitForPageLoaded() + /** + * Wait until the current document has been parsed (DOMContentLoaded, i.e. + * readyState "interactive" or "complete") and has a . + * + * This polls the page state; it does not observe a navigation. Right after + * an action that triggers one, the previous document may still report + * itself as ready, so prefer waiting on navigate()->waitForNavigation() or + * on an element of the next page when you have one. + * + * @throws TimeoutException when the page is still loading after $timeout ms + */ + public function waitForPageLoaded(int $timeout = 30000): void { - $this->waitForNavigation(DomPage::DOM_CONTENT_LOADED, 10000); + $loaded = $this->waitForJsCondition( + "document.readyState !== 'loading' && !!document.body", + $timeout + ); + + if (!$loaded) { + throw new TimeoutException(sprintf('Page did not finish loading within %d ms.', $timeout)); + } } public function getTextContent($selector, $index = 1, $waitForSelector = true, $timeout = 3000) diff --git a/src/Pages/FrontOfficePage.php b/src/Pages/FrontOfficePage.php index 0f3bf12..8faf34b 100644 --- a/src/Pages/FrontOfficePage.php +++ b/src/Pages/FrontOfficePage.php @@ -75,12 +75,12 @@ public function goToPage($page = null, $params = null) if ($hasDebugMoved) { Expect::setWarning('debug-mode'); $this->click('a'); - $this->waitForNavigation(); + $this->waitForPageLoaded(); } } catch (OperationTimedOut | Exception $e) { Expect::setWarning('debug-mode'); $this->click('a'); - $this->waitForNavigation(); + $this->waitForPageLoaded(); } } diff --git a/src/Pages/v9/FrontOffice/Product/Page.php b/src/Pages/v9/FrontOffice/Product/Page.php index d409ff1..362b911 100644 --- a/src/Pages/v9/FrontOffice/Product/Page.php +++ b/src/Pages/v9/FrontOffice/Product/Page.php @@ -33,7 +33,7 @@ public function goToProduct(int $productId = 0) { $this->goToPage('product', $productId); - $this->waitForNavigation(); + $this->waitForPageLoaded(); } /** diff --git a/tests/Unit/Pages/WaitForPageLoadedTest.php b/tests/Unit/Pages/WaitForPageLoadedTest.php new file mode 100644 index 0000000..46c999e --- /dev/null +++ b/tests/Unit/Pages/WaitForPageLoadedTest.php @@ -0,0 +1,107 @@ +fakeDomPage([false, true]); + + $this->makePage($fakePage)->waitForPageLoaded(1000); + + $this->assertCount(2, $fakePage->evaluated, 'waitForPageLoaded() should poll the page until it is ready'); + $this->assertStringContainsString('document.readyState', $fakePage->evaluated[0]); + } + + public function testWaitForPageLoadedThrowsWhenThePageNeverFinishesLoading(): void + { + $fakePage = $this->fakeDomPage([false]); + + $this->expectException(TimeoutException::class); + + $this->makePage($fakePage)->waitForPageLoaded(50); + } + + public function testUnknownMethodIsNotSilentlySwallowed(): void + { + $page = $this->makePage($this->fakeDomPage([true])); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('methodThatDoesNotExist'); + + $page->methodThatDoesNotExist(); + } + + public function testKnownChromePageMethodIsStillProxied(): void + { + $fakePage = $this->fakeDomPage([true]); + + $result = $this->makePage($fakePage)->waitUntilContainsElement('#main', 10); + + $this->assertSame([['#main', 10]], $fakePage->waited); + $this->assertSame($fakePage, $result, '__call should hand back what the chrome-php page returned'); + } + + private function makePage(object $fakePage): CommonPage + { + return new class ($fakePage) extends CommonPage { + private object $fakePage; + + public function __construct(object $fakePage) + { + $this->fakePage = $fakePage; + } + + public function getPage() + { + return $this->fakePage; + } + }; + } + + /** + * Fake chrome-php Page: evaluate() records the expression and returns the + * next scripted readiness value (the last one repeats). + */ + private function fakeDomPage(array $readiness): object + { + return new class ($readiness) { + public array $evaluated = []; + public array $waited = []; + + public function __construct(private array $readiness) + { + } + + public function evaluate($js) + { + $this->evaluated[] = $js; + $value = count($this->readiness) > 1 ? array_shift($this->readiness) : $this->readiness[0]; + + return new class ($value) { + public function __construct(private $value) + { + } + + public function getReturnValue($timeout = null) + { + return $this->value; + } + }; + } + + public function waitUntilContainsElement($selector, int $timeout = 30000) + { + $this->waited[] = [$selector, $timeout]; + + return $this; + } + }; + } +} From 5bec8695d68681656853d6120cf133fce16959ac Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:34:45 +0200 Subject: [PATCH 2/6] fix(cli): exit non-zero when a run throws bin/prestaflow listened to ConsoleEvents::ERROR to print its ERROR/TRACE lines, then called setExitCode(Command::SUCCESS). Symfony treats an exit code of 0 from that event as "handled" and drops the error, so a run that could not even start (Chrome failing to launch, a suites path that does not exist, a mistyped command) ended with status 0 and turned the CI job green. The application now lives in PrestaFlow\Library\Console\Application, which the binary merely runs, so its exit codes can be tested. The ERROR/TRACE rendering moved into doRenderThrowable(); Symfony's own handling then turns the error into exit code 1 (or the exception's code). catchErrors is on, because most failures here are \Error and would otherwise escape as a PHP fatal. Assertion failures still exit 1 through ExecuteSuite's return value, and a clean run still exits 0. The ERROR/TRACE lines now go to stderr, where Symfony writes rendered errors. Co-Authored-By: Claude Opus 5.5 --- bin/prestaflow | 48 +-------------- src/Console/Application.php | 40 ++++++++++++ tests/Unit/Command/ExitCodeTest.php | 95 +++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 45 deletions(-) create mode 100644 src/Console/Application.php create mode 100644 tests/Unit/Command/ExitCodeTest.php diff --git a/bin/prestaflow b/bin/prestaflow index 3fff636..ec99468 100755 --- a/bin/prestaflow +++ b/bin/prestaflow @@ -28,48 +28,6 @@ if (!$autoloadLoaded) { exit(1); } -use PrestaFlow\Library\Command\ExecuteSuite; -use Symfony\Component\Console\Application; -use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\ConsoleEvents; -use Symfony\Component\Console\Event\ConsoleErrorEvent; -use Symfony\Component\Console\Style\SymfonyStyle; -use Symfony\Component\EventDispatcher\EventDispatcher; - -$dispatcher = new EventDispatcher(); - -$dispatcher->addListener(ConsoleEvents::ERROR, function (ConsoleErrorEvent $event): void { - - $input = $event->getInput(); - $output = $event->getOutput(); - - $io = new SymfonyStyle($input, $output); - - $command = $event->getCommand(); - - $error = $event->getError(); - - $io->newLine(); - - $io->writeln(sprintf('ERROR %s', $error->getMessage())); - $io->writeln(sprintf('TRACE %s', $error->getFile() . ':' . $error->getLine())); - - foreach ($error->getTrace() as $trace) { - $io->writeln(sprintf('TRACE %s', ($trace['file'] ?? '[internal]') . ':' . ($trace['line'] ?? '?'))); - } - - // gets the current exit code (the exception code) - $exitCode = $event->getExitCode(); - - $event->setExitCode(Command::SUCCESS); - - // changes the exception to another one - $event->setError(new \LogicException('Caught exception', $exitCode, $event->getError())); -}); - -$application = new Application(); - -$application->add(new ExecuteSuite()); -$application->setDispatcher($dispatcher); - -$application->run(); +// Toute la configuration vit dans PrestaFlow\Library\Console\Application (testée) : +// une exception non rattrapée y donne un code de sortie non nul. +(new PrestaFlow\Library\Console\Application())->run(); diff --git a/src/Console/Application.php b/src/Console/Application.php new file mode 100644 index 0000000..5c70e3f --- /dev/null +++ b/src/Console/Application.php @@ -0,0 +1,40 @@ +add(new ExecuteSuite()); + + // Most failures inside a run are \Error (resolveSuitePaths() throws one): + // without this they would escape run() as a PHP fatal error. + $this->setCatchErrors(true); + } + + protected function doRenderThrowable(\Throwable $e, OutputInterface $output): void + { + $output->writeln(sprintf('ERROR %s', $e->getMessage())); + $output->writeln(sprintf('TRACE %s', $e->getFile() . ':' . $e->getLine())); + + foreach ($e->getTrace() as $trace) { + $output->writeln(sprintf('TRACE %s', ($trace['file'] ?? '[internal]') . ':' . ($trace['line'] ?? '?'))); + } + } +} diff --git a/tests/Unit/Command/ExitCodeTest.php b/tests/Unit/Command/ExitCodeTest.php new file mode 100644 index 0000000..b464ac2 --- /dev/null +++ b/tests/Unit/Command/ExitCodeTest.php @@ -0,0 +1,95 @@ +previousCwd = getcwd(); + $this->tmpDir = sys_get_temp_dir() . '/prestaflow-exit-' . bin2hex(random_bytes(6)); + mkdir($this->tmpDir . '/empty', 0777, true); + chdir($this->tmpDir); + } + + protected function tearDown(): void + { + chdir($this->previousCwd); + exec('rm -rf ' . escapeshellarg($this->tmpDir)); + } + + public function testAMissingSuitesPathExitsNonZero(): void + { + $tester = $this->tester(); + + $exitCode = $this->runCommand($tester, ['command' => 'run', 'folder' => $this->tmpDir . '/does-not-exist']); + + $this->assertNotSame(0, $exitCode); + $this->assertStringContainsString('ERROR', $tester->getErrorOutput()); + $this->assertStringContainsString('does-not-exist', $tester->getErrorOutput()); + } + + public function testAnUnknownCommandExitsNonZero(): void + { + $this->assertNotSame(0, $this->runCommand($this->tester(), ['command' => 'no-such-command'])); + } + + public function testAnEmptySuitesFolderStillExitsZero(): void + { + $this->assertSame(0, $this->runCommand($this->tester(), ['command' => 'run', 'folder' => $this->tmpDir . '/empty'])); + } + + /** + * End to end on the real entry point, so the binary cannot drift away from + * the application it is supposed to run. + */ + public function testTheBinaryExitsNonZeroOnAMissingSuitesPath(): void + { + $bin = dirname(__DIR__, 3) . '/bin/prestaflow'; + + $process = proc_open( + [PHP_BINARY, $bin, 'run', $this->tmpDir . '/does-not-exist'], + [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes, + $this->tmpDir + ); + $stdout = stream_get_contents($pipes[1]); + $stderr = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + $exitCode = proc_close($process); + + $this->assertNotSame(0, $exitCode, "stdout:\n" . $stdout . "\nstderr:\n" . $stderr); + $this->assertStringContainsString('does-not-exist', $stdout . $stderr); + } + + /** + * ExecuteSuite writes to console sections, which only a ConsoleOutput + * offers: capturing stderr separately gives the tester one. + */ + private function runCommand(ApplicationTester $tester, array $input): int + { + return $tester->run($input, ['capture_stderr_separately' => true]); + } + + private function tester(): ApplicationTester + { + $application = new Application(); + $application->setAutoExit(false); + + return new ApplicationTester($application); + } +} From 41206d24ff12cc60c407412f19064e3ab9496b2b Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:35:13 +0200 Subject: [PATCH 3/6] fix(tests): read PRESTAFLOW_EXTRA_HEADERS from the process environment too presetExtraHeadersFromEnv() read $_ENV directly. When PHP runs with a variables_order without "E", the usual case on CI, a variable set by the runner (a GitHub Actions env: block, for instance) never reaches $_ENV, so the headers were silently dropped: a WAF bypass header configured in CI did nothing and the run hit the challenge page instead. It now goes through Env::get(), like every other PRESTAFLOW_* read: $_ENV first (values loaded from .env), then getenv(). It was the last direct read of a PRESTAFLOW_* variable in src/; the remaining $_ENV writes in TestsSuite normalise booleans after reading through Env. Co-Authored-By: Claude Opus 5.5 --- src/Tests/TestsSuite.php | 2 +- tests/Unit/Tests/ExtraHeadersFromEnvTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Tests/TestsSuite.php b/src/Tests/TestsSuite.php index 4e5af19..551663e 100644 --- a/src/Tests/TestsSuite.php +++ b/src/Tests/TestsSuite.php @@ -635,7 +635,7 @@ protected function presetBasicAuth(): void */ protected function presetExtraHeadersFromEnv(): void { - $raw = $_ENV['PRESTAFLOW_EXTRA_HEADERS'] ?? null; + $raw = Env::get('PRESTAFLOW_EXTRA_HEADERS'); if ($raw === null || $raw === '') { return; } diff --git a/tests/Unit/Tests/ExtraHeadersFromEnvTest.php b/tests/Unit/Tests/ExtraHeadersFromEnvTest.php index 18c0808..d9ee38f 100644 --- a/tests/Unit/Tests/ExtraHeadersFromEnvTest.php +++ b/tests/Unit/Tests/ExtraHeadersFromEnvTest.php @@ -36,6 +36,7 @@ protected function tearDown(): void } } TestsSuite::$extraHttpHeaders = $this->headersBackup; + putenv('PRESTAFLOW_EXTRA_HEADERS'); } private function invoke(): void @@ -124,4 +125,18 @@ public function testEmptyKeyIsRejected(): void $this->invoke(); $this->assertSame(['X-Ok' => 'v'], TestsSuite::$extraHttpHeaders); } + + /** + * CI runners set env vars on the process; with a variables_order without + * "E" they never reach $_ENV, only getenv() sees them. + */ + public function testProcessEnvironmentIsReadWhenAbsentFromSuperglobal(): void + { + unset($_ENV['PRESTAFLOW_EXTRA_HEADERS']); + putenv('PRESTAFLOW_EXTRA_HEADERS={"X-CI-Bypass":"from-process"}'); + + $this->invoke(); + + $this->assertSame(['X-CI-Bypass' => 'from-process'], TestsSuite::$extraHttpHeaders); + } } From 1562b922a0a98f7161b91835aec19ad4e1d222a5 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:36:13 +0200 Subject: [PATCH 4/6] fix(composer): expose bin/prestaflow as a Composer binary composer.json had no "bin" key, so installing the library as a dependency never created vendor/bin/prestaflow: the command every guide tells users to run did not exist, and projects had to call the script inside vendor/prestaflow/php-library/bin themselves. bin/prestaflow already found the right autoloader in both installed layouts, through $_composer_autoload_path (set by Composer's vendor/bin proxy) and through ../../../autoload.php (direct call inside vendor/). The new test pins that down for both, running from an unrelated directory so the getcwd() fallback cannot be what makes them pass. Checked by hand too, with a throwaway project requiring the library from a path repository. Co-Authored-By: Claude Opus 5.5 --- composer.json | 3 + tests/Unit/Command/BinaryInstallTest.php | 94 ++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 tests/Unit/Command/BinaryInstallTest.php diff --git a/composer.json b/composer.json index 71bc1a1..e80fe7c 100644 --- a/composer.json +++ b/composer.json @@ -3,6 +3,9 @@ "description": "PrestaFlow is an open-source set of prebuilt tests components, ready-to-use examples made for the PrestaShop E-commerce software.", "type": "library", "license": "MIT", + "bin": [ + "bin/prestaflow" + ], "autoload": { "psr-4": { "PrestaFlow\\Library\\": "src/", diff --git a/tests/Unit/Command/BinaryInstallTest.php b/tests/Unit/Command/BinaryInstallTest.php new file mode 100644 index 0000000..125ca3d --- /dev/null +++ b/tests/Unit/Command/BinaryInstallTest.php @@ -0,0 +1,94 @@ +libRoot = dirname(__DIR__, 3); + $this->project = sys_get_temp_dir() . '/prestaflow-install-' . bin2hex(random_bytes(6)); + + // Installed layout: /vendor/prestaflow/php-library/bin/prestaflow. + mkdir($this->project . '/vendor/prestaflow/php-library/bin', 0777, true); + mkdir($this->project . '/vendor/bin', 0777, true); + copy($this->libRoot . '/bin/prestaflow', $this->project . '/vendor/prestaflow/php-library/bin/prestaflow'); + + // The project's autoloader: leaves a marker, then delegates to the real + // one so the library classes resolve. + file_put_contents($this->project . '/vendor/autoload.php', sprintf( + "libRoot . '/vendor/autoload.php', true) + )); + } + + protected function tearDown(): void + { + exec('rm -rf ' . escapeshellarg($this->project)); + } + + public function testComposerJsonExposesTheBinary(): void + { + $composer = json_decode(file_get_contents($this->libRoot . '/composer.json'), true); + + $this->assertSame(['bin/prestaflow'], $composer['bin'] ?? null); + } + + public function testInstalledBinaryLoadsTheProjectAutoloader(): void + { + [$exitCode, $output] = $this->runPhp($this->project . '/vendor/prestaflow/php-library/bin/prestaflow'); + + $this->assertSame(0, $exitCode, $output); + $this->assertStringContainsString('PROJECT_AUTOLOAD', $output); + $this->assertStringContainsString('run', $output); + } + + /** + * Composer's vendor/bin proxy sets $_composer_autoload_path, then includes + * the real script. + */ + public function testComposerBinProxyLoadsTheProjectAutoloader(): void + { + $proxy = $this->project . '/vendor/bin/prestaflow'; + file_put_contents($proxy, "runPhp($proxy); + + $this->assertSame(0, $exitCode, $output); + $this->assertStringContainsString('PROJECT_AUTOLOAD', $output); + $this->assertStringContainsString('run', $output); + } + + /** + * @return array{0: int, 1: string} + */ + private function runPhp(string $script): array + { + // Run from an unrelated directory: the getcwd() fallback must not be + // what makes these pass. + $process = proc_open( + [PHP_BINARY, $script, 'list', '--raw'], + [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes, + sys_get_temp_dir() + ); + $output = stream_get_contents($pipes[1]) . stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + + return [proc_close($process), $output]; + } +} From 46149a2f583d471824b3f17d58e9fb8eb74ed3c0 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:53:54 +0200 Subject: [PATCH 5/6] feat(cli): run only the suite folders named by --suites / PRESTAFLOW_SUITES The GitHub Action turns its `suites` input into PRESTAFLOW_SUITES, but the library never read it: a workflow asking for BackOffice ran every suite. `prestaflow run ` now takes the comma-separated names from --suites, or from PRESTAFLOW_SUITES (read through Env::get) when the option is absent, as sub-folders of . Nested paths such as FrontOffice/Checkout are allowed, each folder is scanned recursively as before, and the run takes the union without duplicates. --group and --draft then apply to what is left. A name that matches no folder fails the run with the missing names and the available sub-folders: a typo in a workflow must not become a green job that ran zero tests. Absolute paths and ".." are refused, since the filter is only meant to narrow . Combining a filter with a single suite file is an error for the same reason. Unset or empty, nothing changes. Co-Authored-By: Claude Opus 5.5 --- README.md | 21 +++ src/Command/ExecuteSuite.php | 94 +++++++++++- tests/Unit/Command/SuitesFilterTest.php | 186 ++++++++++++++++++++++++ 3 files changed, 298 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/Command/SuitesFilterTest.php diff --git a/README.md b/README.md index 0799320..7473fcd 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,27 @@ Resolution priority: fluent `onVersion()` → `$psVersion` property → `PRESTAF `onVersion()` throws `InvalidArgumentException` on malformed input (expected format: `1.7`, `1.7.8`, `1.7.8.11`, `9`, `9.0`, `9.0.1`, etc.). +## Run only some suite folders + +`prestaflow run ` runs every suite under ``. To run only some of +its sub-folders, list them, comma-separated, relative to ``: + +```bash +./vendor/bin/prestaflow run tests --suites=BackOffice,FrontOffice/Checkout +PRESTAFLOW_SUITES=BackOffice,FrontOffice/Checkout ./vendor/bin/prestaflow run tests +``` + +- Each name is a sub-folder of ``, scanned recursively; nested paths such + as `FrontOffice/Checkout` are allowed. The run takes the union of the folders. +- `--suites` wins over `PRESTAFLOW_SUITES`. The GitHub Action sets + `PRESTAFLOW_SUITES` from its `suites` input. +- A name that matches no folder fails the run (non-zero exit code), listing the + missing names and the available sub-folders. A typo never becomes a green + job that ran zero tests. +- Absolute paths and `..` are refused: the filter can only narrow ``. +- `--group` and `--draft` still apply, on the suites of the selected folders. +- Unset or empty: every suite under `` runs, as before. + ## Run a suite against a throwaway shop `docker-compose.yml` boots a disposable PrestaShop from the official diff --git a/src/Command/ExecuteSuite.php b/src/Command/ExecuteSuite.php index b9afe06..68cec7d 100644 --- a/src/Command/ExecuteSuite.php +++ b/src/Command/ExecuteSuite.php @@ -55,6 +55,7 @@ protected function configure(): void ->addOption('visual-report', null, InputOption::VALUE_OPTIONAL, 'Écrit un rapport de régression visuelle (HTML)', false) ->addOption('visual-report-tz', null, InputOption::VALUE_REQUIRED, 'Fuseau horaire du stamp du rapport visuel (ex. Europe/Brussels). Défaut : env PRESTAFLOW_TZ ou UTC.') ->addOption('draft', 'd', InputOption::VALUE_NEGATABLE, 'Draft mode') + ->addOption('suites', null, InputOption::VALUE_REQUIRED, 'Comma-separated sub-folders of the suites path to run (e.g. BackOffice,FrontOffice/Checkout). Overrides env PRESTAFLOW_SUITES.') ->addArgument('folder', InputArgument::OPTIONAL, 'The folder name', 'tests') ->addOption( 'group', @@ -138,7 +139,11 @@ public function execute(InputInterface $input, OutputInterface $output): int $junitPath = ($junitOption === false) ? null : ($junitOption ?: 'prestaflow/junit.xml'); try { - $testSuites = $this->resolveSuitePaths((string) $input->getArgument('folder')); + // --suites wins over PRESTAFLOW_SUITES (set by the GitHub Action). + $suitesFilter = $this->parseSuitesFilter( + $input->getOption('suites') ?? Env::get('PRESTAFLOW_SUITES') + ); + $testSuites = $this->resolveSuitePaths((string) $input->getArgument('folder'), $suitesFilter); } catch (Error $e) { $this->sections['progressIndicator']->finish('Finished'); $this->sections['progressBar']->clear(); @@ -443,14 +448,24 @@ public function candidatePaths(string $argument): array * * @throws Error when the argument matches neither a directory nor a suite file */ - public function resolveSuitePaths(string $argument): array + public function resolveSuitePaths(string $argument, array $subFolders = []): array { foreach ($this->candidatePaths($argument) as $path) { if (is_dir($path)) { - return $this->getTestsSuites($path); + return $subFolders === [] + ? $this->getTestsSuites($path) + : $this->getTestsSuitesIn($path, $subFolders); } if (is_file($path)) { + if ($subFolders !== []) { + throw new Error(sprintf( + '[%s] is a single suite file: a suites filter (%s) needs a folder', + $path, + implode(', ', $subFolders) + )); + } + if (!str_ends_with($path, '.php')) { throw new Error(sprintf('[%s] is not a PHP suite file', $path)); } @@ -462,6 +477,79 @@ public function resolveSuitePaths(string $argument): array throw new Error(sprintf('The suites path [%s] doesn\'t seem to exist', $argument)); } + /** + * Parse a suites filter (--suites or PRESTAFLOW_SUITES): comma-separated + * sub-folders of the suites path, nested ones allowed (FrontOffice/Checkout). + * Names are trimmed, empty ones dropped, duplicates removed. + * + * @return array an empty array means "no filter" + * + * @throws Error on an absolute path or a `..` segment: the filter may only + * narrow the suites path, never leave it + */ + public function parseSuitesFilter(?string $raw): array + { + $names = []; + foreach (explode(',', (string) $raw) as $name) { + $name = trim(str_replace('\\', '/', $name)); + if ($name === '') { + continue; + } + + if (str_starts_with($name, '/') || preg_match('#^[A-Za-z]:#', $name)) { + throw new Error(sprintf('Suites filter [%s]: absolute paths are not allowed, name a sub-folder of the suites path', $name)); + } + + $segments = array_values(array_filter(explode('/', $name), fn ($segment) => $segment !== '' && $segment !== '.')); + if (in_array('..', $segments, true)) { + throw new Error(sprintf('Suites filter [%s]: ".." is not allowed, name a sub-folder of the suites path', $name)); + } + if ($segments === []) { + continue; + } + + $names[] = implode('/', $segments); + } + + return array_values(array_unique($names)); + } + + /** + * Suites of the given sub-folders of $root (the union, without duplicates). + * + * @return array + * + * @throws Error when a name matches no folder, so that a filter typo never + * turns into a green run of zero tests + */ + public function getTestsSuitesIn(string $root, array $subFolders): array + { + $missing = array_values(array_filter($subFolders, fn ($name) => !is_dir($root . '/' . $name))); + if ($missing !== []) { + $available = array_values(array_filter( + scandir($root) ?: [], + fn ($entry) => $entry !== '.' && $entry !== '..' && is_dir($root . '/' . $entry) + )); + sort($available); + + throw new Error(sprintf( + 'Suites filter: no folder [%s] under [%s]. Available sub-folders: %s', + implode(', ', $missing), + $root, + $available === [] ? '(none)' : implode(', ', $available) + )); + } + + $testSuites = []; + foreach ($subFolders as $name) { + foreach ($this->getTestsSuites($root . '/' . $name) as $suitePath) { + $testSuites[] = $suitePath; + } + } + + return array_values(array_unique($testSuites)); + } + public function getTestsSuites($folderPath) { $testSuites = []; diff --git a/tests/Unit/Command/SuitesFilterTest.php b/tests/Unit/Command/SuitesFilterTest.php new file mode 100644 index 0000000..62e6dd4 --- /dev/null +++ b/tests/Unit/Command/SuitesFilterTest.php @@ -0,0 +1,186 @@ +tmpDir = sys_get_temp_dir() . '/prestaflow-suites-' . bin2hex(random_bytes(6)); + $this->root = $this->tmpDir . '/Suites'; + + foreach (['BackOffice', 'FrontOffice/Checkout', 'Empty'] as $dir) { + mkdir($this->root . '/' . $dir, 0777, true); + } + foreach (['Top.php', 'BackOffice/Login.php', 'FrontOffice/Home.php', 'FrontOffice/Checkout/Guest.php'] as $file) { + file_put_contents($this->root . '/' . $file, 'previousCwd = getcwd(); + chdir($this->tmpDir); + + $this->previousEnv = [ + 'env' => $_ENV['PRESTAFLOW_SUITES'] ?? null, + 'process' => getenv('PRESTAFLOW_SUITES'), + ]; + unset($_ENV['PRESTAFLOW_SUITES']); + putenv('PRESTAFLOW_SUITES'); + } + + protected function tearDown(): void + { + chdir($this->previousCwd); + exec('rm -rf ' . escapeshellarg($this->tmpDir)); + + if ($this->previousEnv['env'] === null) { + unset($_ENV['PRESTAFLOW_SUITES']); + } else { + $_ENV['PRESTAFLOW_SUITES'] = $this->previousEnv['env']; + } + putenv($this->previousEnv['process'] === false ? 'PRESTAFLOW_SUITES' : 'PRESTAFLOW_SUITES=' . $this->previousEnv['process']); + } + + public function testParseTrimsAndDropsEmptyNamesAndDuplicates(): void + { + $this->assertSame( + ['BackOffice', 'FrontOffice/Checkout'], + (new ExecuteSuite())->parseSuitesFilter(' BackOffice, ,FrontOffice/Checkout/ ,BackOffice,') + ); + } + + public function testParseOfNothingIsNoFilter(): void + { + $this->assertSame([], (new ExecuteSuite())->parseSuitesFilter(null)); + $this->assertSame([], (new ExecuteSuite())->parseSuitesFilter(' , ')); + } + + public function testParseRejectsParentSegments(): void + { + $this->expectException(Error::class); + $this->expectExceptionMessageMatches('/\.\./'); + + (new ExecuteSuite())->parseSuitesFilter('BackOffice/../../etc'); + } + + public function testParseRejectsAbsolutePaths(): void + { + $this->expectException(Error::class); + $this->expectExceptionMessageMatches('/absolute/'); + + (new ExecuteSuite())->parseSuitesFilter('/etc'); + } + + public function testNoFilterKeepsTheWholeTree(): void + { + $this->assertSame( + $this->sorted((new ExecuteSuite())->resolveSuitePaths($this->root)), + $this->sorted((new ExecuteSuite())->resolveSuitePaths($this->root, [])) + ); + } + + public function testFilterKeepsOnlyTheNamedSubFoldersRecursively(): void + { + $resolved = (new ExecuteSuite())->resolveSuitePaths($this->root, ['FrontOffice']); + + $this->assertSame([ + $this->root . '/FrontOffice/Checkout/Guest.php', + $this->root . '/FrontOffice/Home.php', + ], $this->sorted($resolved)); + } + + public function testFilterAcceptsNestedPathsAndReturnsTheUnionWithoutDuplicates(): void + { + $resolved = (new ExecuteSuite())->resolveSuitePaths($this->root, ['FrontOffice/Checkout', 'BackOffice', 'FrontOffice']); + + $this->assertSame([ + $this->root . '/BackOffice/Login.php', + $this->root . '/FrontOffice/Checkout/Guest.php', + $this->root . '/FrontOffice/Home.php', + ], $this->sorted($resolved)); + $this->assertSame(count($resolved), count(array_unique($resolved))); + } + + public function testUnknownNamesFailListingMissingAndAvailableFolders(): void + { + try { + (new ExecuteSuite())->resolveSuitePaths($this->root, ['BackOffice', 'Nope', 'Front']); + $this->fail('An unknown sub-folder must fail the run'); + } catch (Error $e) { + $this->assertStringContainsString('Nope', $e->getMessage()); + $this->assertStringContainsString('Front', $e->getMessage()); + $this->assertStringContainsString('BackOffice, Empty, FrontOffice', $e->getMessage()); + } + } + + public function testFilterOnASingleSuiteFileIsRefused(): void + { + $this->expectException(Error::class); + + (new ExecuteSuite())->resolveSuitePaths($this->root . '/Top.php', ['BackOffice']); + } + + public function testEnvVariableFiltersTheRunAndFailsOnAnUnknownFolder(): void + { + putenv('PRESTAFLOW_SUITES=Nope'); + + $tester = $this->tester(); + $exitCode = $tester->run(['command' => 'run', 'folder' => $this->root], ['capture_stderr_separately' => true]); + + $this->assertNotSame(0, $exitCode); + $this->assertStringContainsString('Nope', $tester->getErrorOutput()); + } + + public function testCliOptionWinsOverTheEnvVariable(): void + { + putenv('PRESTAFLOW_SUITES=Nope'); + + // Empty/ holds no suite: the run succeeds with "Tests folder is empty", + // which proves the unknown name from the environment was ignored. + $exitCode = $this->tester()->run( + ['command' => 'run', 'folder' => $this->root, '--suites' => 'Empty'], + ['capture_stderr_separately' => true] + ); + + $this->assertSame(0, $exitCode); + } + + public function testCliOptionAloneFailsOnAnUnknownFolder(): void + { + $exitCode = $this->tester()->run( + ['command' => 'run', 'folder' => $this->root, '--suites' => 'Missing'], + ['capture_stderr_separately' => true] + ); + + $this->assertNotSame(0, $exitCode); + } + + private function tester(): ApplicationTester + { + $application = new Application(); + $application->setAutoExit(false); + + return new ApplicationTester($application); + } + + private function sorted(array $paths): array + { + sort($paths); + + return $paths; + } +} From 43c9521076b065865a7d9d0ea7f7b0d56221b02a Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:55:24 +0200 Subject: [PATCH 6/6] fix(cli): fail a filtered run that selects no suite A --suites / PRESTAFLOW_SUITES filter naming a folder that exists but holds no suite still ended with "Tests folder is empty" and exit code 0. Asking for specific suites and running none is a mistake in the filter, not an empty project, and on CI it shows up as a green job that tested nothing. When a filter is set and the run ends up with zero suites, whether the folders were empty or --group / --draft removed everything, it now stops with "Suites filter [...] selected no suite under [...]" and exit code 1. Without a filter an empty folder still exits 0. The filter's test fixtures now declare a namespace. Bare " --- README.md | 2 ++ src/Command/ExecuteSuite.php | 20 +++++++++++++++++ tests/Unit/Command/SuitesFilterTest.php | 30 ++++++++++++++++++++----- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7473fcd..cafa7e4 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,8 @@ PRESTAFLOW_SUITES=BackOffice,FrontOffice/Checkout ./vendor/bin/prestaflow run te - A name that matches no folder fails the run (non-zero exit code), listing the missing names and the available sub-folders. A typo never becomes a green job that ran zero tests. +- A filtered run that ends up with no suite at all (an empty folder, or + nothing left after `--group` / `--draft`) fails too, for the same reason. - Absolute paths and `..` are refused: the filter can only narrow ``. - `--group` and `--draft` still apply, on the suites of the selected folders. - Unset or empty: every suite under `` runs, as before. diff --git a/src/Command/ExecuteSuite.php b/src/Command/ExecuteSuite.php index 68cec7d..b96e51e 100644 --- a/src/Command/ExecuteSuite.php +++ b/src/Command/ExecuteSuite.php @@ -154,6 +154,7 @@ public function execute(InputInterface $input, OutputInterface $output): int $this->sections['progressIndicator']->finish('Finished'); $this->sections['progressBar']->clear(); + $this->failOnEmptyFilteredRun($suitesFilter, (string) $input->getArgument('folder')); $this->success('Tests folder is empty', newLine: true); return Command::SUCCESS; }; @@ -260,6 +261,7 @@ public function execute(InputInterface $input, OutputInterface $output): int } if (!$nbSuites) { + $this->failOnEmptyFilteredRun($suitesFilter, (string) $input->getArgument('folder')); $this->success('Tests folder is empty', newLine: true); return Command::SUCCESS; }; @@ -514,6 +516,24 @@ public function parseSuitesFilter(?string $raw): array return array_values(array_unique($names)); } + /** + * A run narrowed by --suites / PRESTAFLOW_SUITES that ends up with no suite + * is a mistake in the filter, not an empty project: fail it rather than + * report a green run of zero tests. Unfiltered, an empty folder still passes. + * + * @throws Error when $suitesFilter is not empty + */ + protected function failOnEmptyFilteredRun(array $suitesFilter, string $root): void + { + if ($suitesFilter !== []) { + throw new Error(sprintf( + 'Suites filter [%s] selected no suite under [%s]', + implode(', ', $suitesFilter), + $root + )); + } + } + /** * Suites of the given sub-folders of $root (the union, without duplicates). * diff --git a/tests/Unit/Command/SuitesFilterTest.php b/tests/Unit/Command/SuitesFilterTest.php index 62e6dd4..737057a 100644 --- a/tests/Unit/Command/SuitesFilterTest.php +++ b/tests/Unit/Command/SuitesFilterTest.php @@ -28,7 +28,7 @@ protected function setUp(): void mkdir($this->root . '/' . $dir, 0777, true); } foreach (['Top.php', 'BackOffice/Login.php', 'FrontOffice/Home.php', 'FrontOffice/Checkout/Guest.php'] as $file) { - file_put_contents($this->root . '/' . $file, 'root . '/' . $file, "previousCwd = getcwd(); @@ -149,14 +149,34 @@ public function testCliOptionWinsOverTheEnvVariable(): void { putenv('PRESTAFLOW_SUITES=Nope'); - // Empty/ holds no suite: the run succeeds with "Tests folder is empty", - // which proves the unknown name from the environment was ignored. - $exitCode = $this->tester()->run( + $tester = $this->tester(); + $tester->run( + ['command' => 'run', 'folder' => $this->root, '--suites' => 'BackOffice'], + ['capture_stderr_separately' => true] + ); + + // The unknown name from the environment was never looked at. + $this->assertStringNotContainsString('Nope', $tester->getErrorOutput()); + } + + public function testAFilteredRunThatSelectsNoSuiteFails(): void + { + $tester = $this->tester(); + $exitCode = $tester->run( ['command' => 'run', 'folder' => $this->root, '--suites' => 'Empty'], ['capture_stderr_separately' => true] ); - $this->assertSame(0, $exitCode); + $this->assertSame(1, $exitCode); + $this->assertStringContainsString('Suites filter [Empty] selected no suite', $tester->getErrorOutput()); + } + + public function testAnUnfilteredEmptyFolderStillSucceeds(): void + { + $this->assertSame(0, $this->tester()->run( + ['command' => 'run', 'folder' => $this->root . '/Empty'], + ['capture_stderr_separately' => true] + )); } public function testCliOptionAloneFailsOnAnUnknownFolder(): void