Skip to content

Commit 33a862f

Browse files
committed
refactor(magics): replace fragile psutil socket check with client close mock spy
1 parent 80d94f6 commit 33a862f

1 file changed

Lines changed: 21 additions & 61 deletions

File tree

packages/bigquery-magics/tests/system/test_bigquery.py

Lines changed: 21 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -14,77 +14,42 @@
1414

1515
"""System tests for Jupyter/IPython connector."""
1616

17-
import contextlib
18-
import gc
1917
import re
20-
import time
18+
from unittest import mock
2119

20+
import google.cloud.bigquery
2221
import pandas
23-
import psutil
2422
from IPython.testing import globalipapp
2523
from IPython.utils import io
2624

2725

28-
@contextlib.contextmanager
29-
def patch_tracked_requests():
30-
"""Context manager to patch google-auth requests and track/close their HTTP sessions.
31-
32-
This prevents socket leaks in system tests that use Workload Identity or metadata server auth.
33-
"""
34-
import google.auth.transport.requests
35-
36-
original_init = google.auth.transport.requests.Request.__init__
37-
tracked_requests = []
38-
39-
def patched_init(self, session=None):
40-
original_init(self, session=session)
41-
if session is None:
42-
tracked_requests.append(self)
43-
44-
google.auth.transport.requests.Request.__init__ = patched_init
45-
try:
46-
yield tracked_requests
47-
finally:
48-
google.auth.transport.requests.Request.__init__ = original_init
49-
for req in tracked_requests:
50-
if hasattr(req, "session") and req.session is not None:
51-
req.session.close()
52-
53-
5426
def test_bigquery_magic():
5527
globalipapp.start_ipython()
5628
ip = globalipapp.get_ipython()
57-
current_process = psutil.Process()
5829

59-
# GC to ensure clean starting state
60-
gc.collect()
61-
conn_count_start = len(current_process.net_connections())
30+
ip.extension_manager.load_extension("bigquery_magics")
31+
sql = """
32+
SELECT
33+
CONCAT(
34+
'https://stackoverflow.com/questions/',
35+
CAST(id as STRING)) as url,
36+
view_count
37+
FROM `bigquery-public-data.stackoverflow.posts_questions`
38+
WHERE tags like '%google-bigquery%'
39+
ORDER BY view_count DESC
40+
LIMIT 10
41+
"""
42+
original_close = google.cloud.bigquery.Client.close
43+
with mock.patch.object(
44+
google.cloud.bigquery.Client, "close", autospec=True
45+
) as mock_close:
46+
mock_close.side_effect = original_close
6247

63-
with patch_tracked_requests():
64-
ip.extension_manager.load_extension("bigquery_magics")
65-
sql = """
66-
SELECT
67-
CONCAT(
68-
'https://stackoverflow.com/questions/',
69-
CAST(id as STRING)) as url,
70-
view_count
71-
FROM `bigquery-public-data.stackoverflow.posts_questions`
72-
WHERE tags like '%google-bigquery%'
73-
ORDER BY view_count DESC
74-
LIMIT 10
75-
"""
7648
with io.capture_output() as captured:
7749
result = ip.run_cell_magic("bigquery", "--use_rest_api", sql)
7850

79-
# Force garbage collection to sweep unreferenced socket objects
80-
gc.collect()
81-
82-
# Wait a bit for the asynchronous channel teardown to complete and the socket to be closed.
83-
for _ in range(30):
84-
conn_count_end = len(current_process.net_connections())
85-
if conn_count_end <= conn_count_start:
86-
break
87-
time.sleep(0.1)
51+
# Verify that client close is explicitly called to release sockets.
52+
assert mock_close.called
8853

8954
lines = re.split("\n|\r", captured.stdout)
9055
# Removes blanks & terminal code (result of display clearing)
@@ -94,8 +59,3 @@ def test_bigquery_magic():
9459
assert isinstance(result, pandas.DataFrame)
9560
assert len(result) == 10 # verify row count
9661
assert list(result) == ["url", "view_count"] # verify column names
97-
98-
# NOTE: For some reason, the number of open sockets is sometimes one *less*
99-
# than expected when running system tests on Kokoro, thus using the <= assertion.
100-
# That's still fine, however, since the sockets are apparently not leaked.
101-
assert conn_count_end <= conn_count_start # system resources are released

0 commit comments

Comments
 (0)