From 1871d825d247873b006a9083232f8fbd66d2c1ed Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Fri, 25 Sep 2026 09:05:58 +0200 Subject: [PATCH 1/4] feat(themes): let a theme override a selector the whole area inherits desktopLogo, userInfoLink and accountLink are declared once on FrontOfficePage and inherited by all 31 front-office pages, so the catalog had nowhere to theme them: the chain "FrontOffice\Page" strips to a single usable segment and is dropped by the existing guard. The only expressible fix was to repeat each key in every page block, which is what Home already did for desktopLogo. A reserved "_common" block per area now merges as the least specific tier, beaten by any page block including a parent's. It cannot collide with a page name: page segments come from class namespaces and never start with an underscore. Measured against a live 9.2 shop rather than guessed: the three selectors miss on hummingbird on every one of 18 front-office pages probed, and the replacements were verified present before being written. Resolving them through the real page objects and querying the running shop gives 60 matches out of 60 across 15 pages and both themes; dropping the block turns 30 of those red. Three of the six new tests failed before this change. The other three pin which tier wins when blocks conflict -- they passed while _common did nothing, so they are regression guards, not evidence. Co-Authored-By: Claude Opus 5 --- src/Pages/CommonPage.php | 12 +++ src/Themes/hummingbird.json | 6 +- tests/Unit/Pages/ThemeInheritanceTest.php | 122 ++++++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/src/Pages/CommonPage.php b/src/Pages/CommonPage.php index d6207c9..4c6905c 100644 --- a/src/Pages/CommonPage.php +++ b/src/Pages/CommonPage.php @@ -280,6 +280,18 @@ protected function themePageChains(array $pageNames): array { $chains = []; + // The area's shared block, least specific of all. A selector declared + // on the area base (FrontOfficePage's desktopLogo, userInfoLink, ...) + // is inherited by every page in the area, but its own chain -- + // "FrontOffice\Page" -- is dropped by the guard below, so there was + // nowhere to theme it once. The alternative was repeating it in all 31 + // page blocks. "_common" cannot collide with a page name: page segments + // come from class namespaces and never start with an underscore. + $area = $pageNames[0] ?? ''; + if (is_string($area) && $area !== '' && $area !== 'Page') { + $chains[] = [$area, '_common']; + } + // 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) { diff --git a/src/Themes/hummingbird.json b/src/Themes/hummingbird.json index ecc4c62..b84cf18 100644 --- a/src/Themes/hummingbird.json +++ b/src/Themes/hummingbird.json @@ -1,8 +1,12 @@ { "FrontOffice": { + "_common": { + "desktopLogo": ".header-bottom__logo", + "userInfoLink": ".ps-customersignin", + "accountLink": ".ps-customersignin a[href*=\"my-account\"]" + }, "Home": { "homePageSection": "#content.page-content--home", - "desktopLogo": ".header-bottom__logo", "allProductsLink": ".ps-featuredproducts .module-products__buttons a" }, "Product": { diff --git a/tests/Unit/Pages/ThemeInheritanceTest.php b/tests/Unit/Pages/ThemeInheritanceTest.php index ba1403e..b235589 100644 --- a/tests/Unit/Pages/ThemeInheritanceTest.php +++ b/tests/Unit/Pages/ThemeInheritanceTest.php @@ -241,5 +241,127 @@ public function testNestedArraysAreNeverMergedAsSelectors(): void $this->assertSame('.from-category', $selectors['productArticle']); $this->assertArrayNotHasKey('Nested', $selectors); } + + /* + * A selector declared on the AREA base (FrontOfficePage: desktopLogo, + * userInfoLink, ...) is inherited by every page in that area, but the + * catalog had nowhere to express it: the chain "FrontOffice\\Page" + * strips to a single usable segment and is dropped by trap 1, so the + * only way to theme such a selector was to repeat it in all 31 page + * blocks. Measured on a real 9.2 shop, desktopLogo and userInfoLink + * miss on hummingbird on every one of 18 front-office pages, which is + * exactly the shape that duplication would have to cover. + * + * "_common" is the reserved block for that. It cannot collide with a + * page name: page segments come from class namespaces and never start + * with an underscore. + */ + public function testACommonBlockAppliesToAPageWithNoBlockOfItsOwn(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + '_common' => ['desktopLogo' => '.header-bottom__logo'], + ], + ]); + + $page = new ProductPage(['THEME' => 'hummingbird'], ['desktopLogo' => '#_desktop_logo']); + $page->themeDirs = [$dir]; + + $this->assertSame('.header-bottom__logo', $page->getSelectors()['desktopLogo']); + } + + /** The common block is the least specific tier: any page block beats it. */ + public function testAPageBlockBeatsTheCommonBlock(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + '_common' => ['desktopLogo' => '.from-common'], + 'Product' => ['desktopLogo' => '.from-product'], + ], + ]); + + $page = new ProductPage(['THEME' => 'hummingbird'], ['desktopLogo' => '#_desktop_logo']); + $page->themeDirs = [$dir]; + + $this->assertSame('.from-product', $page->getSelectors()['desktopLogo']); + } + + /** ... and a parent PAGE block beats it too, not just the concrete one. */ + public function testAParentPageBlockBeatsTheCommonBlock(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + '_common' => ['productArticle' => '.from-common'], + 'Listing' => ['productArticle' => '.from-listing'], + ], + ]); + + $page = new CategoryPage(['THEME' => 'hummingbird'], ['productArticle' => '.base']); + $page->themeDirs = [$dir]; + + $this->assertSame('.from-listing', $page->getSelectors()['productArticle']); + } + + /** Keys the page block does not mention still come through. */ + public function testCommonAndPageBlocksAreMergedNotReplaced(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + '_common' => ['desktopLogo' => '.logo', 'userInfoLink' => '.user'], + 'Product' => ['desktopLogo' => '.product-logo'], + ], + ]); + + $page = new ProductPage( + ['THEME' => 'hummingbird'], + ['desktopLogo' => '#_desktop_logo', 'userInfoLink' => '#_desktop_user_info'] + ); + $page->themeDirs = [$dir]; + + $selectors = $page->getSelectors(); + + $this->assertSame('.product-logo', $selectors['desktopLogo']); + $this->assertSame('.user', $selectors['userInfoLink']); + } + + /** + * A common block belongs to its area. Without this the reserved key + * would become a global, and a BackOffice override would start + * rewriting FrontOffice selectors. + */ + public function testACommonBlockDoesNotLeakAcrossAreas(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'BackOffice' => [ + '_common' => ['desktopLogo' => '.back-office-logo'], + ], + ]); + + $page = new ProductPage(['THEME' => 'hummingbird'], ['desktopLogo' => '#_desktop_logo']); + $page->themeDirs = [$dir]; + + $this->assertSame('#_desktop_logo', $page->getSelectors()['desktopLogo']); + } + + /** Trap 2 still applies inside the reserved block. */ + public function testNestedArraysInTheCommonBlockAreNotMergedAsSelectors(): void + { + $dir = $this->writeTheme('hummingbird', [ + 'FrontOffice' => [ + '_common' => [ + 'desktopLogo' => '.logo', + 'Nested' => ['desktopLogo' => '.too-deep'], + ], + ], + ]); + + $page = new ProductPage(['THEME' => 'hummingbird'], []); + $page->themeDirs = [$dir]; + + $selectors = $page->getSelectors(); + + $this->assertSame('.logo', $selectors['desktopLogo']); + $this->assertArrayNotHasKey('Nested', $selectors); + } } } From 9cdd6356d74e539f1215ae36f0ae467538462350 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Fri, 25 Sep 2026 09:06:07 +0200 Subject: [PATCH 2/4] test(smoke): assert the header chrome the whole front office inherits Nothing asserted desktopLogo, userInfoLink or accountLink, because no page declares them -- they come from the area base. A whole theme's worth of header markup sat outside the matrix. Two steps, both on Stores, a page that declares no selectors of its own, so everything it resolves comes from the area. Each was checked able to fail: removing the theme block reddens that step and only that step. accountLink needs a session. Probed anonymously it missed on BOTH themes, the one shape that reads as "no divergence" while hiding one; against a logged-in session it misses on hummingbird and matches on Classic. The session comes from Registration, not EnsureTestAccount. The latter logs in with the FO_EMAIL / FO_PASSWD defaults, which match PrestaShop's demo customer, so it only works on a shop carrying demo data; its register-if-missing fallback cannot stand in, because 9.2 rejects that same password -- the shop answers "The minimum score must be: Strong". On a shop without the fixture, such as the duplicated second shop, both branches fail. An earlier green here proved nothing: it was the fixture passing, not the scenario. Verified 11/11 on hummingbird and on Classic. The two steps are inserted at the end on purpose: the suite is sequential, and putting a navigation mid-chain broke the step that followed it. Co-Authored-By: Claude Opus 5 --- src/Tests/Suites/Smoke/FrontOfficeSmoke.php | 42 +++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php index b22de64..f1af80a 100644 --- a/src/Tests/Suites/Smoke/FrontOfficeSmoke.php +++ b/src/Tests/Suites/Smoke/FrontOfficeSmoke.php @@ -30,6 +30,10 @@ public function init() $this->importPage('FrontOffice\Product'); $this->importPage('FrontOffice\Cart'); $this->importPage('FrontOffice\Category'); + // A page with no selectors of its own: everything it resolves comes + // from the area base, which is exactly what the theme _common block + // has to reach. + $this->importPage('FrontOffice\Stores'); extract($this->pages); @@ -90,6 +94,44 @@ public function init() $frontOfficeCategoryPage->goToProduct(1); Expect::that($frontOfficeProductPage->getPrice() > 0)->equals(true); + }) + /* + * The header chrome is declared once on FrontOfficePage and inherited + * by all 31 front-office pages, so nothing page-specific ever asserted + * it. Measured on a live 9.2 shop, both selectors miss on hummingbird + * on every page -- a whole theme's worth of markup that no suite + * touched. Asserting them here puts the area-wide selectors under the + * same matrix as everything else, on a page that declares none of its + * own. + */ + ->it('the shared header chrome resolves on this theme', function () use ($frontOfficeStoresPage) { + $frontOfficeStoresPage->goToPage('stores'); + + Expect::that($frontOfficeStoresPage->elementIsVisible($frontOfficeStoresPage->getSelector('desktopLogo'), 5000))->equals(true); + Expect::that($frontOfficeStoresPage->elementIsVisible($frontOfficeStoresPage->getSelector('userInfoLink'), 5000))->equals(true); + }) + /* + * accountLink only exists once a session is open, so every anonymous + * probe reported it missing on BOTH themes -- the one shape that looks + * like "no divergence" and hides one. Measured against a logged-in + * session it misses on hummingbird and matches on Classic, which is why + * it needs the scenario below rather than another anonymous step. + * + * Registration, not EnsureTestAccount: the latter logs in with the + * FO_EMAIL / FO_PASSWD defaults, which match PrestaShop's demo customer + * (pub@prestashop.com / 123456789) and therefore only work on a shop + * that carries demo data. Its register-if-missing fallback cannot stand + * in, because 9.2 rejects that same password -- the shop answers "The + * minimum score must be: Strong" -- so on a shop without the fixture, + * such as a duplicated second shop, both branches fail. Registration + * creates its own account with a unique address and a policy-compliant + * password, so it needs nothing from the shop's fixtures. + */ + ->scenario(\PrestaFlow\Library\Scenarios\Registration::class) + ->it('the logged-in header chrome resolves on this theme', function () use ($frontOfficeStoresPage) { + $frontOfficeStoresPage->goToPage('stores'); + + Expect::that($frontOfficeStoresPage->elementIsVisible($frontOfficeStoresPage->getSelector('accountLink'), 5000))->equals(true); }); } } From e59ca85ad19299d9b567d4bd9ae0b469b0db0aec Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Fri, 25 Sep 2026 09:10:54 +0200 Subject: [PATCH 3/4] fix(pages): reach account creation on 1.7, where /registration does not exist The /registration route arrived in 8.0. Asking a 1.7.8.11 shop for it lands on pagenotfound, so goToRegistration() had been walking into a 404 on every 1.7 run. The v7 page was a bare stub inheriting the v9 route, and nothing exercised it: the Registration scenario was not in either smoke suite, so the gap sat in "supported" territory without a single run behind it. 1.7 reaches the form through the login page's create_account flag and renders it as body#authentication, so the two selectors anchored on body#registration move with the route. Everything else is already identical -- #field-firstname, #field-lastname, #field-email, #field-password, #field-birthday, [data-link-action="save-customer"] and the two required consent boxes all exist on 1.7.8.11 -- which is why only these three things are overridden. The URL is built from the login URL rather than hardcoded, so a project that remapped "login" in its own Urls catalogue keeps its route. Passing the flag through goToPage()'s $params would have dropped it: substitution only replaces {placeholders} the template already carries. Found by running the widened front-office suite against 1.7 before pushing, not in CI. All four matrix combinations now pass 11/11; reverting this file to the stub reddens 1.7 alone. The unit test guards the fragile half: the overrides are spread on top of the parent map, and written as a plain array instead they would drop every other field the form needs. Removing the spread fails it. Co-Authored-By: Claude Opus 5 --- .../v7/FrontOffice/Registration/Page.php | 30 +++++++++ .../Pages/V7RegistrationOverridesTest.php | 61 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 tests/Unit/Pages/V7RegistrationOverridesTest.php diff --git a/src/Pages/v7/FrontOffice/Registration/Page.php b/src/Pages/v7/FrontOffice/Registration/Page.php index ff7f17e..d610797 100644 --- a/src/Pages/v7/FrontOffice/Registration/Page.php +++ b/src/Pages/v7/FrontOffice/Registration/Page.php @@ -4,6 +4,36 @@ use PrestaFlow\Library\Pages\v9\FrontOffice\Registration\Page as V9Page; +/** + * 1.7 has no /registration route: it was introduced in 8.0, and asking a + * 1.7.8 shop for it lands on pagenotfound. Account creation lives behind the + * login page's create_account flag, and the resulting page is body#authentication, + * not body#registration -- so the two selectors anchored on that id have to move + * with the route. + * + * Everything else the form needs is already identical: #field-firstname, + * #field-lastname, #field-email, #field-password, #field-birthday, + * [data-link-action="save-customer"] and the two required consent boxes are all + * present on 1.7.8.11, which is why only these three things are overridden. + */ class Page extends V9Page { + public function defineSelectors() + { + return [ + ...parent::defineSelectors(), + 'registrationForm' => 'body#authentication', + 'requiredConsentCheckbox' => 'body#authentication input[type="checkbox"][required]', + ]; + } + + public function goToRegistration(): void + { + // Built from the login URL rather than hardcoded, so a project that + // remapped "login" in its own Urls catalogue keeps its route. Passing + // the flag through goToPage()'s $params would drop it: substitution + // only replaces {placeholders} the template already carries, and the + // login template has none. + $this->goToUrl($this->getPageURL('login') . '?create_account=1'); + } } diff --git a/tests/Unit/Pages/V7RegistrationOverridesTest.php b/tests/Unit/Pages/V7RegistrationOverridesTest.php new file mode 100644 index 0000000..8df3d00 --- /dev/null +++ b/tests/Unit/Pages/V7RegistrationOverridesTest.php @@ -0,0 +1,61 @@ + ['URL' => 'http://shop.test/'], 'THEME' => 'classic'], []); + } + + public function testTheTwoIdAnchoredSelectorsMoveToTheAuthenticationPage(): void + { + $selectors = $this->page(V7Registration::class)->selectors; + + $this->assertSame('body#authentication', $selectors['registrationForm']); + $this->assertSame( + 'body#authentication input[type="checkbox"][required]', + $selectors['requiredConsentCheckbox'] + ); + } + + /** THE fragile part: everything the parent declares must survive. */ + public function testEveryOtherFieldIsInheritedFromV9(): void + { + $v9 = $this->page(V9Registration::class)->selectors; + $v7 = $this->page(V7Registration::class)->selectors; + + $moved = ['registrationForm', 'requiredConsentCheckbox']; + + foreach ($v9 as $key => $value) { + if (in_array($key, $moved, true)) { + continue; + } + + $this->assertArrayHasKey($key, $v7, sprintf('v7 dropped the "%s" selector', $key)); + $this->assertSame($value, $v7[$key], sprintf('v7 changed "%s" without reason', $key)); + } + } + + /** The overrides are additions, not a replacement of the parent map. */ + public function testTheOverrideDoesNotShrinkTheSelectorMap(): void + { + $this->assertCount( + count($this->page(V9Registration::class)->selectors), + $this->page(V7Registration::class)->selectors + ); + } +} From d72e410655a7880e720e379fbbeb02074bc21b92 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Fri, 25 Sep 2026 10:01:22 +0200 Subject: [PATCH 4/4] ci: make an upload that captures nothing say so A red row on 2026-09-25 printed a screenshot path in the runner log and still produced an empty artifact. `if-no-files-found: ignore` let the upload step report success while carrying nothing, so the one run that needed evidence left none, and the failure had to be chased by guesswork instead. The library side is not at fault: locally the capture writes a valid PNG, it wrote one for a flaky failure too, and when it genuinely cannot capture it says why -- "Screenshot capture failed: The session is destroyed." is printed as a Debug line on the failing test. That message is also the root cause of the flakiness itself: the Chrome session dies mid-run, which kills the step and makes the capture impossible for the same reason. It explains why the failing step varies between runs. Why the runner had a path, no capture-failure note, and no artifact is still unexplained. Listing the directory before the upload, and warning instead of ignoring, is what makes the next occurrence diagnosable rather than another round of inference. Co-Authored-By: Claude Opus 5 --- .github/workflows/live-smoke.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/live-smoke.yml b/.github/workflows/live-smoke.yml index 3b9cc82..53044ff 100644 --- a/.github/workflows/live-smoke.yml +++ b/.github/workflows/live-smoke.yml @@ -121,10 +121,24 @@ jobs: php bin/prestaflow run src/Tests/Suites/Smoke/BackOfficeSmoke.php || status=1 exit $status + # Says what is on disk before the upload decides there is nothing to + # take. A red row on 2026-09-25 printed a screenshot path in the runner + # log and still produced an empty artifact, and `if-no-files-found: + # ignore` made the upload report success while carrying nothing — so the + # one run that needed evidence left none. Locally the capture writes a + # valid PNG, including on the flaky failures, so the gap is here rather + # than in the library. + - name: List what the run captured + if: failure() + run: | + echo "cwd: $(pwd)" + ls -laR prestaflow/screens/ 2>&1 || echo "prestaflow/screens/ does not exist" + - name: Upload failure screenshots if: failure() uses: actions/upload-artifact@v4 with: name: screenshots-${{ matrix.ps }}-${{ matrix.theme }} path: prestaflow/screens/ - if-no-files-found: ignore + # `warn`, not `ignore`: an upload that finds nothing has to say so. + if-no-files-found: warn