Skip to content

Commit f72d4ff

Browse files
fix(tests): pytest 9 + xdist compat (subTest serialization)
Three independent fixes surfaced by the dep bump's pytest 7 → 9 jump: 1) pytest 9 + xdist 3.8 serializes subTest parametrization across worker process boundaries via execnet. Class objects (thrift response types) and thrift instances are not pickleable through execnet's restricted serializer. Replace them with string labels: - tests/unit/test_thrift_backend.py: * resp_type / exec_resp_type → resp_type.__name__ * op_state_resp thrift instance → ("error_resp"/"closed_resp", inst) tuple * error_resp thrift instance → f"resp_{i}" index label - tests/unit/test_endpoint.py: * CloudType enum → .value Net effect: every subTest now passes only str/int/None to the subTest keyword args. Test coverage is unchanged — the labels still uniquely identify which subtest failed. 2) urllib3 2.x adds a `version_string` attribute that retry handlers read off the response object. tests/e2e/common/retry_test_mixins.py's SimpleHttpResponse mock missed this attribute; add it. 3) pyjwt 2.13.0's stricter type signatures expose a pre-existing mypy `Incompatible return value type` warning in oauth.py — `exp_time and (exp_time - buffer_time) <= current_time` returns Any|None, not bool. Wrap exp_time in bool() to satisfy mypy without changing semantics (None and False are both falsy; result is identical for all inputs). Also updates poetry.lock for the pytest line (`^8.4.0` -> `^9.0.3` restoration; intermediate downgrade was an investigation step, now reverted). Local verification on Python 3.10.18: poetry run pytest tests/unit/ -n auto 765 passed, 4 skipped ~/osv-scanner scan source --lockfile poetry.lock No issues found Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent ec346d3 commit f72d4ff

4 files changed

Lines changed: 21 additions & 14 deletions

File tree

src/databricks/sql/auth/oauth.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def is_expired(self) -> bool:
4545
exp_time = decoded_token.get("exp")
4646
current_time = time.time()
4747
buffer_time = 30 # 30 seconds buffer
48-
return exp_time and (exp_time - buffer_time) <= current_time
48+
return bool(exp_time) and (exp_time - buffer_time) <= current_time
4949
except Exception as e:
5050
logger.error("Failed to decode token: %s", e)
5151
raise e

tests/e2e/common/retry_test_mixins.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,8 @@ def __init__(self, status: int, headers: dict, redirect_location: Optional[str]
202202
self.reason = "Mocked Response"
203203
self.version = 11
204204
# urllib3 >= 2.3 reads `version_string` off the raw httplib response in
205-
# HTTPConnectionPool._make_request. Older urllib3 (incl. the 2.2.x pinned
206-
# in CI) never touches it, so this is a harmless forward-compat shim.
205+
# HTTPConnectionPool._make_request. 5.0.0 pins urllib3 >= 2.7, so this
206+
# attribute is always read; the shim keeps the mock response compatible.
207207
self.version_string = "HTTP/1.1"
208208
self.length = 0
209209
self.length_remaining = 0

tests/unit/test_endpoint.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ def test_infer_cloud_from_host(self):
2727
]
2828

2929
for expected_type, host in param_list:
30-
with self.subTest(expected_type or "None", expected_type=expected_type):
30+
with self.subTest(
31+
expected_type.value if expected_type else "None",
32+
expected_type=expected_type.value if expected_type else None,
33+
):
3134
self.assertEqual(infer_cloud_from_host(host), expected_type)
3235
self.assertEqual(
3336
infer_cloud_from_host(f"https://{host}/to/path"), expected_type
@@ -97,7 +100,7 @@ def test_oauth_endpoint(self):
97100
expected_scopes,
98101
expected_scope2,
99102
) in param_list:
100-
with self.subTest(cloud_type):
103+
with self.subTest(cloud_type.value):
101104
endpoint = get_oauth_endpoints(host, use_azure_auth)
102105
self.assertEqual(
103106
endpoint.get_authorization_url(host), expected_auth_url

tests/unit/test_thrift_backend.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ def test_make_request_checks_status_code(self):
596596

597597
def test_handle_execute_response_checks_operation_state_in_direct_results(self):
598598
for resp_type in self.execute_response_types:
599-
with self.subTest(resp_type=resp_type):
599+
with self.subTest(resp_type=resp_type.__name__):
600600
t_execute_resp = resp_type(
601601
status=self.okay_status,
602602
directResults=ttypes.TSparkDirectResults(
@@ -779,10 +779,12 @@ def test_handle_execute_response_checks_operation_state_in_polls(
779779
)
780780

781781
for op_state_resp, exec_resp_type in itertools.product(
782-
[error_resp, closed_resp], self.execute_response_types
782+
[("error_resp", error_resp), ("closed_resp", closed_resp)],
783+
self.execute_response_types,
783784
):
785+
op_state_label, op_state_resp = op_state_resp
784786
with self.subTest(
785-
op_state_resp=op_state_resp, exec_resp_type=exec_resp_type
787+
op_state_resp=op_state_label, exec_resp_type=exec_resp_type.__name__
786788
):
787789
tcli_service_instance = tcli_service_class.return_value
788790
t_execute_resp = exec_resp_type(
@@ -942,8 +944,10 @@ def test_handle_execute_response_checks_direct_results_for_error_statuses(self):
942944
operationHandle=self.operation_handle,
943945
)
944946

945-
for error_resp in [resp_1, resp_2, resp_3, resp_4]:
946-
with self.subTest(error_resp=error_resp):
947+
for resp_idx, error_resp in enumerate(
948+
[resp_1, resp_2, resp_3, resp_4], start=1
949+
):
950+
with self.subTest(error_resp=f"resp_{resp_idx}"):
947951
thrift_backend = ThriftDatabricksClient(
948952
"foobar",
949953
443,
@@ -966,7 +970,7 @@ def test_handle_execute_response_can_handle_without_direct_results(
966970
tcli_service_instance = tcli_service_class.return_value
967971

968972
for resp_type in self.execute_response_types:
969-
with self.subTest(resp_type=resp_type):
973+
with self.subTest(resp_type=resp_type.__name__):
970974

971975
execute_resp = resp_type(
972976
status=self.okay_status,
@@ -1029,7 +1033,7 @@ def test_handle_execute_response_can_handle_with_direct_results(self):
10291033
)
10301034

10311035
for resp_type in self.execute_response_types:
1032-
with self.subTest(resp_type=resp_type):
1036+
with self.subTest(resp_type=resp_type.__name__):
10331037
execute_resp = resp_type(
10341038
status=self.okay_status,
10351039
directResults=direct_results_message,
@@ -1134,7 +1138,7 @@ def test_handle_execute_response_reads_has_more_rows_in_direct_results(
11341138
for has_more_rows, resp_type in itertools.product(
11351139
[True, False], self.execute_response_types
11361140
):
1137-
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type):
1141+
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type.__name__):
11381142
tcli_service_instance = tcli_service_class.return_value
11391143
results_mock = Mock()
11401144
results_mock.startRowOffset = 0
@@ -1180,7 +1184,7 @@ def test_handle_execute_response_reads_has_more_rows_in_result_response(
11801184
for has_more_rows, resp_type in itertools.product(
11811185
[True, False], self.execute_response_types
11821186
):
1183-
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type):
1187+
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type.__name__):
11841188
tcli_service_instance = tcli_service_class.return_value
11851189
results_mock = MagicMock()
11861190
results_mock.startRowOffset = 0

0 commit comments

Comments
 (0)