Skip to content

Commit ac3c86f

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 dfac273 commit ac3c86f

5 files changed

Lines changed: 21 additions & 27 deletions

File tree

poetry.lock

Lines changed: 1 addition & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ def __init__(self, status: int, headers: dict, redirect_location: Optional[str]
187187
self.msg = self.headers # For urllib3~=1.0.0 compatibility
188188
self.reason = "Mocked Response"
189189
self.version = 11
190+
self.version_string = "HTTP/1.1" # urllib3 v2.x reads this attribute
190191
self.length = 0
191192
self.length_remaining = 0
192193
self._redirect_location = redirect_location

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(
@@ -687,10 +687,12 @@ def test_handle_execute_response_checks_operation_state_in_polls(
687687
)
688688

689689
for op_state_resp, exec_resp_type in itertools.product(
690-
[error_resp, closed_resp], self.execute_response_types
690+
[("error_resp", error_resp), ("closed_resp", closed_resp)],
691+
self.execute_response_types,
691692
):
693+
op_state_label, op_state_resp = op_state_resp
692694
with self.subTest(
693-
op_state_resp=op_state_resp, exec_resp_type=exec_resp_type
695+
op_state_resp=op_state_label, exec_resp_type=exec_resp_type.__name__
694696
):
695697
tcli_service_instance = tcli_service_class.return_value
696698
t_execute_resp = exec_resp_type(
@@ -850,8 +852,10 @@ def test_handle_execute_response_checks_direct_results_for_error_statuses(self):
850852
operationHandle=self.operation_handle,
851853
)
852854

853-
for error_resp in [resp_1, resp_2, resp_3, resp_4]:
854-
with self.subTest(error_resp=error_resp):
855+
for resp_idx, error_resp in enumerate(
856+
[resp_1, resp_2, resp_3, resp_4], start=1
857+
):
858+
with self.subTest(error_resp=f"resp_{resp_idx}"):
855859
thrift_backend = ThriftDatabricksClient(
856860
"foobar",
857861
443,
@@ -874,7 +878,7 @@ def test_handle_execute_response_can_handle_without_direct_results(
874878
tcli_service_instance = tcli_service_class.return_value
875879

876880
for resp_type in self.execute_response_types:
877-
with self.subTest(resp_type=resp_type):
881+
with self.subTest(resp_type=resp_type.__name__):
878882

879883
execute_resp = resp_type(
880884
status=self.okay_status,
@@ -937,7 +941,7 @@ def test_handle_execute_response_can_handle_with_direct_results(self):
937941
)
938942

939943
for resp_type in self.execute_response_types:
940-
with self.subTest(resp_type=resp_type):
944+
with self.subTest(resp_type=resp_type.__name__):
941945
execute_resp = resp_type(
942946
status=self.okay_status,
943947
directResults=direct_results_message,
@@ -1041,7 +1045,7 @@ def test_handle_execute_response_reads_has_more_rows_in_direct_results(
10411045
for has_more_rows, resp_type in itertools.product(
10421046
[True, False], self.execute_response_types
10431047
):
1044-
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type):
1048+
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type.__name__):
10451049
tcli_service_instance = tcli_service_class.return_value
10461050
results_mock = Mock()
10471051
results_mock.startRowOffset = 0
@@ -1087,7 +1091,7 @@ def test_handle_execute_response_reads_has_more_rows_in_result_response(
10871091
for has_more_rows, resp_type in itertools.product(
10881092
[True, False], self.execute_response_types
10891093
):
1090-
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type):
1094+
with self.subTest(has_more_rows=has_more_rows, resp_type=resp_type.__name__):
10911095
tcli_service_instance = tcli_service_class.return_value
10921096
results_mock = MagicMock()
10931097
results_mock.startRowOffset = 0

0 commit comments

Comments
 (0)