Stabilize test suite: fix flaky timing tests, add rerun support, and handle Windows resource leaks

CHANGES/11992.contrib.rst

@@ -0,0 +1 @@
+Fixed flaky performance tests by using appropriate fixed thresholds that account for CI variability -- by :user:`rodrigobnogueira`.

CONTRIBUTORS.txt

@@ -318,6 +318,7 @@ Raúl Cumplido
 Required Field
 Robert Lu
 Robert Nikolich
+Rodrigo Nogueira
 Roman Markeloff
 Roman Podoliaka
 Roman Postnov

tests/conftest.py

@@ -49,6 +49,18 @@
     TRUSTME = False
 
 
+def pytest_configure(config: pytest.Config) -> None:
+    # On Windows with Python 3.10/3.11, proxy.py's threaded mode can leave
+    # sockets not fully released by the time pytest's unraisableexception
+    # plugin collects warnings during teardown. Suppress these warnings
+    # since they are not actionable and only affect older Python versions.
+    if os.name == "nt" and sys.version_info[:2] in ((3, 10), (3, 11)):
+        config.addinivalue_line(
+            "filterwarnings",
+            "ignore:Exception ignored in.*socket.*:pytest.PytestUnraisableExceptionWarning",
+        )
+
+
 try:
     if sys.platform == "win32":
         import winloop as uvloop
@@ -431,13 +443,14 @@ async def make_client_request(
     loop: asyncio.AbstractEventLoop,
 ) -> AsyncIterator[Callable[[str, URL, Unpack[ClientRequestArgs]], ClientRequest]]:
     """Fixture to help creating test ClientRequest objects with defaults."""
-    request = session = None
+    requests: list[ClientRequest] = []
+    sessions: list[ClientSession] = []
 
     def maker(
         method: str, url: URL, **kwargs: Unpack[ClientRequestArgs]
     ) -> ClientRequest:
-        nonlocal request, session
         session = ClientSession()
+        sessions.append(session)
         default_args: ClientRequestArgs = {
             "loop": loop,
             "params": {},
@@ -462,14 +475,15 @@ def maker(
             "server_hostname": None,
         }
         request = ClientRequest(method, url, **(default_args | kwargs))
+        requests.append(request)
         return request
 
     yield maker
 
-    if request is not None:
-        await request._close()
-        assert session is not None
-        await session.close()
+    await asyncio.gather(
+        *(request._close() for request in requests),
+        *(session.close() for session in sessions),
+    )
 
 
 @pytest.fixture

tests/test_client_middleware_digest_auth.py

@@ -1331,13 +1331,21 @@ async def handler(request: Request) -> Response:
     assert auth_algorithms[0] == "MD5-sess"  # Not "MD5-SESS"
 
 
+_REGEX_TIME_THRESHOLD_SECONDS = 0.08
+
+
 def test_regex_performance() -> None:
+    """Test that the regex pattern doesn't suffer from ReDoS issues."""
     value = "0" * 54773 + "\\0=a"
+
     start = time.perf_counter()
     matches = _HEADER_PAIRS_PATTERN.findall(value)
-    end = time.perf_counter()
+    elapsed = time.perf_counter() - start
 
-    # If this is taking more than 10ms, there's probably a performance/ReDoS issue.
-    assert (end - start) < 0.01
-    # This example probably shouldn't produce a match either.
+    # If this is taking more time, there's probably a performance/ReDoS issue.
+    assert elapsed < _REGEX_TIME_THRESHOLD_SECONDS, (
+        f"Regex took {elapsed * 1000:.1f}ms, "
+        f"expected <{_REGEX_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
+    )
+    # This example shouldn't produce a match either.
     assert not matches

tests/test_cookie_helpers.py

@@ -637,14 +637,21 @@ def test_cookie_pattern_matches_partitioned_attribute(test_string: str) -> None:
     assert match.group("key").lower() == "partitioned"
 
 
+_COOKIE_PATTERN_TIME_THRESHOLD_SECONDS = 0.08
+
+
 def test_cookie_pattern_performance() -> None:
+    """Test that the cookie pattern doesn't suffer from ReDoS issues."""
     value = "a" + "=" * 21651 + "\x00"
     start = time.perf_counter()
     match = helpers._COOKIE_PATTERN.match(value)
-    end = time.perf_counter()
+    elapsed = time.perf_counter() - start
 
-    # If this is taking more than 10ms, there's probably a performance/ReDoS issue.
-    assert (end - start) < 0.01
+    # If this is taking more time, there's probably a performance/ReDoS issue.
+    assert elapsed < _COOKIE_PATTERN_TIME_THRESHOLD_SECONDS, (
+        f"Pattern took {elapsed * 1000:.1f}ms, "
+        f"expected <{_COOKIE_PATTERN_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
+    )
     # This example shouldn't produce a match either.
     assert match is None
 

tests/test_imports.py

@@ -28,20 +28,7 @@ def test_web___all__(pytester: pytest.Pytester) -> None:
     result.assert_outcomes(passed=0, errors=0)
 
 
-_IS_CI_ENV = os.getenv("CI") == "true"
-_XDIST_WORKER_COUNT = int(os.getenv("PYTEST_XDIST_WORKER_COUNT", 0))
-_IS_XDIST_RUN = _XDIST_WORKER_COUNT > 1
-
-_TARGET_TIMINGS_BY_PYTHON_VERSION = {
-    "3.12": (
-        # 3.12+ is expected to be a bit slower due to performance trade-offs,
-        # and even slower under pytest-xdist, especially in CI
-        _XDIST_WORKER_COUNT * 100 * (1 if _IS_CI_ENV else 1.53)
-        if _IS_XDIST_RUN
-        else 295
-    ),
-}
-_TARGET_TIMINGS_BY_PYTHON_VERSION["3.13"] = _TARGET_TIMINGS_BY_PYTHON_VERSION["3.12"]
+_IMPORT_TIME_THRESHOLD_MS = 400 if sys.version_info >= (3, 12) else 300
 
 
 @pytest.mark.internal
@@ -61,23 +48,15 @@ def test_import_time(pytester: pytest.Pytester) -> None:
     old_path = os.environ.get("PYTHONPATH")
     os.environ["PYTHONPATH"] = os.pathsep.join([str(root)] + sys.path)
 
-    best_time_ms = 1000
     cmd = "import timeit; print(int(timeit.timeit('import aiohttp', number=1) * 1000))"
     try:
-        for _ in range(3):
-            r = pytester.run(sys.executable, "-We", "-c", cmd)
-
-            assert not r.stderr.str()
-            runtime_ms = int(r.stdout.str())
-            if runtime_ms < best_time_ms:
-                best_time_ms = runtime_ms
+        r = pytester.run(sys.executable, "-We", "-c", cmd)
+        assert not r.stderr.str(), r.stderr.str()
+        runtime_ms = int(r.stdout.str())
     finally:
         if old_path is None:
             os.environ.pop("PYTHONPATH")
         else:
             os.environ["PYTHONPATH"] = old_path
 
-    expected_time = _TARGET_TIMINGS_BY_PYTHON_VERSION.get(
-        f"{sys.version_info.major}.{sys.version_info.minor}", 200
-    )
-    assert best_time_ms < expected_time
+    assert runtime_ms < _IMPORT_TIME_THRESHOLD_MS

tests/test_web_request.py

@@ -600,14 +600,20 @@ def test_single_forwarded_header() -> None:
     assert req.forwarded[0]["proto"] == "identifier"
 
 
+_FORWARDED_RE_TIME_THRESHOLD_SECONDS = 0.08
+
+
 def test_forwarded_re_performance() -> None:
     value = "{" + "f" * 54773 + "z\x00a=v"
     start = time.perf_counter()
     match = _FORWARDED_PAIR_RE.match(value)
-    end = time.perf_counter()
+    elapsed = time.perf_counter() - start
 
-    # If this is taking more than 10ms, there's probably a performance/ReDoS issue.
-    assert (end - start) < 0.01
+    # If this is taking more time, there's probably a performance/ReDoS issue.
+    assert elapsed < _FORWARDED_RE_TIME_THRESHOLD_SECONDS, (
+        f"Regex took {elapsed * 1000:.1f}ms, "
+        f"expected <{_FORWARDED_RE_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
+    )
     # This example shouldn't produce a match either.
     assert match is None