Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
16 changes: 11 additions & 5 deletions .claude/skills/playwright-roll/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,17 @@ After the script settles, run `pre-commit run --all-files` once more to confirm

### 6. Add tests

For each PORT, port the upstream test that covers it, as an async test and a matching sync test. Conventions:
Copy the upstream registry of the new release, then bring the Python tests in line with it:

- The upstream file decides the Python file: `tests/page/expect-misc.spec.ts` becomes `tests/async/test_expect_misc.py` and `tests/sync/test_expect_misc.py`. Create the file if it does not exist.
- The upstream title decides the test name: the `describe` title and the test title, in snake_case. `test('should support pseudo element')` in `test.describe('toHaveCSS')` becomes `test_to_have_css_should_support_pseudo_element`.
- When no upstream test covers the change, extend an existing test before you add a new one.
```sh
cp "$PW_SRC_DIR/tests/registry.md" tests/upstream_registry.md
python scripts/check_test_registry.py --report
```

- **New identifiers**: port each new upstream test, as an async test and a matching sync test. `python scripts/check_test_registry.py --missing <code or file glob>` prints the file and the expected name of each. The upstream file decides the Python file (`tests/page/expect-misc.spec.ts` becomes `tests/async/page/test_expect_misc.py` and `tests/sync/page/test_expect_misc.py`); the identifier and the title decide the name (see [Tests](../../../AGENTS.md#tests)).
- **Changed titles**: the checker names the expected name; rename the Python test and port the changed body.
- **Removed identifiers**: delete the Python test (the checker reports it as not in the registry).
- **Not ported**: a test that can not exist in Python goes in `tests/upstream_not_ported.txt`, with the reason. The checker fails on a rule that no longer matches a test.
- Use `from playwright.async_api import …`, **not** `from playwright._impl._page import Page` (the impl class doesn't have the public wrappers like `expect_console_message`).
- For event-info objects, `await message_info.value` (it's an `async` property).
- Don't write tests that hang the page (e.g. `page.evaluate(... fetch slow ...)` followed by `page.close()` from a fixture) — the request task gets a `TargetClosedError`. Use `page.on("event", handler)` to capture state at event time instead.
Expand All @@ -308,7 +314,7 @@ mypy playwright # 2 pre-existing errors in _json_pipe.py and _ar
pytest --browser chromium tests/async/<files-you-touched> tests/sync/<files-you-touched>
```

Then a smoke regression on a few neighboring suites (`tests/async/test_browser*.py`, `test_cdp_session.py`, `test_tracing.py`, `test_dialog.py`, `test_page_*.py`) to make sure nothing inherent to the port shifted.
Then run `python scripts/check_test_registry.py`, and a smoke regression on a few neighboring suites (for example `tests/async/library/test_browser*.py`, `tests/async/library/chromium/test_session.py`, `tests/async/library/test_tracing.py`, `tests/async/page/test_page_*.py`) to make sure nothing inherent to the port shifted.

## Reference: `python_rules.js` tables

Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ jobs:
pip install -e .
- name: Lint
run: pre-commit run --show-diff-on-failure --color=always --all-files
- name: Check the tests against the upstream test registry
run: python scripts/check_test_registry.py --report

build:
name: Build
Expand Down
3 changes: 2 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
# The test assets are copied from upstream byte for byte; the hooks must not change them.
exclude: ^tests/assets/
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
exclude: tests/assets/har-sha1-main-response.txt
- id: check-yaml
- id: check-toml
- id: requirements-txt-fixer
Expand Down
16 changes: 9 additions & 7 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Python bindings for [Playwright](https://playwright.dev). The Python client talk
- `_protocol_types.py` has the TypedDicts for initializers, results, events and nested types, named as upstream's `utils/generate_channels.js` names them (`PageInitializer`, `FrameGotoResult`, `PageRouteEvent`, …).
- `_protocol.py` has the parameters of every command. With `PW_CHECK_PROTOCOL=1`, `playwright/_impl/_protocol_check.py` checks each message that the client sends against it. The test suite sets this, and fails a test that sends an unknown command, an unknown or missing parameter, or a value of the wrong type.
- `scripts/codegen/compare_api.py` — compares two generated files (signatures, annotations, call bodies, docstrings) and lists every difference. Use it to check that a generator change does not change the API.
- `tests/async/`, `tests/sync/` — pytest suites, ported from the upstream tests. Most new tests are added to the async file with a sync mirror. See [Tests](#tests).
- `tests/async/`, `tests/sync/` — pytest suites, ported test for test from the upstream tests, which `tests/upstream_registry.md` lists by identifier. See [Tests](#tests).
- `DRIVER_VERSION` — the single source of truth for which Playwright release the driver is assembled from (one line, the `playwright-core` npm version, e.g. `1.61.0`, no `v` prefix). Read by `setup.py`, `scripts/build_driver.py`, and CI. The wheel build downloads `playwright-core` at this version from npm plus the matching Node.js binary and assembles the per-platform bundles — no source build. The version is baked into the staged bundle filenames (`driver/playwright-<version>-<suffix>.zip`), so it doubles as the build cache key.
- `NODE_VERSION` — the Node.js version bundled with the driver (one line, e.g. `24.16.0`). Maintained at roll time by `scripts/update_node_version.py` (latest LTS, mirroring upstream's `utils/build/update-playwright-node.mjs`).
- `scripts/build_driver.py` — assembles the per-platform driver bundles into `driver/` by downloading the `playwright-core` npm package (`DRIVER_VERSION`) and the official Node.js binaries (`NODE_VERSION`). Fetches `playwright-core` with `npm pack` (needs Node.js/npm on PATH; honours a root `.npmrc`) and the Node.js binaries over plain HTTP; invoked from `setup.py`'s `bdist_wheel` with the target platform's suffix (no arg builds all six).
Expand Down Expand Up @@ -50,14 +50,16 @@ The public API comes from the upstream docs. After `./scripts/update_api.sh`, ru

## Tests

The tests are a port of the upstream tests in `microsoft/playwright` (`tests/page/`, `tests/library/`). Keep them close to upstream, so that each upstream test has one place in this repo.
The tests are a port of the upstream tests in `microsoft/playwright` (`tests/page/`, `tests/library/`), test for test. Upstream gives each test an identifier, the first token of its title: `it('CH-001 should check the box', ...)`. `tests/upstream_registry.md` is a copy of upstream's `tests/registry.md`, which lists every identifier with its file and title.

**First rule: follow the upstream names and files.**
**First rule: every upstream test is ported, or listed as not ported.**

- When a change needs a test, find the upstream test for the same behavior and port it. Do not write a new test when an upstream test exists.
- Put the test in the Python file that matches the upstream file: `tests/page/expect-misc.spec.ts` becomes `tests/async/test_expect_misc.py`. If that file does not exist, create it.
- Name the test after the upstream title, in snake_case, with the `describe` title in front: `test('should support pseudo element')` in `test.describe('toHaveCSS')` becomes `test_to_have_css_should_support_pseudo_element`.
- Add the sync mirror in `tests/sync/`, with the same file name and test name.
- Port each upstream test twice, to `tests/async/` and to `tests/sync/`, in the file that matches the upstream file: `tests/page/page-check.spec.ts` becomes `tests/async/page/test_page_check.py` and `tests/sync/page/test_page_check.py`; `tests/library/chromium/oopif.spec.ts` becomes `tests/async/library/chromium/test_oopif.py`.
- Name the test `test_<ID>_<title>`: the identifier with `_`, then the `describe` titles and the title in snake_case. `test('EX-181 toHaveCount pass')` in `test.describe('toHaveCount')` becomes `test_EX_181_to_have_count_to_have_count_pass`. A test that a loop declares several times is one function with `@pytest.mark.parametrize`.
- Port faithfully: the same steps, assertions and skip conditions. A test that fails because of a bug in the Python client stays, marked `@pytest.mark.xfail(reason="python: ...", strict=True)`.
- A test that can not exist in Python (a JavaScript-only API, Node.js client internals, tools that run in the driver, `@playwright/test` features) goes in `tests/upstream_not_ported.txt`, with the reason.
- `python scripts/check_test_registry.py` checks all of this. `--missing <code or file glob>` prints the expected names of the tests still to port; `--report` prints the coverage per subsystem.
- Do not write a new test when an upstream test exists. `tests/async/legacy/` and `tests/sync/legacy/` hold older ports named after old upstream titles; do not add tests there.

**Second rule: keep the number of tests small.** This applies to every test, and most of all to Python-only behavior that has no upstream test (the sync API, asyncio, the driver).

Expand Down
Loading
Loading