test: use upstream's per-test timeout, and fix two races of the test infrastructure

A test times out after 30 seconds, as upstream, and the fixtures are not counted
(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-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. A test
that times out fails with its stack (the signal method); the thread method killed
the xdist worker. CI and test.sh drop their --timeout flags.

Two races that the full suites on 28 workers 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.

Author: pavelfeldman

.github/workflows/ci.yml

@@ -107,23 +107,23 @@ jobs:
         python -m build --wheel
         python -m playwright install --with-deps ${{ matrix.browser }}
     - name: Common Tests
-      run: pytest tests/common --browser=${{ matrix.browser }} --timeout 90
+      run: pytest tests/common --browser=${{ matrix.browser }}
     - name: Test Reference count
       run: pytest tests/test_reference_count_async.py --browser=${{ matrix.browser }}
     - name: Test Wheel Installation
       run: pytest tests/test_installation.py --browser=${{ matrix.browser }}
     - name: Test Sync API
       if: matrix.os != 'ubuntu-latest'
-      run: pytest tests/sync --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
+      run: pytest tests/sync --browser=${{ matrix.browser }} -n auto
     - name: Test Sync API
       if: matrix.os == 'ubuntu-latest'
-      run: xvfb-run pytest tests/sync --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
+      run: xvfb-run pytest tests/sync --browser=${{ matrix.browser }} -n auto
     - name: Test Async API
       if: matrix.os != 'ubuntu-latest'
-      run: pytest tests/async --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
+      run: pytest tests/async --browser=${{ matrix.browser }} -n auto
     - name: Test Async API
       if: matrix.os == 'ubuntu-latest'
-      run: xvfb-run pytest tests/async --browser=${{ matrix.browser }} --timeout 90 --timeout-method thread -n auto
+      run: xvfb-run pytest tests/async --browser=${{ matrix.browser }} -n auto
 
   test-stable:
     name: Stable
@@ -154,19 +154,19 @@ jobs:
         python -m build --wheel
         python -m playwright install ${{ matrix.browser-channel }} --with-deps
     - name: Common Tests
-      run: pytest tests/common --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90
+      run: pytest tests/common --browser=chromium --browser-channel=${{ matrix.browser-channel }}
     - name: Test Sync API
       if: matrix.os != 'ubuntu-latest'
-      run: pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
+      run: pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
     - name: Test Sync API
       if: matrix.os == 'ubuntu-latest'
-      run: xvfb-run pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
+      run: xvfb-run pytest tests/sync --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
     - name: Test Async API
       if: matrix.os != 'ubuntu-latest'
-      run: pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
+      run: pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
     - name: Test Async API
       if: matrix.os == 'ubuntu-latest'
-      run: xvfb-run pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} --timeout 90 --timeout-method thread -n auto
+      run: xvfb-run pytest tests/async --browser=chromium --browser-channel=${{ matrix.browser-channel }} -n auto
 
   test_examples:
     name: Examples

AGENTS.md

@@ -56,7 +56,7 @@ The tests are a port of the upstream tests in `microsoft/playwright` (`tests/pag
 
 - 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)`.
+- 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)`.
 - 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. A Python-only test goes in `tests/async/python/` or `tests/sync/python/`, in the file named after the upstream file of the feature.

pyproject.toml

@@ -60,12 +60,21 @@ version_file = "playwright/_repo_version.py"
 
 [tool.pytest.ini_options]
 addopts = "-Wall -rsx -vv -s"
+# The timeout of a test is 30 seconds, as upstream; a test marked slow gets three
+# times that, as test.slow() gives upstream (tests/conftest.py). The timeout covers
+# the test function; the fixtures (a browser launch) are not counted. The signal
+# method fails the test with its stack and keeps the worker; the thread method
+# kills the process (on Windows, pytest-timeout falls back to it).
+timeout = 30
+timeout_method = "signal"
+timeout_func_only = true
 markers = [
     "skip_browser",
     "only_browser",
     "skip_platform",
     "only_platform",
-    "skip_protocol_check"
+    "skip_protocol_check",
+    "slow: the test gets three times the timeout, as test.slow() upstream",
 ]
 junit_family = "xunit2"
 asyncio_mode = "auto"

test.sh

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

tests/async/library/test_browsertype_connect.py

@@ -1298,6 +1298,7 @@ def handle_upload(request: TestServerRequest) -> None:
     upload_file.unlink()
 
 
+@pytest.mark.slow
 @pytest.mark.parametrize("kind", KINDS)
 async def test_BT_077_should_upload_a_folder(
     kind: str,
@@ -1783,6 +1784,7 @@ async def wait_for_navigation() -> Error:
     assert TARGET_CLOSED_ERROR_MESSAGE in (await wait_for_navigation()).message
 
 
+@pytest.mark.slow
 async def test_BT_091_launch_server_only_should_be_able_to_reconnect_to_a_browser_12_times_without_warnings(
     connect: Callable[..., Awaitable[Browser]],
     start_remote_server: RemoteServerStarter,

tests/async/library/test_client_certificates.py

@@ -1099,6 +1099,7 @@ def respond(peer: PeerInfo) -> Response:
     server.close()
 
 
+@pytest.mark.slow
 async def test_CC_040_browser_persistent_context_validate_input(
     launch_persistent: LaunchPersistent,
 ) -> None:

tests/async/library/test_screencast.py

@@ -43,6 +43,9 @@
 
 from ..utils import utils
 
+# All the tests of the file are slow upstream (test.slow() at the top of the file).
+pytestmark = pytest.mark.slow
+
 
 async def ensure_some_frames(page: Page) -> None:
     """Port of ensureSomeFrames() in tests/config/utils.ts."""

tests/async/library/test_signals.py

@@ -35,6 +35,9 @@
 from tests.helpers.client_process import ClientProcess
 from tests.server import Server
 
+# All the tests of the file are slow upstream (test.slow() at the top of the file).
+pytestmark = pytest.mark.slow
+
 FLAVOR = "async"
 
 skip_on_windows = pytest.mark.skipif(
@@ -169,9 +172,9 @@ def test_SI_006_signals_should_close_the_browser_on_sighup(
     client.kill(signal.SIGHUP)
     assert client.out("exitCode") == "0"
     assert client.out("signal") == "null"
-    # Node.js handles SIGHUP and exits with 0. Python does not handle SIGHUP, so the
-    # signal ends the process.
-    assert client.child_exit_code() == -signal.SIGHUP
+    # The child ignores SIGHUP and ends when the browser has closed, as the Node.js
+    # child does; see client_process_impl.py.
+    assert client.child_exit_code() == 0
 
 
 @skip_on_windows

tests/async/page/test_page_filechooser.py

@@ -28,6 +28,7 @@
 from ..utils import Utils
 
 
+@pytest.mark.slow
 async def test_FC_001_should_upload_multiple_large_files(
     page: Page, server: Server, tmp_path: Path
 ) -> None:

tests/async/page/test_page_set_input_files.py

@@ -199,6 +199,7 @@ async def test_IF_008_should_upload_a_file_after_popup(
     assert await page.evaluate("e => e.files[0].name", input) == "file-to-upload.txt"
 
 
+@pytest.mark.slow
 async def test_IF_009_should_upload_large_file(
     page: Page, server: Server, tmp_path: Path
 ) -> None:
@@ -239,6 +240,7 @@ async def test_IF_010_should_throw_an_error_if_the_file_does_not_exist(
     assert "i actually do not exist.txt" in str(exc_info.value)
 
 
+@pytest.mark.slow
 async def test_IF_011_should_upload_large_file_with_relative_path(
     page: Page, server: Server, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None: