Skip to content

Flashlight test infrastructure, and three defects it uncovered - #62

Merged
PrestaEdit merged 12 commits into
mainfrom
dev
Sep 24, 2026
Merged

PrestaEdit merged 12 commits into
mainfrom
dev

Conversation

@PrestaEdit

@PrestaEdit PrestaEdit commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Replaces the hand-built reference container with reproducible PrestaShop shops from the official prestashop-flashlight images, and runs the front-office smoke suite against 1.7.8.11, 8.2.8 and 9.2.0.

What the green matrix means — and what it does not

All three rows pass. An earlier version of this description predicted 1.7 and 8.2 would be red, and said so in bold. That prediction was wrong, and the reasoning behind it is worth recording because it is the same mistake this PR fixes elsewhere.

The argument was: the v7 and v8 page objects are nine-line stubs inheriting the v9 selectors — 42 files each, 378 lines against v9's 2222 — therefore support for those versions is declarative. That is an inference drawn from a file listing, not a measurement. The measurement says otherwise: for Home, Listing and Product under the classic theme, the markup has not meaningfully changed between 1.7.8 and 9.2. Inheritance is a working strategy for those pages, not an empty shell.

The runs are real, not vacuous — the 1.7 job waits for a 302 on /admin-dev/, then:

PASS the home page renders
PASS reach the product listing from the home page
PASS open a product and read its price
Tests: 3 passed (3 assertions)

So read the green narrowly. It proves that three front-office pages work on three versions with no version-specific selector. It does not prove that 1.7 or 8.2 are supported: the suite covers 3 of 31 front-office pages, classic only, and touches no checkout, no back office, no account and no cart.

A green matrix is a strong claim that nobody re-reads. "CI passes on 1.7" will become "we support 1.7" unless the suite grows. Widening it until it finds where the versions actually diverge is the next useful step — while it stays at three pages it will keep producing reassuring, empty green.

Infrastructure

  • docker-compose.yml — three shops, each with its own MariaDB and a provisioned second shop on its own port (8017/8018, 8082/8083, 8092/8093).
  • docker/post-scripts/ — provisioning that runs after PrestaShop starts.
  • src/Tests/Suites/Smoke/FrontOfficeSmoke.php — Home → Listing → Product, no hardcoded product URL or category id, so it is as valid on 1.7 as on 9.2.
  • docs/testing-with-flashlight.md — every command in it has been executed from a torn-down state.

The 9.2 container serves both themes from one boot: hummingbird on 8092, classic on 8093.

Three library defects this uncovered

Each is fixed here, and each is pinned by a test that fails without the fix.

Commit Defect
2df75f8 Listing::goToProduct() concatenated .product-title a in PHP, so no theme file could reach it — listing→product navigation was broken on hummingbird, and AddProductToCart with it. It also used waitForNavigation(), which does not wait for a click-initiated navigation: 2 failures in 4 runs.
4ddc031 Theme overrides were keyed on the concrete class, so a page extending a sibling never saw its parent's overrides. Seven v9 front-office pages are affected.
f1ad516 getPageURL() substituted {index} only when the parameter was an array; every caller passes a scalar, so the placeholder reached the URL percent-encoded.

The third also corrected a false comment in three scenarios — "friendly URLs can't be rebuilt from an id". PrestaShop canonicalises by id (/3-anything → /3-clothes); what could not be rebuilt was our own substitution.

Known gaps, stated rather than hidden

  • The matrix covers the version axis only, and shallowly. Hummingbird is exercised on three pages, not the 25 front-office pages that have no theme map.
  • The second shop is provisioned but not exercised in CI.
  • The same inheritance defect fixed in 4ddc031 still exists in getMessages(), the locale catalogue and customs['selectors'].
  • A failing post-script does not stop the container: it logs and stays healthy. The smoke suite is what proves provisioning worked.

Verification

OK (297 tests, 655 assertions) — Unit. The smoke suite ran 6× green on hummingbird, 5× on classic, and failed identically 3× when told the wrong theme. Provisioning was verified from a clean boot on all three versions.

🤖 Generated with Claude Code

PrestaEdit and others added 11 commits September 24, 2026 13:29
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… for the click

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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…pped

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 <noreply@anthropic.com>
…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 `<h4 id="shop_name">`; 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 -- `<form action="#">` driven by js/admin/login.js -- and the page
   ships `<div id="error" class="hide alert alert-danger"></div>` 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 <noreply@anthropic.com>
PrestaEdit added a commit to PrestaEdit/prestaedit.github.io that referenced this pull request Sep 24, 2026
La série utilise Flashlight partout sans l'avoir jamais présenté. Cette
annexe s'appuie sur PrestaFlow/php-library#62 : choix du tag (les tags
nus 8.2.8 et 9.2.0 n'existent pas), base MariaDB à côté, post-scripts
pour une seconde boutique sur un port dédié, attente sur /admin-dev/,
matrice 1.7 / 8.2 / 9.2 en docker compose et GitHub Actions, et ce
qu'une matrice verte prouve ou non.

Les comportements décrits (script non exécutable signalé dans le log,
échec d'un init-script qui n'arrête pas le conteneur) ont été vérifiés
sur l'image 8.1.7.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…checked

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 `<h4 id="shop_name">`. 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 (`<form action="#">`, js/admin/login.js) and the
   page ships `<div id="error" class="hide alert alert-danger"></div>` 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 <noreply@anthropic.com>
@PrestaEdit
PrestaEdit merged commit 4d7393b into main Sep 24, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant