Prestashop Pr QA

QA a PrestaShop pull request

Run the pull request's own test steps against a running environment twice: once on the code as it was before the PR, once on the PR's code. Record both runs. The deliverable is one verdict (approved, not approved or not reproducible) with the recording that backs it up.

How the environment is observed depends on what the PR changes. Three probes, one set of rules:

Everything after the probe is identical, and that is the point: two phases in the same order, the same scenario in both, the same flake rule, the same reasons to refuse a verdict.

Terms used in this skill

The left column is what the files and the code call things: before/ is a real directory, --phase=before is a real argument, flaky is a real field in phase.json. Renaming them in prose only would leave the words and the data saying different things.

The right column is what they mean, and the right column is what you say out loud. Never make the person you are talking to learn this table.

Who you are talking to

The person running this QA may be a developer, an integrator, a QA engineer or a product manager. They know the environment and the ticket; they do not know this skill, and they should not have to.

• Say the meaning, not the term. "Put the environment back on the code as it was before the fix" rather than "put the environment in its pre-fix state". "I will check the server is really serving the new code" rather than "I will take a canary reading".
• Every question you ask is one action with one reason. Print the exact command, say in one sentence what it is for, and wait. Never ask for two things in the same breath.
• Ask early whether they can change which code is running at all. Someone who cannot run git on that machine is not a problem, but finding out at the third gate is: say so at the start, run the after measurement alone, and cap the verdict at "approved, reproduction not attempted".
• Never guess what you can ask. Three cases, and only three. If the person knows it, ask them: which probe fits, what the test steps are, which versions the environment runs. If the machine knows it, measure it and show the measurement rather than asserting it. If neither knows it, write that down in the report instead of filling the gap with something plausible. A QA report is worth exactly as much as the reader's confidence that nothing in it was assumed.
• Read the verdict back in their words. "The bug happened on the old code and is gone on the new one" is the sentence they need. The table below is how you got there, not what you report.

Rules that do not bend

1. The developer prepares the ground. Anything that writes to their repository is theirs: a checkout, a fetch, a ref. A checkout can destroy uncommitted work, and it is their branch. Print the command, say what it is for, wait. Reading is yours: gh pr view, gh pr diff, git merge-base on refs that already exist.
2. Everything derived from the code, the skill offers to run itself: composer install, the build, cache:clear. The order matters: rebuild first, read the canary second.
3. before runs first, always. Migrations only run forward, so testing the PR first leaves the before run reading a migrated database.
4. The same scenario.js runs in both phases, unchanged, and so does the runner. Three hashes are recorded in each phase, the scenario, the runner and the shared recording rules; any mismatch voids the verdict.
5. Nothing is ever posted to GitHub. The comment is written to a file for the user to paste.
6. Back-office credentials arrive as the QA_BO_EMAIL and QA_BO_PASSWORD environment variables, never as command-line arguments: arguments end up in artifacts that get pasted into a public pull request.
7. The scenario asserts; it never decides. The verdict comes from the table in this file.
8. Evidence never lands in the environment under test or in the checkout. scripts/pick-run-dir.sh enforces this.

What to ask the user

• the pull request: owner/repo#number, or its URL
• the front-office URL of the running shop
• the back-office URL and the admin folder name, if the PR touches the back office. That folder name differs on every installation
• the versions the environment runs, unless a command can print them: PrestaShop, PHP, and the version of the theme, module or library the PR is about. The report states them as fact, so they are asked for or measured, never assumed from a branch name
• whether they can put the environment back on the code as it was before the fix. Without that there is no before measurement, and the verdict is capped at "approved, reproduction not attempted"

Do not ask for paths on disk: scripts/pick-run-dir.sh derives them from the URL. Ask only if it refuses because the shop does not run in Docker.

Workflow

A typical request: QA the PR PrestaShop/hummingbird#1092 against http://localhost:8887. What follows is the whole run, from reading that PR to handing over a comment to paste.

Copy this checklist into your reply and tick items off as you go:

QA progress:
- [ ] 1. Read the PR and its linked issues
- [ ] 2. Set up the run directory, and Playwright for a browser run
- [ ] 3. Write scenario.js from the ticket's steps
- [ ] 4. GATE: show the scenario, ask for the code from before the fix
- [ ] 5. Measure the before phase
- [ ] 6. GATE: ask for the PR's code
- [ ] 7. Measure the after phase
- [ ] 8. Write verdict.json, render report.html, write report.md and comments/

1. Read the PR and its linked issues

gh pr view [number] --repo [owner/repo] \
  --json title,body,baseRefName,headRefOid,files,closingIssuesReferences

Read every linked issue with gh issue view. Labels are deliberately not fetched: this skill never sets one and never reads the queue, so pulling them would only add noise to what you have to read.

Then:

