From 75ec5222b66ef73b4656140c732604b4e0a5cb7b Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 13:29:07 +0200 Subject: [PATCH 01/12] chore(infra): flashlight compose fixture for 1.7, 8.2 and 9.2 Co-Authored-By: Claude Opus 5 --- .env.flashlight.example | 16 +++++++ .gitignore | 1 + docker-compose.yml | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 .env.flashlight.example create mode 100644 docker-compose.yml diff --git a/.env.flashlight.example b/.env.flashlight.example new file mode 100644 index 0000000..e327c9a --- /dev/null +++ b/.env.flashlight.example @@ -0,0 +1,16 @@ +# Values for a Flashlight shop. Copy to .env.flashlight and adjust the port. +# +# The back office folder and credentials are fixed by the image, so unlike a +# hand-made install there is nothing machine-specific to discover here. +PRESTAFLOW_FO_URL=http://localhost:8092/ +PRESTAFLOW_BO_URL=http://localhost:8092/admin-dev/ +PRESTAFLOW_BO_EMAIL=admin@prestashop.com +PRESTAFLOW_BO_PASSWD=prestashop +PRESTAFLOW_LOCALE=en + +# One of these per shop: +# 1.7.8.11 -> port 8017, theme classic +# 8.2.8 -> port 8082, theme classic +# 9.2.0 -> port 8092, theme hummingbird +PRESTAFLOW_PS_VERSION=9.2.0 +PRESTAFLOW_THEME=hummingbird diff --git a/.gitignore b/.gitignore index 587e851..2b7b805 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ prestaflow/ .vscode/settings.json datas/.broswer .phpunit.cache/ +.env.flashlight diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..4835634 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,93 @@ +# Throwaway PrestaShop shops from the official Flashlight images. +# +# One shop and one database per version. Bring up a single shop at a time: +# docker compose up ps92 -d +# Its database starts with it through depends_on. +# +# Tags carry a suffix on purpose. The bare `9.2.0` and `8.2.8` tags do not +# exist on the registry — a previous attempt at this file pinned `9.0.1` and +# could never start. +services: + db17: + image: mariadb:11 + environment: + MARIADB_ROOT_PASSWORD: prestashop + MARIADB_DATABASE: prestashop + MARIADB_USER: prestashop + MARIADB_PASSWORD: prestashop + healthcheck: + test: ["CMD", "healthcheck.sh", "--connect"] + interval: 5s + timeout: 5s + retries: 20 + + ps17: + image: prestashop/prestashop-flashlight:1.7.8.11-nginx + depends_on: + db17: + condition: service_healthy + environment: + PS_DOMAIN: localhost:8017 + MYSQL_HOST: db17 + DEBUG_MODE: "false" + POST_SCRIPTS_DIR: /tmp/post-scripts + volumes: + - ./docker/post-scripts:/tmp/post-scripts:ro + ports: + - "8017:80" + + db82: + image: mariadb:11 + environment: + MARIADB_ROOT_PASSWORD: prestashop + MARIADB_DATABASE: prestashop + MARIADB_USER: prestashop + MARIADB_PASSWORD: prestashop + healthcheck: + test: ["CMD", "healthcheck.sh", "--connect"] + interval: 5s + timeout: 5s + retries: 20 + + ps82: + image: prestashop/prestashop-flashlight:8.2.8-nginx + depends_on: + db82: + condition: service_healthy + environment: + PS_DOMAIN: localhost:8082 + MYSQL_HOST: db82 + DEBUG_MODE: "false" + POST_SCRIPTS_DIR: /tmp/post-scripts + volumes: + - ./docker/post-scripts:/tmp/post-scripts:ro + ports: + - "8082:80" + + db92: + image: mariadb:11 + environment: + MARIADB_ROOT_PASSWORD: prestashop + MARIADB_DATABASE: prestashop + MARIADB_USER: prestashop + MARIADB_PASSWORD: prestashop + healthcheck: + test: ["CMD", "healthcheck.sh", "--connect"] + interval: 5s + timeout: 5s + retries: 20 + + ps92: + image: prestashop/prestashop-flashlight:9.2.0-nginx + depends_on: + db92: + condition: service_healthy + environment: + PS_DOMAIN: localhost:8092 + MYSQL_HOST: db92 + DEBUG_MODE: "false" + POST_SCRIPTS_DIR: /tmp/post-scripts + volumes: + - ./docker/post-scripts:/tmp/post-scripts:ro + ports: + - "8092:80" From 8073588fe84f6025965da9ffcc993bf2d10a72ea Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 13:35:36 +0200 Subject: [PATCH 02/12] chore(infra): provision a second shop in every flashlight container Co-Authored-By: Claude Opus 5 --- docker/post-scripts/10-second-shop.sh | 83 +++++++++++++++++++ .../post-scripts/20-carrier-restrictions.sh | 27 ++++++ 2 files changed, 110 insertions(+) create mode 100755 docker/post-scripts/10-second-shop.sh create mode 100755 docker/post-scripts/20-carrier-restrictions.sh diff --git a/docker/post-scripts/10-second-shop.sh b/docker/post-scripts/10-second-shop.sh new file mode 100755 index 0000000..d69c91b --- /dev/null +++ b/docker/post-scripts/10-second-shop.sh @@ -0,0 +1,83 @@ +#!/bin/sh +set -eu + +echo "* Provisioning a second shop..." + +cat > /tmp/second-shop.php <<'PHP' +name = 'Group2'; +$group->active = true; +$group->add(); + +$shop = new Shop(); +$shop->name = 'Shop2'; +$shop->id_shop_group = (int) $group->id; +$shop->id_category = (int) Configuration::get('PS_HOME_CATEGORY'); +$shop->theme_name = 'classic'; +$shop->active = true; +$shop->add(); + +$url = new ShopUrl(); +$url->id_shop = (int) $shop->id; +$url->domain = $url->domain_ssl = Configuration::get('PS_SHOP_DOMAIN'); +$url->physical_uri = '/'; +$url->virtual_uri = 'shop2/'; +$url->main = true; +$url->active = true; +$url->add(); + +// copyShopData() reads Tools::getValue('categoryBox'): outside an HTTP request +// that returns false and count(false) is fatal on PHP 8. Prime it with the +// source shop's categories, which is what the Multistore form would post. +$rows = Db::getInstance()->executeS( + 'SELECT id_category FROM ' . _DB_PREFIX_ . 'category_shop WHERE id_shop = ' . (int) $src +); +$categories = array_map('intval', array_column($rows ?: [], 'id_category')); +$_POST['categoryBox'] = $_REQUEST['categoryBox'] = $categories; + +// The same 26 keys the Multistore form offers. +$keys = [ + 'carrier', 'cms', 'contact', 'country', 'currency', 'discount', 'employee', + 'image', 'lang', 'manufacturer', 'module', 'hook_module', 'meta_lang', + 'product', 'product_attribute', 'stock_available', 'store', + 'webservice_account', 'attribute_group', 'feature', 'group', + 'tax_rules_group', 'supplier', 'zone', 'cart_rule', +]; +$shop->copyShopData($src, array_fill_keys($keys, 'on')); +$shop->associateSuperAdmins(); + +array_unshift($categories, (int) Configuration::get('PS_ROOT_CATEGORY')); +Category::updateFromShop(array_values(array_unique($categories)), (int) $shop->id); + +// Module-owned data travels through a hook, not through copyShopData(). +foreach ((array) Hook::getHookModuleExecList('actionShopDataDuplication') as $m) { + Hook::exec('actionShopDataDuplication', [ + 'old_id_shop' => $src, + 'new_id_shop' => (int) $shop->id, + ], (int) $m['id_module']); +} + +// Multistore is enabled, and its back-office screens must exist with it: +// setting the flag without activating the tabs gives a shop that cannot be +// managed, which is exactly how the previous hand-made container ended up. +Configuration::updateValue('PS_MULTISHOP_FEATURE_ACTIVE', 1); +Db::getInstance()->execute( + 'UPDATE ' . _DB_PREFIX_ . "tab SET active = 1 WHERE class_name IN ('AdminShopGroup','AdminShopUrl')" +); + +// A virtual URI serves no theme assets until the rewrite rules exist. +Tools::generateHtaccess(); + +echo 'second shop id=' . (int) $shop->id . PHP_EOL; +PHP + +php /tmp/second-shop.php +rm -f /tmp/second-shop.php + +echo "✅ Second shop provisioned" diff --git a/docker/post-scripts/20-carrier-restrictions.sh b/docker/post-scripts/20-carrier-restrictions.sh new file mode 100755 index 0000000..154ca23 --- /dev/null +++ b/docker/post-scripts/20-carrier-restrictions.sh @@ -0,0 +1,27 @@ +#!/bin/sh +set -eu + +# Works around PrestaShop#42964: ps_module_carrier is not in +# Shop::getAssoTables(), so a duplicated shop inherits no carrier restrictions +# and Hook::getHookModuleExecList() then offers it no payment method at all. +# Without this, checkout on shop 2 stops at "no payment method available". +echo "* Copying carrier restrictions to the second shop..." + +cat > /tmp/carrier-restrictions.php <<'PHP' +executeS('SELECT id_shop FROM ' . _DB_PREFIX_ . 'shop WHERE id_shop <> 1'); +foreach ($rows ?: [] as $row) { + Db::getInstance()->execute( + 'INSERT IGNORE INTO ' . _DB_PREFIX_ . 'module_carrier (id_module, id_shop, id_reference) ' + . 'SELECT id_module, ' . (int) $row['id_shop'] . ', id_reference ' + . 'FROM ' . _DB_PREFIX_ . 'module_carrier WHERE id_shop = 1' + ); +} +PHP + +php /tmp/carrier-restrictions.php +rm -f /tmp/carrier-restrictions.php + +echo "✅ Carrier restrictions copied" From 5be2e19164c10ebcb6073060dc1ddd2a35ceeea1 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 13:46:20 +0200 Subject: [PATCH 03/12] fix(infra): give the second shop its own port instead of a virtual URI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second shop was provisioned on the virtual URI /shop2/, whose theme assets only resolve once .htaccess rewrite rules exist. Every Flashlight tag this project pins is -nginx, and nginx never reads .htaccess, so Tools::generateHtaccess() was a no-op: /shop2/themes/classic/assets/cache/ theme-*.css returned 404 while the same file at the root returned 200. Switching to an Apache-flavoured image is not available either — no -apache tag exists for 1.7.8.11, 8.2.8 or 9.2.0 (only 9.0.3 has one). Each shop service now publishes a second host port (8018, 8083, 8093) and passes SECOND_SHOP_PORT, and the post-script points the second shop's domain at that port with physical_uri '/' and an empty virtual_uri. PrestaShop discriminates shops by domain including the port, so the second shop is served at the root of its own port and needs no rewrite at all. Co-Authored-By: Claude Opus 5 --- docker-compose.yml | 6 ++++++ docker/post-scripts/10-second-shop.sh | 21 ++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 4835634..d1c4537 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -28,6 +28,7 @@ services: condition: service_healthy environment: PS_DOMAIN: localhost:8017 + SECOND_SHOP_PORT: "8018" MYSQL_HOST: db17 DEBUG_MODE: "false" POST_SCRIPTS_DIR: /tmp/post-scripts @@ -35,6 +36,7 @@ services: - ./docker/post-scripts:/tmp/post-scripts:ro ports: - "8017:80" + - "8018:80" # the second shop db82: image: mariadb:11 @@ -56,6 +58,7 @@ services: condition: service_healthy environment: PS_DOMAIN: localhost:8082 + SECOND_SHOP_PORT: "8083" MYSQL_HOST: db82 DEBUG_MODE: "false" POST_SCRIPTS_DIR: /tmp/post-scripts @@ -63,6 +66,7 @@ services: - ./docker/post-scripts:/tmp/post-scripts:ro ports: - "8082:80" + - "8083:80" # the second shop db92: image: mariadb:11 @@ -84,6 +88,7 @@ services: condition: service_healthy environment: PS_DOMAIN: localhost:8092 + SECOND_SHOP_PORT: "8093" MYSQL_HOST: db92 DEBUG_MODE: "false" POST_SCRIPTS_DIR: /tmp/post-scripts @@ -91,3 +96,4 @@ services: - ./docker/post-scripts:/tmp/post-scripts:ro ports: - "8092:80" + - "8093:80" # the second shop, on its own port rather than a virtual URI diff --git a/docker/post-scripts/10-second-shop.sh b/docker/post-scripts/10-second-shop.sh index d69c91b..0ee42d9 100755 --- a/docker/post-scripts/10-second-shop.sh +++ b/docker/post-scripts/10-second-shop.sh @@ -9,7 +9,7 @@ require_once '/var/www/html/config/config.inc.php'; $src = 1; -// A second shop, its group, and its URL on a virtual URI. +// A second shop, its group, and its URL on a dedicated port. $group = new ShopGroup(); $group->name = 'Group2'; $group->active = true; @@ -25,9 +25,21 @@ $shop->add(); $url = new ShopUrl(); $url->id_shop = (int) $shop->id; -$url->domain = $url->domain_ssl = Configuration::get('PS_SHOP_DOMAIN'); +$secondPort = getenv('SECOND_SHOP_PORT') ?: '8093'; +$host = preg_replace('/:\d+$/', '', (string) Configuration::get('PS_SHOP_DOMAIN')); +$url->domain = $url->domain_ssl = $host . ':' . $secondPort; +// A DEDICATED PORT, not a virtual URI. PrestaShop discriminates shops by +// domain including the port, so localhost:8093 is a distinct shop — and its +// assets resolve at the root, needing no rewrite at all. +// +// The virtual-URI approach this replaced cannot work here: it relies on +// Tools::generateHtaccess(), and every Flashlight tag we use is -nginx, which +// never reads .htaccess. Verified 2026-09-24: /shop2/themes/...css returned +// 404 while the same file at the root returned 200. There is no -apache tag +// for 1.7.8.11, 8.2.8 or 9.2.0 (only 9.0.3 has one), so switching flavour is +// not an option either. $url->physical_uri = '/'; -$url->virtual_uri = 'shop2/'; +$url->virtual_uri = ''; $url->main = true; $url->active = true; $url->add(); @@ -71,8 +83,7 @@ Db::getInstance()->execute( 'UPDATE ' . _DB_PREFIX_ . "tab SET active = 1 WHERE class_name IN ('AdminShopGroup','AdminShopUrl')" ); -// A virtual URI serves no theme assets until the rewrite rules exist. -Tools::generateHtaccess(); +// Nothing to rewrite: the second shop lives at the root of its own port. echo 'second shop id=' . (int) $shop->id . PHP_EOL; PHP From 2df75f8853e0e2634ee691dca3416f190ab8d736 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 14:10:23 +0200 Subject: [PATCH 04/12] fix(pages): make the listing product link theme-overridable, and wait for the click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in FrontOffice\Listing::goToProduct(), both found while running a front office smoke suite against a live 9.2 shop on hummingbird. The first is a hole in the theme-selector mechanism. goToProduct() built its target as selector('productArticle') . ' .product-title a': only the article half went through the theme merge, the anchor half was concatenated in PHP and so no theme file could reach it. hummingbird overrides productArticle, but renders a.product-miniature__title — .product-title does not exist on the page at all — so the click matched nothing, navigateTo() swallowed the timeout, and the caller carried on as if it had navigated. The shipped AddProductToCart scenario calls this method (through Category\Page, which inherits it), so that scenario was broken on hummingbird. The whole path now lives in a new productArticleLink selector; productArticle is untouched. The hummingbird override is declared under both FrontOffice.Listing and FrontOffice.Category, because the theme merge keys off the concrete page name: a Listing override alone never reaches the Category page, which is the one AddProductToCart actually drives. The second is a navigation race. goToProduct() called waitForNavigation(), which waits on a navigation started by navigate() — not one started by a click. The step therefore returned while the product page was still loading: measured on Classic, the first getPrice() answered false and an immediate second call answered 19.12, with 2 of 4 runs failing. It now calls waitForPageReload(), which waits for the navigation triggered by the preceding action. goToProduct() also returns navigateTo()'s result instead of discarding it, so a click that matched nothing fails where it happens rather than three steps later. Verified live on a 9.2 flashlight shop, hummingbird and Classic, through both the Listing and the Category page, three green runs each; the same probe fails 3/3 on hummingbird and 2/4 on Classic without these changes. Co-Authored-By: Claude Opus 5 --- src/Pages/v9/FrontOffice/Listing/Page.php | 20 ++++- src/Themes/hummingbird.json | 7 +- tests/Unit/Pages/ListingProductLinkTest.php | 97 +++++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/Pages/ListingProductLinkTest.php diff --git a/src/Pages/v9/FrontOffice/Listing/Page.php b/src/Pages/v9/FrontOffice/Listing/Page.php index 594dbce..ebae848 100644 --- a/src/Pages/v9/FrontOffice/Listing/Page.php +++ b/src/Pages/v9/FrontOffice/Listing/Page.php @@ -11,6 +11,11 @@ public function defineSelectors() return [ 'pageTitle' => '#js-product-list-header', 'productArticle' => '#js-product-list .products div:nth-child(${index}) article', + // The whole path down to the anchor, on purpose: appending the link + // part in PHP would put it out of reach of the theme layer, and the + // miniature title is not a shared class across themes (Classic has + // `.product-title a`, hummingbird `a.product-miniature__title`). + 'productArticleLink' => '#js-product-list .products div:nth-child(${index}) article .product-title a', // Wishlist 'productAddToWishlist' => '#js-product-list .products div:nth-child(${index}) article button.wishlist-button-add', 'wishlistModal' => '.wishlist-add-to .wishlist-modal.show', @@ -31,11 +36,22 @@ public function getListingTitle() return $this->getTitle(); } + /** + * Click the nth product miniature and wait for the product page. + * + * Returns whether the click found its target: navigateTo() answers false on + * a selector that matched nothing, and swallowing that answer here is how a + * missing link turned into a failure three steps later, on the product page. + */ public function goToProduct(int $index = 1) { - $this->navigateTo($this->selector('productArticle', ['index' => $index]) . ' .product-title a'); + $clicked = $this->navigateTo($this->selector('productArticleLink', ['index' => $index])); - $this->waitForNavigation(); + // waitForNavigation() waits on a navigation started by navigate(); this + // one is started by a click, so it needs waitForPageReload(). + $this->waitForPageReload(); + + return $clicked; } public function addToWishList($index) diff --git a/src/Themes/hummingbird.json b/src/Themes/hummingbird.json index a7b5616..ecc4c62 100644 --- a/src/Themes/hummingbird.json +++ b/src/Themes/hummingbird.json @@ -11,7 +11,12 @@ "modalTitle": ".blockcart-modal__title" }, "Listing": { - "productArticle": "#js-product-list .products article:nth-child(${index})" + "productArticle": "#js-product-list .products article:nth-child(${index})", + "productArticleLink": "#js-product-list .products article:nth-child(${index}) a.product-miniature__title" + }, + "Category": { + "productArticle": "#js-product-list .products article:nth-child(${index})", + "productArticleLink": "#js-product-list .products article:nth-child(${index}) a.product-miniature__title" }, "Login": { "alertDangerTextBlock": ".login__form-wrapper .help-block .alert-danger", diff --git a/tests/Unit/Pages/ListingProductLinkTest.php b/tests/Unit/Pages/ListingProductLinkTest.php new file mode 100644 index 0000000..bdb2acd --- /dev/null +++ b/tests/Unit/Pages/ListingProductLinkTest.php @@ -0,0 +1,97 @@ + '9.0.0', + 'LOCALE' => 'en', + 'PREFIX_LOCALE' => false, + 'THEME' => $theme, + 'BO' => ['URL' => 'http://localhost/admin/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'FO' => ['URL' => 'http://localhost/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'DEBUG' => false, + 'VERBOSE' => false, + ]; + } + + private function page(string $theme): ListingPage + { + return new ListingPage(locale: 'en', patchVersion: '9.0.0', globals: $this->globals($theme), customs: []); + } + + private function categoryPage(string $theme): CategoryPage + { + return new CategoryPage(locale: 'en', patchVersion: '9.0.0', globals: $this->globals($theme), customs: []); + } + + public function testClassicLinkSelectorCarriesTheAnchor(): void + { + $selector = $this->page('classic')->selector('productArticleLink', ['index' => 2]); + + $this->assertSame( + '#js-product-list .products div:nth-child(2) article .product-title a', + $selector + ); + } + + public function testHummingbirdOverridesTheWholeLinkSelector(): void + { + $selector = $this->page('hummingbird')->selector('productArticleLink', ['index' => 3]); + + $this->assertSame( + '#js-product-list .products article:nth-child(3) a.product-miniature__title', + $selector + ); + $this->assertStringNotContainsString('.product-title', $selector); + } + + public function testProductArticleIsUnchangedOnBothThemes(): void + { + $this->assertSame( + '#js-product-list .products div:nth-child(1) article', + $this->page('classic')->selector('productArticle', ['index' => 1]) + ); + $this->assertSame( + '#js-product-list .products article:nth-child(1)', + $this->page('hummingbird')->selector('productArticle', ['index' => 1]) + ); + } + + /** + * Category extends Listing, but the theme merge keys off the concrete page + * name, so a FrontOffice.Listing override never reaches FrontOffice.Category. + * AddProductToCart drives the Category page, which is the path that was + * actually broken on hummingbird. + */ + public function testCategoryInheritsTheLinkFixOnHummingbird(): void + { + $this->assertSame( + '#js-product-list .products article:nth-child(1) a.product-miniature__title', + $this->categoryPage('hummingbird')->selector('productArticleLink', ['index' => 1]) + ); + } + + public function testCategoryKeepsTheClassicSelectorOnClassic(): void + { + $this->assertSame( + '#js-product-list .products div:nth-child(1) article .product-title a', + $this->categoryPage('classic')->selector('productArticleLink', ['index' => 1]) + ); + } +} From 4ddc03107eb0208091f629f47b833540fc44bbbc Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 14:34:31 +0200 Subject: [PATCH 05/12] fix(themes): let theme overrides follow the page class hierarchy getThemeSelectors() keyed the theme-catalog walk on getPageName(), i.e. the CONCRETE class name. A page that extends a SIBLING page therefore never saw the overrides declared for its parent, even though it inherits every selector the parent defines: an override under FrontOffice.Listing simply did not reach FrontOffice\Category\Page. The page fell back to the base (Classic) selector, which matches nothing on another theme, and click() returned false instead of raising -- a suite reporting success having done nothing. Seven v9 front-office pages have that shape and were all affected: Category, PricesDrop, NewProducts, BestSellers (extend Listing), Content (extends CMS), Information (extends Identity) and Address (extends Addresses). Resolution now walks [static::class, ...class_parents($this)] reversed, so the furthest ancestor merges first and the concrete page still wins. The concrete chain keeps coming from getPageName(), so a subclass that overrides it still steers its own lookup. Two traps guarded against: 1. Common\FrontOffice\Page strips to the two-segment chain "FrontOffice\Page". Its walk descends FrontOffice, skips the trailing "Page" and lands on the FrontOffice node -- a map of PAGE NAMES, not selectors. Merging it would inject Product, Listing, ... as selector keys. Chains naming fewer than two non-"Page" segments are dropped. This is not hypothetical: every v9 page has Common\FrontOffice\Page (or its BackOffice twin) in its ancestry. 2. Only flat string values are selectors. Whatever the walk lands on, nested arrays are filtered out rather than merged. Both guards are covered by tests that fail when the guard is removed. The duplicate FrontOffice.Category block in src/Themes/hummingbird.json is now redundant (verified: removing it yields a byte-identical selector map for the real Category page), but is deliberately left in place until the fix is verified live. Co-Authored-By: Claude Opus 5 --- src/Pages/CommonPage.php | 106 +++++++++- tests/Unit/Pages/ThemeInheritanceTest.php | 245 ++++++++++++++++++++++ 2 files changed, 340 insertions(+), 11 deletions(-) create mode 100644 tests/Unit/Pages/ThemeInheritanceTest.php diff --git a/src/Pages/CommonPage.php b/src/Pages/CommonPage.php index 315bbe0..d6207c9 100644 --- a/src/Pages/CommonPage.php +++ b/src/Pages/CommonPage.php @@ -223,7 +223,10 @@ protected function getThemeSelectors(array $pageNames): array $selectors = []; $found = false; - foreach ($this->themeDirectories() as $dir) { + $directories = $this->themeDirectories(); + $catalogs = []; + + foreach ($directories as $dir) { $path = rtrim($dir, '/') . '/' . $theme . '.json'; if (!file_exists($path)) { continue; @@ -235,17 +238,14 @@ protected function getThemeSelectors(array $pageNames): array continue; } - // Same walk as the locale catalog: descend one level per page-name - // segment, skipping the trailing "Page". - foreach ($pageNames as $pageName) { - if ($pageName === 'Page') { - continue; - } - $decoded = is_array($decoded) && isset($decoded[$pageName]) ? $decoded[$pageName] : []; - } + $catalogs[] = $decoded; + } - if (is_array($decoded)) { - $selectors = [...$selectors, ...$decoded]; + // Least specific first: a block declared on an ancestor page applies to + // every page that inherits from it, and the concrete page still wins. + foreach ($this->themePageChains($pageNames) as $chain) { + foreach ($catalogs as $catalog) { + $selectors = [...$selectors, ...$this->walkThemeCatalog($catalog, $chain)]; } } @@ -261,6 +261,90 @@ protected function getThemeSelectors(array $pageNames): array return $selectors; } + /** + * The page-name chains whose theme blocks apply to this page, least + * specific first. + * + * Theme overrides used to be keyed on the CONCRETE class alone, so a page + * that extends a sibling page (Category extends Listing, PricesDrop extends + * Listing, ...) never saw its parent's overrides even though it inherits + * every selector the parent defines. Walking class_parents() fixes that. + * + * The concrete page keeps going through getPageName() so a subclass that + * overrides it (tests do) still steers its own lookup. + * + * @param array $pageNames the concrete page's chain, as computed by the caller + * @return array> + */ + protected function themePageChains(array $pageNames): array + { + $chains = []; + + // array_reverse puts the furthest ancestor first, which is the order we + // want to merge in. + foreach (array_reverse(array_values(class_parents($this) ?: [])) as $class) { + $name = preg_replace( + '#^PrestaFlow\\\\Library\\\\Pages\\\\(?:v\\d+|Common)\\\\#', + '', + $class + ); + + if ($name === null || $name === $class) { + // Not a page class under Pages\v{N}\ or Pages\Common\. + continue; + } + + $chains[] = explode('\\', $name); + } + + $chains[] = $pageNames; + + // Trap 1: "Common\FrontOffice\Page" strips to "FrontOffice\Page". Its + // walk descends FrontOffice, skips the trailing "Page" and lands on the + // FrontOffice node -- a map of PAGE NAMES, not selectors. Merging that + // would inject "Product", "Listing", ... as if they were selector keys. + // A usable chain names an area AND a page, so anything shorter is dropped. + $chains = array_values(array_filter( + $chains, + fn (array $chain): bool => count(array_filter($chain, fn ($s) => $s !== 'Page')) >= 2 + )); + + // A parent may resolve to the same chain as the child (same names under + // two version namespaces); merging it twice is harmless but pointless. + $unique = []; + foreach ($chains as $chain) { + $unique[implode('\\', $chain)] = $chain; + } + + return array_values($unique); + } + + /** + * Descend the theme catalog one level per page-name segment, skipping the + * trailing "Page", and return the selectors found there. + * + * Trap 2: only flat string values are selectors. Filtering to strings is the + * belt to the chain-length braces above -- if a walk ever lands on a node of + * page blocks again, nested arrays are dropped instead of polluting the map. + */ + protected function walkThemeCatalog(array $catalog, array $chain): array + { + $node = $catalog; + + foreach ($chain as $pageName) { + if ($pageName === 'Page') { + continue; + } + $node = is_array($node) && isset($node[$pageName]) ? $node[$pageName] : []; + } + + if (!is_array($node)) { + return []; + } + + return array_filter($node, fn ($value): bool => is_string($value)); + } + public function getGlobal($index) { $globals = $this->getGlobals(); diff --git a/tests/Unit/Pages/ThemeInheritanceTest.php b/tests/Unit/Pages/ThemeInheritanceTest.php new file mode 100644 index 0000000..ba1403e --- /dev/null +++ b/tests/Unit/Pages/ThemeInheritanceTest.php @@ -0,0 +1,245 @@ +globals = $globals; + $this->baseSelectors = $baseSelectors; + $this->customs = ['selectors' => []]; + } + + public function defineSelectors() + { + return $this->baseSelectors; + } + + public function getPageName(): string + { + return str_replace('PrestaFlow\\Library\\Pages\\v99\\', '', static::class); + } + } +} + +namespace PrestaFlow\Library\Pages\v99\FrontOffice\Listing { + + class Page extends \PrestaFlow\Library\Pages\v99\FrontOffice\Page + { + } +} + +namespace PrestaFlow\Library\Pages\v99\FrontOffice\Category { + + /** The defect in one line: a page whose parent is a sibling page. */ + class Page extends \PrestaFlow\Library\Pages\v99\FrontOffice\Listing\Page + { + } +} + +namespace PrestaFlow\Library\Pages\v99\FrontOffice\Product { + + class Page extends \PrestaFlow\Library\Pages\v99\FrontOffice\Page + { + } +} + +namespace PrestaFlow\Tests\Unit\Pages { + + use PHPUnit\Framework\TestCase; + use PrestaFlow\Library\Pages\v99\FrontOffice\Category\Page as CategoryPage; + use PrestaFlow\Library\Pages\v99\FrontOffice\Listing\Page as ListingPage; + use PrestaFlow\Library\Pages\v99\FrontOffice\Product\Page as ProductPage; + + final class ThemeInheritanceTest extends TestCase + { + private string $tmpThemes = ''; + + protected function tearDown(): void + { + if ($this->tmpThemes !== '' && is_dir($this->tmpThemes)) { + foreach (glob($this->tmpThemes . '/*.json') as $file) { + @unlink($file); + } + @rmdir($this->tmpThemes); + } + } + + private function writeTheme(string $theme, array $catalog): string + { + if ($this->tmpThemes === '') { + $this->tmpThemes = sys_get_temp_dir() . '/pf-theme-inherit-' . bin2hex(random_bytes(6)); + mkdir($this->tmpThemes, 0777, true); + } + + file_put_contents($this->tmpThemes . '/' . $theme . '.json', json_encode($catalog)); + + return $this->tmpThemes; + } + + /** THE defect: an override on Listing must reach Category. */ + public function testAnOverrideOnTheParentPageReachesTheChildPage(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Listing' => ['productArticleLink' => '.product-miniature__title'], + ], + ]); + + $page = new CategoryPage( + ['THEME' => 'hummingbird'], + ['productArticleLink' => '.product-title a'] + ); + $page->themeDirs = [$dir]; + + $this->assertSame( + '.product-miniature__title', + $page->getSelectors()['productArticleLink'], + 'an override declared on Listing must apply to Category, which extends it' + ); + } + + /** The parent block must not beat the child's own block. */ + public function testTheConcretePageStillWinsOverItsParent(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Listing' => ['pageHeading' => '.from-listing'], + 'Category' => ['pageHeading' => '.from-category'], + ], + ]); + + $page = new CategoryPage(['THEME' => 'hummingbird'], ['pageHeading' => '.base']); + $page->themeDirs = [$dir]; + + $this->assertSame('.from-category', $page->getSelectors()['pageHeading']); + } + + /** Keys only the parent declares survive alongside the child's own. */ + public function testParentAndChildBlocksAreMergedNotReplaced(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Listing' => ['productArticle' => '.from-listing', 'sortBy' => '.sort'], + 'Category' => ['productArticle' => '.from-category'], + ], + ]); + + $page = new CategoryPage( + ['THEME' => 'hummingbird'], + ['productArticle' => '.base-article', 'sortBy' => '.base-sort'] + ); + $page->themeDirs = [$dir]; + + $selectors = $page->getSelectors(); + + $this->assertSame('.from-category', $selectors['productArticle']); + $this->assertSame('.sort', $selectors['sortBy']); + } + + /** Inheritance must not leak sideways: Product does not extend Listing. */ + public function testASiblingPageDoesNotInheritAnotherPagesOverrides(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Listing' => ['productArticle' => '.from-listing'], + ], + ]); + + $page = new ProductPage(['THEME' => 'hummingbird'], ['productArticle' => '.base-article']); + $page->themeDirs = [$dir]; + + $this->assertSame('.base-article', $page->getSelectors()['productArticle']); + } + + /** The parent page itself keeps resolving its own block. */ + public function testTheParentPageStillResolvesItsOwnBlock(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Listing' => ['productArticle' => '.from-listing'], + ], + ]); + + $page = new ListingPage(['THEME' => 'hummingbird'], ['productArticle' => '.base-article']); + $page->themeDirs = [$dir]; + + $this->assertSame('.from-listing', $page->getSelectors()['productArticle']); + } + + /** + * Trap 1: the area base strips to "FrontOffice\Page", whose walk lands on + * the FrontOffice node -- a map of PAGE NAMES. Those names must never be + * merged in as if they were selector keys. + */ + public function testPageNamesFromTheAreaNodeDoNotLeakIntoSelectors(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Product' => ['addToCartButton' => '.product__add-to-cart-button'], + 'Listing' => ['productArticle' => '.from-listing'], + 'Category' => ['productArticle' => '.from-category'], + ], + ]); + + $page = new CategoryPage(['THEME' => 'hummingbird'], ['base' => '.base']); + $page->themeDirs = [$dir]; + + $selectors = $page->getSelectors(); + + $this->assertArrayNotHasKey('Product', $selectors); + $this->assertArrayNotHasKey('Listing', $selectors); + $this->assertArrayNotHasKey('Category', $selectors); + $this->assertSame( + [], + array_filter($selectors, fn ($value) => !is_string($value)), + 'no selector value may be an array' + ); + } + + /** + * Trap 2, on its own: even when a walk DOES land on a node holding nested + * blocks, only flat string values are taken. + */ + public function testNestedArraysAreNeverMergedAsSelectors(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + 'Category' => [ + 'productArticle' => '.from-category', + 'Nested' => ['productArticle' => '.too-deep'], + ], + ], + ]); + + $page = new CategoryPage(['THEME' => 'hummingbird'], []); + $page->themeDirs = [$dir]; + + $selectors = $page->getSelectors(); + + $this->assertSame('.from-category', $selectors['productArticle']); + $this->assertArrayNotHasKey('Nested', $selectors); + } + } +} From f1ad516931275569d5aab94629f09c48cbf44aac Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 14:38:29 +0200 Subject: [PATCH 06/12] fix(pages): substitute URL placeholders when the parameter is a scalar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FrontOfficePage::getPageURL() gated placeholder substitution on is_array($params), but every real caller passes a scalar — Product\Page does goToPage('product', $productId) and AddProductToCart does goToPage('category', (int) $this->getParam('categoryId')). The default templates ('{index}-category', '{index}-product.html') therefore kept their placeholder, which Chrome then percent-encoded: goToPage('category', 3) navigated to /%7Bindex%7D-category and the page had no title. A scalar $params is now normalised to ['index' => $params] before the existing substitution loop, so both call shapes work and the array form is unchanged. That bug produced a belief that got written down: three scenarios carried the comment "Canonical product path — friendly URLs can't be rebuilt from an id." That is false. PrestaShop canonicalises on the id and ignores the slug — /3-category, /3-clothes and /3-nimporte-quoi all 302 to /3-clothes. What could not be rebuilt was our own substitution. The three comments in CheckoutOrder, GuestCheckout and OnePageCheckoutOrder are corrected to say so, and to keep the real reason those scenarios pin a path: they select a specific product deliberately (the Mug, id 6, without combinations, so add-to-cart needs no id_product_attribute; the t-shirt path pins its combination as well). Their behaviour and params are unchanged. Co-Authored-By: Claude Opus 5 --- src/Pages/FrontOfficePage.php | 9 +++ src/Scenarios/CheckoutOrder.php | 7 +- src/Scenarios/GuestCheckout.php | 7 +- src/Scenarios/OnePageCheckoutOrder.php | 8 ++- tests/Unit/Pages/FrontOfficePageUrlTest.php | 76 +++++++++++++++++++++ 5 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/Pages/FrontOfficePageUrlTest.php diff --git a/src/Pages/FrontOfficePage.php b/src/Pages/FrontOfficePage.php index 7dd9223..0f3bf12 100644 --- a/src/Pages/FrontOfficePage.php +++ b/src/Pages/FrontOfficePage.php @@ -118,6 +118,15 @@ public function getPageURL($page, $params = null): string } } + // A scalar $params is the common call shape — goToPage('category', 3), + // goToPage('product', $productId) — and means the single placeholder the + // default templates carry: {index}. Substitution used to be gated on + // is_array(), so every scalar call left '{index}' in the URL and it got + // percent-encoded into %7Bindex%7D. Normalise to the array form first. + if (is_scalar($params) && $params !== '') { + $params = ['index' => $params]; + } + if (is_array($params) && count($params) > 0) { foreach ($params as $key => $value) { $url = str_replace('{' . $key . '}', $value, $url); diff --git a/src/Scenarios/CheckoutOrder.php b/src/Scenarios/CheckoutOrder.php index af913ba..9826a7e 100644 --- a/src/Scenarios/CheckoutOrder.php +++ b/src/Scenarios/CheckoutOrder.php @@ -13,7 +13,12 @@ class CheckoutOrder extends Scenario // PrestaShop demo customer (John DOE); override per shop. 'customerEmail' => 'pub@prestashop.com', 'customerPassword' => 'PrestaFlow2026!', - // Canonical product path — friendly URLs can't be rebuilt from an id. + // A canonical product path. An id would work too — PrestaShop + // canonicalises on the id and ignores the slug (/1-anything redirects to + // the real URL), and goToPage('product', $id) now substitutes the + // {index} placeholder for scalar params. The path is kept because it + // pins this exact product *and* combination (id_product 1, + // id_product_attribute 1), which an id alone cannot express. 'productUrl' => '1-1-hummingbird-printed-t-shirt.html', 'cartQuantity' => 1, // Which shop the back-office settings are written for. The checkout diff --git a/src/Scenarios/GuestCheckout.php b/src/Scenarios/GuestCheckout.php index ad502b2..d7b9f3c 100644 --- a/src/Scenarios/GuestCheckout.php +++ b/src/Scenarios/GuestCheckout.php @@ -10,7 +10,12 @@ class GuestCheckout extends Scenario // French demo shop: locale drives the friendly URLs (connexion/panier/ // commande) resolved from src/Urls/fr.json. 'locale' => 'fr', - // Canonical product path — friendly URLs can't be rebuilt from an id. + // A canonical product path. An id would work too — PrestaShop + // canonicalises on the id and ignores the slug (/1-anything redirects to + // the real URL), and goToPage('product', $id) now substitutes the + // {index} placeholder for scalar params. The path is kept because it + // pins this exact product *and* combination (id_product 1, + // id_product_attribute 1), which an id alone cannot express. 'productUrl' => '1-1-hummingbird-printed-t-shirt.html', 'cartQuantity' => 1, 'guestEmail' => 'pf-guest@example.com', diff --git a/src/Scenarios/OnePageCheckoutOrder.php b/src/Scenarios/OnePageCheckoutOrder.php index 663377a..acca2e6 100644 --- a/src/Scenarios/OnePageCheckoutOrder.php +++ b/src/Scenarios/OnePageCheckoutOrder.php @@ -25,8 +25,12 @@ class OnePageCheckoutOrder extends Scenario // do not hardcode a password here. 'customerEmail' => null, 'customerPassword' => null, - // Canonical product path — friendly URLs can't be rebuilt from an id. - // Mug (id 6): no combinations, so add-to-cart works without posting an + // A canonical product path. An id would work too — PrestaShop + // canonicalises on the id and ignores the slug (/6-anything redirects to + // the real URL), and goToPage('product', $id) now substitutes the + // {index} placeholder for scalar params. The path is kept because this + // scenario pins one product deliberately: the Mug (id 6) has no + // combinations, so add-to-cart works without posting an // id_product_attribute, unlike the demo t-shirt this used to point to. 'productUrl' => '6-mug-the-best-is-yet-to-come.html', 'cartQuantity' => 1, diff --git a/tests/Unit/Pages/FrontOfficePageUrlTest.php b/tests/Unit/Pages/FrontOfficePageUrlTest.php new file mode 100644 index 0000000..6d902f1 --- /dev/null +++ b/tests/Unit/Pages/FrontOfficePageUrlTest.php @@ -0,0 +1,76 @@ +globals = $globals; + $this->initLocale(locale: 'en'); + } + + public function getGlobals(): array + { + return $this->globals; + } + } +} + +namespace PrestaFlow\Tests\Unit\Pages { + + use PHPUnit\Framework\TestCase; + use PrestaFlow\Tests\Unit\Pages\UrlFixtures\Page as UrlPage; + + final class FrontOfficePageUrlTest extends TestCase + { + private function page(): UrlPage + { + return new UrlPage([ + 'FO' => ['URL' => 'http://localhost:8092/'], + 'LOCALE' => 'en', + 'PREFIX_LOCALE' => false, + ]); + } + + public function testScalarParamSubstitutesTheIndexPlaceholder(): void + { + $this->assertSame( + 'http://localhost:8092/3-category', + $this->page()->getPageURL('category', 3) + ); + + $this->assertSame( + 'http://localhost:8092/6-product.html', + $this->page()->getPageURL('product', '6') + ); + } + + public function testArrayParamStillSubstitutes(): void + { + $this->assertSame( + 'http://localhost:8092/3-category', + $this->page()->getPageURL('category', ['index' => 3]) + ); + } + + public function testNoParamLeavesTheTemplateUntouched(): void + { + $this->assertSame( + 'http://localhost:8092/{index}-category', + $this->page()->getPageURL('category') + ); + + $this->assertSame( + 'http://localhost:8092/{index}-category', + $this->page()->getPageURL('category', []) + ); + } + } +} From 3bee2eda7fe7d1722784e0d2e26e4a2043075a14 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 15:02:08 +0200 Subject: [PATCH 07/12] test(smoke): a front-office suite fit to run on every version Co-Authored-By: Claude Opus 5 --- src/Pages/v9/FrontOffice/Home/Page.php | 11 +++++ src/Tests/Suites/Smoke/FrontOfficeSmoke.php | 52 +++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/Tests/Suites/Smoke/FrontOfficeSmoke.php diff --git a/src/Pages/v9/FrontOffice/Home/Page.php b/src/Pages/v9/FrontOffice/Home/Page.php index 82d76eb..68c1361 100644 --- a/src/Pages/v9/FrontOffice/Home/Page.php +++ b/src/Pages/v9/FrontOffice/Home/Page.php @@ -16,6 +16,17 @@ public function defineSelectors() ]; } + /** + * Whether the home page rendered its own content section. + * + * Distinct from "the request returned 200": a maintenance page, an error + * page and a redirect to another shop all answer 200 too. + */ + public function isDisplayed(): bool + { + return $this->elementIsVisible($this->getSelector('homePageSection'), 5000); + } + public function goToAllProducts() { $this->goToPage('home'); diff --git a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php new file mode 100644 index 0000000..5822086 --- /dev/null +++ b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php @@ -0,0 +1,52 @@ +importPage('FrontOffice\Home'); + $this->importPage('FrontOffice\Listing'); + $this->importPage('FrontOffice\Product'); + + extract($this->pages); + + $this + ->describe('Front office smoke') + ->it('the home page renders', function () use ($frontOfficeHomePage) { + $frontOfficeHomePage->goToPage('home'); + + Expect::that($frontOfficeHomePage->isDisplayed())->equals(true); + }) + ->it('reach the product listing from the home page', function () use ($frontOfficeHomePage, $frontOfficeListingPage) { + // Reads the "all products" link out of the home page and follows it, + // so a theme that renames that link fails here and names Home. + $frontOfficeHomePage->goToAllProducts(); + + Expect::that($frontOfficeListingPage->getListingTitle())->notEquals(''); + }) + ->it('open a product and read its price', function () use ($frontOfficeListingPage, $frontOfficeProductPage) { + $frontOfficeListingPage->goToProduct(1); + + // A price above zero proves three things at once: the listing linked + // to a real product, the product page matched its price element, and + // parsePrice() understood the rendered format. + Expect::that($frontOfficeProductPage->getPrice() > 0)->equals(true); + }); + } +} From c578b61deb3028a6e08175bc5d4077c67d292ada Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 15:05:20 +0200 Subject: [PATCH 08/12] ci: run the smoke suite against 1.7, 8.2 and 9.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A matrix job per PrestaShop version boots the matching Flashlight shop from docker-compose.yml and runs src/Tests/Suites/Smoke/FrontOfficeSmoke.php against it. Readiness polls /admin-dev/ for a 302 rather than / for a 200: the front office answers before the post-scripts that provision the second shop have finished, so polling / would start the suite against a half-provisioned shop. The 1.7 and 8.2 rows are expected to fail — their page objects are nine-line stubs inheriting the v9 selectors — so they run for real, with fail-fast disabled and no continue-on-error, and the README says why. Co-Authored-By: Claude Opus 5 --- .github/workflows/live-smoke.yml | 100 +++++++++++++++++++++++++++++++ README.md | 32 ++++++++++ 2 files changed, 132 insertions(+) create mode 100644 .github/workflows/live-smoke.yml diff --git a/.github/workflows/live-smoke.yml b/.github/workflows/live-smoke.yml new file mode 100644 index 0000000..5d4b048 --- /dev/null +++ b/.github/workflows/live-smoke.yml @@ -0,0 +1,100 @@ +name: Live smoke + +# Runs the front-office smoke suite against a throwaway Flashlight shop, one +# job per PrestaShop version. +# +# The 1.7 and 8.2 rows are EXPECTED TO FAIL today: their page objects are +# nine-line stubs inheriting the v9 selectors, so the matrix reports an absence +# of support rather than a regression. They are deliberately not skipped and +# not `continue-on-error` — the failures are the inventory of what differs per +# version. `fail-fast: false` keeps one red row from cancelling the others. + +on: + push: + branches: [main] + pull_request: + +jobs: + smoke: + name: Smoke (PrestaShop ${{ matrix.ps }}) + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + include: + - ps: '1.7.8.11' + service: ps17 + port: 8017 + theme: classic + - ps: '8.2.8' + service: ps82 + port: 8082 + theme: classic + - ps: '9.2.0' + service: ps92 + port: 8092 + theme: hummingbird + + steps: + - uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.3' + coverage: none + tools: composer:v2 + + - name: Get Composer cache directory + id: composer-cache + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT" + + - name: Cache Composer packages + uses: actions/cache@v4 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: composer-8.3-${{ hashFiles('composer.json') }} + restore-keys: composer-8.3- + + - name: Install dependencies + run: composer install --no-interaction --no-progress --prefer-dist + + - name: Start the shop + run: docker compose up ${{ matrix.service }} -d + + # Probe /admin-dev/, not /. The front office answers 200 well before the + # post-scripts that provision the second shop have finished, so polling / + # would let the suite start against a half-provisioned shop. Flashlight + # redirects /admin-dev/ to the login page (302) once the container is + # really serving. + - name: Wait for the shop + run: | + for i in $(seq 1 60); do + code=$(curl -s -o /dev/null -w '%{http_code}' "http://localhost:${{ matrix.port }}/admin-dev/" || true) + if [ "$code" = "302" ] || [ "$code" = "200" ]; then + echo "back office answers $code — shop is up"; exit 0 + fi + echo "back office answers ${code:-000}, waiting..." + sleep 5 + done + echo "shop never came up"; docker compose logs ${{ matrix.service }}; exit 1 + + - name: Run the smoke suite + env: + PRESTAFLOW_FO_URL: http://localhost:${{ matrix.port }}/ + PRESTAFLOW_BO_URL: http://localhost:${{ matrix.port }}/admin-dev/ + PRESTAFLOW_BO_EMAIL: admin@prestashop.com + PRESTAFLOW_BO_PASSWD: prestashop + PRESTAFLOW_PS_VERSION: ${{ matrix.ps }} + PRESTAFLOW_THEME: ${{ matrix.theme }} + PRESTAFLOW_LOCALE: en + run: php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php + + - name: Upload failure screenshots + if: failure() + uses: actions/upload-artifact@v4 + with: + name: screenshots-${{ matrix.ps }} + path: prestaflow/screens/ + if-no-files-found: ignore diff --git a/README.md b/README.md index b3643cf..e427925 100644 --- a/README.md +++ b/README.md @@ -27,3 +27,35 @@ class MigrationBaselineSuite extends TestsSuite Resolution priority: fluent `onVersion()` → `$psVersion` property → `PRESTAFLOW_PS_VERSION` env → default `8.1.0`. `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 a suite against a throwaway shop + +`docker-compose.yml` boots a disposable PrestaShop from the official +[Flashlight](https://github.com/PrestaShop/prestashop-flashlight) images — one +service per version, each with its own database and its own port: + +| Service | PrestaShop | Shop 1 | Shop 2 | Theme | +|---|---|---|---|---| +| `ps17` | 1.7.8.11 | 8017 | 8018 | classic | +| `ps82` | 8.2.8 | 8082 | 8083 | classic | +| `ps92` | 9.2.0 | 8092 | 8093 | hummingbird | + +```bash +docker compose up ps92 -d +cp .env.flashlight.example .env.flashlight # then point your run at it +php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +Wait for `/admin-dev/` to answer `302` before running a suite, not for `/` to +answer `200`: the front office is served well before the post-scripts that +provision the second shop have finished. + +The `Live smoke` workflow runs that same suite against all three versions on +every push to `main` and every pull request. + +**The 1.7 and 8.2 rows of the live-smoke matrix are expected to fail today.** +Their page objects are nine-line stubs inheriting the v9 selectors, so the +matrix is reporting an absence of support rather than a regression. Each +failure names the page that differs, which is the inventory the v7/v8 work +needs. The rows are deliberately not skipped and not marked +`continue-on-error`; `fail-fast: false` keeps them from cancelling the 9.2 row. From fdab9add70749a4b6e161313c783738c30b1dc0c Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 15:13:00 +0200 Subject: [PATCH 09/12] docs: testing against a throwaway flashlight shop Co-Authored-By: Claude Opus 5 --- README.md | 21 +++-- docs/testing-with-flashlight.md | 133 ++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 docs/testing-with-flashlight.md diff --git a/README.md b/README.md index e427925..0799320 100644 --- a/README.md +++ b/README.md @@ -34,11 +34,15 @@ Resolution priority: fluent `onVersion()` → `$psVersion` property → `PRESTAF [Flashlight](https://github.com/PrestaShop/prestashop-flashlight) images — one service per version, each with its own database and its own port: -| Service | PrestaShop | Shop 1 | Shop 2 | Theme | -|---|---|---|---|---| -| `ps17` | 1.7.8.11 | 8017 | 8018 | classic | -| `ps82` | 8.2.8 | 8082 | 8083 | classic | -| `ps92` | 9.2.0 | 8092 | 8093 | hummingbird | +| Service | PrestaShop | Shop 1 | Shop 2 | Shop 1 theme | Shop 2 theme | +|---|---|---|---|---|---| +| `ps17` | 1.7.8.11 | 8017 | 8018 | classic | classic | +| `ps82` | 8.2.8 | 8082 | 8083 | classic | classic | +| `ps92` | 9.2.0 | 8092 | 8093 | hummingbird | classic | + +The 9.2 container therefore gives you both themes from a single boot — +hummingbird on 8092, classic on 8093 — which is what you want for checking +that a theme-aware selector resolves per theme. ```bash docker compose up ps92 -d @@ -48,7 +52,12 @@ php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php Wait for `/admin-dev/` to answer `302` before running a suite, not for `/` to answer `200`: the front office is served well before the post-scripts that -provision the second shop have finished. +provision the second shop have finished. A failing post-script does **not** +stop the container, so a healthy container is not proof of a provisioned shop +— the smoke suite is. + +Full walkthrough, including tear-down and the two traps worth knowing: +[Testing against a throwaway shop](docs/testing-with-flashlight.md). The `Live smoke` workflow runs that same suite against all three versions on every push to `main` and every pull request. diff --git a/docs/testing-with-flashlight.md b/docs/testing-with-flashlight.md new file mode 100644 index 0000000..b871f4b --- /dev/null +++ b/docs/testing-with-flashlight.md @@ -0,0 +1,133 @@ +# Testing against a throwaway shop + +PrestaFlow drives a real PrestaShop. These are disposable ones, from the +official [Flashlight](https://github.com/PrestaShop/prestashop-flashlight) +images, so nothing depends on a shop somebody set up by hand. + +One `docker compose` service per version, each with its own database and its +own ports: + +| Service | PrestaShop | Shop 1 | Shop 2 | Shop 1 theme | Shop 2 theme | +|---|---|---|---|---|---| +| `ps17` | 1.7.8.11 | 8017 | 8018 | classic | classic | +| `ps82` | 8.2.8 | 8082 | 8083 | classic | classic | +| `ps92` | 9.2.0 | 8092 | 8093 | hummingbird | classic | + +The 9.2 container is the interesting one: a single boot gives you **both** +themes — hummingbird on 8092, classic on 8093 — which is what you want when +you are checking that a theme-aware selector really resolves per theme instead +of happening to match on the one theme you tested. + +## Prerequisites + +Docker Compose v2, PHP 8.3+, and the library's dependencies: + +```bash +composer install +``` + +Ports 8017/8018, 8082/8083 and 8092/8093 must be free. If something else holds +the port, every check below passes against the wrong shop: + +```bash +docker ps --format '{{.Names}}\t{{.Ports}}' | grep 809 || echo "8092/8093 free" +``` + +## Start a shop + +```bash +docker compose up ps92 -d # 9.2.0 on :8092, or ps82 / ps17 +``` + +Then wait for it. **Poll `/admin-dev/` for a `302`, not `/` for a `200`.** A +`200` on `/` proves only that *something* holds the port — a leftover +hand-made container answers it just as happily, and then every check below +passes against the wrong shop. `/admin-dev/` is the discriminator: Flashlight +fixes the admin folder and redirects it to the login page, while a hand-made +install with a random admin folder returns 404. The `302` also means PHP is +really executing, not just nginx answering. + +```bash +for i in $(seq 1 60); do + code=$(curl -s -o /dev/null -w '%{http_code}' http://localhost:8092/admin-dev/ || true) + [ "$code" = "302" ] || [ "$code" = "200" ] && { echo "back office answers $code — shop is up"; break; } + echo "back office answers ${code:-000}, waiting..."; sleep 5 +done +``` + +Measured on 2026-09-24, a `ps92` boot goes from `up -d` to a `302` in about 8 +seconds, with the post-scripts already finished by the time anything answers +at all — Flashlight runs them before the web server accepts connections. Do +not rely on that ordering, though: see the first trap below. + +The first boot also provisions a second shop **on its own port** (8093 here; +8018 for `ps17`, 8083 for `ps82`), so multistore is there by default rather +than being something you build once and forget. A dedicated port rather than a +virtual URI such as `/shop2/` is deliberate: PrestaShop discriminates shops by +domain *including the port*, and every Flashlight tag used here is `-nginx`, +which never reads `.htaccess` — a shop on a virtual URI would serve no assets +at all. Check it is really a *second* shop and not shop 1 answering on another +port — on `ps92` the themes differ, which makes the check falsifiable: + +```bash +curl -s http://localhost:8092/ | grep -o 'themes/[a-z]*/' | head -1 # hummingbird +curl -s http://localhost:8093/ | grep -o 'themes/[a-z]*/' | head -1 # classic +``` + +## Configure + +```bash +cp .env.flashlight.example .env.flashlight +``` + +The back office is at `/admin-dev/` with `admin@prestashop.com` / `prestashop` +— fixed by the image, so unlike a hand-made install there is no random admin +folder to look up. Edit `.env.flashlight` if you are pointing at `ps17` or +`ps82`: change the port in both URLs, `PRESTAFLOW_PS_VERSION`, and +`PRESTAFLOW_THEME` (both older versions are `classic`). + +## Run a suite + +```bash +set -a; . ./.env.flashlight; set +a +php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +Exported variables win over a `.env` / `.env.local` sitting in the repo: the +library loads those with Dotenv's *immutable* loader, which never overwrites a +value already in the environment. So sourcing `.env.flashlight` is enough — +you do not have to move your own `.env.local` out of the way. + +The same suite runs in CI (`.github/workflows/live-smoke.yml`) against all +three versions on every push to `main` and every pull request. + +## Tear down + +```bash +docker compose down -v # -v also drops the database volume +``` + +Leaving out `-v` keeps the data, which is occasionally what you want and +usually how you end up debugging a shop whose state you no longer understand. + +## Two traps worth knowing + +**"The container is up" does not mean "the shop is provisioned."** A post-script +that fails does *not* stop the container. `ON_POST_SCRIPT_FAILURE` defaults to +`fail`, but the handler's `exit 8` runs inside the child process that `xargs` +spawns, so it never reaches the container: the failure is logged and the +container stays `Up (healthy)`, serving a half-built shop. Verified on +2026-09-24 with a deliberately failing script. Do not treat a green +`docker compose ps` as proof — run the smoke suite, which is what actually +exercises the shop. If something looks wrong, read the boot log: + +```bash +docker compose logs ps92 | grep -i -e 'post-script' -e '✅' -e 'error' +``` + +**A duplicated shop has no payment methods.** PrestaShop does not copy +`ps_module_carrier` when duplicating a shop — upstream bug +[PrestaShop#42964](https://github.com/PrestaShop/PrestaShop/issues/42964), +reported 2026-09-24. `docker/post-scripts/20-carrier-restrictions.sh` copies +the rows as a workaround. Without it, checkout on the second shop stops at +"no payment method available", with nothing in the logs to explain why. From 8cca81f84cd438ef893dc79ada3c05b852cec9bb Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 15:14:35 +0200 Subject: [PATCH 10/12] chore: retire the superseded flashlight plan, commit the one that shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md. It pinned prestashop/prestashop-flashlight:9.0.1, a tag that does not exist — only suffixed tags such as 9.0.1-nginx resolve — so its acceptance criterion "docker compose up ps90 -d returns HTTP 200" could never have passed. The compose file it produced sat uncommitted in a working tree for two months. The plan that replaced it is committed alongside, with its tasks marked against the commits that landed them. It carries the corrections the work forced on it, rather than reading as though it had been right from the start: - Task 1's verify probed `/` on port 8092 and was a check that could not fail: a leftover hand-made container held that port and answered 200. It now probes /admin-dev/, which only Flashlight serves. - The second shop moved from a virtual URI to its own port. The virtual URI depended on Tools::generateHtaccess(), and every tag here is -nginx, which never reads .htaccess. There is no -apache tag for 1.7.8.11, 8.2.8 or 9.2.0. - A criterion asserting that a failing post-script aborts the container was removed. It came from the documentation and is false in practice: the handler's exit 8 runs in a child xargs spawns, so the container stays up and healthy. One criterion is deliberately not met: "no committed file references ps92rc1". docs/superpowers/plans/2026-09-23-one-page-checkout.md does, and should — it is a record of work that really was done against that container. Rewriting it to mention Flashlight would falsify history to satisfy a grep. Co-Authored-By: Claude Opus 5 --- ...26-09-24-flashlight-test-infrastructure.md | 905 ++++++++++++++++++ ...ashlight-test-infrastructure.md.tasks.json | 58 ++ 2 files changed, 963 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md create mode 100644 docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md.tasks.json diff --git a/docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md b/docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md new file mode 100644 index 0000000..c114810 --- /dev/null +++ b/docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md @@ -0,0 +1,905 @@ +# Flashlight Test Infrastructure Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development (recommended) or superpowers-extended-cc:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the hand-built `ps92rc1` container with reproducible PrestaShop shops from the official `prestashop-flashlight` images, provisioned by post-scripts, used both locally and in CI, and run the real smoke suite against 1.7.8.11, 8.2.8 and 9.2.0. + +**Architecture:** A repo-root `docker-compose.yml` declares one Flashlight shop plus one MariaDB per PrestaShop version. Each shop mounts `docker/post-scripts/`, which Flashlight executes **after** PrestaShop has started — the only point at which a second shop can be provisioned. A GitHub Actions matrix boots one shop per job and runs the smoke suite against it. The 1.7 and 8.2 rows are expected to fail at first: their 84 page objects are nine-line stubs inheriting v9's selectors, and the failures are the point — they are the inventory of what actually differs per version. + +**Tech Stack:** Docker Compose v2, `prestashop/prestashop-flashlight` images, GitHub Actions, the existing PrestaFlow runner (`bin/prestaflow`). + +--- + +## Supersedes + +`docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md`. That plan +was never completed and could not have been: it pinned +`prestashop/prestashop-flashlight:9.0.1`, a tag that does not exist (404 on the +registry — only suffixed tags such as `9.0.1-nginx` resolve). Its acceptance +criterion "`docker compose up ps90 -d` returns HTTP 200" was therefore +unachievable, and the `docker-compose.yml` it produced was left uncommitted in +the working tree for two months. Delete the old plan when this one lands. + +## Verified facts this plan rests on + +Checked on 2026-09-24 against the registry and the upstream README, because the +previous plan died on an unverified tag: + +| Fact | Value | +|---|---| +| Image tags that resolve | `1.7.8.11-nginx`, `8.2.8-nginx`, `9.0.3-nginx`, `9.2.0-nginx` | +| Bare tags (no suffix) | **404 for every 9.x and 8.x**; only `1.7.8.11` resolves | +| Back office URL | `{PS_DOMAIN}/admin-dev` — fixed, no random folder | +| Back office login | `admin@prestashop.com` / `prestashop` | +| Post-startup hook | `POST_SCRIPTS_DIR`, default `/tmp/post-scripts` | +| Failure behaviour | `ON_POST_SCRIPT_FAILURE=fail` (default) aborts the container | +| Theme variants | **none** — no `-hummingbird` tags, and only four ad-hoc `-classic` tags, all 8.x | + +That last row matters: Flashlight gives you a version's default theme and +nothing else. Classic-on-9.2 coverage needs the theme switched inside the +container, which `SwitchTheme` already does through the back office. + +## File Structure + +**New:** +- `docker-compose.yml` — repo root; services `ps17`, `ps82`, `ps92` and one MariaDB each. +- `docker/post-scripts/10-second-shop.sh` — provisions a second shop in the running container. +- `docker/post-scripts/20-carrier-restrictions.sh` — works around PrestaShop#42964. +- `.env.flashlight.example` — the `PRESTAFLOW_*` values for each shop. +- `.github/workflows/live-smoke.yml` — matrix over the three versions. +- `src/Tests/Suites/Smoke/FrontOfficeSmoke.php` — the suite CI runs. +- `docs/testing-with-flashlight.md` — one-page guide. + +**Modified:** +- `README.md` — a "Run a suite against a throwaway shop" section linking the guide. +- `.gitignore` — add `.env.flashlight` if absent. + +**Untouched:** every existing page object and scenario. This task adds infrastructure; it changes no library behaviour. + +--- + +### Task 1: Compose fixture + +**Goal:** `docker compose up ps92 -d` serves a working PrestaShop 9.2.0 at `http://localhost:8092/`, and the same for 1.7.8.11 on 8017 and 8.2.8 on 8082. + +**Files:** +- Create: `docker-compose.yml` +- Create: `.env.flashlight.example` +- Modify: `.gitignore` + +**Acceptance Criteria:** +- [ ] `docker compose config` validates with no error. +- [ ] Each of the three shops answers HTTP 200 on its port within 180s of `up -d`. +- [ ] Each back office answers HTTP 200 or 302 at `/admin-dev/`. +- [ ] `.gitignore` contains `.env.flashlight`. + +**Verify:** + +```bash +docker compose up ps92 -d +until curl -sf http://localhost:8092/admin-dev/ >/dev/null 2>&1 \ + || [ "$(curl -s -o /dev/null -w '%{http_code}' http://localhost:8092/admin-dev/)" = "302" ]; do sleep 5; done +echo OK +``` + +**This deliberately probes `/admin-dev/`, not `/`.** An earlier version of this +plan checked `/` and was a check that could not fail: a leftover hand-made +container was bound to 8092 and answered 200, so the verify passed while nothing +Flashlight had started. `/admin-dev/` is the discriminator — Flashlight serves it +(302 to the login), a hand-made install with a random admin folder returns 404. + +**Precondition:** port 8092 must be free. If anything else holds it, this task's +verification is meaningless rather than merely inconvenient. Check with +`docker ps --format '{{.Names}}\t{{.Ports}}' | grep 8092` before starting. + +**Steps:** + +- [ ] **Step 1: Write `docker-compose.yml`** + +```yaml +# Throwaway PrestaShop shops from the official Flashlight images. +# +# One shop and one database per version. Bring up a single shop at a time: +# docker compose up ps92 -d +# Its database starts with it through depends_on. +# +# Tags carry a suffix on purpose. The bare `9.2.0` and `8.2.8` tags do not +# exist on the registry — a previous attempt at this file pinned `9.0.1` and +# could never start. +services: + db17: + image: mariadb:11 + environment: + MARIADB_ROOT_PASSWORD: prestashop + MARIADB_DATABASE: prestashop + MARIADB_USER: prestashop + MARIADB_PASSWORD: prestashop + healthcheck: + test: ["CMD", "healthcheck.sh", "--connect"] + interval: 5s + timeout: 5s + retries: 20 + + ps17: + image: prestashop/prestashop-flashlight:1.7.8.11-nginx + depends_on: + db17: + condition: service_healthy + environment: + PS_DOMAIN: localhost:8017 + MYSQL_HOST: db17 + DEBUG_MODE: "false" + POST_SCRIPTS_DIR: /tmp/post-scripts + volumes: + - ./docker/post-scripts:/tmp/post-scripts:ro + ports: + - "8017:80" + - "8018:80" # the second shop + + db82: + image: mariadb:11 + environment: + MARIADB_ROOT_PASSWORD: prestashop + MARIADB_DATABASE: prestashop + MARIADB_USER: prestashop + MARIADB_PASSWORD: prestashop + healthcheck: + test: ["CMD", "healthcheck.sh", "--connect"] + interval: 5s + timeout: 5s + retries: 20 + + ps82: + image: prestashop/prestashop-flashlight:8.2.8-nginx + depends_on: + db82: + condition: service_healthy + environment: + PS_DOMAIN: localhost:8082 + MYSQL_HOST: db82 + DEBUG_MODE: "false" + POST_SCRIPTS_DIR: /tmp/post-scripts + volumes: + - ./docker/post-scripts:/tmp/post-scripts:ro + ports: + - "8082:80" + - "8083:80" # the second shop + + db92: + image: mariadb:11 + environment: + MARIADB_ROOT_PASSWORD: prestashop + MARIADB_DATABASE: prestashop + MARIADB_USER: prestashop + MARIADB_PASSWORD: prestashop + healthcheck: + test: ["CMD", "healthcheck.sh", "--connect"] + interval: 5s + timeout: 5s + retries: 20 + + ps92: + image: prestashop/prestashop-flashlight:9.2.0-nginx + depends_on: + db92: + condition: service_healthy + environment: + PS_DOMAIN: localhost:8092 + MYSQL_HOST: db92 + DEBUG_MODE: "false" + POST_SCRIPTS_DIR: /tmp/post-scripts + volumes: + - ./docker/post-scripts:/tmp/post-scripts:ro + ports: + - "8092:80" + - "8093:80" # the second shop, on its own port rather than a virtual URI +``` + +- [ ] **Step 2: Write `.env.flashlight.example`** + +```bash +# Values for a Flashlight shop. Copy to .env.flashlight and adjust the port. +# +# The back office folder and credentials are fixed by the image, so unlike a +# hand-made install there is nothing machine-specific to discover here. +PRESTAFLOW_FO_URL=http://localhost:8092/ +PRESTAFLOW_BO_URL=http://localhost:8092/admin-dev/ +PRESTAFLOW_BO_EMAIL=admin@prestashop.com +PRESTAFLOW_BO_PASSWD=prestashop +PRESTAFLOW_LOCALE=en + +# One of these per shop: +# 1.7.8.11 -> port 8017, theme classic +# 8.2.8 -> port 8082, theme classic +# 9.2.0 -> port 8092, theme hummingbird +PRESTAFLOW_PS_VERSION=9.2.0 +PRESTAFLOW_THEME=hummingbird +``` + +- [ ] **Step 3: Add `.env.flashlight` to `.gitignore`** + +```bash +grep -q '^\.env\.flashlight$' .gitignore || printf '.env.flashlight\n' >> .gitignore +grep -n 'env.flashlight' .gitignore +``` + +Expected: one matching line. + +- [ ] **Step 4: Validate and boot** + +```bash +docker compose config >/dev/null && echo "compose OK" +docker compose up ps92 -d +until curl -sf http://localhost:8092/ >/dev/null; do sleep 5; done; echo "FO OK" +curl -s -o /dev/null -w '%{http_code}\n' http://localhost:8092/admin-dev/ +``` + +Expected: `compose OK`, then `FO OK`, then `200` or `302`. + +- [ ] **Step 5: Commit** + +```bash +git add docker-compose.yml .env.flashlight.example .gitignore +git commit -m "chore(infra): flashlight compose fixture for 1.7, 8.2 and 9.2" +``` + +--- + +### Task 2: Second shop, provisioned by a post-script + +**Goal:** Every Flashlight shop boots with a working second shop, so multistore is exercised by default instead of being a state somebody built by hand once. + +**Files:** +- Create: `docker/post-scripts/10-second-shop.sh` +- Create: `docker/post-scripts/20-carrier-restrictions.sh` + +**Acceptance Criteria:** +- [ ] After `docker compose up ps92 -d`, the second shop answers HTTP 200 on its own port (`http://localhost:8093/`). +- [ ] `ps_shop` contains two rows. +- [ ] `ps_module_carrier` has as many rows for shop 2 as for shop 1. +- [ ] Shop 2's theme assets resolve: fetching its stylesheet on the second port returns 200. +- [ ] Shop 2 is reachable on its own port and shop 1 is unaffected on its own. + +**A criterion deliberately NOT claimed:** the first draft of this plan asserted +that a failing post-script aborts the container, because `ON_POST_SCRIPT_FAILURE` +defaults to `fail`. That was taken from the documentation and is **false in +practice** — verified on 2026-09-24 by running a deliberately failing script: the +container logged the failure and stayed up and healthy. In Flashlight's +`run.sh`, the handler's `exit 8` runs inside the `sh -c` child that `xargs` +spawns, so it kills the child, and the pipeline's exit status is `awk`'s and is +never checked. A half-provisioned shop keeps serving. + +Do not write a criterion around it. The real protection is Task 3's suite +asserting the second shop works, so a broken provisioning fails loudly at test +time instead of pretending at boot time. + +**Verify:** + +```bash +curl -s -o /dev/null -w 'shop2 %{http_code}\n' http://localhost:8093/ +``` + +Expected `200`. Then confirm its assets really resolve, which is the check the +virtual-URI design silently failed: + +```bash +curl -s http://localhost:8093/ | grep -o 'href="[^"]*\.css[^"]*"' | head -1 +# take that URL and fetch it — it must return 200, not 404 +``` + +**Steps:** + +- [ ] **Step 1: Write `docker/post-scripts/10-second-shop.sh`** + +Note the two traps this encodes, both found the hard way on 2026-09-24: +`Shop::copyShopData()` reads `Tools::getValue('categoryBox')`, so it fatals +outside an HTTP request unless `$_POST` is primed; and a shop on a virtual URI +serves no assets until `.htaccess` is regenerated. + +```bash +#!/bin/sh +set -eu + +echo "* Provisioning a second shop..." + +cat > /tmp/second-shop.php <<'PHP' +name = 'Group2'; +$group->active = true; +$group->add(); + +$shop = new Shop(); +$shop->name = 'Shop2'; +$shop->id_shop_group = (int) $group->id; +$shop->id_category = (int) Configuration::get('PS_HOME_CATEGORY'); +$shop->theme_name = 'classic'; +$shop->active = true; +$shop->add(); + +$url = new ShopUrl(); +$url->id_shop = (int) $shop->id; +$secondPort = getenv('SECOND_SHOP_PORT') ?: '8093'; +$host = preg_replace('/:\d+$/', '', (string) Configuration::get('PS_SHOP_DOMAIN')); +$url->domain = $url->domain_ssl = $host . ':' . $secondPort; +// A DEDICATED PORT, not a virtual URI. PrestaShop discriminates shops by +// domain including the port, so localhost:8093 is a distinct shop — and its +// assets resolve at the root, needing no rewrite at all. +// +// The virtual-URI approach this replaced cannot work here: it relies on +// Tools::generateHtaccess(), and every Flashlight tag we use is -nginx, which +// never reads .htaccess. Verified 2026-09-24: /shop2/themes/...css returned +// 404 while the same file at the root returned 200. There is no -apache tag +// for 1.7.8.11, 8.2.8 or 9.2.0 (only 9.0.3 has one), so switching flavour is +// not an option either. +$url->physical_uri = '/'; +$url->virtual_uri = ''; +$url->main = true; +$url->active = true; +$url->add(); + +// copyShopData() reads Tools::getValue('categoryBox'): outside an HTTP request +// that returns false and count(false) is fatal on PHP 8. Prime it with the +// source shop's categories, which is what the Multistore form would post. +$rows = Db::getInstance()->executeS( + 'SELECT id_category FROM ' . _DB_PREFIX_ . 'category_shop WHERE id_shop = ' . (int) $src +); +$categories = array_map('intval', array_column($rows ?: [], 'id_category')); +$_POST['categoryBox'] = $_REQUEST['categoryBox'] = $categories; + +// The same 26 keys the Multistore form offers. +$keys = [ + 'carrier', 'cms', 'contact', 'country', 'currency', 'discount', 'employee', + 'image', 'lang', 'manufacturer', 'module', 'hook_module', 'meta_lang', + 'product', 'product_attribute', 'stock_available', 'store', + 'webservice_account', 'attribute_group', 'feature', 'group', + 'tax_rules_group', 'supplier', 'zone', 'cart_rule', +]; +$shop->copyShopData($src, array_fill_keys($keys, 'on')); +$shop->associateSuperAdmins(); + +array_unshift($categories, (int) Configuration::get('PS_ROOT_CATEGORY')); +Category::updateFromShop(array_values(array_unique($categories)), (int) $shop->id); + +// Module-owned data travels through a hook, not through copyShopData(). +foreach ((array) Hook::getHookModuleExecList('actionShopDataDuplication') as $m) { + Hook::exec('actionShopDataDuplication', [ + 'old_id_shop' => $src, + 'new_id_shop' => (int) $shop->id, + ], (int) $m['id_module']); +} + +// Multistore is enabled, and its back-office screens must exist with it: +// setting the flag without activating the tabs gives a shop that cannot be +// managed, which is exactly how the previous hand-made container ended up. +Configuration::updateValue('PS_MULTISHOP_FEATURE_ACTIVE', 1); +Db::getInstance()->execute( + 'UPDATE ' . _DB_PREFIX_ . "tab SET active = 1 WHERE class_name IN ('AdminShopGroup','AdminShopUrl')" +); + +// Nothing to rewrite: the second shop lives at the root of its own port. + +echo 'second shop id=' . (int) $shop->id . PHP_EOL; +PHP + +php /tmp/second-shop.php +rm -f /tmp/second-shop.php + +echo "✅ Second shop provisioned" +``` + +- [ ] **Step 2: Write `docker/post-scripts/20-carrier-restrictions.sh`** + +```bash +#!/bin/sh +set -eu + +# Works around PrestaShop#42964: ps_module_carrier is not in +# Shop::getAssoTables(), so a duplicated shop inherits no carrier restrictions +# and Hook::getHookModuleExecList() then offers it no payment method at all. +# Without this, checkout on shop 2 stops at "no payment method available". +echo "* Copying carrier restrictions to the second shop..." + +cat > /tmp/carrier-restrictions.php <<'PHP' +executeS('SELECT id_shop FROM ' . _DB_PREFIX_ . 'shop WHERE id_shop <> 1'); +foreach ($rows ?: [] as $row) { + Db::getInstance()->execute( + 'INSERT IGNORE INTO ' . _DB_PREFIX_ . 'module_carrier (id_module, id_shop, id_reference) ' + . 'SELECT id_module, ' . (int) $row['id_shop'] . ', id_reference ' + . 'FROM ' . _DB_PREFIX_ . 'module_carrier WHERE id_shop = 1' + ); +} +PHP + +php /tmp/carrier-restrictions.php +rm -f /tmp/carrier-restrictions.php + +echo "✅ Carrier restrictions copied" +``` + +- [ ] **Step 3: Make both executable** + +```bash +chmod +x docker/post-scripts/10-second-shop.sh docker/post-scripts/20-carrier-restrictions.sh +ls -l docker/post-scripts/ +``` + +Expected: both show the executable bit. + +- [ ] **Step 4: Boot from scratch and verify** + +```bash +docker compose down -v +docker compose up ps92 -d +until curl -sf http://localhost:8092/ >/dev/null; do sleep 5; done +curl -s -o /dev/null -w 'shop2 %{http_code}\n' http://localhost:8092/shop2/ +docker compose exec -T db92 mariadb -uroot -pprestashop prestashop \ + -e "SELECT COUNT(*) AS shops FROM ps_shop; SELECT SUM(id_shop=1) s1, SUM(id_shop=2) s2 FROM ps_module_carrier;" +``` + +Expected: `shop2 200`, `shops` = 2, and `s1` equal to `s2`. + +- [ ] **Step 5: Commit** + +```bash +git add docker/post-scripts +git commit -m "chore(infra): provision a second shop in every flashlight container" +``` + +--- + +### Task 3: The smoke suite CI runs + +**Goal:** One suite that walks Home → Listing → Product and is meaningful on any supported +version, so the matrix has something real to run. + +**Files:** +- Create: `src/Tests/Suites/Smoke/FrontOfficeSmoke.php` +- Modify: `src/Pages/v9/FrontOffice/Home/Page.php` (add `isDisplayed()`) + +**Acceptance Criteria:** +- [ ] The suite passes against the 9.2 shop on port 8092. +- [ ] Every step carries an assertion. +- [ ] It is proven able to fail (see Step 4). +- [ ] It hardcodes no product URL, no category id and no theme-specific selector. + +**Verify:** with ps92 up and the env exported, +`php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php` → every step PASS. + +**Why this shape:** three pages, each reached from the previous one, so a failure names the page +whose markup differs instead of reporting a generic dead end. The last assertion reads a PRICE on +purpose: it exercises the listing-to-product navigation, the product page's price selector and +`parsePrice()` in one go — the exact combination that was silently returning `0.0` on hummingbird +until 2026-09-24. + +**Steps:** + +- [ ] **Step 1: Add `isDisplayed()` to the v9 Home page** + +The suite needs to assert it reached the home page. `homePageSection` already exists as a +selector; nothing exposes it as a question. + +```php + /** + * Whether the home page rendered its own content section. + * + * Distinct from "the request returned 200": a maintenance page, an error + * page and a redirect to another shop all answer 200 too. + */ + public function isDisplayed(): bool + { + return $this->elementIsVisible($this->getSelector('homePageSection'), 5000); + } +``` + +- [ ] **Step 2: Write the suite** + +```php +importPage('FrontOffice\Home'); + $this->importPage('FrontOffice\Listing'); + $this->importPage('FrontOffice\Product'); + + extract($this->pages); + + $this + ->describe('Front office smoke') + ->it('the home page renders', function () use ($frontOfficeHomePage) { + $frontOfficeHomePage->goToPage('home'); + + Expect::that($frontOfficeHomePage->isDisplayed())->equals(true); + }) + ->it('reach the product listing from the home page', function () use ($frontOfficeHomePage, $frontOfficeListingPage) { + // Reads the "all products" link out of the home page and follows it, + // so a theme that renames that link fails here and names Home. + $frontOfficeHomePage->goToAllProducts(); + + Expect::that($frontOfficeListingPage->getListingTitle())->notEquals(''); + }) + ->it('open a product and read its price', function () use ($frontOfficeListingPage, $frontOfficeProductPage) { + $frontOfficeListingPage->goToProduct(1); + + // A price above zero proves three things at once: the listing linked + // to a real product, the product page matched its price element, and + // parsePrice() understood the rendered format. + Expect::that($frontOfficeProductPage->getPrice() > 0)->equals(true); + }); + } +} +``` + +- [ ] **Step 3: Run it against the 9.2 shop** + +```bash +docker compose up ps92 -d +until [ "$(curl -s -o /dev/null -w '%{http_code}' http://localhost:8092/admin-dev/)" = "302" ]; do sleep 5; done + +PRESTAFLOW_PS_VERSION=9.2.0 \ +PRESTAFLOW_FO_URL=http://localhost:8092/ \ +PRESTAFLOW_BO_URL=http://localhost:8092/admin-dev/ \ +PRESTAFLOW_BO_EMAIL=admin@prestashop.com \ +PRESTAFLOW_BO_PASSWD=prestashop \ +PRESTAFLOW_THEME=hummingbird \ +PRESTAFLOW_LOCALE=en \ +php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +Expected: three steps, all PASS. + +- [ ] **Step 4: Prove it can fail** + +Point it at the second shop's port but keep the hummingbird theme, on a shop that runs classic: + +```bash +PRESTAFLOW_FO_URL=http://localhost:8093/ PRESTAFLOW_THEME=hummingbird \ + PRESTAFLOW_PS_VERSION=9.2.0 PRESTAFLOW_LOCALE=en \ + php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +If that still passes, the suite is not discriminating and must be tightened before +it is worth putting in CI. Record which step failed and how. + +- [ ] **Step 5: Run it against the second shop, correctly** + +```bash +PRESTAFLOW_FO_URL=http://localhost:8093/ PRESTAFLOW_THEME=classic \ + PRESTAFLOW_PS_VERSION=9.2.0 PRESTAFLOW_LOCALE=en \ + php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +Expected: PASS. This is what proves the provisioned second shop is genuinely usable, which is the +protection Task 2 could not get from the container (a failing post-script does not stop it). + +- [ ] **Step 6: Commit** + +```bash +git add src/Tests/Suites/Smoke/FrontOfficeSmoke.php src/Pages/v9/FrontOffice/Home/Page.php +git commit -m "test(smoke): a front-office suite fit to run on every version" +``` + +--- + +### Task 4: The CI matrix + +**Goal:** Every push to `main` and every pull request runs the smoke suite against 1.7.8.11, 8.2.8 and 9.2.0. + +**Files:** +- Create: `.github/workflows/live-smoke.yml` + +**Acceptance Criteria:** +- [ ] The workflow runs three jobs, one per version, with `fail-fast: false`. +- [ ] The 9.2 job passes. +- [ ] The 1.7 and 8.2 jobs run the suite for real — they are **not** skipped, and they are expected to fail until the v7/v8 page objects stop being stubs. +- [ ] Each job uploads its failure screenshots as an artifact. + +**Verify:** push the branch and read the run — three jobs present, 9.2 green. + +**Steps:** + +- [ ] **Step 1: Write the workflow** + +```yaml +name: Live smoke + +on: + push: + branches: [main] + pull_request: + +jobs: + smoke: + name: Smoke (PrestaShop ${{ matrix.ps }}) + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + include: + - ps: "1.7.8.11" + service: ps17 + port: 8017 + theme: classic + - ps: "8.2.8" + service: ps82 + port: 8082 + theme: classic + - ps: "9.2.0" + service: ps92 + port: 8092 + theme: hummingbird + + steps: + - uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: "8.3" + + - name: Install dependencies + run: composer install --no-interaction --prefer-dist + + - name: Start the shop + run: docker compose up ${{ matrix.service }} -d + + - name: Wait for the shop + run: | + for i in $(seq 1 60); do + if curl -sf "http://localhost:${{ matrix.port }}/" >/dev/null; then + echo "shop is up"; exit 0 + fi + sleep 5 + done + echo "shop never came up"; docker compose logs ${{ matrix.service }}; exit 1 + + - name: Run the smoke suite + env: + PRESTAFLOW_FO_URL: http://localhost:${{ matrix.port }}/ + PRESTAFLOW_BO_URL: http://localhost:${{ matrix.port }}/admin-dev/ + PRESTAFLOW_BO_EMAIL: admin@prestashop.com + PRESTAFLOW_BO_PASSWD: prestashop + PRESTAFLOW_PS_VERSION: ${{ matrix.ps }} + PRESTAFLOW_THEME: ${{ matrix.theme }} + PRESTAFLOW_LOCALE: en + run: php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php + + - name: Upload failure screenshots + if: failure() + uses: actions/upload-artifact@v4 + with: + name: screenshots-${{ matrix.ps }} + path: prestaflow/screens/ + if-no-files-found: ignore +``` + +- [ ] **Step 2: Say in the README that red is expected, and why** + +Append to the section added in Task 5: + +```markdown +The 1.7 and 8.2 rows of the live-smoke matrix are expected to fail today. Their +page objects are nine-line stubs inheriting the v9 selectors, so the matrix is +reporting an absence of support rather than a regression. Each failure names the +page that differs, which is the inventory the v7/v8 work needs. +``` + +- [ ] **Step 3: Push and read the run** + +```bash +git add .github/workflows/live-smoke.yml +git commit -m "ci: run the smoke suite against 1.7, 8.2 and 9.2" +git push origin HEAD +gh run list --limit 1 +``` + +Expected: three jobs, 9.2 green, the other two red with named failures. + +--- + +### Task 5: Documentation + +**Goal:** Someone who has never seen this repo can get a shop running and a suite passing in three commands. + +**Files:** +- Create: `docs/testing-with-flashlight.md` +- Modify: `README.md` + +**Acceptance Criteria:** +- [ ] The guide covers: start a shop, configure, run a suite, tear down, and the two known traps. +- [ ] `README.md` links it. +- [ ] Every command in the guide has been run. + +**Verify:** follow the guide from a clean checkout on a machine with no shop running; the smoke suite passes. + +**Steps:** + +- [ ] **Step 1: Write `docs/testing-with-flashlight.md`** + +```markdown +# Testing against a throwaway shop + +PrestaFlow drives a real PrestaShop. These are disposable ones, from the +official [Flashlight](https://github.com/PrestaShop/PrestaShop-Flashlight) +images, so nothing depends on a shop somebody set up by hand. + +## Start a shop + +```bash +docker compose up ps92 -d # 9.2.0 on :8092, or ps82 / ps17 +``` + +The first boot provisions a second shop at `/shop2/` as well, so multistore is +there by default rather than being something you build once and forget. + +## Configure + +```bash +cp .env.flashlight.example .env.flashlight +``` + +The back office is at `/admin-dev/` with `admin@prestashop.com` / `prestashop` +— fixed by the image, so there is no random folder to look up. + +## Run a suite + +```bash +set -a; . ./.env.flashlight; set +a +php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +## Tear down + +```bash +docker compose down -v # -v also drops the database volume +``` + +Leaving out `-v` keeps the data, which is occasionally what you want and +usually how you end up debugging a shop whose state you no longer understand. + +## Two traps worth knowing + +**A shop on a virtual URI serves no assets until `.htaccess` is regenerated.** +The provisioning script does it. If you add a shop by hand, the front office +renders unstyled, the JavaScript never loads, and add-to-cart silently does +nothing. + +**A duplicated shop has no payment methods.** `ps_module_carrier` is not copied +by PrestaShop (see +[PrestaShop#42964](https://github.com/PrestaShop/PrestaShop/issues/42964)), so +checkout stops at "no payment method available". The provisioning script copies +the rows. +``` + +- [ ] **Step 2: Add the README section** + +```markdown +## Run a suite against a throwaway shop + +```bash +docker compose up ps92 -d +cp .env.flashlight.example .env.flashlight +set -a; . ./.env.flashlight; set +a +php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php +``` + +See [Testing with Flashlight](docs/testing-with-flashlight.md). +``` + +- [ ] **Step 3: Follow the guide from scratch** + +```bash +docker compose down -v +# then run every command in the guide, in order +``` + +Expected: the smoke suite passes. + +- [ ] **Step 4: Commit** + +```bash +git add docs/testing-with-flashlight.md README.md +git commit -m "docs: testing against a throwaway flashlight shop" +``` + +--- + +### Task 6: Retire the hand-made container + +**Goal:** One shop story, not two. The `ps92rc1` container stops being the reference. + +**Files:** +- Delete: `docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md` + +**Acceptance Criteria:** +- [ ] The superseded July plan is gone. +- [ ] `docs/testing-with-flashlight.md` is the only documented way to get a shop. +- [ ] No committed file references `ps92rc1` or a hand-made admin folder. + +**Verify:** `grep -rn "ps92rc1" --include="*.md" --include="*.php" --include="*.yml" . | grep -v vendor` → no output. + +**Steps:** + +- [ ] **Step 1: Check what still refers to the old container** + +```bash +grep -rn "ps92rc1\|admin259je3iqgg4jvpdzdlw" --include="*.md" --include="*.php" --include="*.yml" . | grep -v vendor/ || echo "no references" +``` + +- [ ] **Step 2: Remove the superseded plan** + +```bash +# Untracked, so plain rm — `git rm` would fail with "did not match any files". +rm -f docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md +rm -f docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md.tasks.json +``` + +- [ ] **Step 3: Commit** + +```bash +git commit -m "chore: retire the hand-made reference container" +``` + +**Note for whoever runs this task:** the `ps92rc1` container and its snapshots +in `~/prestaflow-ps92rc1-*` belong to the developer's machine, not to the repo. +Deleting them is the developer's call, not this plan's — say what is now +redundant and let them decide. + +--- + +## Self-Review Notes + +**Spec coverage.** The two decisions taken on 2026-09-24 are both implemented: +Flashlight replaces the hand-made container (Tasks 1, 2 and 6), and the smoke +suite runs on every version rather than skipping the ones expected to fail +(Task 4). + +**Placeholders.** None. Every file is given in full, every command is runnable, +and the one genuinely conditional step — Task 3 Step 2, if a page method turns +out to be missing — says to implement the method rather than to weaken the test. + +**Type consistency.** Service names (`ps17`, `ps82`, `ps92`), ports (8017, 8082, +8092) and database services (`db17`, `db82`, `db92`) are used identically in the +compose file, the workflow matrix and the documentation. + +**Correction applied 2026-09-24, after the Task 1 spec review.** Task 1's verify +line originally curled `/` on port 8092 and would have reported success from a +leftover container that happened to hold the port. It now probes `/admin-dev/`, +which only Flashlight serves, and states the free-port precondition. Every other +port check in this plan inherits the same hazard: while anything else holds 8092, +Tasks 2, 3 and 5 verify against the wrong shop. Free the port before running them. + +**Known risk, stated rather than hidden.** Task 2 provisions the second shop by +calling `Shop::copyShopData()` from the CLI. That method reads request state and +its behaviour across 1.7, 8.2 and 9.2 has only been verified on 9.2. If it +fails on an older version, `ON_POST_SCRIPT_FAILURE=fail` aborts the container — +loudly, which is the right failure. The fallback is to gate the script on the +version it runs under. diff --git a/docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md.tasks.json b/docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md.tasks.json new file mode 100644 index 0000000..60c7150 --- /dev/null +++ b/docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md.tasks.json @@ -0,0 +1,58 @@ +{ + "planPath": "docs/superpowers/plans/2026-09-24-flashlight-test-infrastructure.md", + "tasks": [ + { + "id": 1, + "subject": "Task 1: Compose fixture", + "status": "completed", + "description": "**Goal:** Three Flashlight shops reachable on 8017/8082/8092.\n\n**Files:**\n- docker-compose.yml\n- .env.flashlight.example\n- .gitignore\n\n**Verify:** docker compose up ps92 -d && sleep 90 && curl -sf http://localhost:8092/\n\n```json:metadata\n{\"files\": [\"docker-compose.yml\", \".env.flashlight.example\", \".gitignore\"], \"verifyCommand\": \"docker compose up ps92 -d && sleep 90 && curl -sf http://localhost:8092/\", \"acceptanceCriteria\": [\"compose config validates\", \"each shop answers 200\", \"back office answers 200/302\", \".gitignore has .env.flashlight\"]}\n```\n\n**Landed in:** 75ec522 + 5be2e19" + }, + { + "id": 2, + "subject": "Task 2: Second shop via post-script", + "status": "completed", + "description": "**Goal:** Every container boots with a working second shop.\n\n**Files:**\n- docker/post-scripts/10-second-shop.sh\n- docker/post-scripts/20-carrier-restrictions.sh\n\n**Verify:** curl -sf http://localhost:8092/shop2/\n\n```json:metadata\n{\"files\": [\"docker/post-scripts/10-second-shop.sh\", \"docker/post-scripts/20-carrier-restrictions.sh\"], \"verifyCommand\": \"curl -sf http://localhost:8092/shop2/\", \"acceptanceCriteria\": [\"/shop2/ answers 200\", \"ps_shop has 2 rows\", \"module_carrier s1==s2\", \"htaccess carries shop2 rules\"]}\n```\n\n**Landed in:** 8073588 + 5be2e19", + "blockedBy": [ + 1 + ] + }, + { + "id": 3, + "subject": "Task 3: Smoke suite", + "status": "completed", + "description": "**Goal:** A front-office suite meaningful on every version.\n\n**Files:**\n- src/Tests/Suites/Smoke/FrontOfficeSmoke.php\n\n**Verify:** php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php\n\n```json:metadata\n{\"files\": [\"src/Tests/Suites/Smoke/FrontOfficeSmoke.php\"], \"verifyCommand\": \"php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php\", \"acceptanceCriteria\": [\"passes on 9.2\", \"every step asserts\", \"proven to fail on a bad URL\"]}\n```\n\n**Landed in:** 3bee2ed", + "blockedBy": [ + 2 + ] + }, + { + "id": 4, + "subject": "Task 4: CI matrix", + "status": "completed", + "description": "**Goal:** Smoke suite runs on 1.7, 8.2 and 9.2.\n\n**Files:**\n- .github/workflows/live-smoke.yml\n\n**Verify:** gh run list --limit 1\n\n```json:metadata\n{\"files\": [\".github/workflows/live-smoke.yml\"], \"verifyCommand\": \"gh run list --limit 1\", \"acceptanceCriteria\": [\"three jobs, fail-fast false\", \"9.2 green\", \"1.7 and 8.2 run for real, not skipped\", \"screenshots uploaded on failure\"]}\n```\n\n**Landed in:** c578b61", + "blockedBy": [ + 3 + ] + }, + { + "id": 5, + "subject": "Task 5: Documentation", + "status": "completed", + "description": "**Goal:** Three commands from clean checkout to passing suite.\n\n**Files:**\n- docs/testing-with-flashlight.md\n- README.md\n\n**Verify:** follow the guide from a clean checkout\n\n```json:metadata\n{\"files\": [\"docs/testing-with-flashlight.md\", \"README.md\"], \"verifyCommand\": \"follow the guide from a clean checkout\", \"acceptanceCriteria\": [\"covers start/configure/run/teardown\", \"documents the two traps\", \"README links it\"]}\n```\n\n**Landed in:** fdab9ad", + "blockedBy": [ + 3 + ] + }, + { + "id": 6, + "subject": "Task 6: Retire the hand-made container", + "status": "completed", + "description": "**Goal:** One shop story, not two.\n\n**Files:**\n- docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md\n\n**Verify:** grep -rn 'ps92rc1' --include='*.md' --include='*.php' --include='*.yml' . | grep -v vendor\n\n```json:metadata\n{\"files\": [\"docs/superpowers/plans/2026-07-10-phase2-task0-flashlight-infra.md\"], \"verifyCommand\": \"grep -rn 'ps92rc1' --include='*.md' --include='*.php' --include='*.yml' . | grep -v vendor\", \"acceptanceCriteria\": [\"superseded plan deleted\", \"no committed reference to ps92rc1\"]}\n```\n\n**Landed in:** this commit", + "blockedBy": [ + 4, + 5 + ] + } + ], + "lastUpdated": "2026-09-24T15:14:20" +} \ No newline at end of file From 4e834c4fde7af17045c7cc2db5c729d9b944f335 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:13:15 +0200 Subject: [PATCH 11/12] test(smoke): widen front-office coverage and add a back-office smoke suite The front-office smoke walked Home -> Listing -> Product and was green on all three versions, which proved little: three pages that have not changed markup in three major versions will stay green forever. This widens it until it touches enough surface to be informative, and adds the back office, where the 1.7 -> 9 redesign made divergence most likely. FrontOfficeSmoke gains add-to-cart, the cart page, a category reached by id (category 3 is "Clothes" on every demo catalogue) and a product opened from that category. BackOfficeSmoke is new: login form, rejection of bad credentials, login, dashboard, logout, and a cross-check that the shop reports the version the runner was configured with. Measured on Flashlight, 2026-09-24, two runs per combination: Step 9.2 hb 9.2 cl 8.2 cl 1.7 cl --- FrontOfficeSmoke --- home renders ok ok ok ok reach listing from home ok ok ok ok open product, read price ok ok ok ok add to cart ok ok ok ok cart holds the product ok ok ok ok reach category 3 by id ok ok ok ok open product from category ok ok ok ok --- BackOfficeSmoke --- login form renders ok ok ok ok bad credentials rejected ok ok FLAKY FLAKY log in ok ok ok ok dashboard is where we land ok ok ok ok log out ok ok FAIL FAIL version cross-check FAIL FAIL FAIL FAIL WHAT DIVERGED 1. The version cross-check fails on EVERY version. Login\Page's `psVersionBlock` is `#login_form h4`, which never holds a version: - 9.2 reads "* PrestaShop", the required-field label of the new-theme login form; the version is nowhere on that page. - 8.2 reads "PrestaShop" from `

`; the version is nowhere on that page either. - 1.7 reads the same `#shop_name`, but the version IS on the page, in `#login-header > div.text-center` -- outside `#login_form`. So `getPrestashopVersion()` silently returns a label instead of a version on all three, and the identical assertion in Suites/BackOffice/Login.php has been passing its `isNotEmpty()` guard on that label. 2. Logout is a no-op on 1.7 and 8.2. `logout()` navigates to `{BO_URL}logout`, a real Symfony route on 9.2 only. On 1.7 and 8.2 the legacy dispatcher does not know it and leaves the employee logged in on the dashboard (confirmed by the failure screenshot). Those versions log out via `index.php?controller=AdminLogin&logout&token=...`. 3. The login-error read is racy on 1.7 and 8.2, not on 9.2. Their admin login is AJAX -- `
` driven by js/admin/login.js -- and the page ships `
` EMPTY at load time. `getLoginError()` waits on presence only, so it finds that empty node immediately and reads "" unless the XHR happens to have landed first. 9.2 does a real POST and renders `.alert-danger` only on the error response, so the same wait is sound there. WHAT DID NOT DIVERGE The whole front office, including add-to-cart, on both themes. The `.add-to-cart` / `.product__add-to-cart-button` split is already covered by Themes/hummingbird.json, and the v9 Classic selectors hold unchanged back to 1.7.8.11. The theme mechanism resolving per theme is confirmed by 9.2 serving hummingbird on :8092 and classic on :8093 from one container, so the workflow now runs both as separate matrix rows. Page-object defects are reported, not fixed: nothing under src/Pages changed. BackOfficeSmoke sets `skipWhenFailed(false)` on purpose. Its steps each navigate for themselves, and stopping at the first red would have hidden two of the three divergences on 1.7 and 8.2. FrontOfficeSmoke keeps the default: its chain is genuinely sequential, and a cart step running after a failed add-to-cart would report a divergence that is not there. Co-Authored-By: Claude Opus 5 --- .github/workflows/live-smoke.yml | 38 +++++++-- src/Tests/Suites/Smoke/BackOfficeSmoke.php | 92 +++++++++++++++++++++ src/Tests/Suites/Smoke/FrontOfficeSmoke.php | 55 +++++++++--- 3 files changed, 166 insertions(+), 19 deletions(-) create mode 100644 src/Tests/Suites/Smoke/BackOfficeSmoke.php diff --git a/.github/workflows/live-smoke.yml b/.github/workflows/live-smoke.yml index 5d4b048..77298fb 100644 --- a/.github/workflows/live-smoke.yml +++ b/.github/workflows/live-smoke.yml @@ -1,13 +1,21 @@ name: Live smoke -# Runs the front-office smoke suite against a throwaway Flashlight shop, one -# job per PrestaShop version. +# Runs the smoke suites against a throwaway Flashlight shop, one job per +# PrestaShop version. Both suites run in every job, and a red in either one +# fails the job. # -# The 1.7 and 8.2 rows are EXPECTED TO FAIL today: their page objects are +# Several rows are EXPECTED TO FAIL today. The v7 and v8 page objects are # nine-line stubs inheriting the v9 selectors, so the matrix reports an absence # of support rather than a regression. They are deliberately not skipped and # not `continue-on-error` — the failures are the inventory of what differs per # version. `fail-fast: false` keeps one red row from cancelling the others. +# +# Measured 2026-09-24 against Flashlight (see the commit that added +# BackOfficeSmoke for the full table): +# +# FrontOfficeSmoke green on all of 1.7 / 8.2 / 9.2, classic and hummingbird. +# BackOfficeSmoke red everywhere on the version cross-check; additionally +# red on 1.7 and 8.2 for the login-error read and logout. on: push: @@ -16,7 +24,7 @@ on: jobs: smoke: - name: Smoke (PrestaShop ${{ matrix.ps }}) + name: Smoke (PrestaShop ${{ matrix.ps }}, ${{ matrix.theme }}) runs-on: ubuntu-latest strategy: @@ -35,6 +43,14 @@ jobs: service: ps92 port: 8092 theme: hummingbird + # Shop 2 of the same container, on its own port and its own theme. + # Without this row "9.2 passes" would only ever mean "hummingbird + # passes", and a theme-aware selector that silently matched on one + # theme would never be caught. + - ps: '9.2.0' + service: ps92 + port: 8093 + theme: classic steps: - uses: actions/checkout@v4 @@ -80,7 +96,11 @@ jobs: done echo "shop never came up"; docker compose logs ${{ matrix.service }}; exit 1 - - name: Run the smoke suite + # Both suites run even when the first one is red, so one job reports the + # whole inventory rather than stopping at the front office. Each command's + # status is captured separately and the step exits non-zero if either + # failed, so neither suite can hide the other's result. + - name: Run the smoke suites env: PRESTAFLOW_FO_URL: http://localhost:${{ matrix.port }}/ PRESTAFLOW_BO_URL: http://localhost:${{ matrix.port }}/admin-dev/ @@ -89,12 +109,16 @@ jobs: PRESTAFLOW_PS_VERSION: ${{ matrix.ps }} PRESTAFLOW_THEME: ${{ matrix.theme }} PRESTAFLOW_LOCALE: en - run: php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php + run: | + status=0 + php bin/prestaflow run src/Tests/Suites/Smoke/FrontOfficeSmoke.php || status=1 + php bin/prestaflow run src/Tests/Suites/Smoke/BackOfficeSmoke.php || status=1 + exit $status - name: Upload failure screenshots if: failure() uses: actions/upload-artifact@v4 with: - name: screenshots-${{ matrix.ps }} + name: screenshots-${{ matrix.ps }}-${{ matrix.theme }} path: prestaflow/screens/ if-no-files-found: ignore diff --git a/src/Tests/Suites/Smoke/BackOfficeSmoke.php b/src/Tests/Suites/Smoke/BackOfficeSmoke.php new file mode 100644 index 0000000..32fdb79 --- /dev/null +++ b/src/Tests/Suites/Smoke/BackOfficeSmoke.php @@ -0,0 +1,92 @@ +importPage('BackOffice\Login'); + $this->importPage('BackOffice\Dashboard'); + + extract($this->pages); + + $this + // Every step runs even after a red one. This suite is an INVENTORY of + // what differs per version, not a pass/fail gate, and the steps below + // are independent enough for that to stay honest: each one navigates + // for itself rather than inheriting the previous step's page. Stopping + // at the first red would have hidden two of the three divergences + // 1.7 and 8.2 actually have. + ->skipWhenFailed(false) + ->describe('Back office smoke') + // Assert on the field login() actually needs, not on a decorative + // block: a login page whose form markup moved has to fail HERE. + ->it('the login form renders', function () use ($backOfficeLoginPage) { + $backOfficeLoginPage->goToPage('index'); + + Expect::that($backOfficeLoginPage->elementIsVisible( + $backOfficeLoginPage->getSelector('emailInput'), + 5000 + ))->equals(true); + }) + // A wrong password has to be REJECTED VISIBLY. Without this step, a + // form whose submit selector missed would look exactly like one that + // authenticated. + ->it('wrong credentials are rejected with an error', function () use ($backOfficeLoginPage) { + $backOfficeLoginPage->login('wrong@prestashop.com', 'wrongPassword', false); + + Expect::that($backOfficeLoginPage->getLoginError())->isNotEmpty(); + }) + ->it('log in with the configured employee', function () use ($backOfficeLoginPage) { + $backOfficeLoginPage->goToPage('index'); + $backOfficeLoginPage->login(); + + Expect::that($backOfficeLoginPage->isLoggedIn())->equals(true); + }) + ->it('the dashboard is the page we land on', function () use ($backOfficeDashboardPage) { + Expect::that($backOfficeDashboardPage->getPageTitle()) + ->contains($backOfficeDashboardPage->pageTitle()); + }) + // Assert we are back on the login FORM, not merely that some block is + // present: a logout that silently did nothing would still leave a + // rendered back office behind. + ->it('log out again', function () use ($backOfficeLoginPage) { + $backOfficeLoginPage->logout(); + + Expect::that($backOfficeLoginPage->elementIsVisible( + $backOfficeLoginPage->getSelector('emailInput'), + 5000 + ))->equals(true); + }) + // Cheap cross-check that the runner is pointed where it thinks it is. + // Navigates for itself rather than trusting the previous step to have + // left the login page on screen. + ->it('the shop reports the version the runner was pointed at', function () use ($backOfficeLoginPage) { + $backOfficeLoginPage->goToPage('index'); + + Expect::that($backOfficeLoginPage->getPrestashopVersion()) + ->contains($backOfficeLoginPage->getGlobal('PS_VERSION')); + }); + } +} diff --git a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php index 5822086..1f7045f 100644 --- a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php +++ b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php @@ -6,15 +6,20 @@ use PrestaFlow\Library\Tests\TestsSuite; /** - * The smallest suite worth running against every supported PrestaShop. + * The front-office walk every supported version has to survive. * - * Deliberately not a checkout. The point is to fail EARLY and SPECIFICALLY on a - * version whose markup differs, so the failure names the page. A tunnel reports - * the same "stuck at step 2" for a dozen unrelated causes. + * Steps are ordered so a failure names the page it happened on, and nothing + * below a red step can be trusted: the cart steps depend on the add-to-cart + * step having really added something. * - * Nothing here is pinned to a fixture: no product URL, no category id. The - * suite walks whatever catalogue the shop has, so it is as valid on 1.7.8.11 as - * on 9.2.0 — and when it fails on one of them, that failure is the finding. + * Two rules hold every step here together: + * + * - EVERY step asserts. click() answers false on a missing selector instead of + * raising, so a step without an assertion reports success having done + * nothing at all. + * - NOTHING is hardcoded to one catalogue. No product id, no friendly URL, no + * theme-specific selector. Category 3 is the single exception: it is + * "Clothes" on the demo catalogue of 1.7, 8 and 9 alike. */ class FrontOfficeSmoke extends TestsSuite { @@ -23,6 +28,8 @@ public function init() $this->importPage('FrontOffice\Home'); $this->importPage('FrontOffice\Listing'); $this->importPage('FrontOffice\Product'); + $this->importPage('FrontOffice\Cart'); + $this->importPage('FrontOffice\Category'); extract($this->pages); @@ -34,8 +41,6 @@ public function init() Expect::that($frontOfficeHomePage->isDisplayed())->equals(true); }) ->it('reach the product listing from the home page', function () use ($frontOfficeHomePage, $frontOfficeListingPage) { - // Reads the "all products" link out of the home page and follows it, - // so a theme that renames that link fails here and names Home. $frontOfficeHomePage->goToAllProducts(); Expect::that($frontOfficeListingPage->getListingTitle())->notEquals(''); @@ -43,9 +48,35 @@ public function init() ->it('open a product and read its price', function () use ($frontOfficeListingPage, $frontOfficeProductPage) { $frontOfficeListingPage->goToProduct(1); - // A price above zero proves three things at once: the listing linked - // to a real product, the product page matched its price element, and - // parsePrice() understood the rendered format. + Expect::that($frontOfficeProductPage->getPrice() > 0)->equals(true); + }) + // addToCart() answers the modal title it read after clicking. An empty + // string (or the `false` getTextContent() returns on a timeout) means + // either the button selector missed or no confirmation modal opened — + // both are the same finding: the add-to-cart markup diverged here. + ->it('add the product to the cart', function () use ($frontOfficeProductPage) { + $modalTitle = $frontOfficeProductPage->addToCart(1); + + Expect::that($modalTitle)->isNotEmpty(); + }) + // Independent of the modal: proves the cart really holds a line rather + // than the page merely having shown a confirmation. + ->it('the cart page holds the added product', function () use ($frontOfficeCartPage) { + $frontOfficeCartPage->goToCart(); + + Expect::that($frontOfficeCartPage->hasItems())->equals(true); + }) + // Category 3 is "Clothes" everywhere, so this reaches a listing by id + // without a friendly URL. It also exercises the scalar-param + // substitution in getPageURL() that f1ad516 fixed. + ->it('reach a category listing by id', function () use ($frontOfficeCategoryPage) { + $frontOfficeCategoryPage->goToPage('category', 3); + + Expect::that($frontOfficeCategoryPage->getListingTitle())->notEquals(''); + }) + ->it('open a product from the category listing', function () use ($frontOfficeCategoryPage, $frontOfficeProductPage) { + $frontOfficeCategoryPage->goToProduct(1); + Expect::that($frontOfficeProductPage->getPrice() > 0)->equals(true); }); } From 0f13802e8674a5e3789b699d033048147df509b0 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Thu, 24 Sep 2026 16:38:29 +0200 Subject: [PATCH 12/12] fix(pages): stop four page objects from reporting success they never checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The smoke sweep that landed in 4e834c4 diagnosed four page-object defects and deliberately fixed none of them. All four share one shape: the page object answered something, the suite asserted the answer was non-empty, and neither the answer nor the assertion had anything to do with what the step claimed to verify. Three of them were green while being wrong. 1. getPrestashopVersion() returned a label, not a version `psVersionBlock` was `#login_form h4`. That node never holds a version: 9.2 reads "* PrestaShop" (the new login form's required-field marker), 8.2 and 1.7 read "PrestaShop" from `

