From 57a0d4be741af03028cbd1f899f404677b1464dd Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 7 Oct 2026 15:51:48 -0700 Subject: [PATCH 1/2] feat: record traces of playwright.request API contexts --tracing only covered browser contexts, so API tests using playwright.request.new_context() produced no trace. With APIRequestContext.tracing public since playwright 1.60, contexts created through playwright.request.new_context() are now traced per test: - tracing starts when the context is created; - each test records its own chunk, so a session-scoped context shared by many tests yields one trace per test, and chunks without any action (a long-lived context the test did not use) are dropped; - the traces go through the same retain-on-failure handling, file naming and report attachments as browser context traces. Tests that use Playwright but no page or context now get the artifacts recorder through a small autouse fixture when tracing is enabled. Fixes #137 --- .../pytest_playwright.py | 127 +++++++++++++++++- .../pytest_playwright/pytest_playwright.py | 123 ++++++++++++++++- tests/test_asyncio.py | 78 +++++++++++ tests/test_sync.py | 72 ++++++++++ 4 files changed, 396 insertions(+), 4 deletions(-) diff --git a/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py b/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py index 20e91a4..b2550c8 100644 --- a/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py +++ b/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py @@ -20,6 +20,7 @@ import os import sys import warnings +import zipfile from pathlib import Path from typing import ( Any, @@ -34,6 +35,7 @@ Optional, Protocol, Sequence, + Set, Tuple, Union, Pattern, @@ -52,6 +54,7 @@ except ImportError: _soft_scope = None from playwright.async_api import ( + APIRequestContext, Browser, BrowserContext, BrowserType, @@ -438,11 +441,19 @@ async def _artifacts_recorder( playwright: Playwright, pytestconfig: Any, _pw_artifacts_folder: tempfile.TemporaryDirectory, + _pw_api_request_contexts: Optional["_APIRequestContexts"], ) -> AsyncGenerator["ArtifactsRecorder", None]: artifacts_recorder = ArtifactsRecorder( - pytestconfig, request, output_path, playwright, _pw_artifacts_folder + pytestconfig, + request, + output_path, + playwright, + _pw_artifacts_folder, + _pw_api_request_contexts, ) + await artifacts_recorder.start_api_request_tracing() yield artifacts_recorder + await artifacts_recorder.stop_api_request_tracing() await artifacts_recorder.collect_videos() if _test_failed_before_teardown(request.node): artifacts_recorder.did_finish_test(failed=True) @@ -452,9 +463,31 @@ async def _artifacts_recorder( setattr(request.node, _ARTIFACTS_RECORDER_ATTR, artifacts_recorder) +@pytest.fixture(autouse=True) +def _pw_trace_api_requests(request: pytest.FixtureRequest) -> None: + # Tests that only use playwright.request (no page or context) don't request + # the artifacts recorder otherwise, and it records API request traces too. + if ( + "playwright" in request.fixturenames + and request.config.getoption("--tracing") != "off" + ): + request.getfixturevalue("_artifacts_recorder") + + +@pytest.fixture(scope="session") +def _pw_api_request_contexts(pytestconfig: Any) -> Optional["_APIRequestContexts"]: + if pytestconfig.getoption("--tracing") == "off": + return None + return _APIRequestContexts() + + @pytest_asyncio.fixture(scope="session") -async def playwright() -> AsyncGenerator[Playwright, None]: +async def playwright( + _pw_api_request_contexts: Optional["_APIRequestContexts"], +) -> AsyncGenerator[Playwright, None]: pw = await async_playwright().start() + if _pw_api_request_contexts: + _pw_api_request_contexts.instrument(pw) yield pw await pw.stop() @@ -811,8 +844,10 @@ def __init__( output_path: str, playwright: Playwright, pw_artifacts_folder: tempfile.TemporaryDirectory, + api_request_contexts: Optional["_APIRequestContexts"] = None, ) -> None: self._request = request + self._api_request_contexts = api_request_contexts self._pytestconfig = pytestconfig self._playwright = playwright self._output_path = output_path @@ -943,6 +978,94 @@ async def on_will_close_browser_context(self, context: BrowserContext) -> None: except Error: pass + async def start_api_request_tracing(self) -> None: + contexts = self._api_request_contexts + if contexts is None: + return + contexts.recorder = self + for context in contexts.live: + if context not in contexts.tracing_chunk: + await context.tracing.start_chunk( + title=slugify(self._request.node.nodeid) + ) + contexts.tracing_chunk.add(context) + + async def stop_api_request_tracing(self) -> None: + contexts = self._api_request_contexts + if contexts is None: + return + contexts.recorder = None + for context in contexts.live: + await self.on_will_dispose_api_request_context(context) + + async def on_will_dispose_api_request_context( + self, context: APIRequestContext + ) -> None: + contexts = self._api_request_contexts + if contexts is None or context not in contexts.tracing_chunk: + return + contexts.tracing_chunk.discard(context) + trace_path = Path(self._pw_artifacts_folder.name) / _create_guid() + try: + await context.tracing.stop_chunk(path=trace_path) + except Error: + return + if not _trace_has_actions(trace_path): + # A long-lived context (e.g. session-scoped) the test did not use. + os.remove(trace_path) + return + self._traces.append(str(trace_path)) + + +class _APIRequestContexts: + # Tracks contexts created via playwright.request.new_context() so that, like + # browser contexts, their traces are recorded per test. + + def __init__(self) -> None: + self.live: List[APIRequestContext] = [] + # Contexts with a trace chunk in progress. + self.tracing_chunk: Set[APIRequestContext] = set() + # Recorder of the running test, if any. + self.recorder: Optional[ArtifactsRecorder] = None + + def instrument(self, playwright: Playwright) -> None: + original_new_context = playwright.request.new_context + + async def _new_context(*args: Any, **kwargs: Any) -> APIRequestContext: + context = await original_new_context(*args, **kwargs) + if getattr(context, "tracing", None) is None: + # APIRequestContext.tracing requires playwright>=1.60. + return context + original_dispose = context.dispose + + async def _dispose_wrapper(*args: Any, **kwargs: Any) -> None: + if context in self.live: + self.live.remove(context) + if self.recorder is not None: + await self.recorder.on_will_dispose_api_request_context(context) + self.tracing_chunk.discard(context) + await original_dispose(*args, **kwargs) + + context.dispose = _dispose_wrapper + await context.tracing.start(screenshots=True, snapshots=True, sources=True) + self.live.append(context) + self.tracing_chunk.add(context) + return context + + playwright.request.new_context = _new_context + + +def _trace_has_actions(trace_path: Path) -> bool: + try: + with zipfile.ZipFile(trace_path) as trace: + return any( + b'"type":"before"' in trace.read(name) + for name in trace.namelist() + if name.endswith(".trace") + ) + except (OSError, zipfile.BadZipFile): + return True + def _create_guid() -> str: return hashlib.sha256(os.urandom(16)).hexdigest() diff --git a/pytest-playwright/pytest_playwright/pytest_playwright.py b/pytest-playwright/pytest_playwright/pytest_playwright.py index 5135eb7..3d8b794 100644 --- a/pytest-playwright/pytest_playwright/pytest_playwright.py +++ b/pytest-playwright/pytest_playwright/pytest_playwright.py @@ -20,6 +20,7 @@ import os import sys import warnings +import zipfile from pathlib import Path from typing import ( Any, @@ -32,6 +33,7 @@ Optional, Protocol, Sequence, + Set, Tuple, Union, Pattern, @@ -50,6 +52,7 @@ except ImportError: _soft_scope = None from playwright.sync_api import ( + APIRequestContext, Browser, BrowserContext, BrowserType, @@ -435,11 +438,19 @@ def _artifacts_recorder( playwright: Playwright, pytestconfig: Any, _pw_artifacts_folder: tempfile.TemporaryDirectory, + _pw_api_request_contexts: Optional["_APIRequestContexts"], ) -> Generator["ArtifactsRecorder", None, None]: artifacts_recorder = ArtifactsRecorder( - pytestconfig, request, output_path, playwright, _pw_artifacts_folder + pytestconfig, + request, + output_path, + playwright, + _pw_artifacts_folder, + _pw_api_request_contexts, ) + artifacts_recorder.start_api_request_tracing() yield artifacts_recorder + artifacts_recorder.stop_api_request_tracing() artifacts_recorder.collect_videos() if _test_failed_before_teardown(request.node): artifacts_recorder.did_finish_test(failed=True) @@ -449,9 +460,31 @@ def _artifacts_recorder( setattr(request.node, _ARTIFACTS_RECORDER_ATTR, artifacts_recorder) +@pytest.fixture(autouse=True) +def _pw_trace_api_requests(request: pytest.FixtureRequest) -> None: + # Tests that only use playwright.request (no page or context) don't request + # the artifacts recorder otherwise, and it records API request traces too. + if ( + "playwright" in request.fixturenames + and request.config.getoption("--tracing") != "off" + ): + request.getfixturevalue("_artifacts_recorder") + + +@pytest.fixture(scope="session") +def _pw_api_request_contexts(pytestconfig: Any) -> Optional["_APIRequestContexts"]: + if pytestconfig.getoption("--tracing") == "off": + return None + return _APIRequestContexts() + + @pytest.fixture(scope="session") -def playwright() -> Generator[Playwright, None, None]: +def playwright( + _pw_api_request_contexts: Optional["_APIRequestContexts"], +) -> Generator[Playwright, None, None]: pw = sync_playwright().start() + if _pw_api_request_contexts: + _pw_api_request_contexts.instrument(pw) yield pw pw.stop() @@ -808,8 +841,10 @@ def __init__( output_path: str, playwright: Playwright, pw_artifacts_folder: tempfile.TemporaryDirectory, + api_request_contexts: Optional["_APIRequestContexts"] = None, ) -> None: self._request = request + self._api_request_contexts = api_request_contexts self._pytestconfig = pytestconfig self._playwright = playwright self._output_path = output_path @@ -940,6 +975,90 @@ def on_will_close_browser_context(self, context: BrowserContext) -> None: except Error: pass + def start_api_request_tracing(self) -> None: + contexts = self._api_request_contexts + if contexts is None: + return + contexts.recorder = self + for context in contexts.live: + if context not in contexts.tracing_chunk: + context.tracing.start_chunk(title=slugify(self._request.node.nodeid)) + contexts.tracing_chunk.add(context) + + def stop_api_request_tracing(self) -> None: + contexts = self._api_request_contexts + if contexts is None: + return + contexts.recorder = None + for context in contexts.live: + self.on_will_dispose_api_request_context(context) + + def on_will_dispose_api_request_context(self, context: APIRequestContext) -> None: + contexts = self._api_request_contexts + if contexts is None or context not in contexts.tracing_chunk: + return + contexts.tracing_chunk.discard(context) + trace_path = Path(self._pw_artifacts_folder.name) / _create_guid() + try: + context.tracing.stop_chunk(path=trace_path) + except Error: + return + if not _trace_has_actions(trace_path): + # A long-lived context (e.g. session-scoped) the test did not use. + os.remove(trace_path) + return + self._traces.append(str(trace_path)) + + +class _APIRequestContexts: + # Tracks contexts created via playwright.request.new_context() so that, like + # browser contexts, their traces are recorded per test. + + def __init__(self) -> None: + self.live: List[APIRequestContext] = [] + # Contexts with a trace chunk in progress. + self.tracing_chunk: Set[APIRequestContext] = set() + # Recorder of the running test, if any. + self.recorder: Optional[ArtifactsRecorder] = None + + def instrument(self, playwright: Playwright) -> None: + original_new_context = playwright.request.new_context + + def _new_context(*args: Any, **kwargs: Any) -> APIRequestContext: + context = original_new_context(*args, **kwargs) + if getattr(context, "tracing", None) is None: + # APIRequestContext.tracing requires playwright>=1.60. + return context + original_dispose = context.dispose + + def _dispose_wrapper(*args: Any, **kwargs: Any) -> None: + if context in self.live: + self.live.remove(context) + if self.recorder is not None: + self.recorder.on_will_dispose_api_request_context(context) + self.tracing_chunk.discard(context) + original_dispose(*args, **kwargs) + + context.dispose = _dispose_wrapper + context.tracing.start(screenshots=True, snapshots=True, sources=True) + self.live.append(context) + self.tracing_chunk.add(context) + return context + + playwright.request.new_context = _new_context + + +def _trace_has_actions(trace_path: Path) -> bool: + try: + with zipfile.ZipFile(trace_path) as trace: + return any( + b'"type":"before"' in trace.read(name) + for name in trace.namelist() + if name.endswith(".trace") + ) + except (OSError, zipfile.BadZipFile): + return True + def _create_guid() -> str: return hashlib.sha256(os.urandom(16)).hexdigest() diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index 354ecdf..08bf20d 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -19,6 +19,7 @@ import sys from typing import Optional from xml.etree import ElementTree +import zipfile import pytest @@ -1246,6 +1247,52 @@ async def test_failing(page, new_context): assert "data:video/webm;base64," in report +def test_traces_api_request_contexts( + testdir: pytest.Testdir, test_server: HTTPTestServer +) -> None: + # https://github.com/microsoft/playwright-pytest/issues/137 + testdir.makeconftest( + f""" + import pytest_asyncio + + @pytest_asyncio.fixture(scope="session") + async def api_request_context(playwright): + context = await playwright.request.new_context(base_url="{test_server.PREFIX}") + yield context + await context.dispose() + + @pytest_asyncio.fixture + async def function_api_request_context(playwright): + context = await playwright.request.new_context(base_url="{test_server.PREFIX}") + yield context + await context.dispose() + """ + ) + testdir.makepyfile( + """ + import pytest + + @pytest.mark.asyncio + async def test_session_context_1(api_request_context): + await api_request_context.get("/one") + + @pytest.mark.asyncio + async def test_session_context_2(api_request_context): + await api_request_context.get("/two") + + @pytest.mark.asyncio + async def test_function_context(function_api_request_context): + await function_api_request_context.get("/three") + + @pytest.mark.asyncio + async def test_failing(api_request_context): + await api_request_context.get("/four") + raise Exception("Failed") + """ + ) + _assert_api_request_traces(testdir) + + def test_should_work_with_test_names_which_exceeds_256_characters( testdir: pytest.Testdir, ) -> None: @@ -1295,6 +1342,37 @@ def _assert_folder_structure(root: str, expected: str) -> None: raise AssertionError("Actual tree does not match expected tree") +def _assert_api_request_traces(testdir: pytest.Testdir) -> None: + __tracebackhide__ = True + test_results_dir = os.path.join(testdir.tmpdir, "test-results") + prefix = os.path.join(test_results_dir, "test-traces-api-request-contexts-py-test-") + + result = testdir.runpytest("--tracing", "on") + result.assert_outcomes(passed=3, failed=1) + # Each test gets a trace with only its own requests, including tests that + # share a session-scoped context. + assert _trace_request_urls(prefix + "session-context-1/trace.zip") == ["/one"] + assert _trace_request_urls(prefix + "session-context-2/trace.zip") == ["/two"] + assert _trace_request_urls(prefix + "function-context/trace.zip") == ["/three"] + assert _trace_request_urls(prefix + "failing/trace.zip") == ["/four"] + + result = testdir.runpytest("--tracing", "retain-on-failure") + result.assert_outcomes(passed=3, failed=1) + _assert_folder_structure( + test_results_dir, + """ +- test-traces-api-request-contexts-py-test-failing: + - trace.zip +""", + ) + + +def _trace_request_urls(trace_path: str) -> list: + with zipfile.ZipFile(trace_path) as trace: + events = [json.loads(line) for line in trace.read("trace.trace").splitlines()] + return [event["params"]["url"] for event in events if event["type"] == "before"] + + def test_is_able_to_set_expect_timeout_via_conftest(testdir: pytest.Testdir) -> None: testdir.makeconftest( """ diff --git a/tests/test_sync.py b/tests/test_sync.py index 6ca84c5..7f4c854 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -20,6 +20,7 @@ import sys from typing import Optional from xml.etree import ElementTree +import zipfile import pytest @@ -1214,6 +1215,46 @@ def test_failing(page, new_context): assert "data:video/webm;base64," in report +def test_traces_api_request_contexts( + testdir: pytest.Testdir, test_server: HTTPTestServer +) -> None: + # https://github.com/microsoft/playwright-pytest/issues/137 + testdir.makeconftest( + f""" + import pytest + + @pytest.fixture(scope="session") + def api_request_context(playwright): + context = playwright.request.new_context(base_url="{test_server.PREFIX}") + yield context + context.dispose() + + @pytest.fixture + def function_api_request_context(playwright): + context = playwright.request.new_context(base_url="{test_server.PREFIX}") + yield context + context.dispose() + """ + ) + testdir.makepyfile( + """ + def test_session_context_1(api_request_context): + api_request_context.get("/one") + + def test_session_context_2(api_request_context): + api_request_context.get("/two") + + def test_function_context(function_api_request_context): + function_api_request_context.get("/three") + + def test_failing(api_request_context): + api_request_context.get("/four") + raise Exception("Failed") + """ + ) + _assert_api_request_traces(testdir) + + def test_should_work_with_test_names_which_exceeds_256_characters( testdir: pytest.Testdir, ) -> None: @@ -1261,6 +1302,37 @@ def _assert_folder_structure(root: str, expected: str) -> None: raise AssertionError("Actual tree does not match expected tree") +def _assert_api_request_traces(testdir: pytest.Testdir) -> None: + __tracebackhide__ = True + test_results_dir = os.path.join(testdir.tmpdir, "test-results") + prefix = os.path.join(test_results_dir, "test-traces-api-request-contexts-py-test-") + + result = testdir.runpytest("--tracing", "on") + result.assert_outcomes(passed=3, failed=1) + # Each test gets a trace with only its own requests, including tests that + # share a session-scoped context. + assert _trace_request_urls(prefix + "session-context-1/trace.zip") == ["/one"] + assert _trace_request_urls(prefix + "session-context-2/trace.zip") == ["/two"] + assert _trace_request_urls(prefix + "function-context/trace.zip") == ["/three"] + assert _trace_request_urls(prefix + "failing/trace.zip") == ["/four"] + + result = testdir.runpytest("--tracing", "retain-on-failure") + result.assert_outcomes(passed=3, failed=1) + _assert_folder_structure( + test_results_dir, + """ +- test-traces-api-request-contexts-py-test-failing: + - trace.zip +""", + ) + + +def _trace_request_urls(trace_path: str) -> list: + with zipfile.ZipFile(trace_path) as trace: + events = [json.loads(line) for line in trace.read("trace.trace").splitlines()] + return [event["params"]["url"] for event in events if event["type"] == "before"] + + def test_is_able_to_set_expect_timeout_via_conftest(testdir: pytest.Testdir) -> None: testdir.makeconftest( """ From 1e0724e55d92472f915671de84fc8e530bab4134 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 7 Oct 2026 18:17:50 -0700 Subject: [PATCH 2/2] review: title the first trace chunk of API contexts, only drop empty traces of pre-existing contexts, let stop_chunk errors surface, stream the trace check; test mixed page+API traces and --tracing off --- pytest-playwright-asyncio/pyproject.toml | 2 +- .../pytest_playwright.py | 67 +++++++++++---- pytest-playwright/pyproject.toml | 2 +- .../pytest_playwright/pytest_playwright.py | 67 +++++++++++---- tests/test_asyncio.py | 85 ++++++++++++++++--- tests/test_sync.py | 82 +++++++++++++++--- 6 files changed, 247 insertions(+), 58 deletions(-) diff --git a/pytest-playwright-asyncio/pyproject.toml b/pytest-playwright-asyncio/pyproject.toml index 38f559d..82363dd 100644 --- a/pytest-playwright-asyncio/pyproject.toml +++ b/pytest-playwright-asyncio/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ dynamic = ["version"] dependencies = [ "playwright>=1.18", - "pytest>=6.2.4,<10.0.0", + "pytest>=8.2.0,<10.0.0", "python-slugify>=6.0.0,<10.0.0", "pytest-asyncio>=0.24.0", ] diff --git a/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py b/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py index b2550c8..f1ebce0 100644 --- a/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py +++ b/pytest-playwright-asyncio/pytest_playwright_asyncio/pytest_playwright.py @@ -186,6 +186,8 @@ def pytest_configure(config: Any) -> None: # Set on a test item by _artifacts_recorder, picked up for the teardown report. _ARTIFACTS_RECORDER_ATTR = "_playwright_artifacts_recorder" +# Node id of the test being set up, run or torn down, see _APIRequestContexts. +_CURRENT_TEST_NODEID_KEY = pytest.StashKey[str]() # Making test result information available in fixtures @@ -324,6 +326,7 @@ def _get_skiplist(item: Any, values: List[str], value_name: str) -> List[str]: def pytest_runtest_setup(item: Any) -> None: + item.config.stash[_CURRENT_TEST_NODEID_KEY] = item.nodeid if not hasattr(item, "callspec"): return browser_name = item.callspec.params.get("browser_name") @@ -478,7 +481,7 @@ def _pw_trace_api_requests(request: pytest.FixtureRequest) -> None: def _pw_api_request_contexts(pytestconfig: Any) -> Optional["_APIRequestContexts"]: if pytestconfig.getoption("--tracing") == "off": return None - return _APIRequestContexts() + return _APIRequestContexts(pytestconfig) @pytest_asyncio.fixture(scope="session") @@ -856,6 +859,8 @@ def __init__( self._all_pages: List[Page] = [] self._screenshots: List[str] = [] self._traces: List[str] = [] + # API request contexts that existed before this test, see start_api_request_tracing. + self._shared_api_request_contexts: Set[APIRequestContext] = set() # (temporary path, file name in the output folder) self._videos: List[Tuple[str, str]] = [] self.finished = False @@ -983,6 +988,10 @@ async def start_api_request_tracing(self) -> None: if contexts is None: return contexts.recorder = self + # Contexts from before this test, e.g. session-scoped ones: their trace is + # only kept when the test used them. Contexts the test creates itself keep + # their trace, like browser contexts do. + self._shared_api_request_contexts = set(contexts.live) for context in contexts.live: if context not in contexts.tracing_chunk: await context.tracing.start_chunk( @@ -1004,14 +1013,15 @@ async def on_will_dispose_api_request_context( contexts = self._api_request_contexts if contexts is None or context not in contexts.tracing_chunk: return + # Forgotten first: should stop_chunk fail (the error is reported like one + # from a browser context), the next start_chunk discards what is still + # being recorded. contexts.tracing_chunk.discard(context) trace_path = Path(self._pw_artifacts_folder.name) / _create_guid() - try: - await context.tracing.stop_chunk(path=trace_path) - except Error: - return - if not _trace_has_actions(trace_path): - # A long-lived context (e.g. session-scoped) the test did not use. + await context.tracing.stop_chunk(path=trace_path) + if context in self._shared_api_request_contexts and not _trace_has_actions( + trace_path + ): os.remove(trace_path) return self._traces.append(str(trace_path)) @@ -1021,7 +1031,8 @@ class _APIRequestContexts: # Tracks contexts created via playwright.request.new_context() so that, like # browser contexts, their traces are recorded per test. - def __init__(self) -> None: + def __init__(self, config: Any) -> None: + self._config = config self.live: List[APIRequestContext] = [] # Contexts with a trace chunk in progress. self.tracing_chunk: Set[APIRequestContext] = set() @@ -1029,6 +1040,8 @@ def __init__(self) -> None: self.recorder: Optional[ArtifactsRecorder] = None def instrument(self, playwright: Playwright) -> None: + # playwright.request is one APIRequest instance for the lifetime of the + # Playwright object, so wrapping its method here covers all contexts. original_new_context = playwright.request.new_context async def _new_context(*args: Any, **kwargs: Any) -> APIRequestContext: @@ -1041,13 +1054,25 @@ async def _new_context(*args: Any, **kwargs: Any) -> APIRequestContext: async def _dispose_wrapper(*args: Any, **kwargs: Any) -> None: if context in self.live: self.live.remove(context) - if self.recorder is not None: - await self.recorder.on_will_dispose_api_request_context(context) - self.tracing_chunk.discard(context) - await original_dispose(*args, **kwargs) + try: + if self.recorder is not None: + await self.recorder.on_will_dispose_api_request_context(context) + self.tracing_chunk.discard(context) + finally: + # Dispose even if saving the trace failed; that error still + # propagates. + await original_dispose(*args, **kwargs) context.dispose = _dispose_wrapper - await context.tracing.start(screenshots=True, snapshots=True, sources=True) + # Contexts are usually created by fixtures, before the artifacts + # recorder of the test exists, so the title comes from the hook. + nodeid = self._config.stash.get(_CURRENT_TEST_NODEID_KEY, None) + await context.tracing.start( + title=slugify(nodeid) if nodeid else None, + screenshots=True, + snapshots=True, + sources=True, + ) self.live.append(context) self.tracing_chunk.add(context) return context @@ -1056,13 +1081,19 @@ async def _dispose_wrapper(*args: Any, **kwargs: Any) -> None: def _trace_has_actions(trace_path: Path) -> bool: + # Looks for an action ("before" event) in the trace. The format (one JSON + # event per line in *.trace) is Playwright's own and may change, in which + # case, or on any error, the trace is kept. try: with zipfile.ZipFile(trace_path) as trace: - return any( - b'"type":"before"' in trace.read(name) - for name in trace.namelist() - if name.endswith(".trace") - ) + for name in trace.namelist(): + if not name.endswith(".trace"): + continue + with trace.open(name) as events: + for event in events: + if b'"type":"before"' in event: + return True + return False except (OSError, zipfile.BadZipFile): return True diff --git a/pytest-playwright/pyproject.toml b/pytest-playwright/pyproject.toml index cf78300..8c560b4 100644 --- a/pytest-playwright/pyproject.toml +++ b/pytest-playwright/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ dynamic = ["version"] dependencies = [ "playwright>=1.18", - "pytest>=6.2.4,<10.0.0", + "pytest>=7.0.0,<10.0.0", "python-slugify>=6.0.0,<10.0.0", ] diff --git a/pytest-playwright/pytest_playwright/pytest_playwright.py b/pytest-playwright/pytest_playwright/pytest_playwright.py index 3d8b794..fce034a 100644 --- a/pytest-playwright/pytest_playwright/pytest_playwright.py +++ b/pytest-playwright/pytest_playwright/pytest_playwright.py @@ -183,6 +183,8 @@ def pytest_configure(config: Any) -> None: # Set on a test item by _artifacts_recorder, picked up for the teardown report. _ARTIFACTS_RECORDER_ATTR = "_playwright_artifacts_recorder" +# Node id of the test being set up, run or torn down, see _APIRequestContexts. +_CURRENT_TEST_NODEID_KEY = pytest.StashKey[str]() # Making test result information available in fixtures @@ -321,6 +323,7 @@ def _get_skiplist(item: Any, values: List[str], value_name: str) -> List[str]: def pytest_runtest_setup(item: Any) -> None: + item.config.stash[_CURRENT_TEST_NODEID_KEY] = item.nodeid if not hasattr(item, "callspec"): return browser_name = item.callspec.params.get("browser_name") @@ -475,7 +478,7 @@ def _pw_trace_api_requests(request: pytest.FixtureRequest) -> None: def _pw_api_request_contexts(pytestconfig: Any) -> Optional["_APIRequestContexts"]: if pytestconfig.getoption("--tracing") == "off": return None - return _APIRequestContexts() + return _APIRequestContexts(pytestconfig) @pytest.fixture(scope="session") @@ -853,6 +856,8 @@ def __init__( self._all_pages: List[Page] = [] self._screenshots: List[str] = [] self._traces: List[str] = [] + # API request contexts that existed before this test, see start_api_request_tracing. + self._shared_api_request_contexts: Set[APIRequestContext] = set() # (temporary path, file name in the output folder) self._videos: List[Tuple[str, str]] = [] self.finished = False @@ -980,6 +985,10 @@ def start_api_request_tracing(self) -> None: if contexts is None: return contexts.recorder = self + # Contexts from before this test, e.g. session-scoped ones: their trace is + # only kept when the test used them. Contexts the test creates itself keep + # their trace, like browser contexts do. + self._shared_api_request_contexts = set(contexts.live) for context in contexts.live: if context not in contexts.tracing_chunk: context.tracing.start_chunk(title=slugify(self._request.node.nodeid)) @@ -997,14 +1006,15 @@ def on_will_dispose_api_request_context(self, context: APIRequestContext) -> Non contexts = self._api_request_contexts if contexts is None or context not in contexts.tracing_chunk: return + # Forgotten first: should stop_chunk fail (the error is reported like one + # from a browser context), the next start_chunk discards what is still + # being recorded. contexts.tracing_chunk.discard(context) trace_path = Path(self._pw_artifacts_folder.name) / _create_guid() - try: - context.tracing.stop_chunk(path=trace_path) - except Error: - return - if not _trace_has_actions(trace_path): - # A long-lived context (e.g. session-scoped) the test did not use. + context.tracing.stop_chunk(path=trace_path) + if context in self._shared_api_request_contexts and not _trace_has_actions( + trace_path + ): os.remove(trace_path) return self._traces.append(str(trace_path)) @@ -1014,7 +1024,8 @@ class _APIRequestContexts: # Tracks contexts created via playwright.request.new_context() so that, like # browser contexts, their traces are recorded per test. - def __init__(self) -> None: + def __init__(self, config: Any) -> None: + self._config = config self.live: List[APIRequestContext] = [] # Contexts with a trace chunk in progress. self.tracing_chunk: Set[APIRequestContext] = set() @@ -1022,6 +1033,8 @@ def __init__(self) -> None: self.recorder: Optional[ArtifactsRecorder] = None def instrument(self, playwright: Playwright) -> None: + # playwright.request is one APIRequest instance for the lifetime of the + # Playwright object, so wrapping its method here covers all contexts. original_new_context = playwright.request.new_context def _new_context(*args: Any, **kwargs: Any) -> APIRequestContext: @@ -1034,13 +1047,25 @@ def _new_context(*args: Any, **kwargs: Any) -> APIRequestContext: def _dispose_wrapper(*args: Any, **kwargs: Any) -> None: if context in self.live: self.live.remove(context) - if self.recorder is not None: - self.recorder.on_will_dispose_api_request_context(context) - self.tracing_chunk.discard(context) - original_dispose(*args, **kwargs) + try: + if self.recorder is not None: + self.recorder.on_will_dispose_api_request_context(context) + self.tracing_chunk.discard(context) + finally: + # Dispose even if saving the trace failed; that error still + # propagates. + original_dispose(*args, **kwargs) context.dispose = _dispose_wrapper - context.tracing.start(screenshots=True, snapshots=True, sources=True) + # Contexts are usually created by fixtures, before the artifacts + # recorder of the test exists, so the title comes from the hook. + nodeid = self._config.stash.get(_CURRENT_TEST_NODEID_KEY, None) + context.tracing.start( + title=slugify(nodeid) if nodeid else None, + screenshots=True, + snapshots=True, + sources=True, + ) self.live.append(context) self.tracing_chunk.add(context) return context @@ -1049,13 +1074,19 @@ def _dispose_wrapper(*args: Any, **kwargs: Any) -> None: def _trace_has_actions(trace_path: Path) -> bool: + # Looks for an action ("before" event) in the trace. The format (one JSON + # event per line in *.trace) is Playwright's own and may change, in which + # case, or on any error, the trace is kept. try: with zipfile.ZipFile(trace_path) as trace: - return any( - b'"type":"before"' in trace.read(name) - for name in trace.namelist() - if name.endswith(".trace") - ) + for name in trace.namelist(): + if not name.endswith(".trace"): + continue + with trace.open(name) as events: + for event in events: + if b'"type":"before"' in event: + return True + return False except (OSError, zipfile.BadZipFile): return True diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index 08bf20d..d34e3ea 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -13,6 +13,7 @@ # limitations under the License. import importlib.util +import json import os import signal import subprocess @@ -1284,6 +1285,20 @@ async def test_session_context_2(api_request_context): async def test_function_context(function_api_request_context): await function_api_request_context.get("/three") + @pytest.mark.asyncio + async def test_unused_function_context(function_api_request_context): + pass + + @pytest.mark.asyncio + async def test_page_and_api_context(page, api_request_context): + await page.set_content("
hello
") + await api_request_context.get("/five") + + @pytest.mark.asyncio + async def test_instrumented_only_with_tracing(playwright, pytestconfig): + instrumented = playwright.request.new_context.__name__ != "new_context" + assert instrumented == (pytestconfig.getoption("--tracing") != "off") + @pytest.mark.asyncio async def test_failing(api_request_context): await api_request_context.get("/four") @@ -1348,16 +1363,47 @@ def _assert_api_request_traces(testdir: pytest.Testdir) -> None: prefix = os.path.join(test_results_dir, "test-traces-api-request-contexts-py-test-") result = testdir.runpytest("--tracing", "on") - result.assert_outcomes(passed=3, failed=1) + result.assert_outcomes(passed=6, failed=1) + _assert_folder_structure( + test_results_dir, + """ +- test-traces-api-request-contexts-py-test-failing: + - trace.zip +- test-traces-api-request-contexts-py-test-function-context: + - trace.zip +- test-traces-api-request-contexts-py-test-page-and-api-context-chromium: + - trace-1.zip + - trace-2.zip +- test-traces-api-request-contexts-py-test-session-context-1: + - trace.zip +- test-traces-api-request-contexts-py-test-session-context-2: + - trace.zip +- test-traces-api-request-contexts-py-test-unused-function-context: + - trace.zip +""", + ) # Each test gets a trace with only its own requests, including tests that - # share a session-scoped context. - assert _trace_request_urls(prefix + "session-context-1/trace.zip") == ["/one"] - assert _trace_request_urls(prefix + "session-context-2/trace.zip") == ["/two"] - assert _trace_request_urls(prefix + "function-context/trace.zip") == ["/three"] - assert _trace_request_urls(prefix + "failing/trace.zip") == ["/four"] + # share a session-scoped context, and every trace is titled after its test. + for name, urls in [ + ("session-context-1", ["/one"]), + ("session-context-2", ["/two"]), + ("function-context", ["/three"]), + ("failing", ["/four"]), + # A context the test created but did not use keeps its trace. + ("unused-function-context", []), + ]: + assert _trace_request_urls(prefix + name + "/trace.zip") == urls + assert _trace_title(prefix + name + "/trace.zip") == os.path.basename( + prefix + name + ) + # Browser trace first, then the API trace. + folder = prefix + "page-and-api-context-chromium/" + assert _trace_request_urls(folder + "trace-2.zip") == ["/five"] + assert _trace_title(folder + "trace-1.zip") == _trace_title(folder + "trace-2.zip") + assert _trace_title(folder + "trace-1.zip") == os.path.basename(folder[:-1]) result = testdir.runpytest("--tracing", "retain-on-failure") - result.assert_outcomes(passed=3, failed=1) + result.assert_outcomes(passed=6, failed=1) _assert_folder_structure( test_results_dir, """ @@ -1366,11 +1412,30 @@ def _assert_api_request_traces(testdir: pytest.Testdir) -> None: """, ) + result = testdir.runpytest() + result.assert_outcomes(passed=6, failed=1) + assert not os.path.exists(test_results_dir) -def _trace_request_urls(trace_path: str) -> list: + +def _trace_events(trace_path: str) -> list: with zipfile.ZipFile(trace_path) as trace: - events = [json.loads(line) for line in trace.read("trace.trace").splitlines()] - return [event["params"]["url"] for event in events if event["type"] == "before"] + return [json.loads(line) for line in trace.read("trace.trace").splitlines()] + + +def _trace_request_urls(trace_path: str) -> list: + return [ + event["params"]["url"] + for event in _trace_events(trace_path) + if event["type"] == "before" + ] + + +def _trace_title(trace_path: str) -> str: + return next( + event["title"] + for event in _trace_events(trace_path) + if event["type"] == "context-options" + ) def test_is_able_to_set_expect_timeout_via_conftest(testdir: pytest.Testdir) -> None: diff --git a/tests/test_sync.py b/tests/test_sync.py index 7f4c854..9528fc5 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -13,6 +13,7 @@ # limitations under the License. import importlib.util +import json import os from pathlib import Path import signal @@ -1247,6 +1248,17 @@ def test_session_context_2(api_request_context): def test_function_context(function_api_request_context): function_api_request_context.get("/three") + def test_unused_function_context(function_api_request_context): + pass + + def test_page_and_api_context(page, api_request_context): + page.set_content("
hello
") + api_request_context.get("/five") + + def test_instrumented_only_with_tracing(playwright, pytestconfig): + instrumented = playwright.request.new_context.__name__ != "new_context" + assert instrumented == (pytestconfig.getoption("--tracing") != "off") + def test_failing(api_request_context): api_request_context.get("/four") raise Exception("Failed") @@ -1308,16 +1320,47 @@ def _assert_api_request_traces(testdir: pytest.Testdir) -> None: prefix = os.path.join(test_results_dir, "test-traces-api-request-contexts-py-test-") result = testdir.runpytest("--tracing", "on") - result.assert_outcomes(passed=3, failed=1) + result.assert_outcomes(passed=6, failed=1) + _assert_folder_structure( + test_results_dir, + """ +- test-traces-api-request-contexts-py-test-failing: + - trace.zip +- test-traces-api-request-contexts-py-test-function-context: + - trace.zip +- test-traces-api-request-contexts-py-test-page-and-api-context-chromium: + - trace-1.zip + - trace-2.zip +- test-traces-api-request-contexts-py-test-session-context-1: + - trace.zip +- test-traces-api-request-contexts-py-test-session-context-2: + - trace.zip +- test-traces-api-request-contexts-py-test-unused-function-context: + - trace.zip +""", + ) # Each test gets a trace with only its own requests, including tests that - # share a session-scoped context. - assert _trace_request_urls(prefix + "session-context-1/trace.zip") == ["/one"] - assert _trace_request_urls(prefix + "session-context-2/trace.zip") == ["/two"] - assert _trace_request_urls(prefix + "function-context/trace.zip") == ["/three"] - assert _trace_request_urls(prefix + "failing/trace.zip") == ["/four"] + # share a session-scoped context, and every trace is titled after its test. + for name, urls in [ + ("session-context-1", ["/one"]), + ("session-context-2", ["/two"]), + ("function-context", ["/three"]), + ("failing", ["/four"]), + # A context the test created but did not use keeps its trace. + ("unused-function-context", []), + ]: + assert _trace_request_urls(prefix + name + "/trace.zip") == urls + assert _trace_title(prefix + name + "/trace.zip") == os.path.basename( + prefix + name + ) + # Browser trace first, then the API trace. + folder = prefix + "page-and-api-context-chromium/" + assert _trace_request_urls(folder + "trace-2.zip") == ["/five"] + assert _trace_title(folder + "trace-1.zip") == _trace_title(folder + "trace-2.zip") + assert _trace_title(folder + "trace-1.zip") == os.path.basename(folder[:-1]) result = testdir.runpytest("--tracing", "retain-on-failure") - result.assert_outcomes(passed=3, failed=1) + result.assert_outcomes(passed=6, failed=1) _assert_folder_structure( test_results_dir, """ @@ -1326,11 +1369,30 @@ def _assert_api_request_traces(testdir: pytest.Testdir) -> None: """, ) + result = testdir.runpytest() + result.assert_outcomes(passed=6, failed=1) + assert not os.path.exists(test_results_dir) -def _trace_request_urls(trace_path: str) -> list: + +def _trace_events(trace_path: str) -> list: with zipfile.ZipFile(trace_path) as trace: - events = [json.loads(line) for line in trace.read("trace.trace").splitlines()] - return [event["params"]["url"] for event in events if event["type"] == "before"] + return [json.loads(line) for line in trace.read("trace.trace").splitlines()] + + +def _trace_request_urls(trace_path: str) -> list: + return [ + event["params"]["url"] + for event in _trace_events(trace_path) + if event["type"] == "before" + ] + + +def _trace_title(trace_path: str) -> str: + return next( + event["title"] + for event in _trace_events(trace_path) + if event["type"] == "context-options" + ) def test_is_able_to_set_expect_timeout_via_conftest(testdir: pytest.Testdir) -> None: