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

Fix ResourceWarning unclosed socket and re-enable and fix tornado tests #801

Merged
merged 28 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f414435
Fix ResourceWarning unclosed socket
Aug 7, 2023
17c78bf
Merge branch 'master' of github.com:kevin1024/vcrpy into fix-resource…
graingert Dec 15, 2023
556fd01
enable filterwarnings=error
graingert Dec 15, 2023
fa789e9
fix a KeyError
graingert Dec 15, 2023
bddec2e
use socketserver.ThreadingTCPServer as a contextmanager
graingert Dec 15, 2023
3919cb2
remember to close the VCRHTTPSConnection
graingert Dec 15, 2023
f075c8b
close aiohttp session on errors
graingert Dec 15, 2023
895ae20
use asyncio.run to run coroutines
graingert Dec 15, 2023
97de8a0
ignore warning from dateutil
graingert Dec 15, 2023
73d11e8
fix httpx resource warnings
graingert Dec 15, 2023
cf76592
remove redundant contextlib import
graingert Dec 15, 2023
356ff41
fix sync do_request().stream
graingert Dec 15, 2023
80614db
fix resource warning due to pytest-asyncio
graingert Dec 15, 2023
5cff354
Revert "fix a KeyError"
graingert Dec 15, 2023
d76c243
Revert "Fix ResourceWarning unclosed socket"
graingert Dec 15, 2023
d39c26b
remember to close removed connections
graingert Dec 15, 2023
8e13af2
use context manager for requests.Session
graingert Dec 15, 2023
cc4d03c
close unremoved connections (pool already removed the connection)
graingert Dec 15, 2023
ee6e790
Merge branch 'master' into fix-resource-warning-2
graingert Dec 15, 2023
5104b1f
Merge branch 'master' of github.com:kevin1024/vcrpy into fix-resource…
graingert Jan 23, 2024
666686b
restore pytest-tornado
graingert Jan 23, 2024
a093fb1
add new deprecation warnings for tornado tests
graingert Jan 23, 2024
c6667ac
restore scheme fixture for tests
graingert Jan 23, 2024
db1f5b0
tornado 6 changes raise_error behaviour
graingert Jan 23, 2024
6d7a842
fix test_tornado_exception_can_be_caught RuntimeError: generator rais…
graingert Jan 23, 2024
b7f6c2f
mark tornado tests as online
graingert Jan 23, 2024
42b4a5d
move off of mockbin on tornado tests also
graingert Jan 23, 2024
784b2dc
tornado test_redirects is no longer an online test
graingert Jan 23, 2024
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
12 changes: 12 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,19 @@ ignore-regex = "\\\\[fnrstv]"
# ignore-words-list = ''

[tool.pytest.ini_options]
addopts = [
"--strict-config",
"--strict-markers",
]
markers = ["online"]
filterwarnings = [
"error",
'''ignore:datetime\.datetime\.utcfromtimestamp\(\) is deprecated and scheduled for removal in a future version.*:DeprecationWarning''',
'''ignore:There is no current event loop:DeprecationWarning''',
'''ignore:make_current is deprecated; start the event loop first:DeprecationWarning''',
'''ignore:clear_current is deprecated:DeprecationWarning''',
'''ignore:the \(type, exc, tb\) signature of throw\(\) is deprecated, use the single-arg signature instead.:DeprecationWarning''',
Comment on lines +16 to +19
Copy link
Collaborator Author

@graingert graingert Jan 23, 2024

Choose a reason for hiding this comment

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

I'll fix these deprecation warnings in a followup PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool!

]

