Skip to content

Commit 86208da

Browse files
committed
test: use upstream's per-test timeout, and fix what the full matrix showed
A test times out after 30 seconds, as upstream, and the fixtures count, as upstream counts them (pyproject.toml). A test marked slow gets three times that, as test.slow() gives upstream: the ports of the tests that call it (IF-009, IF-011, FC-001, CC-040, BT-076, BT-077, BT-091, all of screencast.spec.ts and signals.spec.ts), and the Python-only tests that wait longer by design (the pause tests, the subprocess and multiprocessing tests). The wheel installation test gets its own bound. CI and test.sh drop their --timeout flags. The timeout method is thread, which ends the process of a test that hangs; xdist replaces the worker. The signal method can not be used: asyncio's Handle._run() catches and logs every BaseException but SystemExit and KeyboardInterrupt, so a timeout that fires while the event loop runs a callback is lost and the test hangs on (the jobs that ran into the 90-minute limit). tests/conftest.py cancels the timer before the report of a failure is built, where it fired inside pytest when a test failed just before its deadline, and gives the teardown a timer of its own. Races and failures that the full suites on every bot showed: - find_free_port() reserves a port with a dual-stack bind, and a listener that gets 127.0.0.1 but loses ::1 to another process takes a new port instead of going on with IPv4 only. The IPv6-only server of the socks tests could take a port that the test server of another worker held on 127.0.0.1, and the browser, which resolves localhost to ::1 first, reached the wrong server. - The child of the signal tests closes the browser on SIGHUP and ends when it has closed, as the Node.js child does. The Python client has no SIGHUP handler, so the signal ended the child at once, and the driver, left without a client, killed the browser and could crash on its closed pipe before it logged the exit; SI-006 failed one time in two on WebKit. - The child of the signal tests gets a short temporary directory: under tmp_path, the socket of Chrome's ProcessSingleton in the user data dir was longer than a Unix socket path allows, and the stable channels crashed at launch. - A zone whose coroutine is closed from another context (GeneratorExit) can not reset its ContextVar token; the reset raised in every job. - CI runs Linux on ubuntu-22.04, where headless Firefox has WebGL (CA-008, CA-009); the 24.04 image has none. The async step runs after a failed sync step too, so that each job reports both suites. The ports are not changed to pass: AGENTS.md says so now, and tests/transform.md has the rules of the port of the tests. transform.md, the rules of the port of the client, moves to playwright/_impl.
1 parent 36d698e commit 86208da

34 files changed

Lines changed: 439 additions & 100 deletions

‎.claude/skills/playwright-roll/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ A few rules of thumb that catch most "actually a PORT" cases:
154154

155155
#### PORT
156156

157-
Implement the change in `playwright/_impl/<module>.py`, the line-by-line port of `$PW_SRC_DIR/packages/playwright-core/src/client/<module>.ts` (snake_case: `browserContext.ts` is `browser_context.py`). The client change that implements the new API is usually in the same upstream commit as the docs change, or close to it: port it line by line with the rules of [`transform.md`](../../../transform.md), and walk the whole client range as section 14 of that file says (`git -C "$PW_SRC_DIR" log <prev>..<rev> -- packages/playwright-core/src/client packages/isomorphic packages/protocol`). A few reminders:
157+
Implement the change in `playwright/_impl/<module>.py`, the line-by-line port of `$PW_SRC_DIR/packages/playwright-core/src/client/<module>.ts` (snake_case: `browserContext.ts` is `browser_context.py`). The client change that implements the new API is usually in the same upstream commit as the docs change, or close to it: port it line by line with the rules of [`playwright/_impl/transform.md`](../../../playwright/_impl/transform.md), and walk the whole client range as section 14 of that file says (`git -C "$PW_SRC_DIR" log <prev>..<rev> -- packages/playwright-core/src/client packages/isomorphic packages/protocol`). A few reminders:
158158

159159
| Upstream JS | Python |
160160
|---|---|

