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.

Sign up for GitHub

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

Add tests for memory leaks #106

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6346e99
Add tests for memory
sethmlarson Apr 23, 2023
90d8530
Skip memray tests if not installed
sethmlarson Apr 23, 2023
fc3e952
Ignore 'limit_memory' marker not being available
sethmlarson Apr 23, 2023
ba3f080
Move memray posargs separately to be less confusing
sethmlarson Apr 24, 2023
07b2680
Test out memory limiting to see if we can trip the limit
sethmlarson Apr 24, 2023
cd1ff09
Merge branch 'main' into memory-testing
sethmlarson Dec 15, 2023
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
View file
Open in desktop
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"
3 changes: 3 additions & 0 deletions noxfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,14 @@ def test(session):

session.install("-rdev-requirements.txt", ".")
session.run("pip", "freeze")
memray_supported = not session.run("python", "-c", '"import pytest_memray"')

session.run(
"pytest",
"-v",
"-s",
"-rs",
*("--memray", "--hide-memray-summary") if memray_supported else (),
sethmlarson marked this conversation as resolved.
Show resolved Hide resolved
"--no-flaky-report",
"--max-runs=3",
*(session.posargs or ("tests/",)),
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
View file
Open in desktop
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
View file
Open in desktop
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.

Copy link

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