Skip to content

Commit 8826895

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 d99ba3b commit 8826895

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
@@ -308,6 +308,8 @@ Raúl Cumplido
308308
Required Field
309309
Robert Lu
310310
Robert Nikolich
311+
Rodrigo Nogueira
312+
Roman Markeloff
311313
Roman Podoliaka
312314
Roman Postnov
313315
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
@@ -1331,12 +1331,18 @@ async def handler(request: Request) -> Response:
13311331

13321332

13331333
def test_regex_performance() -> None:
1334+
"""Test that the regex pattern doesn't suffer from ReDoS issues."""
1335+
REGEX_TIME_THRESHOLD_SECONDS = 0.08
13341336
value = "0" * 54773 + "\\0=a"
1337+
13351338
start = time.perf_counter()
13361339
matches = _HEADER_PAIRS_PATTERN.findall(value)
1337-
end = time.perf_counter()
1340+
elapsed = time.perf_counter() - start
13381341

1339-
# If this is taking more than 10ms, there's probably a performance/ReDoS issue.
1340-
assert (end - start) < 0.01
1341-
# This example probably shouldn't produce a match either.
1342+
# If this is taking more time, there's probably a performance/ReDoS issue.
1343+
assert elapsed < REGEX_TIME_THRESHOLD_SECONDS, (
1344+
f"Regex took {elapsed * 1000:.1f}ms, "
1345+
f"expected <{REGEX_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
1346+
)
1347+
# This example shouldn't produce a match either.
13421348
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
@@ -557,13 +557,17 @@ def test_single_forwarded_header() -> None:
557557

558558

559559
def test_forwarded_re_performance() -> None:
560+
FORWARDED_RE_TIME_THRESHOLD_SECONDS = 0.08
560561
value = "{" + "f" * 54773 + "z\x00a=v"
561562
start = time.perf_counter()
562563
match = _FORWARDED_PAIR_RE.match(value)
563-
end = time.perf_counter()
564+
elapsed = time.perf_counter() - start
564565

565-
# If this is taking more than 10ms, there's probably a performance/ReDoS issue.
566-
assert (end - start) < 0.01
566+
# If this is taking more time, there's probably a performance/ReDoS issue.
567+
assert elapsed < FORWARDED_RE_TIME_THRESHOLD_SECONDS, (
568+
f"Regex took {elapsed * 1000:.1f}ms, "
569+
f"expected <{FORWARDED_RE_TIME_THRESHOLD_SECONDS * 1000:.0f}ms - potential ReDoS issue"
570+
)
567571
# This example shouldn't produce a match either.
568572
assert match is None
569573

0 commit comments

Comments
 (0)