[tool.ruff]
select = [
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def run_tests(self):
"pytest-asyncio",
"pytest-cov",
"pytest-httpbin",
"pytest-tornado",
"pytest",
"requests>=2.22.0",
"tornado",
Expand Down
36 changes: 18 additions & 18 deletions tests/integration/aiohttp_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,24 @@


async def aiohttp_request(loop, method, url, output="text", encoding="utf-8", content_type=None, **kwargs):
session = aiohttp.ClientSession(loop=loop)
response_ctx = session.request(method, url, **kwargs)

response = await response_ctx.__aenter__()
if output == "text":
content = await response.text()
elif output == "json":
content_type = content_type or "application/json"
content = await response.json(encoding=encoding, content_type=content_type)
elif output == "raw":
content = await response.read()
elif output == "stream":
content = await response.content.read()

response_ctx._resp.close()
await session.close()

return response, content
async with aiohttp.ClientSession(loop=loop) as session:
response_ctx = session.request(method, url, **kwargs)

response = await response_ctx.__aenter__()
if output == "text":
content = await response.text()
elif output == "json":
content_type = content_type or "application/json"
content = await response.json(encoding=encoding, content_type=content_type)
elif output == "raw":
content = await response.read()
elif output == "stream":
content = await response.content.read()

response_ctx._resp.close()
await session.close()

return response, content


def aiohttp_app():
Expand Down
15 changes: 10 additions & 5 deletions tests/integration/test_aiohttp.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import contextlib
import logging
import urllib.parse

Expand All @@ -14,10 +13,10 @@


def run_in_loop(fn):
with contextlib.closing(asyncio.new_event_loop()) as loop:
asyncio.set_event_loop(loop)
task = loop.create_task(fn(loop))
return loop.run_until_complete(task)
async def wrapper():
return await fn(asyncio.get_running_loop())

return asyncio.run(wrapper())


def request(method, url, output="text", **kwargs):
Expand Down Expand Up @@ -260,6 +259,12 @@ def test_aiohttp_test_client_json(aiohttp_client, tmpdir):
assert cassette.play_count == 1


def test_cleanup_from_pytest_asyncio():
# work around https://github.com/pytest-dev/pytest-asyncio/issues/724
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

asyncio.get_event_loop().close()
asyncio.set_event_loop(None)


@pytest.mark.online
def test_redirect(tmpdir, httpbin):
url = httpbin.url + "/redirect/2"
Expand Down
26 changes: 19 additions & 7 deletions tests/integration/test_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,37 @@ class DoSyncRequest(BaseDoRequest):
_client_class = httpx.Client

def __enter__(self):
self._client = self._make_client()
return self

def __exit__(self, *args):
pass
self._client.close()
del self._client

@property
def client(self):
try:
return self._client
except AttributeError:
self._client = self._make_client()
return self._client
except AttributeError as e:
raise ValueError('To access sync client, use "with do_request() as client"') from e

def __call__(self, *args, **kwargs):
return self.client.request(*args, timeout=60, **kwargs)
if hasattr(self, "_client"):
return self.client.request(*args, timeout=60, **kwargs)

# Use one-time context and dispose of the client afterwards
with self:
return self.client.request(*args, timeout=60, **kwargs)

def stream(self, *args, **kwargs):
with self.client.stream(*args, **kwargs) as response:
return b"".join(response.iter_bytes())
if hasattr(self, "_client"):
with self.client.stream(*args, **kwargs) as response:
return b"".join(response.iter_bytes())

# Use one-time context and dispose of the client afterwards
with self:
with self.client.stream(*args, **kwargs) as response:
return b"".join(response.iter_bytes())


class DoAsyncRequest(BaseDoRequest):
Expand Down
12 changes: 6 additions & 6 deletions tests/integration/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ def do_GET(self):

@pytest.fixture(scope="session")
def proxy_server():
httpd = socketserver.ThreadingTCPServer(("", 0), Proxy)
proxy_process = threading.Thread(target=httpd.serve_forever)
proxy_process.start()
yield "http://{}:{}".format(*httpd.server_address)
httpd.shutdown()
proxy_process.join()
with socketserver.ThreadingTCPServer(("", 0), Proxy) as httpd:
proxy_process = threading.Thread(target=httpd.serve_forever)
proxy_process.start()
yield "http://{}:{}".format(*httpd.server_address)
httpd.shutdown()
proxy_process.join()


def test_use_proxy(tmpdir, httpbin, proxy_server):
Expand Down
32 changes: 30 additions & 2 deletions tests/integration/test_tornado.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@
# whether the current version of Tornado supports the raise_error argument for
# fetch().
supports_raise_error = tornado.version_info >= (4,)
raise_error_for_response_code_only = tornado.version_info >= (6,)


@pytest.fixture(params=["https", "http"])
def scheme(request):
"""Fixture that returns both http and https."""
return request.param


@pytest.fixture(params=["simple", "curl", "default"])
Expand Down Expand Up @@ -44,6 +51,7 @@ def post(client, url, data=None, **kwargs):
return client.fetch(http.HTTPRequest(url, method="POST", **kwargs))


@pytest.mark.online
@pytest.mark.gen_test
def test_status_code(get_client, scheme, tmpdir):
"""Ensure that we can read the status code"""
Expand All @@ -56,6 +64,7 @@ def test_status_code(get_client, scheme, tmpdir):
assert 1 == cass.play_count


@pytest.mark.online
@pytest.mark.gen_test
def test_headers(get_client, scheme, tmpdir):
"""Ensure that we can read the headers back"""
Expand All @@ -68,6 +77,7 @@ def test_headers(get_client, scheme, tmpdir):
assert 1 == cass.play_count


@pytest.mark.online
@pytest.mark.gen_test
def test_body(get_client, tmpdir, scheme):
"""Ensure the responses are all identical enough"""
Expand All @@ -94,6 +104,7 @@ def test_effective_url(get_client, tmpdir, httpbin):
assert 1 == cass.play_count


@pytest.mark.online
@pytest.mark.gen_test
def test_auth(get_client, tmpdir, scheme):
"""Ensure that we can handle basic auth"""
Expand All @@ -109,6 +120,7 @@ def test_auth(get_client, tmpdir, scheme):
assert 1 == cass.play_count


@pytest.mark.online
@pytest.mark.gen_test
def test_auth_failed(get_client, tmpdir, scheme):
"""Ensure that we can save failed auth statuses"""
Expand All @@ -132,6 +144,7 @@ def test_auth_failed(get_client, tmpdir, scheme):
assert 1 == cass.play_count


@pytest.mark.online
@pytest.mark.gen_test
def test_post(get_client, tmpdir, scheme):
"""Ensure that we can post and cache the results"""
Expand All @@ -148,9 +161,9 @@ def test_post(get_client, tmpdir, scheme):


@pytest.mark.gen_test
def test_redirects(get_client, tmpdir, scheme):
def test_redirects(get_client, tmpdir, httpbin):
"""Ensure that we can handle redirects"""
url = scheme + "://mockbin.org/redirect/301?url=bytes/1024"
url = httpbin + "/redirect-to?url=bytes/1024&status_code=301"
with vcr.use_cassette(str(tmpdir.join("requests.yaml"))):
content = (yield get(get_client(), url)).body

Expand All @@ -159,6 +172,7 @@ def test_redirects(get_client, tmpdir, scheme):
assert cass.play_count == 1


@pytest.mark.online
@pytest.mark.gen_test
def test_cross_scheme(get_client, tmpdir, scheme):
"""Ensure that requests between schemes are treated separately"""
Expand All @@ -178,6 +192,7 @@ def test_cross_scheme(get_client, tmpdir, scheme):
assert cass.play_count == 2


@pytest.mark.online
@pytest.mark.gen_test
def test_gzip(get_client, tmpdir, scheme):
"""
Expand All @@ -203,6 +218,7 @@ def test_gzip(get_client, tmpdir, scheme):
assert 1 == cass.play_count


@pytest.mark.online
@pytest.mark.gen_test
def test_https_with_cert_validation_disabled(get_client, tmpdir):
cass_path = str(tmpdir.join("cert_validation_disabled.yaml"))
Expand Down Expand Up @@ -233,6 +249,10 @@ def callback(chunk):


@pytest.mark.skipif(not supports_raise_error, reason="raise_error unavailable in tornado <= 3")
@pytest.mark.skipif(
raise_error_for_response_code_only,
reason="raise_error only ignores HTTPErrors due to response code",
)
@pytest.mark.gen_test
def test_unsupported_features_raise_error_disabled(get_client, tmpdir):
"""Ensure that the exception for an AsyncHTTPClient feature not being
Expand All @@ -252,6 +272,7 @@ def callback(chunk):
assert "not yet supported by VCR" in str(response.error)


@pytest.mark.online
@pytest.mark.gen_test
def test_cannot_overwrite_cassette_raises_in_future(get_client, tmpdir):
"""Ensure that CannotOverwriteExistingCassetteException is raised inside
Expand All @@ -268,6 +289,10 @@ def test_cannot_overwrite_cassette_raises_in_future(get_client, tmpdir):


@pytest.mark.skipif(not supports_raise_error, reason="raise_error unavailable in tornado <= 3")
@pytest.mark.skipif(
raise_error_for_response_code_only,
reason="raise_error only ignores HTTPErrors due to response code",
)
@pytest.mark.gen_test
def test_cannot_overwrite_cassette_raise_error_disabled(get_client, tmpdir):
"""Ensure that CannotOverwriteExistingCassetteException is not raised if
Expand Down Expand Up @@ -303,6 +328,7 @@ def test_tornado_exception_can_be_caught(get_client):
assert e.code == 404


@pytest.mark.online
@pytest.mark.gen_test
def test_existing_references_get_patched(tmpdir):
from tornado.httpclient import AsyncHTTPClient
Expand All @@ -316,6 +342,7 @@ def test_existing_references_get_patched(tmpdir):
assert cass.play_count == 1


@pytest.mark.online
@pytest.mark.gen_test
def test_existing_instances_get_patched(get_client, tmpdir):
"""Ensure that existing instances of AsyncHTTPClient get patched upon
Expand All @@ -331,6 +358,7 @@ def test_existing_instances_get_patched(get_client, tmpdir):
assert cass.play_count == 1


@pytest.mark.online
@pytest.mark.gen_test
def test_request_time_is_set(get_client, tmpdir):
"""Ensures that the request_time on HTTPResponses is set."""
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/test_wild.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ def test_flickr_should_respond_with_200(tmpdir):
def test_cookies(tmpdir, httpbin):
testfile = str(tmpdir.join("cookies.yml"))
with vcr.use_cassette(testfile):
s = requests.Session()
s.get(httpbin.url + "/cookies/set?k1=v1&k2=v2")
assert s.cookies.keys() == ["k1", "k2"]
with requests.Session() as s:
s.get(httpbin.url + "/cookies/set?k1=v1&k2=v2")
assert s.cookies.keys() == ["k1", "k2"]

r2 = s.get(httpbin.url + "/cookies")
assert sorted(r2.json()["cookies"].keys()) == ["k1", "k2"]
r2 = s.get(httpbin.url + "/cookies")
assert sorted(r2.json()["cookies"].keys()) == ["k1", "k2"]


@pytest.mark.online
Expand Down
9 changes: 5 additions & 4 deletions tests/unit/test_stubs.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import contextlib
from unittest import mock

from pytest import mark
Expand All @@ -16,7 +17,7 @@ def test_setting_of_attributes_get_propagated_to_real_connection(self):
@mark.online
@mock.patch("vcr.cassette.Cassette.can_play_response_for", return_value=False)
def testing_connect(*args):
vcr_connection = VCRHTTPSConnection("www.google.com")
vcr_connection.cassette = Cassette("test", record_mode=mode.ALL)
vcr_connection.real_connection.connect()
assert vcr_connection.real_connection.sock is not None
with contextlib.closing(VCRHTTPSConnection("www.google.com")) as vcr_connection:
vcr_connection.cassette = Cassette("test", record_mode=mode.ALL)
vcr_connection.real_connection.connect()
assert vcr_connection.real_connection.sock is not None
16 changes: 1 addition & 15 deletions vcr/cassette.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import copy
import inspect
import logging
import sys
from asyncio import iscoroutinefunction

import wrapt
Expand Down Expand Up @@ -126,20 +125,7 @@ def _handle_generator(self, fn):
duration of the generator.
"""
with self as cassette:
coroutine = fn(cassette)
# We don't need to catch StopIteration. The caller (Tornado's
# gen.coroutine, for example) will handle that.
to_yield = next(coroutine)
while True:
try:
to_send = yield to_yield
except Exception:
to_yield = coroutine.throw(*sys.exc_info())
else:
try:
to_yield = coroutine.send(to_send)
except StopIteration:
break
yield from fn(cassette)

def _handle_function(self, fn):
with self as cassette:
Expand Down
Loading