Skip to content

Commit

Permalink
test(leak): Fix unclosed socket leaks
Browse files Browse the repository at this point in the history
Fix or workaround `ResourceWarning: unclosed <socket.socket ...>` warnings when running
the tests.  Close the requests sessions at test cleanup time.

I don't understand why these are reported as leaks.  At least some of the instances I
spot-checked were reported as leaks *after* `response.json()` had been called.  AFAIK,
[`requests` should close the socket when it's been fully
read](https://docs.python-requests.org/en/latest/user/advanced/#body-content-workflow)
which should happen when parsing the JSON.

Regardless, I did confirm that these leaks were indeed coming from this package's use of
sessions so it's ours to workaround or fix.  I don't like the approach in this change in
that it places a testing concern inside the session class but I also think the root
issue here is how much test set up is repeated across the test modules.  It seemed
better to take the expedient approach for this issue and tackle the issue of refactoring
test set up separately.

With this change and the previous deprecations warnings change, the bulk of warnings
output while running the tests have been removed.  This means there's a *lot* less to
ignore in the test output and we should not be training ourselves to ignore test output.
  • Loading branch information
rpatterson committed Dec 28, 2021
1 parent fcacc31 commit f209cf5
Show file tree
Hide file tree
Showing 43 changed files with 83 additions and 60 deletions.
3 changes: 3 additions & 0 deletions news/1302.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Resolve the bulk of deprecation and resource leak warnings when running the full test
suite.
[rpatterson]
19 changes: 18 additions & 1 deletion src/plone/restapi/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,29 @@ class RelativeSession(requests.Session):
base if their URL is relative (doesn't begin with a HTTP[S] scheme).
"""

def __init__(self, base_url):
def __init__(self, base_url, test=None):
"""
Capture the base URL. Optionally also capture a test case for cleanup.
Apparently, network sockets created by the `requests` library can remain open
even after the full body of the response has been read, despite [the
docs](https://docs.python-requests.org/en/latest/user/advanced/#body-content-workflow). In
particular, this results in `ResourceWarning: unclosed <socket.socket ...>` leak
warnings when running the tests. If the `test` kwarg is passed, it will be used
to register future cleanup calls to close this session and thus also the
sockets. If passed, it must be an object with a `addCleanup(func)` callable
attribute, such as instances of `unittest.TestCase`.
"""
super().__init__()

if not base_url.endswith("/"):
base_url += "/"
self.__base_url = base_url

if hasattr(test, "addCleanup"):
# Avoid `ResourceWarning: unclosed <socket.socket ...>`
test.addCleanup(self.close)

def request(self, method, url, **kwargs):
if urlparse(url).scheme not in ("http", "https"):
url = url.lstrip("/")
Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_batching.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
self.request = self.portal.REQUEST

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_blocks_searchable_text.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_content_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_copymove.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def setUp(self):
email="[email protected]", username="memberuser", password="secret"
)

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
12 changes: 6 additions & 6 deletions src/plone/restapi/tests/test_documentation.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def setUp(self):
pushGlobalRegistry(getSite())
register_static_uuid_utility(prefix="SomeUUID")

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down Expand Up @@ -794,7 +794,7 @@ def test_documentation_users(self):
save_request_and_response_for_docs("users", response)

def test_documentation_users_as_anonymous(self):
logged_out_api_session = RelativeSession(self.portal_url)
logged_out_api_session = RelativeSession(self.portal_url, test=self)
logged_out_api_session.headers.update({"Accept": "application/json"})

response = logged_out_api_session.get("@users")
Expand All @@ -819,7 +819,7 @@ def test_documentations_users_as_unauthorized_user(self):
)
transaction.commit()

standard_api_session = RelativeSession(self.portal_url)
standard_api_session = RelativeSession(self.portal_url, test=self)
standard_api_session.headers.update({"Accept": "application/json"})
standard_api_session.auth = ("noam", "password")

Expand Down Expand Up @@ -858,7 +858,7 @@ def test_documentation_users_anonymous_get(self):
)
transaction.commit()

logged_out_api_session = RelativeSession(self.portal_url)
logged_out_api_session = RelativeSession(self.portal_url, test=self)
logged_out_api_session.headers.update({"Accept": "application/json"})

response = logged_out_api_session.get("@users/noam")
Expand Down Expand Up @@ -890,7 +890,7 @@ def test_documentation_users_unauthorized_get(self):

transaction.commit()

logged_out_api_session = RelativeSession(self.portal_url)
logged_out_api_session = RelativeSession(self.portal_url, test=self)
logged_out_api_session.headers.update({"Accept": "application/json"})
logged_out_api_session.auth = ("noam-fake", "secret")

Expand All @@ -915,7 +915,7 @@ def test_documentation_users_authorized_get(self):
)
transaction.commit()

logged_out_api_session = RelativeSession(self.portal_url)
logged_out_api_session = RelativeSession(self.portal_url, test=self)
logged_out_api_session.headers.update({"Accept": "application/json"})
logged_out_api_session.auth = ("noam", "secret")
response = logged_out_api_session.get("@users/noam")
Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_error_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def setUp(self):
self.portal = self.layer["portal"]
self.portal_url = self.portal.absolute_url()

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/tests/test_expansion.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down Expand Up @@ -421,7 +421,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
3 changes: 3 additions & 0 deletions src/plone/restapi/tests/test_functional_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def test_api_login_grants_zmi(self):
Logging in via the API also grants access to the Zope root ZMI.
"""
session = requests.Session()
self.addCleanup(session.close)
login_resp = session.post(
self.portal_url + "/@login",
headers={"Accept": "application/json"},
Expand Down Expand Up @@ -112,6 +113,7 @@ def test_zmi_login_grants_api(self):
Logging in via the Zope root ZMI also grants access to the API.
"""
session = requests.Session()
self.addCleanup(session.close)
basic_auth_headers = {
"Authorization": "Basic {}".format(
base64.b64encode(
Expand Down Expand Up @@ -161,6 +163,7 @@ def test_cookie_login_grants_api(self):
Logging in via the Plone login form also grants access to the API.
"""
session = requests.Session()
self.addCleanup(session.close)
challenge_resp = session.get(self.private_document_url)
self.assertEqual(
challenge_resp.status_code,
Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_locking.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def setUp(self):
]
alsoProvides(self.doc, ITTWLockable)

self.api_session = RelativeSession(self.doc.absolute_url())
self.api_session = RelativeSession(self.doc.absolute_url(), test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def setUp(self):
self.portal = self.layer["portal"]
self.portal_url = self.portal.absolute_url()

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (TEST_USER_NAME, TEST_USER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def setUp(self):
self.portal = self.layer["portal"]
self.portal_url = self.portal.absolute_url()

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def setUp(self):
self.request = self.portal.REQUEST
self.catalog = getToolByName(self.portal, "portal_catalog")

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/tests/test_services_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

self.anon_api_session = RelativeSession(self.portal_url)
self.anon_api_session = RelativeSession(self.portal_url, test=self)
self.anon_api_session.headers.update({"Accept": "application/json"})

self.portal_actions = api.portal.get_tool(name="portal_actions")
Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/tests/test_services_breadcrumbs.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down Expand Up @@ -71,7 +71,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
self.request = self.layer["request"]

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/tests/test_services_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def setUp(self):
api.user.create(username="jos", password="josjos", email="[email protected]")

# Admin session
self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

# User session
self.user_session = RelativeSession(self.portal_url)
self.user_session = RelativeSession(self.portal_url, test=self)
self.user_session.headers.update({"Accept": "application/json"})
self.user_session.auth = ("jos", "jos")

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services_contextnavigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services_controlpanels.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def setUp(self):
self.request = self.portal.REQUEST
self.catalog = getToolByName(self.portal, "portal_catalog")

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/tests/test_services_email_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ def setUp(self):
registry["plone.email_from_address"] = "[email protected]"
registry["plone.email_from_name"] = "Plone test site"

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)
self.anon_api_session = RelativeSession(self.portal_url)
self.anon_api_session = RelativeSession(self.portal_url, test=self)
self.anon_api_session.headers.update({"Accept": "application/json"})

transaction.commit()
Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/tests/test_services_email_send.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ def setUp(self):
registry["plone.email_from_address"] = "[email protected]"
registry["plone.email_from_name"] = "Plone test site"

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)
self.anon_api_session = RelativeSession(self.portal_url)
self.anon_api_session = RelativeSession(self.portal_url, test=self)
self.anon_api_session.headers.update({"Accept": "application/json"})

transaction.commit()
Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down
6 changes: 3 additions & 3 deletions src/plone/restapi/tests/test_services_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)

Expand Down Expand Up @@ -153,7 +153,7 @@ def setUp(self):
api.content.transition(self.doc, "publish")
self.endpoint_url = f"{self.doc.absolute_url()}/@history"

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.auth = (TEST_USER_NAME, TEST_USER_PASSWORD)
# forbid access to `workflowHistory`
Expand All @@ -178,7 +178,7 @@ def setUp(self):
self.portal_url = self.portal.absolute_url()
setRoles(self.portal, TEST_USER_ID, ["Manager"])

self.api_session = RelativeSession(self.portal_url)
self.api_session = RelativeSession(self.portal_url, test=self)
self.api_session.headers.update({"Accept": "application/json"})
self.api_session.headers.update({"Accept-Language": "es"})
self.api_session.auth = (SITE_OWNER_NAME, SITE_OWNER_PASSWORD)
Expand Down
Loading

0 comments on commit f209cf5

Please sign in to comment.