• Classify the PR as bugfix or new feature.
• Find the test steps. They sit in a table row of the PR body whose label is not standardised: How to test?, How to test, sometimes missing. Reproduction steps and the affected version are usually in the linked issue, not the PR. If they exist in neither, ask the person before reading the diff: they often know the reproduction by heart, and steps that come from a human beat steps reverse-engineered from a patch. Only if they cannot say, derive them from the diff, and then say in the report that they were inferred and from what.
• Propose the probe, then confirm it. The diff suggests one: templates, assets or a controller's output mean browser; a console command, an install or upgrade script, a composer script mean cli; a webservice resource or a JSON endpoint means http. Say which one you propose and why, in one sentence, and let them correct you. They know whether the ticket is about the page or the command; the diff only knows which files moved. A PR can need two, and then it gets two runs and one report per probe.
• Stop here if no probe can observe the change: CI config, documentation, tests only, a pure refactor. Say which it is, and that there is no verdict to give. Inventing steps for such a PR manufactures a verdict out of nothing.
• Check whether the PR silently depends on another PR before blaming the code. The probe is in references/prestashop.md.
• If the diff is one-way, ask now whether this PR has already run here. A checkout does not reset the database, and at the gate the question is too late. See references/prestashop.md.

2. Set up the run directory, and Playwright for a browser run

SKILL_DIR is the directory this file was read from. Both scripts print their reasoning to stderr and their one result to stdout.

RUN=$(sh "$SKILL_DIR/scripts/pick-run-dir.sh" "[front-office URL]" \
        "$HOME/prestashop-pr-qa/[owner]/[repo]/pr-[number]")

# Only for a browser run. A command-line or HTTP run needs neither Playwright nor npm.
NODE_PATH=$(sh "$SKILL_DIR/scripts/playwright-lab.sh"); export NODE_PATH

A run that never touches HTTP has no URL to give: pass - instead, and the guard skips the Docker lookup rather than falling back to port 80 and guarding whatever unrelated container publishes it. The checkout is still protected, and so is any directory you name after the run directory.

If pick-run-dir.sh exits non-zero, stop and read what it printed. It refuses when the evidence would land somewhere it could be committed or served over HTTP. When the shop does not run in Docker it asks for the served directory; pass it as a third argument.

One directory per owner, per repository, per pull request, as in PrestaShop/hummingbird/pr-1092. Each segment is exactly one identifier, so a repository whose name contains a hyphen stays readable, and ls ~/prestashop-pr-qa/[owner]/ answers "which repositories have I QA'd" without parsing anything.

Tell the user the run directory now, and again at the end, so the files are findable when the comment gets pasted. It holds:

The run directory is reused across passes on the same PR; a phase directory is not. The runner empties before/ or after/ itself before measuring it, so an earlier pass's video can never be read as this run's proof. Move a pass aside first (mv before before-01) if it is worth keeping. Whatever you do find in a phase directory is only evidence if its phase.json hashes match the scenario.js and the runner about to run; otherwise treat that phase as absent and say so.

3. Write scenario.js from the ticket's steps

Write it from the ticket's steps first. Read the diff only afterwards, and only to find which page to open and whether a build is needed. One step() per test step. The template, the assertion kinds and the runner's API are in references/runner.md.

In a browser run, call clip() once, in the step where the symptom is visible, naming a container that exists in both phases. The report leads with that clipped pair, which is what makes it readable. A command-line or HTTP run has nothing to clip: its evidence is the paired transcript. See references/runner.md.

If the ticket is about mobile, declare viewport: 'mobile' in the scenario, so the bug is measured at the width where it was reported. Browser runs only. See references/runner.md.

Then derive surfaces, the pages this PR touches on both sides of the shop, and propose the list before writing it into the scenario. On a back-office PR this is what catches the breakage the ticket never thought to mention. See references/runner.md.

Then check every bug assertion against the tokens the diff adds. The recipe is in references/runner.md. A bug assertion naming a class, id or attribute the PR introduces proves nothing: on the code from before the fix that selector is simply absent, the check fails, and the run claims a reproduction it never made. Rewrite it in the words of the ticket.

4. GATE: show the scenario, ask for the code from before the fix

Show the scenario. Then print the exact commands that put the environment back on the code from before the fix, with the reason for each:

Offer to run everything except git, so the rebuild happens before the canary reading. git stays with the developer, because a checkout can destroy uncommitted work. Then wait.

Skip the before phase, saying why, when a checkout cannot reset the database and no snapshot exists. Step 1 is where that was decided. For a new feature there is nothing to reproduce: go to step 6.

Name any fixture data or configuration change out loud before creating it, and list it in the report.

5. Measure the before phase

curl -s -L '[url]' | grep -c '[string the PR introduces]'   # the canary reading

# browser
( cd "$RUN" && node "$SKILL_DIR/scripts/run.js" \
    --scenario=./scenario.js --phase=before --out=. --url="[FO URL]" --bo-url="[BO URL]" )
# command line
( cd "$RUN" && node "$SKILL_DIR/scripts/run-cli.js" \
    --scenario=./scenario.js --phase=before --out=. --cwd="[the checkout under test]" )