‎.github/workflows/ci.yml‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ jobs:
4343
strategy:
4444
fail-fast: false
4545
matrix:
46-
os: [ubuntu-latest, windows-latest, macos-latest]
46+
# ubuntu-22.04, not ubuntu-latest: the 24.04 image has no WebGL for headless
47+
# Firefox (CA-008, CA-009).
48+
os: [ubuntu-22.04, windows-latest, macos-latest]
4749
python-version: ['3.14']
4850
browser: [chromium, firefox, webkit]
4951
exclude:
@@ -58,7 +60,7 @@ jobs:
5860
- os: macos-latest
5961
python-version: '3.10'
6062
browser: chromium
61-
- os: ubuntu-latest
63+
- os: ubuntu-22.04
6264
python-version: '3.10'
6365
browser: chromium
6466
- os: macos-15-xlarge
@@ -70,7 +72,7 @@ jobs:
7072
- os: macos-latest
7173
python-version: '3.11'
7274
browser: chromium
73-
- os: ubuntu-latest
75+
- os: ubuntu-22.04
7476
python-version: '3.11'
7577
browser: chromium
7678
- os: windows-latest
@@ -79,7 +81,7 @@ jobs:
7981
- os: macos-latest
8082
python-version: '3.12'
8183
browser: chromium
82-
- os: ubuntu-latest
84+
- os: ubuntu-22.04
8385
python-version: '3.12'
8486
browser: chromium
8587
- os: windows-latest
@@ -88,7 +90,7 @@ jobs:
8890
- os: macos-latest
8991
python-version: '3.13'
9092
browser: chromium
91-
- os: ubuntu-latest
93+
- os: ubuntu-22.04
9294
python-version: '3.13'
9395
browser: chromium
9496
runs-on: ${{ matrix.os }}
@@ -99,6 +101,7 @@ jobs:
99101
with:
100102
python-version: ${{ matrix.python-version }}
101103
- name: Install dependencies & browsers
104+
id: install
102105
run: |
103106
python -m pip install --upgrade pip
104107
pip install -r local-requirements.txt
@@ -107,31 +110,32 @@ jobs:
107110
python -m build --wheel
108111
python -m playwright install --with-deps ${{ matrix.browser }}
109112
- name: Common Tests
110-
run: pytest tests/common --browser=${{ matrix.browser }} --timeout 90
113+
run: pytest tests/common --browser=${{ matrix.browser }}
111114
- name: Test Reference count
112115
run: pytest tests/test_reference_count_async.py --browser=${{ matrix.browser }}
113116
- name: Test Wheel Installation
114117
run: pytest tests/test_installation.py --browser=${{ matrix.browser }}
115118
- name: Test Sync API
116-
if: matrix.os != 'ubuntu-latest'
117-
run: pytest tests/sync --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
119+
if: matrix.os != 'ubuntu-22.04'
120+
run: pytest tests/sync --browser=${{ matrix.browser }} -n auto
118121
- name: Test Sync API
119-
if: matrix.os == 'ubuntu-latest'
120-
run: xvfb-run pytest tests/sync --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
122+
if: matrix.os == 'ubuntu-22.04'
123+
run: xvfb-run pytest tests/sync --browser=${{ matrix.browser }} -n auto
121124
- name: Test Async API
122-
if: matrix.os != 'ubuntu-latest'
123-
run: pytest tests/async --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
125+
# Also after a failed sync step: the two suites report on their own.
126+
if: ${{ !cancelled() && steps.install.outcome == 'success' && matrix.os != 'ubuntu-22.04' }}
127+
run: pytest tests/async --browser=${{ matrix.browser }} -n auto
124128
- name: Test Async API
125-
if: matrix.os == 'ubuntu-latest'
126-
run: xvfb-run pytest tests/async --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
129+
if: ${{ !cancelled() && steps.install.outcome == 'success' && matrix.os == 'ubuntu-22.04' }}
130+
run: xvfb-run pytest tests/async --browser=${{ matrix.browser }} -n auto
127131

128132
test-stable:
129133
name: Stable
130134
timeout-minutes: 90
131135
strategy:
132136
fail-fast: false
133137
matrix:
134-
os: [ubuntu-latest, windows-latest, macos-latest]
138+
os: [ubuntu-22.04, windows-latest, macos-latest]
135139
browser-channel: [chrome]
136140
include:
137141
- os: windows-latest
@@ -146,6 +150,7 @@ jobs:
146150
with:
147151
python-version: "3.10"
148152
- name: Install dependencies & browsers
153+
id: install
149154
run: |
150155
python -m pip install --upgrade pip
151156
pip install -r local-requirements.txt
@@ -154,19 +159,20 @@ jobs:
154159
python -m build --wheel
155160
python -m playwright install ${{ matrix.browser-channel }} --with-deps
156161
- name: Common Tests
157-
run: pytest tests/common --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90
162+
run: pytest tests/common --browser=chromium --browser-channel=${{ matrix.browser-channel }}
158163
- name: Test Sync API
159-
if: matrix.os != 'ubuntu-latest'
160-
run: pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
164+
if: matrix.os != 'ubuntu-22.04'
165+
run: pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
161166
- name: Test Sync API
162-
if: matrix.os == 'ubuntu-latest'
163-
run: xvfb-run pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
167+
if: matrix.os == 'ubuntu-22.04'
168+
run: xvfb-run pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
164169
- name: Test Async API
165-
if: matrix.os != 'ubuntu-latest'
166-
run: pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
170+
# Also after a failed sync step: the two suites report on their own.
171+
if: ${{ !cancelled() && steps.install.outcome == 'success' && matrix.os != 'ubuntu-22.04' }}
172+
run: pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
167173
- name: Test Async API
168-
if: matrix.os == 'ubuntu-latest'
169-
run: xvfb-run pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
174+
if: ${{ !cancelled() && steps.install.outcome == 'success' && matrix.os == 'ubuntu-22.04' }}
175+
run: xvfb-run pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
170176

171177
test_examples:
172178
name: Examples
@@ -181,6 +187,7 @@ jobs:
181187
with:
182188
python-version: '3.10'
183189
- name: Install dependencies & browsers
190+
id: install
184191
run: |
185192
pip install -r requirements.txt
186193
python -m playwright install --with-deps chromium

‎AGENTS.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Python bindings for [Playwright](https://playwright.dev). The Python client talk
88

99
## Layout
1010

11-
- `playwright/_impl/` — the client, a line-by-line port of upstream's JS client (`packages/playwright-core/src/client/*.ts`): one module per client file in snake_case (`browserContext.ts` is `browser_context.py`), members in the JS order. `transform.md` (repo root) has the rules of the port; each difference from the JS code has a `# Python:` comment. `isomorphic/`, `protocol/` and `utils/` hold the ports of the parts of `packages/isomorphic`, `packages/protocol` and `packages/utils` that the client uses. The modules whose names start with an underscore are Python only, and the generated API imports most of them by name: `_api_classes.py` (the implementation of each API class), `_async_base.py` / `_sync_base.py` (the base classes of the API objects), `_impl_to_api_mapping.py`, `_errors.py` (the public error classes), `_form_data.py`, `_helper.py`. `_greenlets.py` runs the sync API (see `transform.md`, section 13), `_driver.py` finds the bundled driver, and pytest-playwright uses `_assertions.py` (the soft-assertion scope); the matchers are in `assertions.py`.
11+
- `playwright/_impl/` — the client, a line-by-line port of upstream's JS client (`packages/playwright-core/src/client/*.ts`): one module per client file in snake_case (`browserContext.ts` is `browser_context.py`), members in the JS order. `playwright/_impl/transform.md` has the rules of the port; each difference from the JS code has a `# Python:` comment. `isomorphic/`, `protocol/` and `utils/` hold the ports of the parts of `packages/isomorphic`, `packages/protocol` and `packages/utils` that the client uses. The modules whose names start with an underscore are Python only, and the generated API imports most of them by name: `_api_classes.py` (the implementation of each API class), `_async_base.py` / `_sync_base.py` (the base classes of the API objects), `_impl_to_api_mapping.py`, `_errors.py` (the public error classes), `_form_data.py`, `_helper.py`. `_greenlets.py` runs the sync API (see `transform.md`, section 13), `_driver.py` finds the bundled driver, and pytest-playwright uses `_assertions.py` (the soft-assertion scope); the matchers are in `assertions.py`.
1212
- `playwright/async_api/_generated.py`, `playwright/sync_api/_generated.py`, and the public TypedDicts in `playwright/_impl/_api_structures.py` — **auto-generated** from the upstream API docs. Never edit by hand; rerun `./scripts/update_api.sh` after a roll or after changing `scripts/codegen/`.
1313
- `scripts/codegen/generate_api.js` — the generator. A Node.js script that loads the upstream doc parser (`utils/doclint/api_parser.js`) from `$PW_SRC_DIR`, filters the docs for Python, and writes the whole public API: signatures, types, docstrings, event overloads, the calls into `_impl/`, and the TypedDicts. It does not read `_impl/`.
1414
- `scripts/codegen/python_rules.js` — the Python facts that the docs do not contain (positional options, struct `total=`, hidden members and options). Policy: the shipped Python API wins; each entry is a candidate for an upstream docs fix, and the generator fails on entries that no longer match anything. Every entry needs a one-line rationale comment above it.
@@ -56,7 +56,8 @@ The tests are a port of the upstream tests in `microsoft/playwright` (`tests/pag
5656

5757
- 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`.
5858
- 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`.
59-
- 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)`.
59+
- Port faithfully: the same steps, assertions and skip conditions. The timeout of a test is 30 seconds, as upstream (`pyproject.toml`); where upstream calls `test.slow()`, mark the port `@pytest.mark.slow`, which triples it. A Python-only test that waits longer than 30 seconds by design gets the same mark. A test that fails because of a bug in the Python client stays, marked `@pytest.mark.xfail(reason="python: ...", strict=True)`.
60+
- **Never change a port to make it pass.** No skip, condition, mark, wait or assertion that upstream does not have, for any reason: not a browser channel, not a CI image, not an operating system, not flakiness. A port that fails shows a bug in the client, the driver or the environment: fix that, or report it with the evidence. The one allowed mark is the `xfail(reason="python: ...", strict=True)` above, for a bug in the Python client that is not fixed yet. `tests/transform.md` has the rules of the port.
6061
- 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.
6162
- `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.
6263
- Do not write a new test when an upstream test exists. A Python-only test goes in `tests/async/python/` or `tests/sync/python/`, in the file named after the upstream file of the feature.

‎playwright/_impl/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
finds the bundled driver, _protocol_check.py and _protocol.py (generated) check the
2929
messages in the tests, and pytest-playwright uses _assertions.py.
3030
31-
The rules of the port are in transform.md, in the root of the repository. What
31+
The rules of the port are in transform.md, next to this file. What
3232
differs from the JS code is marked with a "Python:" comment. In short:
3333
3434
- Names are snake_case. Implementation methods take the arguments that the
File renamed without changes.

‎playwright/_impl/utils/zones.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,12 @@ async def run_async(self, func: Callable[[], Awaitable[R]]) -> R:
5757
try:
5858
return await func()
5959
finally:
60-
_async_local_storage.reset(token)
60+
try:
61+
_async_local_storage.reset(token)
62+
except ValueError:
63+
# Python: the coroutine was closed (GeneratorExit) from another
64+
# context, where the token is not valid; its context is gone with it.
65+
pass
6166

6267
def data(self, type: ZoneType) -> Any:
6368
return self._data.get(type)

‎pyproject.toml‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,31 @@ packages = [
5555
]
5656
include-package-data = true
5757

58+
[tool.setuptools.exclude-package-data]
59+
# The rules of the port, for the people who write it; not for the wheel.
60+
"playwright._impl" = ["transform.md"]
61+
5862
[tool.setuptools_scm]
5963
version_file = "playwright/_repo_version.py"
6064

6165
[tool.pytest.ini_options]
6266
addopts = "-Wall -rsx -vv -s"
67+
# The timeout of a test is 30 seconds, as upstream; a test marked slow gets three
68+
# times that, as test.slow() gives upstream (tests/conftest.py). The fixtures count,
69+
# as upstream counts them. The thread method ends the process of a test that hangs;
70+
# xdist replaces the worker. The signal method can not be used: it raises inside the
71+
# running code, and asyncio's Handle._run() catches and logs every BaseException but
72+
# SystemExit and KeyboardInterrupt, so a timeout that fires while the event loop runs
73+
# a callback is lost and the test hangs on.
74+
timeout = 30
75+
timeout_method = "thread"
6376
markers = [
6477
"skip_browser",
6578
"only_browser",
6679
"skip_platform",
6780
"only_platform",
68-
"skip_protocol_check"
81+
"skip_protocol_check",
82+
"slow: the test gets three times the timeout, as test.slow() upstream",
6983
]
7084
junit_family = "xunit2"
7185
asyncio_mode = "auto"

‎test.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ python=python
1414
export PYTHONPATH="scripts${PYTHONPATH:+:$PYTHONPATH}"
1515
# --maxschedchunk=1: a worker gets one test at a time, as soon as it is free, so
1616
# that the slow tests at the end of the collection spread over all workers.
17-
args=(--browser "$browser" -n "$workers" --maxschedchunk=1 --timeout 120 --timeout-method thread -p pytest_line_reporter -rfE --disable-warnings)
17+
args=(--browser "$browser" -n "$workers" --maxschedchunk=1 -p pytest_line_reporter -rfE --disable-warnings)
1818
status=0
1919
xvfb-run -a "$python" -m pytest tests/sync "${args[@]}" "$@" || status=$?
2020
xvfb-run -a "$python" -m pytest tests/async "${args[@]}" "$@" || status=$?

‎tests/async/library/test_browsertype_connect.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,7 @@ async def handle(route: Route) -> None:
12471247
assert await response.json() == {"foo": "bar"}
12481248

12491249

1250+
@pytest.mark.slow
12501251
@pytest.mark.parametrize("kind", KINDS)
12511252
async def test_BT_076_should_upload_large_file(
12521253
kind: str,
@@ -1298,6 +1299,7 @@ def handle_upload(request: TestServerRequest) -> None:
12981299
upload_file.unlink()
12991300

13001301

1302+
@pytest.mark.slow
13011303
@pytest.mark.parametrize("kind", KINDS)
13021304
async def test_BT_077_should_upload_a_folder(
13031305
kind: str,
@@ -1783,6 +1785,7 @@ async def wait_for_navigation() -> Error:
17831785
assert TARGET_CLOSED_ERROR_MESSAGE in (await wait_for_navigation()).message
17841786

17851787

1788+
@pytest.mark.slow
17861789
async def test_BT_091_launch_server_only_should_be_able_to_reconnect_to_a_browser_12_times_without_warnings(
17871790
connect: Callable[..., Awaitable[Browser]],
17881791
start_remote_server: RemoteServerStarter,

‎tests/async/library/test_client_certificates.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,7 @@ def respond(peer: PeerInfo) -> Response:
10991099
server.close()
11001100

11011101

1102+
@pytest.mark.slow
11021103
async def test_CC_040_browser_persistent_context_validate_input(
11031104
launch_persistent: LaunchPersistent,
11041105
) -> None:

0 commit comments

Comments
 (0)