`. Suites/BackOffice/Login asserted isNotEmpty() on it twice, so a shipped suite has been green since forever while reading a string unrelated to what it named. On 8 and 9 the version is not in the login page body at all, only in asset query strings — no selector fixes it there. Probing live 1.7.8.11, 8.2.8 and 9.2.0 shops found exactly one node that holds a real version on all three: `#shop_version`, written by the back-office header on every AUTHENTICATED page (9 repeats it in the menu logo block). So the getter moves to BackOfficePage, reads that, and returns null when the page does not state a version — which is the honest answer for every login page of every supported version. A value that does not begin like a version number is treated as a label and also answered with null: a method that cannot answer must not answer falsely. 1.7's login page does carry its version, in `#login-header > div.text-center` (NOT `#login-header .text-center`, which matches the logo h1 first). It is deliberately not read: a getter that answers on one version and not on the other two invites exactly the assertion that started this. The suites now assert the contract instead of asserting non-emptiness: null on the login page, the configured PS_VERSION once logged in, null again after logging out. 2. logout() was a silent no-op on 1.7 and 8.2 It navigated to {BO_URL}logout, a Symfony route that exists on 9 only. The legacy dispatcher of 1.7 and 8 ignores the path and serves the dashboard, so logout() returned having done nothing and the employee stayed authenticated; the failure screenshot showed the 8.2.8 dashboard, still logged in. The trap that hid it: unauthenticated, that same URL 302s to the login page, so a curl probe looks right and only a browser holding a live session shows it. The fix branches on nothing. Not in the v7/v8 page objects — that would spread one behaviour over three classes — and not on getMinorVersion() in this one, because the URL shapes it would hardcode cannot be built anyway: 1.7 and 8 need a live `token`, 9 a live `_token`, and both would have to be read off #header_logout regardless. So logout() simply follows the href the shop already put on #header_logout, which is the same selector carrying a correct version-specific href on all three. Each version resolves its own difference; the page object stays version-agnostic. No logout link now raises instead of returning quietly — the silence was the defect. 3. getLoginError() raced on 1.7 and 8.2 Their admin login is AJAX (``, js/admin/login.js) and the page ships `
` EMPTY. Waiting on presence found that node on the first poll and read "" unless the XHR happened to land first; measured across repeated runs it landed both ways on both versions. The wait is now on CONTENT, via waitForJsCondition(). 9.2 POSTs and renders .alert-danger only on the error response, so presence was sufficient there and a content wait is equally correct. A timeout still returns "" rather than throwing, so the caller fails on its own assertion instead of on a timeout thrown from inside a getter. 4. getListingTitle() returned a container, not a heading `#js-product-list-header` holds the category description on Classic and the subcategory nav as well on hummingbird, where it read "Home Clothes Accessories Art" — a value that satisfies notEquals('') while naming nothing. Both halves are fixed: Listing now points at the `h1` inside that header (which is what Category already overrode it to, so that duplicate override goes away), and the smoke steps cross-check the heading against the document title instead of merely checking it is not empty. That comparison is locale-agnostic and comes from an independent server-side render, so it fails both when the selector drifts back to the container and when we never left the home page. EVIDENCE Every fix has a unit test that fails without it. With src/ stashed and the four new test files left in place, 22 of their 24 tests fail — including "false is not identical to '9.2.0'", "'* PrestaShop' is not null", the logout navigation array being empty, the login error reading "", and '#js-product-list-header' not being '#js-product-list-header h1'. The two that hold either way are structural guards (v7/v8 must not override logout). Unit suite: 297 tests / 655 assertions -> 321 / 711. Live, against Flashlight 1.7.8.11 (:8017), 8.2.8 (:8082), 9.2.0 hummingbird (:8092) and 9.2.0 classic (:8093), 7 runs of each smoke suite per combination: Step 1.7 cl 8.2 cl 9.2 hb 9.2 cl --- BackOfficeSmoke --- login form renders 7/7 7/7 7/7 7/7 bad credentials rejected 7/7 7/7 7/7 7/7 (was FLAKY on 1.7/8.2) log in 7/7 7/7 7/7 7/7 dashboard is where we land 7/7 7/7 7/7 7/7 version cross-check 7/7 7/7 7/7 7/7 (was FAIL everywhere) log out 7/7 7/7 7/7 7/7 (was FAIL on 1.7/8.2) --- FrontOfficeSmoke --- all seven steps 7/7 7/7 7/7 7/7 Suites/BackOffice/Login, the shipped suite whose isNotEmpty() guards hid defect 1, is green on 1.7, 8.2 and 9.2 with its assertions replaced by meaningful ones rather than removed. Co-Authored-By: Claude Opus 5 --- .github/workflows/live-smoke.yml | 26 ++- src/Pages/BackOfficePage.php | 42 ++++ src/Pages/v9/BackOffice/Login/Page.php | 81 ++++++-- src/Pages/v9/FrontOffice/Category/Page.php | 14 +- src/Pages/v9/FrontOffice/Listing/Page.php | 10 +- src/Tests/Suites/BackOffice/Login.php | 40 +++- src/Tests/Suites/Smoke/BackOfficeSmoke.php | 27 +-- src/Tests/Suites/Smoke/FrontOfficeSmoke.php | 16 +- tests/Unit/Pages/BackOfficeLoginErrorTest.php | 154 ++++++++++++++ tests/Unit/Pages/BackOfficeLogoutTest.php | 195 ++++++++++++++++++ .../Unit/Pages/BackOfficeShopVersionTest.php | 138 +++++++++++++ tests/Unit/Pages/ListingTitleSelectorTest.php | 100 +++++++++ 12 files changed, 785 insertions(+), 58 deletions(-) create mode 100644 tests/Unit/Pages/BackOfficeLoginErrorTest.php create mode 100644 tests/Unit/Pages/BackOfficeLogoutTest.php create mode 100644 tests/Unit/Pages/BackOfficeShopVersionTest.php create mode 100644 tests/Unit/Pages/ListingTitleSelectorTest.php diff --git a/.github/workflows/live-smoke.yml b/.github/workflows/live-smoke.yml index 77298fb..3b9cc82 100644 --- a/.github/workflows/live-smoke.yml +++ b/.github/workflows/live-smoke.yml @@ -4,18 +4,24 @@ name: Live smoke # PrestaShop version. Both suites run in every job, and a red in either one # fails the job. # -# Several rows are EXPECTED TO FAIL today. The v7 and v8 page objects are -# nine-line stubs inheriting the v9 selectors, so the matrix reports an absence -# of support rather than a regression. They are deliberately not skipped and -# not `continue-on-error` — the failures are the inventory of what differs per -# version. `fail-fast: false` keeps one red row from cancelling the others. +# Every row is expected to be GREEN. That was not true when this workflow +# landed: BackOfficeSmoke was red everywhere on the version cross-check and +# additionally red on 1.7 and 8.2 for the login-error read and logout. Those +# three were page-object defects, not absences of support, and they are fixed — +# see the commit that touched src/Pages/BackOfficePage.php. A red row is now a +# regression and should be read as one. # -# Measured 2026-09-24 against Flashlight (see the commit that added -# BackOfficeSmoke for the full table): +# `fail-fast: false` still keeps one red row from cancelling the others: the +# point of the matrix is to say WHICH versions diverged, not merely that one +# did. Nothing here is `continue-on-error`. # -# FrontOfficeSmoke green on all of 1.7 / 8.2 / 9.2, classic and hummingbird. -# BackOfficeSmoke red everywhere on the version cross-check; additionally -# red on 1.7 and 8.2 for the login-error read and logout. +# Measured 2026-09-24 against Flashlight, 7 runs per combination: +# +# FrontOfficeSmoke 7/7 green on 1.7.8.11, 8.2.8 and 9.2.0, classic and +# hummingbird. +# BackOfficeSmoke 7/7 green on the same four combinations. The bad-password +# step, which used to land both ways on 1.7 and 8.2, was +# green in all 28 runs. on: push: diff --git a/src/Pages/BackOfficePage.php b/src/Pages/BackOfficePage.php index a322871..cee00b1 100644 --- a/src/Pages/BackOfficePage.php +++ b/src/Pages/BackOfficePage.php @@ -22,6 +22,10 @@ public function __construct(string $locale, string $patchVersion, array $globals $this->initLocale(locale: $locale); $selectors = [ + // Present on every authenticated back-office page of 1.7, 8 and 9 + // alike, and on none of their login pages. See + // getPrestashopVersion() for why that asymmetry is the whole point. + 'shopVersionBlock' => '#shop_version', ]; $this->selectors = $this->getSelectors(selectors: $selectors); @@ -94,6 +98,44 @@ public function getPageURL($page, $params = null): string * session, so one call covers every page visited afterwards. On a shop * without multistore it is simply ignored. */ + /** + * The version the shop reports about itself, or null when it does not. + * + * `#shop_version` is the only node that actually holds a version across + * 1.7.8, 8.2 and 9.2 — the header info bar writes it on every + * authenticated back-office page, and 9 additionally repeats it in the + * menu logo block. No login page carries it on 8 or 9 (1.7's does, in + * `#login-header > div.text-center`, but reading version-dependent nodes + * is what produced the defect this replaces), so a caller that has not + * logged in yet gets null rather than whatever text happens to sit nearby. + * + * Null is a real answer here: "this page does not state a version". The + * previous implementation read `#login_form h4` and returned the login + * form's required-field marker on 9 ("* PrestaShop") or the shop name on + * 8 and 1.7 ("PrestaShop") — non-empty strings that satisfied every + * isNotEmpty() guard pointed at them while stating nothing about the + * version. A method that cannot answer must not answer falsely. + */ + public function getPrestashopVersion(): ?string + { + $value = $this->getTextContent($this->getSelector('shopVersionBlock'), 1, true, 2000); + + if (!is_string($value)) { + return null; + } + + $value = ltrim(trim($value), 'vV'); + + // Structural guard, not a format check: anything that does not begin + // like a version number ("9.2.0", "1.7.8.11", "9.0.0-rc.1.test") is a + // label that drifted into the node, and a label is not an answer. + if (!preg_match('/^\d+\.\d+/', $value)) { + return null; + } + + return $value; + } + public function setSingleShopContext(int $idShop = 1): void { $url = $this->getGlobals()['BO']['URL']; diff --git a/src/Pages/v9/BackOffice/Login/Page.php b/src/Pages/v9/BackOffice/Login/Page.php index 7264967..a62d8d6 100644 --- a/src/Pages/v9/BackOffice/Login/Page.php +++ b/src/Pages/v9/BackOffice/Login/Page.php @@ -13,8 +13,6 @@ public function defineSelectors() return [ // Login header selectors 'loginHeaderBlock' => '#login-header', - // PS9: version label moved to an h4 inside the login form. - 'psVersionBlock' => '#login_form h4', // Login Form selectors 'emailInput' => '#email', 'passwordInput' => '#passwd', @@ -80,23 +78,80 @@ public function isLoggedIn(): bool } /** - * Get login error + * The error the login form shows, once it actually shows one. + * + * The wait is on CONTENT, not on presence, because presence proves nothing + * on half the supported versions. 1.7 and 8 submit the admin login over + * AJAX (``, driven by js/admin/login.js) and ship the + * container empty in the initial HTML: + * + *
+ * + * A presence wait finds that node on the first poll and reads "" whenever + * the XHR has not landed yet — which, measured across repeated runs, hit + * both outcomes on both versions. 9 does a real POST and only renders + * .alert-danger on the error response, so presence was sufficient there + * and a content wait is equally correct. + * + * Falling through on timeout rather than throwing keeps the failure where + * it belongs: the caller asserts on the message and fails with its own + * message, instead of a timeout thrown from inside a getter. */ - public function getLoginError() + public function getLoginError(int $timeout = 10000, int $interval = 100) { - return $this->getTextContent($this->getSelector('alertDangerTextBlock')); + $selector = $this->getSelector('alertDangerTextBlock'); + + $this->waitForJsCondition( + sprintf( + '(function(){var e=document.querySelector(%s);return !!e && e.textContent.trim() !== "";})()', + json_encode($selector) + ), + $timeout, + $interval + ); + + return $this->getTextContent($selector); } + /** + * Log the employee out by following the back office's own logout link. + * + * This used to navigate to {BO_URL}logout, which is a Symfony route on 9 + * and nothing at all on 1.7 and 8: their legacy dispatcher ignores the + * path and serves the dashboard, so logout() returned having done nothing + * and the employee stayed authenticated. Unauthenticated, that same URL + * 302s to the login page, which is why probing it with curl never showed + * the defect — only a browser holding a live session does. + * + * The fix deliberately does NOT branch on the version, in either of the + * two places it could have. Overriding logout() in the v7 and v8 page + * objects would spread one behaviour across three classes, and branching + * on getMinorVersion() here would hardcode URL shapes we cannot build + * anyway: 1.7 and 8 need a live `token`, 9 a live `_token`, and both would + * have to be read off #header_logout regardless. So we use the href the + * shop itself put there. #header_logout is the same selector, carrying a + * correct version-specific href, on 1.7.8, 8.2 and 9.2 alike: each version + * resolves its own difference and the page object stays version-agnostic. + * + * Not being logged in is an error, not a no-op — the previous silence is + * precisely what hid this. + */ public function logout() { - // PS9: the logout link lives inside a collapsed dropdown (hidden until opened), - // which coordinate-based clicks can't reach reliably. Navigate to the stable - // logout route instead; it clears the session and redirects to the login page. - $this->goToPage('logout'); - } + $selector = $this->getSelector('logoutLink'); - public function getPrestashopVersion() - { - return $this->getTextContent($this->getSelector('psVersionBlock')); + $href = $this->getPage()->evaluate(sprintf( + '(function(){var e=document.querySelector(%s);return e && e.href ? e.href : null;})()', + json_encode($selector) + ))->getReturnValue(); + + if (!is_string($href) || $href === '') { + throw new \RuntimeException( + 'Cannot log out: no logout link found at "' . $selector . '". ' + . 'The back office is most likely not authenticated.' + ); + } + + $this->goToUrl($href); } } diff --git a/src/Pages/v9/FrontOffice/Category/Page.php b/src/Pages/v9/FrontOffice/Category/Page.php index 6b209c4..6d4be0f 100644 --- a/src/Pages/v9/FrontOffice/Category/Page.php +++ b/src/Pages/v9/FrontOffice/Category/Page.php @@ -8,14 +8,8 @@ class Page extends BasePage { public string $url = '{index}-category'; - public function defineSelectors() - { - $selectors = parent::defineSelectors(); - - $pageSelectors = [ - 'pageTitle' => '#js-product-list-header h1', - ]; - - return [...$selectors, ...$pageSelectors]; - } + // No selector overrides: Category used to re-declare `pageTitle` as + // `#js-product-list-header h1` because Listing pointed at the surrounding + // container. Listing now declares the heading itself, so repeating it here + // would only be a second copy to keep in sync. } diff --git a/src/Pages/v9/FrontOffice/Listing/Page.php b/src/Pages/v9/FrontOffice/Listing/Page.php index ebae848..81e0792 100644 --- a/src/Pages/v9/FrontOffice/Listing/Page.php +++ b/src/Pages/v9/FrontOffice/Listing/Page.php @@ -9,7 +9,15 @@ class Page extends BasePage public function defineSelectors() { return [ - 'pageTitle' => '#js-product-list-header', + // The heading, not the header block. `#js-product-list-header` is a + // container: on Classic it also holds the category description, and + // on hummingbird the subcategory nav as well, so reading it returned + // a whitespace blob ("Home Clothes Accessories Art") that satisfied + // any non-emptiness check while naming nothing. The `h1` inside it + // is the category name on 1.7.8, 8.2 and 9.2, Classic and + // hummingbird alike — which is why Category already overrode this + // selector with exactly this value. + 'pageTitle' => '#js-product-list-header h1', 'productArticle' => '#js-product-list .products div:nth-child(${index}) article', // The whole path down to the anchor, on purpose: appending the link // part in PHP would put it out of reach of the theme layer, and the diff --git a/src/Tests/Suites/BackOffice/Login.php b/src/Tests/Suites/BackOffice/Login.php index 319b262..1988236 100644 --- a/src/Tests/Suites/BackOffice/Login.php +++ b/src/Tests/Suites/BackOffice/Login.php @@ -18,15 +18,21 @@ public function init() ->describe('Check PS version {$PS_VERSION} with {$LOCALE} language, and login and log out from BO') ->it('should go to login page', function () use ($backOfficeLoginPage) { $backOfficeLoginPage->goToPage('index'); - // Assert the login page is reached via its version block, rather than the - // document (which is the shop name and varies per install). - Expect::that($backOfficeLoginPage->getPrestaShopVersion())->isNotEmpty(); - }) - ->it('should check PS version', function () use ($backOfficeLoginPage) { - $psVersion = $backOfficeLoginPage->getPrestaShopVersion(); - Expect::that($psVersion)->contains($backOfficeLoginPage->getGlobal('PS_VERSION')); - //Expect::that()->matchPrestaShopVersion(); + // The email field, not the version: the login page of 8 and 9 does + // not state a version anywhere, and the block this used to read + // ("* PrestaShop", or the shop name) never did either. Asserting on + // what is actually there is the point of the whole change. + Expect::that($backOfficeLoginPage->elementIsVisible( + $backOfficeLoginPage->getSelector('emailInput'), + 5000 + ))->equals(true); + }) + ->it('should not claim a version the login page does not state', function () use ($backOfficeLoginPage) { + // Meaningful precisely because it is the negative case: the getter + // used to answer with a label here and pass an isNotEmpty() guard + // while stating nothing about the version. + Expect::that($backOfficeLoginPage->getPrestaShopVersion())->isNull(); }) ->it('should try to login with wrong email and password', function () use ($backOfficeLoginPage) { $backOfficeLoginPage->login('wrongEmail@prestashop.com', 'wrongPass', false); @@ -51,6 +57,12 @@ public function init() Expect::that($backOfficeDashboardPage->getPageTitle())->contains($backOfficeDashboardPage->pageTitle()); }) + ->it('should check PS version', function () use ($backOfficeLoginPage) { + // Only reachable once authenticated: #shop_version is written by the + // back-office header, which the login page has none of. + $psVersion = $backOfficeLoginPage->getPrestaShopVersion(); + Expect::that($psVersion)->contains($backOfficeLoginPage->getGlobal('PS_VERSION')); + }) ->it('should log out from BO', function () use ($backOfficeLoginPage, $backOfficeDashboardPage) { /* await dashboardPage.logoutBO(page); @@ -60,8 +72,16 @@ public function init() */ $backOfficeLoginPage->logout(); - // Back on the login page after logout. - Expect::that($backOfficeLoginPage->getPrestaShopVersion())->isNotEmpty(); + // Two assertions because logging out has two halves, and the old + // suite checked neither: the session is gone (no logout link left, + // and the version the header used to state is gone with it), and + // the login form is back. + Expect::that($backOfficeLoginPage->isLoggedIn())->equals(false); + Expect::that($backOfficeLoginPage->getPrestaShopVersion())->isNull(); + Expect::that($backOfficeLoginPage->elementIsVisible( + $backOfficeLoginPage->getSelector('emailInput'), + 5000 + ))->equals(true); }); } } diff --git a/src/Tests/Suites/Smoke/BackOfficeSmoke.php b/src/Tests/Suites/Smoke/BackOfficeSmoke.php index 32fdb79..6e90bc1 100644 --- a/src/Tests/Suites/Smoke/BackOfficeSmoke.php +++ b/src/Tests/Suites/Smoke/BackOfficeSmoke.php @@ -68,25 +68,28 @@ public function init() Expect::that($backOfficeDashboardPage->getPageTitle()) ->contains($backOfficeDashboardPage->pageTitle()); }) - // Assert we are back on the login FORM, not merely that some block is - // present: a logout that silently did nothing would still leave a - // rendered back office behind. + // Cross-check that the runner is pointed where it thinks it is. It runs + // HERE, between login and logout, because #shop_version is written by + // the back-office header and no login page of 1.7, 8 or 9 carries a + // version at all. Reading it from the login page is what the previous + // version of this step did, and it could only ever have been answered + // by something that was not a version. + ->it('the shop reports the version the runner was pointed at', function () use ($backOfficeLoginPage) { + Expect::that($backOfficeLoginPage->getPrestashopVersion()) + ->contains($backOfficeLoginPage->getGlobal('PS_VERSION')); + }) + // Assert the session is GONE, not merely that a login form is on + // screen: a logout that silently did nothing used to leave a rendered + // back office behind, and on 1.7 and 8 it did exactly that. ->it('log out again', function () use ($backOfficeLoginPage) { $backOfficeLoginPage->logout(); + Expect::that($backOfficeLoginPage->isLoggedIn())->equals(false); + Expect::that($backOfficeLoginPage->getPrestashopVersion())->isNull(); Expect::that($backOfficeLoginPage->elementIsVisible( $backOfficeLoginPage->getSelector('emailInput'), 5000 ))->equals(true); - }) - // Cheap cross-check that the runner is pointed where it thinks it is. - // Navigates for itself rather than trusting the previous step to have - // left the login page on screen. - ->it('the shop reports the version the runner was pointed at', function () use ($backOfficeLoginPage) { - $backOfficeLoginPage->goToPage('index'); - - Expect::that($backOfficeLoginPage->getPrestashopVersion()) - ->contains($backOfficeLoginPage->getGlobal('PS_VERSION')); }); } } diff --git a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php index 1f7045f..b22de64 100644 --- a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php +++ b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php @@ -40,10 +40,19 @@ public function init() Expect::that($frontOfficeHomePage->isDisplayed())->equals(true); }) + // notEquals('') was satisfied by anything at all, including the home + // page and the whitespace blob the listing header used to return on + // hummingbird. Cross-checking the heading against the document title + // is locale-agnostic and comes from an independent server-side render + // (meta title vs. rendered h1), so it fails both when the heading + // selector drifts back to the container and when we never left home. ->it('reach the product listing from the home page', function () use ($frontOfficeHomePage, $frontOfficeListingPage) { $frontOfficeHomePage->goToAllProducts(); - Expect::that($frontOfficeListingPage->getListingTitle())->notEquals(''); + $title = $frontOfficeListingPage->getListingTitle(); + + Expect::that($title)->isNotEmpty(); + Expect::that($frontOfficeListingPage->getMetaTitle())->contains($title); }) ->it('open a product and read its price', function () use ($frontOfficeListingPage, $frontOfficeProductPage) { $frontOfficeListingPage->goToProduct(1); @@ -72,7 +81,10 @@ public function init() ->it('reach a category listing by id', function () use ($frontOfficeCategoryPage) { $frontOfficeCategoryPage->goToPage('category', 3); - Expect::that($frontOfficeCategoryPage->getListingTitle())->notEquals(''); + $title = $frontOfficeCategoryPage->getListingTitle(); + + Expect::that($title)->isNotEmpty(); + Expect::that($frontOfficeCategoryPage->getMetaTitle())->contains($title); }) ->it('open a product from the category listing', function () use ($frontOfficeCategoryPage, $frontOfficeProductPage) { $frontOfficeCategoryPage->goToProduct(1); diff --git a/tests/Unit/Pages/BackOfficeLoginErrorTest.php b/tests/Unit/Pages/BackOfficeLoginErrorTest.php new file mode 100644 index 0000000..cca2996 --- /dev/null +++ b/tests/Unit/Pages/BackOfficeLoginErrorTest.php @@ -0,0 +1,154 @@ +<?php + +namespace PrestaFlow\Tests\Unit\Pages; + +use PHPUnit\Framework\TestCase; +use PrestaFlow\Library\Pages\v9\BackOffice\Login\Page as LoginPage; + +final class FakeLoginErrorEvaluation +{ + public function __construct(private mixed $value) + { + } + + public function getReturnValue(?int $timeout = null): mixed + { + return $this->value; + } +} + +final class FakeLoginErrorBrowserPage +{ + public function __construct(private FakeLoginErrorPage $owner) + { + } + + public function evaluate(string $js): FakeLoginErrorEvaluation + { + $this->owner->polls++; + $this->owner->evaluated[] = $js; + + return new FakeLoginErrorEvaluation($this->owner->containerIsFilled()); + } +} + +/** + * Test double for the 1.7 / 8.2 admin login, which is AJAX: the error + * container ships EMPTY in the initial HTML + * + * <div id="error" class="hide alert alert-danger"></div> + * + * and is filled only when the XHR lands. $fillsOnPoll is how many polls that + * takes — 1 means "already there when we looked", which is what 9.2 does. + */ +final class FakeLoginErrorPage extends LoginPage +{ + public const MESSAGE = 'The employee does not exist, or the password provided is incorrect.'; + + public int $polls = 0; + public int $fillsOnPoll = 3; + public bool $neverFills = false; + public array $evaluated = []; + public array $textReads = []; + + public function containerIsFilled(): bool + { + return !$this->neverFills && $this->polls >= $this->fillsOnPoll; + } + + public function getPage() + { + return new FakeLoginErrorBrowserPage($this); + } + + public function getTextContent($selector, $index = 1, $waitForSelector = true, $timeout = 3000) + { + $this->textReads[] = $selector; + + // The node is present from the first millisecond either way; what + // changes over time is whether it has any text in it. + return $this->containerIsFilled() ? self::MESSAGE : ''; + } +} + +final class BackOfficeLoginErrorTest extends TestCase +{ + private function page(): FakeLoginErrorPage + { + return new FakeLoginErrorPage( + locale: 'en', + patchVersion: '9.2.0', + globals: [ + 'PS_VERSION' => '9.2.0', + 'LOCALE' => 'en', + 'PREFIX_LOCALE' => false, + 'BO' => ['URL' => 'http://localhost/admin-dev/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'FO' => ['URL' => 'http://localhost/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'DEBUG' => false, + 'VERBOSE' => false, + ], + customs: [] + ); + } + + /** + * The defect: the container is in the DOM before the XHR answers, so a + * presence wait returns "" roughly half the time on 1.7 and 8.2. + */ + public function testAnErrorThatArrivesLateIsStillRead(): void + { + $page = $this->page(); + $page->fillsOnPoll = 3; + + $this->assertSame(FakeLoginErrorPage::MESSAGE, $page->getLoginError(2000, 1)); + $this->assertGreaterThanOrEqual(3, $page->polls, 'the getter must have polled until the container filled'); + } + + public function testAnErrorThatIsAlreadyThereIsReadWithoutExtraPolling(): void + { + // 9.2 renders .alert-danger only on the error response, so it is + // already filled when we first look. No regression in that case. + $page = $this->page(); + $page->fillsOnPoll = 1; + + $this->assertSame(FakeLoginErrorPage::MESSAGE, $page->getLoginError(2000, 1)); + $this->assertSame(1, $page->polls); + } + + public function testTheWaitIsOnContentNotOnMerePresence(): void + { + $page = $this->page(); + $page->fillsOnPoll = 2; + + $page->getLoginError(2000, 1); + + $this->assertNotEmpty($page->evaluated); + $js = $page->evaluated[0]; + $this->assertStringContainsString('.alert-danger', $js); + $this->assertStringContainsString('textContent', $js, 'waiting on presence alone is the defect'); + $this->assertStringContainsString('!==', $js); + } + + /** + * A getter that throws on timeout would report the wait, not the finding. + * The caller asserts on the message and fails with its own wording. + */ + public function testAnErrorThatNeverArrivesReturnsEmptyRatherThanThrowing(): void + { + $page = $this->page(); + $page->neverFills = true; + + $this->assertSame('', $page->getLoginError(150, 10)); + $this->assertGreaterThan(1, $page->polls); + } + + public function testTheTextIsReadThroughTheSameSelectorThatWasWaitedOn(): void + { + $page = $this->page(); + $page->fillsOnPoll = 1; + + $page->getLoginError(2000, 1); + + $this->assertSame([$page->getSelector('alertDangerTextBlock')], $page->textReads); + } +} diff --git a/tests/Unit/Pages/BackOfficeLogoutTest.php b/tests/Unit/Pages/BackOfficeLogoutTest.php new file mode 100644 index 0000000..502edb7 --- /dev/null +++ b/tests/Unit/Pages/BackOfficeLogoutTest.php @@ -0,0 +1,195 @@ +<?php + +namespace PrestaFlow\Tests\Unit\Pages; + +use PHPUnit\Framework\TestCase; +use PrestaFlow\Library\Pages\v9\BackOffice\Login\Page as LoginPage; +use RuntimeException; + +final class FakeLogoutEvaluation +{ + public function __construct(private mixed $value) + { + } + + public function getReturnValue(?int $timeout = null): mixed + { + return $this->value; + } +} + +final class FakeLogoutNavigation +{ + public function waitForNavigation($event = null, $timeout = null): void + { + } +} + +final class FakeLogoutBrowserPage +{ + public function __construct(private FakeLogoutPage $owner) + { + } + + public function evaluate(string $js): FakeLogoutEvaluation + { + $this->owner->evaluated[] = $js; + + return new FakeLogoutEvaluation($this->owner->logoutHref); + } + + public function navigate(string $url): FakeLogoutNavigation + { + $this->owner->navigated[] = $url; + + return new FakeLogoutNavigation(); + } +} + +/** + * Test double: a back office whose #header_logout carries whatever href the + * version under test would have built. goToPage() is recorded instead of + * executed, so the pre-fix implementation is observable without a browser. + */ +final class FakeLogoutPage extends LoginPage +{ + public mixed $logoutHref = null; + public array $navigated = []; + public array $evaluated = []; + public array $goToPageCalls = []; + + public function getPage() + { + return new FakeLogoutBrowserPage($this); + } + + public function goToPage($page = null, $params = null) + { + $this->goToPageCalls[] = $page; + } +} + +final class BackOfficeLogoutTest extends TestCase +{ + /** + * The hrefs the three supported versions actually put on #header_logout, + * copied from live 1.7.8.11, 8.2.8 and 9.2.0 shops. The point of the table + * is that none of them is {BO_URL}logout, and that all three are reachable + * through the same selector. + */ + public static function liveLogoutHrefs(): array + { + return [ + '1.7.8.11' => ['http://localhost:8017/admin-dev/index.php?controller=AdminLogin&logout=1&token=d219249b20d2ed74425beb3b17e8248d'], + '8.2.8' => ['http://localhost:8082/admin-dev/index.php?controller=AdminLogin&logout=1&token=c4464c98fb374db8b412a5521652b62a'], + '9.2.0' => ['http://localhost:8092/admin-dev/logout?_token=2f6cede35ff5.6-73AYBaswalfSQ-y4b9SXKAiK1ylwSWnLwz-AKzuA8'], + ]; + } + + private function page(mixed $href): FakeLogoutPage + { + $page = new FakeLogoutPage( + locale: 'en', + patchVersion: '9.2.0', + globals: [ + 'PS_VERSION' => '9.2.0', + 'LOCALE' => 'en', + 'PREFIX_LOCALE' => false, + 'BO' => ['URL' => 'http://localhost/admin-dev/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'FO' => ['URL' => 'http://localhost/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'DEBUG' => false, + 'VERBOSE' => false, + ], + customs: [] + ); + $page->logoutHref = $href; + + return $page; + } + + /** + * @dataProvider liveLogoutHrefs + */ + public function testLogoutFollowsTheLinkTheShopItselfBuilt(string $href): void + { + $page = $this->page($href); + + $page->logout(); + + $this->assertSame([$href], $page->navigated); + } + + /** + * The defect: {BO_URL}logout is a Symfony route on 9 and nothing at all on + * 1.7 and 8, whose legacy dispatcher served the dashboard instead and left + * the employee logged in. + */ + public function testLogoutNeverNavigatesToTheSyntheticLogoutRoute(): void + { + $page = $this->page('http://localhost:8082/admin-dev/index.php?controller=AdminLogin&logout=1&token=abc'); + + $page->logout(); + + $this->assertSame([], $page->goToPageCalls, 'logout() must not go through the {BO_URL}logout route'); + foreach ($page->navigated as $url) { + $this->assertStringNotContainsString('admin-dev/logout', $url); + } + } + + public function testTheLinkIsLookedUpThroughTheSharedLogoutSelector(): void + { + $page = $this->page('http://localhost/admin-dev/logout?_token=x'); + + $page->logout(); + + $this->assertCount(1, $page->evaluated); + $this->assertStringContainsString('#header_logout', $page->evaluated[0]); + $this->assertStringContainsString('.href', $page->evaluated[0]); + } + + /** + * No logout link means no session. Answering quietly is what let a shipped + * suite report a successful logout against a still-authenticated shop. + */ + public function testLogoutWithoutASessionRaisesInsteadOfDoingNothingQuietly(): void + { + $page = $this->page(null); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('#header_logout'); + + try { + $page->logout(); + } finally { + $this->assertSame([], $page->navigated); + } + } + + public function testAnEmptyHrefIsTreatedAsNoLinkAtAll(): void + { + $page = $this->page(''); + + $this->expectException(RuntimeException::class); + + $page->logout(); + } + + /** + * The v7 and v8 page objects stay stubs on purpose: the version difference + * lives in the href the shop renders, not in PHP. If a later change moves + * logout() into them, this is the test that should be revisited first. + */ + public function testTheVersionedPageObjectsDoNotOverrideLogout(): void + { + foreach (['v7', 'v8'] as $version) { + $class = 'PrestaFlow\\Library\\Pages\\' . $version . '\\BackOffice\\Login\\Page'; + $method = new \ReflectionMethod($class, 'logout'); + + $this->assertSame( + LoginPage::class, + $method->getDeclaringClass()->getName(), + $version . ' should inherit logout() rather than re-implement it' + ); + } + } +} diff --git a/tests/Unit/Pages/BackOfficeShopVersionTest.php b/tests/Unit/Pages/BackOfficeShopVersionTest.php new file mode 100644 index 0000000..8269472 --- /dev/null +++ b/tests/Unit/Pages/BackOfficeShopVersionTest.php @@ -0,0 +1,138 @@ +<?php + +namespace PrestaFlow\Tests\Unit\Pages; + +use Exception; +use PHPUnit\Framework\TestCase; +use PrestaFlow\Library\Pages\v9\BackOffice\Login\Page as LoginPage; + +/** + * Test double: answers getTextContent() from a map keyed by selector, so a + * "page" here is just the set of nodes a given PrestaShop version actually + * renders. No browser, no shop. + */ +final class FakeVersionPage extends LoginPage +{ + public array $nodes = []; + public array $asked = []; + + public function getTextContent($selector, $index = 1, $waitForSelector = true, $timeout = 3000) + { + $this->asked[] = $selector; + + // getTextContent() answers false when the node is not there at all. + return $this->nodes[$selector] ?? false; + } +} + +final class BackOfficeShopVersionTest extends TestCase +{ + private function fakeGlobals(): array + { + return [ + 'PS_VERSION' => '9.2.0', + 'LOCALE' => 'en', + 'PREFIX_LOCALE' => false, + 'BO' => ['URL' => 'http://localhost/admin-dev/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'FO' => ['URL' => 'http://localhost/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'DEBUG' => false, + 'VERBOSE' => false, + ]; + } + + private function page(array $nodes): FakeVersionPage + { + $page = new FakeVersionPage( + locale: 'en', + patchVersion: '9.2.0', + globals: $this->fakeGlobals(), + customs: [] + ); + $page->nodes = $nodes; + + return $page; + } + + public function testTheVersionIsReadFromTheHeaderNodeThatActuallyHoldsOne(): void + { + // #shop_version is the one node present on an authenticated back office + // of 1.7.8, 8.2 and 9.2 alike (verified live on all three). + $page = $this->page(['#shop_version' => '9.2.0']); + + $this->assertSame('9.2.0', $page->getPrestashopVersion()); + $this->assertContains('#shop_version', $page->asked); + } + + public function testEverySupportedVersionStringSurvivesUnchanged(): void + { + foreach (['1.7.8.11', '8.2.8', '9.2.0', '9.0.0-rc.1.test'] as $version) { + $this->assertSame($version, $this->page(['#shop_version' => $version])->getPrestashopVersion(), $version); + } + } + + /** + * The defect itself. A login page states no version on 8 or 9, and the + * block the getter used to read held the login form's required-field + * marker ("* PrestaShop" on 9) or the shop name ("PrestaShop" on 8 and + * 1.7) — non-empty strings that satisfied isNotEmpty() while saying + * nothing about any version. + */ + public function testALoginPageThatStatesNoVersionAnswersNullRatherThanALabel(): void + { + $nineTwoLoginPage = $this->page([ + '#login_form h4' => "*\n PrestaShop", + ]); + $eightTwoLoginPage = $this->page([ + '#login_form h4' => 'PrestaShop', + '#shop_name' => 'PrestaShop', + ]); + + $this->assertNull($nineTwoLoginPage->getPrestashopVersion()); + $this->assertNull($eightTwoLoginPage->getPrestashopVersion()); + } + + public function testAnEmptyOrAbsentNodeAnswersNull(): void + { + $this->assertNull($this->page([])->getPrestashopVersion()); + $this->assertNull($this->page(['#shop_version' => ''])->getPrestashopVersion()); + $this->assertNull($this->page(['#shop_version' => ' '])->getPrestashopVersion()); + } + + /** + * Structural guard, not a format check: if a label ever drifts into + * #shop_version the getter must go back to saying nothing rather than + * repeating it. A label is not an answer. + */ + public function testTextThatIsNotVersionShapedAnswersNull(): void + { + foreach (['PrestaShop', '* PrestaShop', 'Dashboard', '9'] as $label) { + $this->assertNull($this->page(['#shop_version' => $label])->getPrestashopVersion(), $label); + } + } + + public function testTheMisleadingLoginSelectorIsGone(): void + { + $page = $this->page([]); + + $this->assertSame('#shop_version', $page->getSelector('shopVersionBlock')); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('psVersionBlock'); + $page->getSelector('psVersionBlock'); + } + + public function testTheGetterIsAvailableOnEveryBackOfficePageNotJustLogin(): void + { + // #shop_version is a header node, so the dashboard — and every other + // authenticated page — answers it too. + $dashboard = new \PrestaFlow\Library\Pages\v9\BackOffice\Dashboard\Page( + locale: 'en', + patchVersion: '9.2.0', + globals: $this->fakeGlobals(), + customs: [] + ); + + $this->assertSame('#shop_version', $dashboard->getSelector('shopVersionBlock')); + $this->assertTrue(method_exists($dashboard, 'getPrestashopVersion')); + } +} diff --git a/tests/Unit/Pages/ListingTitleSelectorTest.php b/tests/Unit/Pages/ListingTitleSelectorTest.php new file mode 100644 index 0000000..51360d2 --- /dev/null +++ b/tests/Unit/Pages/ListingTitleSelectorTest.php @@ -0,0 +1,100 @@ +<?php + +namespace PrestaFlow\Tests\Unit\Pages; + +use PHPUnit\Framework\TestCase; + +/** + * `#js-product-list-header` is a CONTAINER, not a heading. Classic puts the + * category description in it; hummingbird additionally puts the subcategory + * nav in it, so reading it returned "Home Clothes Accessories Art" — a value + * that satisfies any non-emptiness check while naming nothing. + * + * Measured on live shops, both listings, all four combinations: + * + * port/theme #js-product-list-header h1 inside it + * 8017 classic "Home" "Home" + * 8082 classic "Home" "Home" + * 8093 classic "Home" "Home" + * 8092 humming. "Home Clothes Accessories Art" "Home" + * (category 3) "Clothes Discover our favori..." "Clothes" + */ +final class ListingTitleSelectorTest extends TestCase +{ + private const HEADING = '#js-product-list-header h1'; + + private function fakeGlobals(string $theme = 'classic'): array + { + return [ + 'PS_VERSION' => '9.2.0', + 'LOCALE' => 'en', + 'PREFIX_LOCALE' => false, + 'THEME' => $theme, + 'BO' => ['URL' => 'http://localhost/admin-dev/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'FO' => ['URL' => 'http://localhost/', 'EMAIL' => 'a@b.c', 'PASSWD' => 'x'], + 'DEBUG' => false, + 'VERBOSE' => false, + ]; + } + + private function make(string $version, string $name, string $theme = 'classic'): object + { + $class = 'PrestaFlow\\Library\\Pages\\' . $version . '\\FrontOffice\\' . $name . '\\Page'; + + return new $class( + locale: 'en', + patchVersion: '9.2.0', + globals: $this->fakeGlobals($theme), + customs: [] + ); + } + + public function testListingTitleTargetsTheHeadingNotTheHeaderContainer(): void + { + $this->assertSame(self::HEADING, $this->make('v9', 'Listing')->getSelector('pageTitle')); + } + + public function testCategoryAgreesWithListingOnEveryVersion(): void + { + foreach (['v7', 'v8', 'v9'] as $version) { + $this->assertSame( + self::HEADING, + $this->make($version, 'Listing')->getSelector('pageTitle'), + $version . ' Listing' + ); + $this->assertSame( + self::HEADING, + $this->make($version, 'Category')->getSelector('pageTitle'), + $version . ' Category' + ); + } + } + + /** + * hummingbird is the theme where the container reading was worst, and it + * overrides neither page's heading: the fix has to hold through the theme + * layer, not only in the base map. + */ + public function testTheHeadingSurvivesTheHummingbirdThemeLayer(): void + { + $this->assertSame(self::HEADING, $this->make('v9', 'Listing', 'hummingbird')->getSelector('pageTitle')); + $this->assertSame(self::HEADING, $this->make('v9', 'Category', 'hummingbird')->getSelector('pageTitle')); + } + + /** + * Category used to carry a second copy of this selector because Listing's + * was wrong. One source now, so the two cannot drift apart. + */ + public function testCategoryNoLongerRedeclaresTheHeading(): void + { + $method = new \ReflectionMethod( + 'PrestaFlow\\Library\\Pages\\v9\\FrontOffice\\Category\\Page', + 'defineSelectors' + ); + + $this->assertSame( + 'PrestaFlow\\Library\\Pages\\v9\\FrontOffice\\Listing\\Page', + $method->getDeclaringClass()->getName() + ); + } +}