Skip to main content

all: use contextlib.closing to ensure resources are closed properly

ID
087b9be
date
2026-11-01 14:33:27+00:00
author
Alex Chan <alex@alexwlchan.net>
parent
b26fde7
message
all: use contextlib.closing to ensure resources are closed properly
changed files
2 files, 73 additions, 97 deletions

Changed files

python/chives/browser_fixtures.py (13225 → 13003)

diff --git a/python/chives/browser_fixtures.py b/python/chives/browser_fixtures.py
index a39ff78..925ec42 100644
--- a/python/chives/browser_fixtures.py
+++ b/python/chives/browser_fixtures.py
@@ -45,6 +45,7 @@ or warnings, and it doesn't make any requests for external resources.
 """
 
 from collections.abc import Iterator
+from contextlib import closing
 import os
 from pathlib import Path
 import urllib.parse
@@ -97,10 +98,8 @@ def browser() -> Iterator[Browser]:
     if os.environ.get("SKIP_PLAYWRIGHT", "").lower() in {"1", "true", "yes"}:
         pytest.skip(reason="skip slow Playwright tests")
 
-    with sync_playwright() as p:
-        browser = p.webkit.launch()
+    with sync_playwright() as p, closing(p.webkit.launch()) as browser:
         yield browser
-        browser.close()
 
 
 @pytest.fixture(scope="function")
@@ -112,31 +111,27 @@ def page(browser: Browser) -> Iterator[Page]:
     that no console warnings, console errors, or unhandled exceptions
     occurred during the page session.
     """
-    p = browser.new_page()
+    with closing(browser.new_page()) as p:
+        # Capture anything that gets logged to the console.
+        console_messages = []
+        p.on("console", lambda msg: console_messages.append(msg))
 
-    # Capture anything that gets logged to the console.
-    console_messages = []
-    p.on("console", lambda msg: console_messages.append(msg))
+        # Capture any page errors
+        page_errors = []
+        p.on("pageerror", lambda err: page_errors.append(err))
 
-    # Capture any page errors
-    page_errors = []
-    p.on("pageerror", lambda err: page_errors.append(err))
+        yield p
 
-    yield p
+        # Check there weren't any console errors logged to the page.
+        console_errors = [
+            msg.text
+            for msg in console_messages
+            if msg.type == "error" or msg.type == "warning"
+        ]
+        assert console_errors == [], console_errors
 
-    # Check there weren't any console errors logged to the page.
-    console_errors = [
-        msg.text
-        for msg in console_messages
-        if msg.type == "error" or msg.type == "warning"
-    ]
-    assert console_errors == [], console_errors
-
-    # Check there weren't any page errors
-    assert page_errors == [], page_errors
-
-    # Close the page
-    p.close()
+        # Check there weren't any page errors
+        assert page_errors == [], page_errors
 
 
 class PageError(Exception):
