Skip to content

Commit c4ee123

Browse files
authored
test: use upstream's per-test timeout, and fix what the full matrix showed (#3281)
1 parent 36d698e commit c4ee123

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)