|
13 | 13 | TExecuteStatementResp, |
14 | 14 | TOperationHandle, |
15 | 15 | THandleIdentifier, |
| 16 | + TOperationState, |
16 | 17 | TOperationType, |
17 | 18 | ) |
18 | 19 | from databricks.sql.thrift_backend import ThriftBackend |
|
23 | 24 | from databricks.sql.exc import RequestError, CursorAlreadyClosedError |
24 | 25 | from databricks.sql.types import Row |
25 | 26 |
|
| 27 | +from databricks.sql.utils import ExecuteResponse |
26 | 28 | from tests.unit.test_fetches import FetchTests |
27 | 29 | from tests.unit.test_thrift_backend import ThriftBackendTestSuite |
28 | 30 | from tests.unit.test_arrow_queue import ArrowQueueSuite |
@@ -168,22 +170,74 @@ def test_useragent_header(self, mock_client_class): |
168 | 170 | http_headers = mock_client_class.call_args[0][3] |
169 | 171 | self.assertIn(user_agent_header_with_entry, http_headers) |
170 | 172 |
|
171 | | - @patch("%s.client.ThriftBackend" % PACKAGE_NAME, ThriftBackendMockFactory.new()) |
172 | | - @patch("%s.client.ResultSet" % PACKAGE_NAME) |
173 | | - def test_closing_connection_closes_commands(self, mock_result_set_class): |
| 173 | + @patch("%s.client.ThriftBackend" % PACKAGE_NAME) |
| 174 | + def test_closing_connection_closes_commands(self, mock_thrift_client_class): |
174 | 175 | # Test once with has_been_closed_server side, once without |
175 | 176 | for closed in (True, False): |
176 | 177 | with self.subTest(closed=closed): |
177 | | - mock_result_set_class.return_value = Mock() |
| 178 | + initial_state = ( |
| 179 | + TOperationState.FINISHED_STATE |
| 180 | + if not closed |
| 181 | + else TOperationState.CLOSED_STATE |
| 182 | + ) |
| 183 | + |
| 184 | + # Mock the execute response with controlled state |
| 185 | + mock_execute_response = Mock(spec=ExecuteResponse) |
| 186 | + mock_execute_response.status = initial_state |
| 187 | + mock_execute_response.has_been_closed_server_side = closed |
| 188 | + mock_execute_response.is_staging_operation = False |
| 189 | + |
| 190 | + # Mock the backend that will be used by the real ThriftResultSet |
| 191 | + mock_backend = Mock(spec=ThriftBackend) |
| 192 | + |
| 193 | + # Configure the decorator's mock to return our specific mock_backend |
| 194 | + mock_thrift_client_class.return_value = mock_backend |
| 195 | + |
| 196 | + # Create connection and cursor |
178 | 197 | connection = databricks.sql.connect(**self.DUMMY_CONNECTION_ARGS) |
179 | 198 | cursor = connection.cursor() |
180 | | - cursor.execute("SELECT 1;") |
| 199 | + |
| 200 | + # Verify initial state |
| 201 | + self.assertEqual( |
| 202 | + mock_execute_response.has_been_closed_server_side, closed |
| 203 | + ) |
| 204 | + self.assertEqual(mock_execute_response.status, initial_state) |
| 205 | + |
| 206 | + # Mock execute_command to return our execute response |
| 207 | + cursor.thrift_backend.execute_command = Mock( |
| 208 | + return_value=mock_execute_response |
| 209 | + ) |
| 210 | + |
| 211 | + # Execute a command - this should set cursor.active_result_set to our real result set |
| 212 | + cursor.execute("SELECT 1") |
| 213 | + |
| 214 | + # Verify that cursor.execute() set up the result set correctly |
| 215 | + active_result_set = cursor.active_result_set |
| 216 | + self.assertEqual(active_result_set.has_been_closed_server_side, closed) |
| 217 | + |
| 218 | + # Close the connection - this should trigger the real close chain: |
| 219 | + # connection.close() -> cursor.close() -> result_set.close() |
181 | 220 | connection.close() |
182 | 221 |
|
183 | | - self.assertTrue( |
184 | | - mock_result_set_class.return_value.has_been_closed_server_side |
| 222 | + # Verify the REAL close logic worked through the chain: |
| 223 | + # 1. has_been_closed_server_side should always be True after close() |
| 224 | + self.assertTrue(active_result_set.has_been_closed_server_side) |
| 225 | + |
| 226 | + # 2. op_state should always be CLOSED after close() |
| 227 | + self.assertEqual( |
| 228 | + active_result_set.op_state, |
| 229 | + connection.thrift_backend.CLOSED_OP_STATE, |
185 | 230 | ) |
186 | | - mock_result_set_class.return_value.close.assert_called_once_with() |
| 231 | + |
| 232 | + # 3. Backend close_command should be called appropriately |
| 233 | + if not closed: |
| 234 | + # Should have called backend.close_command during the close chain |
| 235 | + mock_backend.close_command.assert_called_once_with( |
| 236 | + mock_execute_response.command_handle |
| 237 | + ) |
| 238 | + else: |
| 239 | + # Should NOT have called backend.close_command (already closed) |
| 240 | + mock_backend.close_command.assert_not_called() |
187 | 241 |
|
188 | 242 | @patch("%s.client.ThriftBackend" % PACKAGE_NAME) |
189 | 243 | def test_cant_open_cursor_on_closed_connection(self, mock_client_class): |
|
0 commit comments