@@ -160,72 +155,55 @@ def assert_page_is_self_contained(ctx: BrowserContext | Browser, url: str) -> No
     Note: This only covers the initial page load and will not catch dynamic
     requests triggered later by user interaction or JavaScript.
     """
-    page = None
-    try:
-        page = ctx.new_page()
-        _assert_page_is_self_contained(page, url)
-    finally:
-        if page is not None:
-            page.close()
-        else:  # pragma: no cover
-            pass
-
-
-def _assert_page_is_self_contained(page: Page, url: str) -> None:
-    """
-    Validate page integrity on an existing Playwright page.
-
-    See `assert_local_page_integrity` for details on checks performed and
-    exceptions raised.
-    """
-    # Capture all console logs, page errors, and requests.
-    console_messages = []
-    page.on("console", lambda msg: console_messages.append(msg))
-
-    page_errors = []
-    page.on("pageerror", lambda err: page_errors.append(err))
-
-    requests = []
-    page.on("request", lambda req: requests.append(req))
-
-    page.goto(url, wait_until="networkidle")
-
-    # 2. Force a layout/paint to trigger CSS asset downloads (like
-    # background images).
-    if not _is_svg(url):
-        page.evaluate("document.body.offsetHeight")
-
-    # Wait for the fonts to load, so we get any errors about missing fonts.
-    # Otherwise we can miss the error because we return too quickly.
-    # See https://developer.mozilla.org/en-US/docs/Web/API/Document/fonts#doing_operation_after_fonts_are_loaded
-    page.evaluate("document.fonts.ready")
-
-    # Give the browser a small buffer tick to register the requests
-    page.wait_for_timeout(500)
-
-    # Check there weren't any page errors
-    if page_errors:
-        raise PageError(f"Unexpected page errors: {page_errors}")
-
-    # Check the console logs for errors and warnings.
-    console_errors = [
-        msg for msg in console_messages if _is_important_message(url, requests, msg)
-    ]
-
-    if console_errors:
-        raise PageError(f"Unexpected console warnings/errors: {console_errors}")
-
-    # Check there weren't any non-GET requests
-    non_get_requests = [r for r in requests if r.method != "GET"]
-    if non_get_requests:  # pragma: no cover
-        raise PageError(f"Page made non-GET requests: {non_get_requests}")
-
-    # Check there weren't any HTTP requests to external resources
-    external_requests = [r for r in requests if _is_external_request(r, url)]
-    if external_requests:
-        raise PageError(
-            f"Page made requests for external resources: {external_requests}"
-        )
+    with closing(ctx.new_page()) as page:
+        # Capture all console logs, page errors, and requests.
+        console_messages = []
+        page.on("console", lambda msg: console_messages.append(msg))
+
+        page_errors = []
+        page.on("pageerror", lambda err: page_errors.append(err))
+
+        requests = []
+        page.on("request", lambda req: requests.append(req))
+
+        page.goto(url, wait_until="networkidle")
+
+        # 2. Force a layout/paint to trigger CSS asset downloads (like
+        # background images).
+        if not _is_svg(url):
+            page.evaluate("document.body.offsetHeight")
+
+        # Wait for the fonts to load, so we get any errors about missing fonts.
+        # Otherwise we can miss the error because we return too quickly.
+        # See https://developer.mozilla.org/en-US/docs/Web/API/Document/fonts#doing_operation_after_fonts_are_loaded
+        page.evaluate("document.fonts.ready")
+
+        # Give the browser a small buffer tick to register the requests
+        page.wait_for_timeout(500)
+
+        # Check there weren't any page errors
+        if page_errors:
+            raise PageError(f"Unexpected page errors: {page_errors}")
+
+        # Check the console logs for errors and warnings.
+        console_errors = [
+            msg for msg in console_messages if _is_important_message(url, requests, msg)
+        ]
+
+        if console_errors:
+            raise PageError(f"Unexpected console warnings/errors: {console_errors}")
+
+        # Check there weren't any non-GET requests
+        non_get_requests = [r for r in requests if r.method != "GET"]
+        if non_get_requests:  # pragma: no cover
+            raise PageError(f"Page made non-GET requests: {non_get_requests}")
+
+        # Check there weren't any HTTP requests to external resources
+        external_requests = [r for r in requests if _is_external_request(r, url)]
+        if external_requests:
+            raise PageError(
+                f"Page made requests for external resources: {external_requests}"
+            )
 
 
 def _file_url_exists(url: str, suffixes: set[str] | None = None) -> bool:

python/chives/urls.py (4628 → 4610)

diff --git a/python/chives/urls.py b/python/chives/urls.py
index 9c4413c..0a495f0 100644
--- a/python/chives/urls.py
+++ b/python/chives/urls.py
@@ -1,5 +1,6 @@
 """Code for manipulating and tidying URLs."""
 
+from contextlib import closing
 import json
 from pathlib import Path
 import re
@@ -78,16 +79,14 @@ def is_mastodon_host(hostname: str) -> bool:
     nodeinfo_url = f"https://{hostname}/.well-known/nodeinfo"
 
     try:
-        nodeinfo_resp = urllib.request.urlopen(nodeinfo_url, context=ssl_context)
+        with closing(urllib.request.urlopen(nodeinfo_url, context=ssl_context)) as resp:
+            nodeinfo = json.load(resp)
     except HTTPError as err:
         err.close()
         return False
     except SSLCertVerificationError:
         return False
 
-    nodeinfo = json.loads(nodeinfo_resp.read())
-    nodeinfo_resp.close()
-
     # Then we try to call $.links[0].href, which should return something
     # like:
     #
@@ -101,9 +100,8 @@ def is_mastodon_host(hostname: str) -> bool:
     except KeyError, IndexError:  # pragma: no cover
         return False
 
-    link_resp = urllib.request.urlopen(link_href, context=ssl_context)
-    link_info = json.loads(link_resp.read())
-    link_resp.close()
+    with closing(urllib.request.urlopen(link_href, context=ssl_context)) as resp:
+        link_info = json.load(resp)
 
     try:
         return bool(link_info["software"]["name"] == "mastodon")