Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for memory leaks #106

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ trustme
requests
flaky
httpx
pytest-memray; python_version<"3.12" and sys_platform!="win32" and implementation_name=="cpython"
6 changes: 6 additions & 0 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,17 @@ def test(session):

session.install("-rdev-requirements.txt", ".")
session.run("pip", "freeze")
memray_supported = not session.run("python", "-c", '"import pytest_memray"')
memray_posargs: tuple[str, ...] = (
("--memray", "--hide-memray-summary") if memray_supported else ()
)

session.run(
"pytest",
"-v",
"-s",
"-rs",
*memray_posargs,
"--no-flaky-report",
"--max-runs=3",
*(session.posargs or ("tests/",)),
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@ asyncio_mode = "strict"
filterwarnings = [
"error",
]
markers = [
"limit_memory"
]
31 changes: 31 additions & 0 deletions tests/test_memory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import asyncio
import ssl

import pytest
import urllib3

import truststore

from .conftest import Server

try:
import pytest_memray # noqa: F401

memray_installed = True
except ImportError:
memray_installed = False


@pytest.mark.skipif(not memray_installed, reason="Memray isn't installed")
@pytest.mark.limit_memory("1MB")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is worth something, but for catching leaks what we care about is not the high water mark but how much RAM is still used after all tests are done...right?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I was trying to figure out how we can assert that no allocations were leaked but wasn't able to immediately find anything, I see this PR which looks like what we need? Maybe I'll add a ping of @pablogsal in case there's something I'm missing from the docs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sethmlarson Okay yeah, that looks like the right place for it. We can follow up and add it once that PR lands.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are still iterating a bit over thar PR because is quite difficult to isolate from weird one-off allocations that the interpreter does and caches. Stay tuned. Meanwhile you can use the high watermark limit + a lot of iterations of the test as a proxy if you want

@pytest.mark.asyncio
async def test_memory_limit(server: Server) -> None:
def run_requests():
ctx = truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
for _ in range(10000):
with urllib3.PoolManager(ssl_context=ctx) as http:
http.request("GET", server.base_url)
http.clear() # Close connections so we get new ones.

thread = asyncio.to_thread(run_requests)
await thread