Skip to content

Commit d86cd29

Browse files
authored
chore: roll bot (#3287)
1 parent 845652c commit d86cd29

14 files changed

Lines changed: 485 additions & 423 deletions

File tree

Lines changed: 40 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -1,214 +1,87 @@
11
---
22
name: playwright-roll
3-
description: Roll Playwright Python to a new microsoft/playwright commit. Pins the commit in DRIVER_VERSION, builds the driver and generates the API from .upstream/, and ports the upstream diff of the client and the tests line by line.
3+
description: Roll Playwright Python to a new microsoft/playwright commit. Pins the commit, generates the API and builds the driver from it, and ports the upstream diff of the client and the tests.
44
---
55

6-
# Rolling Playwright Python
6+
# Rolling Playwright
77

8-
`DRIVER_VERSION` holds one `microsoft/playwright` commit. `.upstream/` (gitignored) is
9-
a full clone of `microsoft/playwright` at that commit. Everything comes from it:
10-
11-
- the driver: `python -m build --wheel` builds it from `.upstream/`;
12-
- the generated files: `./scripts/update_api.sh` writes the API and the protocol files,
13-
and copies the test registry;
14-
- the hand-written port: `playwright/_impl/` ports the client
15-
(`packages/playwright-core/src/client` and the parts of `packages/isomorphic`,
16-
`packages/protocol/src` and `packages/utils` that it uses), and `tests/async/`,
17-
`tests/sync/` port `tests/page/` and `tests/library/`.
18-
19-
A roll moves `DRIVER_VERSION` from the old commit to the new one, and ports the diff
20-
between the two. Do not commit, branch or edit in `.upstream/`.
21-
22-
## 0. Set up
23-
24-
Follow steps 1 and 2 of `CONTRIBUTING.md`: the venv, the dependencies, and
25-
`python scripts/upstream.py checkout`. Activate the venv before every `pip`, `pytest`
26-
or `pre-commit`. In the agent harness, `sudo` is denied: if `python3-venv` is
27-
missing, use `uv venv env`, and install browsers without `--with-deps`.
8+
Read `AGENTS.md` first. A roll moves `DRIVER_VERSION` to a new upstream commit and
9+
ports everything that changed in between.
2810

2911
## 1. Pin
3012

3113
```sh
32-
python scripts/upstream.py pin <ref> # a branch (main, release-1.66), a tag (v1.66.0) or a commit
33-
python scripts/update_node_version.py # NODE_VERSION: the current Node.js LTS
14+
python scripts/upstream.py pin main # or a branch, a tag, a commit
15+
python scripts/update_node_version.py
3416
old=$(git show HEAD:DRIVER_VERSION)
3517
new=$(cat DRIVER_VERSION)
3618
```
3719

38-
`pin` writes the commit to `DRIVER_VERSION` and checks it out in `.upstream/`. The
39-
driver reports the version of `packages/playwright-core/package.json` at that commit:
40-
`1.66.0` on a release tag, `1.67.0-next` on `main`.
41-
42-
## 2. Build
43-
44-
```sh
45-
python -m build --wheel # the driver from .upstream/, and the wheel
46-
playwright install chromium
47-
```
48-
49-
## 3. Generate
20+
## 2. Generate
5021

5122
```sh
5223
./scripts/update_api.sh
5324
mypy playwright
5425
```
5526

56-
The API generator (`scripts/codegen/generate_api.js`) reads the docs in
57-
`.upstream/docs/src/api`, keeps what is for Python, applies
58-
`scripts/codegen/python_rules.js`, and writes every member with its call into
59-
`_impl/`. It never reads `_impl/`. So a new member or option in the docs is a mypy
60-
error in `_generated.py`, and a new parameter in the protocol spec is a mypy error at
61-
the channel call in `_impl/`: each `ChannelOwner` subclass declares its channel type
62-
(`_channel: "PageChannel"`), so mypy checks every call. Step 4 fixes them. Until then,
63-
the client may fail to talk to the new driver.
64-
65-
To see what changed in the public API:
27+
The mypy errors are the to-do list: new members, options and protocol parameters that
28+
`_impl/` does not have yet. Until they are ported, the client may not work with the new
29+
driver. To see what changed in the public API:
6630

6731
```sh
6832
git show HEAD:playwright/async_api/_generated.py > /tmp/old.py
6933
python scripts/codegen/compare_api.py /tmp/old.py playwright/async_api/_generated.py --show all
7034
```
7135

72-
## 4. Port
36+
## 3. Build
7337

7438
```sh
75-
python scripts/upstream.py changes
76-
```
77-
78-
`changes` lists each upstream file that changed between `$old` and `$new`, in groups,
79-
with the Python file to port it to. The files that upstream generates from the
80-
protocol spec (`channels.d.ts`, `validator.ts`) are in the "Generated" group with the
81-
docs and the spec: step 3 regenerated their Python counterparts. A spec file that
82-
`tests/upstream_not_ported.txt` lists shows its reason instead of a Python file. For
83-
each other file, read the diff and port each hunk, in order:
84-
85-
```sh
86-
git -C .upstream diff "$old" "$new" -- <upstream file>
39+
python -m build --wheel
40+
playwright install chromium firefox webkit
8741
```
8842

89-
**Client.** Port each hunk line by line into the Python module, with the rules of
90-
[`playwright/_impl/transform.md`](../../../playwright/_impl/transform.md). Change the
91-
lines that the hunk changes, so that a reviewer can read the two diffs side by side.
92-
Mark each difference from the JS with `# Python:`, and remove a mark whose reason is
93-
gone.
94-
95-
- A new client file (`(no Python port yet)`): a new module, named in snake_case
96-
(`browserContext.ts` is `browser_context.py`), with the docstring
97-
`"""Port of <upstream path>.` and the members in the JS order. A new `ChannelOwner`
98-
goes in `_object_factories()` in `connection.py`. A new class of the public API goes
99-
in `_api_classes.py`. A plain wrapper class (like `Clock`) sets `self._loop` and
100-
`self._dispatcher_fiber` from its parent.
101-
- A deleted or renamed file: delete or rename the module.
102-
- Shared code that Python does not port: port it only when the client starts to use it.
103-
104-
**Tests.** Port each hunk line by line into the async test and the sync test that
105-
`changes` names, with the rules of [`tests/transform.md`](../../../tests/transform.md):
106-
zero deviations, and never change a port to make it pass.
107-
`python scripts/check_test_registry.py --missing <code or file>` prints the name of a
108-
new test. A test that can not exist in Python goes in `tests/upstream_not_ported.txt`,
109-
with the reason. Run each ported file on the three browsers, the async and the sync
110-
files in separate runs (a sync test can not run in a process where an async test has
111-
run: `Runner.run() cannot be called from a running event loop`):
43+
## 4. Port
11244

11345
```sh
114-
xvfb-run -a pytest tests/async/page/test_page_click.py --browser firefox
115-
xvfb-run -a pytest tests/sync/page/test_page_click.py --browser firefox
46+
python scripts/upstream.py changes
11647
```
11748

118-
A failure is a bug in the client, the driver or the environment, not in the test.
49+
It lists each changed upstream file with the Python file to port it to. For each one,
50+
read `git -C .upstream diff $old $new -- <file>` and port it hunk by hunk:
11951

120-
**Test assets.** Copy each changed file from `.upstream/tests/assets/` to
121-
`tests/assets/`; delete the deleted ones. Only these files: a few files in
122-
`tests/assets/` are Python only.
52+
- client code: into the module it names; `(no Python port yet)` is a new module;
53+
- tests: into the async and the sync file it names. New test names come from
54+
`python scripts/check_test_registry.py --missing <id>`;
55+
- test assets and snapshot images: copy them to the place it names;
56+
- the "Generated" group: step 2 did it.
12357

124-
**Snapshot images.** Upstream keeps the image of `toMatchSnapshot()` next to the spec
125-
(`page-screenshot.spec.ts-snapshots/<name>-<browser>.png`); the port keeps it in
126-
`tests/golden-<browser>/<name>.png`. `changes` names the golden file of each changed
127-
image: copy it there. The BiDi and the Firefox Nightly images are not used.
58+
Changes in the server or in the injected scripts need nothing: they come with the new
59+
driver. A test marked `xfail(reason="driver: ...")` may pass now; remove the mark then.
12860

129-
Then run `mypy playwright` until it is clean.
61+
Run each changed test file on the three browsers. Then run `mypy playwright` until it
62+
is clean.
13063

13164
## 5. Check
13265

13366
```sh
134-
pre-commit run --all-files
135-
python scripts/check_test_registry.py
13667
./test.sh
137-
./scripts/update_api.sh && git status # no new changes: the generated files come from .upstream/
138-
playwright install chromium firefox webkit
13968
python scripts/update_versions.py # the browser versions in README.md
69+
./scripts/lint.sh # last: the roll is done when it passes
14070
```
14171

142-
`./test.sh` runs every test, on many workers. For a failure in a file that the roll did
143-
not change:
144-
145-
1. Run the test alone. A test that passes alone failed from the load of the full run.
146-
2. Run it against the old driver. Its bundle stays in `driver/`
147-
(`playwright-<old>-<suffix>.zip`): extract it into `playwright/driver/`, keeping the
148-
file modes, run the test, then `python -m build --wheel` puts the new driver back.
149-
A test that fails with the old driver too is not a regression of the roll: list it
150-
in the PR.
72+
A failure in a file that the roll did not change may be a flake, or older than the
73+
roll. Find out which, and say so in the PR.
15174

152-
Title the PR `chore: roll Playwright to <version> (<first 10 characters of $new>)`,
153-
with the version from step 1. List what you ported and what you could not finish, and
154-
put `git -C .upstream log --oneline --reverse "$old..$new"` of the changed paths in a
155-
`<details>` block.
75+
Title the PR `chore: roll Playwright to <version> (<first 10 characters of $new>)`, with
76+
the version from `.upstream/packages/playwright-core/package.json`. In the description,
77+
list what you ported and what is left, and put the output of
78+
`python scripts/upstream.py log` in a `<details>` block. The roll workflow adds that
79+
list itself.
15680

157-
## Reference: the docs decide the public API
158-
159-
A new member or option comes from the docs into `_generated.py`; the client hunk
160-
implements it.
161-
162-
- **For Python**: no `langs` filter, or `langs` includes `python`. Port it.
163-
- **Not for Python**: `* langs: js` (or another filter without Python). The generator
164-
leaves it out. Mark the client member `# Not ported: <member> langs: js`.
165-
- **Different in Python**: the shipped Python API wins. Fix the upstream docs with a
166-
Python-only declaration (`* langs: python`). Put in `python_rules.js` only what the
167-
docs can not say, with a one-line `//` comment above the entry that says why.
168-
169-
**`langs` is the only filter.** Do not decide by the name of an API: the 1.59 roll left
170-
out Screencast, Debugger and `pickLocator` because they sounded like tooling, and they
171-
were Python APIs. An empty `langs` means every language. To check it:
172-
173-
```sh
174-
node .upstream/utils/doclint/generateApiJson.js > /tmp/api.json
175-
python -c "import json; c = {x['name']: x for x in json.load(open('/tmp/api.json'))}['Page']; print([(m['name'], m.get('langs', {})) for m in c['members']])"
176-
```
81+
## When it breaks
17782

178-
Other docs changes:
179-
180-
- A new inline `[Object]` needs a Python name in the docs (`` "Name" <[Object]> `` or
181-
`* alias-python:`). The generator writes its TypedDict into `_api_structures.py`;
182-
re-export it from `playwright/async_api/__init__.py` and `playwright/sync_api/__init__.py`.
183-
- A renamed positional parameter: rename it in `_impl/`.
184-
- A member now in the docs that `python_rules.js` lists: remove the entry. The
185-
generator fails on entries that match nothing.
186-
187-
`python_rules.js` tables (keys use docs names, `Class.member.arg`; `*` matches one segment):
188-
189-
| Table | Use |
190-
|---|---|
191-
| `handWrittenClasses` | Classes written by hand (`Error`, `TimeoutError`, `FormData`). |
192-
| `contextManagerClasses`, `tracebackHideClasses` | Base-class and `__tracebackhide__` choices. |
193-
| `extraMembers`, `extraOptions` | Python-only members and hidden options. |
194-
| `positionalOptions` | Option names that are positional everywhere (`predicate`). |
195-
| `structTotal` | `total=` of a TypedDict, where it differs from the default. |
196-
197-
## Reference: failure modes
198-
199-
| Symptom | Fix |
200-
|---|---|
201-
| `.upstream/ is not at <commit>` | `python scripts/upstream.py checkout`. |
202-
| `.upstream has local changes` | `git -C .upstream status`; discard the changes. |
203-
| mypy: `"XImpl" has no attribute "y"` in `_generated.py` | A new member in the docs: port the client hunk. |
204-
| mypy: `Unexpected keyword argument "z"` for an impl method | A new option: port the client hunk (`Optional[X]`, no default, passed to the channel call), or fix the docs if Python must not have it. |
205-
| mypy: `Missing named argument "z"` for a channel method | A new protocol parameter: port the client hunk that passes it. |
206-
| generator: `No Python name for the struct at X.y.z` | Name the `[Object]` in the docs. |
207-
| generator: `The struct X has different fields at A and B` | Give one place its own name in the docs. |
208-
| generator: `Unknown docs type "T"` | Add it to `toPy` in `generate_api.js`. |
209-
| generator: `required parameter z follows an optional one` | Fix the docs with a Python-only `### param:` or `### option:`. |
210-
| generator: `the entry "…" no longer matches anything` | Delete or re-key the `python_rules.js` entry. |
211-
| `AttributeError: … '_loop'` on a new wrapper class | Set `self._loop` and `self._dispatcher_fiber` in its `__init__`. |
212-
| `Protocol violations: X.y: params.z: …` at test teardown | The client sends what the spec does not allow: port the hunk that renamed or removed it. |
213-
| A `binary` field arrives as `str` | Decode it with `base64.b64decode()`. |
214-
| `timeout: expected float, got undefined` at launch | `playwright/driver/` is older than the pin: `python -m build --wheel`. |
83+
- `missing ... argument`, or `Protocol violations` at the end of a test: a protocol change
84+
that the client does not follow yet. Port its hunk.
85+
- `timeout: expected float, got undefined` at launch: `playwright/driver/` is older
86+
than the pin. Build again.
87+
- `.upstream/ is not at ...`: `python scripts/upstream.py checkout`.

‎.github/workflows/ci.yml‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,9 @@ jobs:
3939
- name: Get Playwright
4040
run: python scripts/upstream.py checkout
4141
- name: Lint
42-
run: pre-commit run --show-diff-on-failure --color=always --all-files
43-
- name: Check the tests against the upstream test registry
44-
run: python scripts/check_test_registry.py --report
45-
- name: Check that the generated files are up to date
46-
run: |
47-
./scripts/update_api.sh
48-
git diff --exit-code
42+
run: ./scripts/lint.sh
43+
env:
44+
PRE_COMMIT_COLOR: always
4945

5046
build:
5147
name: Build

0 commit comments

Comments
 (0)