# HTTP
( cd "$RUN" && node "$SKILL_DIR/scripts/run-http.js" \
    --scenario=./scenario.js --phase=before --out=. --url="[base URL]" )

The cd stays inside a subshell: a bare cd moves the session out of the repository and breaks the git and gh commands in the steps that follow.

curl has no cache, no service worker and no profile, so it reports what the server actually serves. A back-office change has to be read in the browser instead. See references/prestashop.md.

The canary proves the environment really changed between the phases, so it follows the probe too: for a cli run it is the command printing something the PR introduces, for an http run it is the endpoint answering differently. Record both readings in verdict.json either way.

6. GATE: ask for the PR's code

Print the commands, wait, then take the second canary reading.

7. Measure the after phase

Same as step 5 with --phase=after (the runner clears the directory itself), then compare the two canary readings. Identical readings mean the environment never changed state: a stale Symfony or Smarty cache, an untouched opcache, or a URL served from a different directory than the one that was switched. That is a harness error: say so and stop, with no verdict.

8. Write the verdict, then the reports

Apply the verdict table below, and write the judgement once, into verdict.json. Then render the page, which reads that file and both phase.json and invents nothing:

node "$SKILL_DIR/scripts/report.js" --run="$RUN"

Write report.md too: the same facts as text, for grepping and for tools that do not render HTML. Then write comments/, tell the user the pbcopy command for each target, where report.html is, and which state the environment was left in with the command that restores it.

The shape of verdict.json, what belongs in each report and the rules for the text that gets pasted on GitHub are in references/reporting.md. Three of those rules matter enough to repeat here, because breaking them is published rather than merely untidy: nothing is ever posted automatically, nothing in a comment addresses the person pasting it, and no comment carries a path from your disk.

Verdict

A check that fails in both phases is pre-existing: report it, do not hold it against the PR.

A surface the PR touches that answered in before and fails in after is 🔴 not approved on its own, even when every bug assertion passed. The fix working on the page the ticket names does not license breaking the page next to it.

The pages the PR touches

The smoke pass covers what the ticket never mentions. This covers what it does, and they are not the same question. A back-office change reaches the front office through the model the two share, and that is where a migration breaks something nobody looked at: the migrated page is perfect and the front-office page built on the same model renders nothing.

The scenario declares them as surfaces, one entry per page, prefixed by the side it lives on: surfaces: ['bo:/<the route under test>', 'fo:<the page built on the same code>'].

Derive the list from the diff, then ask. Say which pages you propose and why, in one sentence, and let the user correct you: they know whether the ticket has a front-office side. Each surface is opened in both phases and asked whether it answers, is not a fatal page, and rendered anything. The recipe, and the one case that is a failed measurement rather than a failed page, are in references/runner.md.

The regression net

Every phase also runs checks the ticket never asked for. The smoke pass runs under every probe. In a browser run it is joined by the front page plus up to two of the pages the scenario opened, re-visited at 375 and 768 wide. A command-line or HTTP run has no viewport, so the net there is the smoke pass alone, and the report says so.

The smoke pass follows where. A front-office or mixed PR gets the front page, a product page, the cart, and the back office when its URL was given. A back-office PR loses the cart and the product page, which it cannot break: what is left is the back office, proving the container still builds, and the front page, proving the shop still answers. What that PR does touch is covered by surfaces above. Each page is asked one thing: did it answer under 400, and is the body not a fatal error page. In a browser run that list is not configurable, on purpose: one the agent could choose would drift towards the ticket, which is exactly what it must not cover. A command-line or HTTP scenario declares its own with smoke. The narrow pass asks only whether the shop still works at that width: the page responds, it renders visible content, it does not scroll sideways. It never judges the design.

Everything is measured in both phases, which is the whole point: the comparison is what makes a finding attributable.

Nothing else about the narrow layouts is asserted, on purpose: a cramped price, a button on two lines, a stretched image are for a human to judge. The mobile-*.png and tablet-*.png pairs sit in the run directory for exactly that, and the report points at them instead of pretending to judge them.

A flake means two different things depending on the phase:

• in before, the symptom appeared and then cleared. It was still observed, so the row stays reproduced, with the word intermittent on line 1 of the verdict. An intermittent bug is a real bug, and the reporter needs to know it was not seen every time.
• in after, the settled reading is the one that counts. The phase passes, and the report says the first reading was taken too early.

Neither voids the verdict. phase.json marks both as flaky: true, and the before case as intermittent.

Refuse to give any verdict at all, which is a harness error and never a PR failure, when:

• scenarioSha256 differs between the two phases
• runnerSha256 or recordSha256 differs between them (two different programs, or two different sets of recording rules, judged them; they are not comparable)
• the two canary readings are identical
• a precondition failed
• a selector matched nothing
• a bug assertion names markup the PR introduces

Severity markers

Troubleshooting

Bundled files