Skip to content

Commit daa78ca

Browse files
rodrigobnogueiraDreamsorcerer
authored andcommitted
Stabilize test suite: fix flaky timing tests, and handle Windows resource leaks (#11992)
(cherry picked from commit 32924e8)
1 parent 47d045d commit daa78ca

File tree

8 files changed

+61
-39
lines changed

8 files changed

+61
-39
lines changed

CHANGES/11992.contrib.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed flaky performance tests by using appropriate fixed thresholds that account for CI variability -- by :user:`rodrigobnogueira`.

CONTRIBUTORS.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,8 @@ Raúl Cumplido
310310
Required Field
311311
Robert Lu
312312
Robert Nikolich
313+
Rodrigo Nogueira
314+
Roman Markeloff
313315
Roman Podoliaka
314316
Roman Postnov
315317
Rong Zhang

tests/conftest.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@
4141
TRUSTME = False
4242

4343

44+
def pytest_configure(config: pytest.Config) -> None:
45+
# On Windows with Python 3.10/3.11, proxy.py's threaded mode can leave
46+
# sockets not fully released by the time pytest's unraisableexception
47+
# plugin collects warnings during teardown. Suppress these warnings
48+
# since they are not actionable and only affect older Python versions.
49+
if os.name == "nt" and sys.version_info < (3, 12):
50+
config.addinivalue_line(
51+
"filterwarnings",
52+
"ignore:Exception ignored in.*socket.*:pytest.PytestUnraisableExceptionWarning",
53+
)
54+
55+
4456
try:
4557
if sys.platform == "win32":
4658
import winloop as uvloop

tests/test_client_middleware_digest_auth.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,12 +1332,18 @@ async def handler(request: Request) -> Response:
13321332

13331333

13341334
def test_regex_performance() -> None:
1335+
"""Test that the regex pattern doesn't suffer from ReDoS issues."""
1336+
REGEX_TIME_THRESHOLD_SECONDS = 0.08
13351337
value = "0" * 54773 + "\\0=a"
1338+
13361339
start = time.perf_counter()
13371340
matches = _HEADER_PAIRS_PATTERN.findall(value)
1338-
end = time.perf_counter()
1341+
elapsed = time.perf_counter() - start
13391342

1340-
# If this is taking more than 10ms, there's probably a performance/ReDoS issue.
1341-
assert (end - start) < 0.01
1342-
# This example probably shouldn't produce a match either.
1343+
# If this is taking more time, there's probably a performance/ReDoS issue.
1344+
assert elapsed < REGEX_TIME_THRESHOLD_SECONDS, (
1345+
f"Regex took {elapsed * 1000:.1f}ms, "
1346+
f"expected <{REGEX_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
1347+
)
1348+
# This example shouldn't produce a match either.
13431349
assert not matches

tests/test_cookie_helpers.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -638,13 +638,18 @@ def test_cookie_pattern_matches_partitioned_attribute(test_string: str) -> None:
638638

639639

640640
def test_cookie_pattern_performance() -> None:
641+
"""Test that the cookie pattern doesn't suffer from ReDoS issues."""
642+
COOKIE_PATTERN_TIME_THRESHOLD_SECONDS = 0.08
641643
value = "a" + "=" * 21651 + "\x00"
642644
start = time.perf_counter()
643645
match = helpers._COOKIE_PATTERN.match(value)
644-
end = time.perf_counter()
646+
elapsed = time.perf_counter() - start
645647

646-
# If this is taking more than 10ms, there's probably a performance/ReDoS issue.
647-
assert (end - start) < 0.01
648+
# If this is taking more time, there's probably a performance/ReDoS issue.
649+
assert elapsed < COOKIE_PATTERN_TIME_THRESHOLD_SECONDS, (
650+
f"Pattern took {elapsed * 1000:.1f}ms, "
651+
f"expected <{COOKIE_PATTERN_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
652+
)
648653
# This example shouldn't produce a match either.
649654
assert match is None
650655

tests/test_imports.py

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,6 @@ def test_web___all__(pytester: pytest.Pytester) -> None:
2828
result.assert_outcomes(passed=0, errors=0)
2929

3030

31-
_IS_CI_ENV = os.getenv("CI") == "true"
32-
_XDIST_WORKER_COUNT = int(os.getenv("PYTEST_XDIST_WORKER_COUNT", 0))
33-
_IS_XDIST_RUN = _XDIST_WORKER_COUNT > 1
34-
35-
_TARGET_TIMINGS_BY_PYTHON_VERSION = {
36-
"3.12": (
37-
# 3.12+ is expected to be a bit slower due to performance trade-offs,
38-
# and even slower under pytest-xdist, especially in CI
39-
_XDIST_WORKER_COUNT * 100 * (1 if _IS_CI_ENV else 1.53)
40-
if _IS_XDIST_RUN
41-
else 295
42-
),
43-
}
44-
_TARGET_TIMINGS_BY_PYTHON_VERSION["3.13"] = _TARGET_TIMINGS_BY_PYTHON_VERSION["3.12"]
45-
46-
4731
@pytest.mark.internal
4832
@pytest.mark.dev_mode
4933
@pytest.mark.skipif(
@@ -57,7 +41,10 @@ def test_import_time(pytester: pytest.Pytester) -> None:
5741
Obviously, the time may vary on different machines and may need to be adjusted
5842
from time to time, but this should provide an early warning if something is
5943
added that significantly increases import time.
44+
45+
Runs 3 times and keeps the minimum time to reduce flakiness.
6046
"""
47+
IMPORT_TIME_THRESHOLD_MS = 300 if sys.version_info >= (3, 12) else 200
6148
root = Path(__file__).parent.parent
6249
old_path = os.environ.get("PYTHONPATH")
6350
os.environ["PYTHONPATH"] = os.pathsep.join([str(root)] + sys.path)
@@ -67,18 +54,12 @@ def test_import_time(pytester: pytest.Pytester) -> None:
6754
try:
6855
for _ in range(3):
6956
r = pytester.run(sys.executable, "-We", "-c", cmd)
70-
71-
assert not r.stderr.str()
72-
runtime_ms = int(r.stdout.str())
73-
if runtime_ms < best_time_ms:
74-
best_time_ms = runtime_ms
57+
assert not r.stderr.str(), r.stderr.str()
58+
best_time_ms = min(best_time_ms, int(r.stdout.str()))
7559
finally:
7660
if old_path is None:
7761
os.environ.pop("PYTHONPATH")
7862
else:
7963
os.environ["PYTHONPATH"] = old_path
8064

81-
expected_time = _TARGET_TIMINGS_BY_PYTHON_VERSION.get(
82-
f"{sys.version_info.major}.{sys.version_info.minor}", 200
83-
)
84-
assert best_time_ms < expected_time
65+
assert best_time_ms < IMPORT_TIME_THRESHOLD_MS

tests/test_proxy_functional.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from aiohttp.client_exceptions import ClientConnectionError
2020
from aiohttp.helpers import IS_MACOS, IS_WINDOWS
2121
from aiohttp.pytest_plugin import AiohttpServer
22+
from aiohttp.test_utils import TestServer
2223

2324
ASYNCIO_SUPPORTS_TLS_IN_TLS = sys.version_info >= (3, 11)
2425

@@ -216,24 +217,34 @@ async def test_https_proxy_unsupported_tls_in_tls(
216217
# otherwise this test will fail because the proxy will die with an error.
217218
async def test_uvloop_secure_https_proxy(
218219
client_ssl_ctx: ssl.SSLContext,
220+
ssl_ctx: ssl.SSLContext,
219221
secure_proxy_url: URL,
220222
uvloop_loop: asyncio.AbstractEventLoop,
221223
) -> None:
222224
"""Ensure HTTPS sites are accessible through a secure proxy without warning when using uvloop."""
225+
payload = str(uuid4())
226+
227+
async def handler(request: web.Request) -> web.Response:
228+
return web.Response(text=payload)
229+
230+
app = web.Application()
231+
app.router.add_route("GET", "/", handler)
232+
server = TestServer(app, host="127.0.0.1")
233+
await server.start_server(ssl=ssl_ctx)
234+
235+
url = URL.build(scheme="https", host=server.host, port=server.port)
223236
conn = aiohttp.TCPConnector(force_close=True)
224237
sess = aiohttp.ClientSession(connector=conn)
225238
try:
226-
url = URL("https://example.com")
227-
228239
async with sess.get(
229240
url, proxy=secure_proxy_url, ssl=client_ssl_ctx
230241
) as response:
231242
assert response.status == 200
232-
# Ensure response body is read to completion
233-
await response.read()
243+
assert await response.text() == payload
234244
finally:
235245
await sess.close()
236246
await conn.close()
247+
await server.close()
237248
await asyncio.sleep(0)
238249
await asyncio.sleep(0.1)
239250

tests/test_web_request.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -641,13 +641,17 @@ def test_single_forwarded_header() -> None:
641641

642642

643643
def test_forwarded_re_performance() -> None:
644+
FORWARDED_RE_TIME_THRESHOLD_SECONDS = 0.08
644645
value = "{" + "f" * 54773 + "z\x00a=v"
645646
start = time.perf_counter()
646647
match = _FORWARDED_PAIR_RE.match(value)
647-
end = time.perf_counter()
648+
elapsed = time.perf_counter() - start
648649

649-
# If this is taking more than 10ms, there's probably a performance/ReDoS issue.
650-
assert (end - start) < 0.01
650+
# If this is taking more time, there's probably a performance/ReDoS issue.
651+
assert elapsed < FORWARDED_RE_TIME_THRESHOLD_SECONDS, (
652+
f"Regex took {elapsed * 1000:.1f}ms, "
653+
f"expected <{FORWARDED_RE_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
654+
)
651655
# This example shouldn't produce a match either.
652656
assert match is None
653657

0 commit comments

Comments
 (0)