Visual suites: data-driven VisualTestsSuite, selector masks, per-run browser options - #68
Merged
Merged
Conversation
…owing 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…t 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…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 <path>` 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 <path>. 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 <path>. 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 <noreply@anthropic.com>
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 "<?php" files hit an existing "Undefined array key 0" warning in ExecuteSuite's namespace parsing, which this change does not address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The keepAlive browser was found again through a socket file at one fixed path per machine ($TMPDIR/prestaflow-.browser), shared by every PrestaFlow process. A run reconnected to whatever Chrome that file named, and ExecuteSuite ended every run by closing that browser and deleting the file. So any run that finished while another was in flight, even a browser-free one such as a smoke suite, closed the other run's Chrome under it. The victim failed with "The page was closed and is not available anymore", typically on its first navigation, or silently lost its session when it relaunched a browser between two steps (an emptied cart, a checkout that no longer matches). Whether it happened depended on what else was running on the machine, not on the PrestaShop version under test. ExecuteSuite now scopes the socket and options files to the run (pid plus a random token) before it starts and releases only that browser at the end. Callers outside ExecuteSuite keep the shared path, and the reconnection they may rely on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…element
elementIsVisible() caught every Exception and returned false. When the
Chrome a run drives is closed under it (another PrestaFlow process
releasing the shared browser, a crash), chrome-php throws TargetDestroyed
("The session is destroyed."), and isVisible() answered "not visible".
A suite checking a block on the home page then failed with "false must be
the same as true": a lost browser disguised as a regression of the shop,
which is how psflowdemo's DisplayHome looked broken on 1.7.8.11 while the
block was served, hooked and rendered. TargetDestroyed now propagates, so
the step fails with the browser error the runner already reports. A missing
element still answers false.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lpers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Laravel 11+ hosts (the desktop app) require symfony/console ^7; the ^6.0-only constraint made the library uninstallable there. PHP 8.1 still resolves 6.x. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge
devintomain.Includes #67 (visual suites groundwork):
TestsSuite::useBrowserOptions()/browserOptions()/resetBrowser(): per-combination window size + user agent, clean shutdown of the (run-scoped) keepAlive browser; fixes swappedWINDOW_SIZE_*defaults.CommonPage::visualCheckpoint(..., array $masks = [])+scrollBelow()/waitForStable()helpers.VisualDevicespresets +VisualTestsSuitedata-driven base class (one run = one device × locale combination).Plus the other commits already on
devsince the last merge (run-scoped browser files,--suitesfilter, CLI exit codes, page fixes).🤖 Generated with Claude Code