fixed (most) tests by accounting for normalised Session interface

have not considered how to fix the protocol version check yet

Author: varun-edachali-dbx

tests/e2e/test_driver.py

@@ -824,9 +824,7 @@ def test_close_connection_closes_cursors(self):
             status_request = ttypes.TGetOperationStatusReq(
                 operationHandle=ars.command_id, getProgressUpdate=False
             )
-            op_status_at_server = ars.backend._client.GetOperationStatus(
-                status_request
-            )
+            op_status_at_server = ars.backend._client.GetOperationStatus(status_request)
             assert (
                 op_status_at_server.operationState
                 != ttypes.TOperationState.CLOSED_STATE
@@ -856,17 +854,19 @@ def test_closing_a_closed_connection_doesnt_fail(self, caplog):
                     raise KeyboardInterrupt("Simulated interrupt")
         finally:
             if conn is not None:
-                assert not conn.open, "Connection should be closed after KeyboardInterrupt"
+                assert (
+                    not conn.open
+                ), "Connection should be closed after KeyboardInterrupt"
 
     def test_cursor_close_properly_closes_operation(self):
         """Test that Cursor.close() properly closes the active operation handle on the server."""
         with self.connection() as conn:
             cursor = conn.cursor()
             try:
                 cursor.execute("SELECT 1 AS test")
-                assert cursor.active_op_handle is not None
+                assert cursor.active_command_id is not None
                 cursor.close()
-                assert cursor.active_op_handle is None
+                assert cursor.active_command_id is None
                 assert not cursor.open
             finally:
                 if cursor.open:
@@ -883,26 +883,28 @@ def test_cursor_close_properly_closes_operation(self):
                         raise KeyboardInterrupt("Simulated interrupt")
         finally:
             if cursor is not None:
-                assert not cursor.open, "Cursor should be closed after KeyboardInterrupt"
+                assert (
+                    not cursor.open
+                ), "Cursor should be closed after KeyboardInterrupt"
 
     def test_nested_cursor_context_managers(self):
         """Test that nested cursor context managers properly close operations on the server."""
         with self.connection() as conn:
             with conn.cursor() as cursor1:
                 cursor1.execute("SELECT 1 AS test1")
-                assert cursor1.active_op_handle is not None
+                assert cursor1.active_command_id is not None
 
                 with conn.cursor() as cursor2:
                     cursor2.execute("SELECT 2 AS test2")
-                    assert cursor2.active_op_handle is not None
+                    assert cursor2.active_command_id is not None
 
                 # After inner context manager exit, cursor2 should be not open
                 assert not cursor2.open
-                assert cursor2.active_op_handle is None
+                assert cursor2.active_command_id is None
 
             # After outer context manager exit, cursor1 should be not open
             assert not cursor1.open
-            assert cursor1.active_op_handle is None
+            assert cursor1.active_command_id is None
 
     def test_cursor_error_handling(self):
         """Test that cursor close handles errors properly to prevent orphaned operations."""
@@ -911,7 +913,7 @@ def test_cursor_error_handling(self):
 
             cursor.execute("SELECT 1 AS test")
 
-            op_handle = cursor.active_op_handle
+            op_handle = cursor.active_command_id
 
             assert op_handle is not None
 

tests/unit/test_client.py

@@ -22,6 +22,7 @@
 from databricks.sql import InterfaceError, DatabaseError, Error, NotSupportedError
 from databricks.sql.exc import RequestError, CursorAlreadyClosedError
 from databricks.sql.types import Row
+from databricks.sql.client import CommandId
 
 from tests.unit.test_fetches import FetchTests
 from tests.unit.test_thrift_backend import ThriftBackendTestSuite
@@ -81,7 +82,10 @@ class ClientTestSuite(unittest.TestCase):
         "access_token": "tok",
     }
 
-    @patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME, ThriftDatabricksClientMockFactory.new())
+    @patch(
+        "%s.session.ThriftDatabricksClient" % PACKAGE_NAME,
+        ThriftDatabricksClientMockFactory.new(),
+    )
     @patch("%s.client.ResultSet" % PACKAGE_NAME)
     def test_closing_connection_closes_commands(self, mock_result_set_class):
         # Test once with has_been_closed_server side, once without
@@ -286,10 +290,10 @@ def test_get_columns_parameters_passed_to_thrift_backend(self, mock_thrift_backe
     def test_cancel_command_calls_the_backend(self):
         mock_thrift_backend = Mock()
         cursor = client.Cursor(Mock(), mock_thrift_backend)
-        mock_op_handle = Mock()
-        cursor.active_op_handle = mock_op_handle
+        mock_command_id = Mock()
+        cursor.active_command_id = mock_command_id
         cursor.cancel()
-        mock_thrift_backend.cancel_command.assert_called_with(mock_op_handle)
+        mock_thrift_backend.cancel_command.assert_called_with(mock_command_id)
 
     @patch("databricks.sql.client.logger")
     def test_cancel_command_will_issue_warning_for_cancel_with_no_executing_command(
@@ -505,7 +509,10 @@ def test_staging_operation_response_is_handled(
 
         mock_handle_staging_operation.call_count == 1
 
-    @patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME, ThriftDatabricksClientMockFactory.new())
+    @patch(
+        "%s.session.ThriftDatabricksClient" % PACKAGE_NAME,
+        ThriftDatabricksClientMockFactory.new(),
+    )
     def test_access_current_query_id(self):
         operation_id = "EE6A8778-21FC-438B-92D8-96AC51EE3821"
 
@@ -514,9 +521,13 @@ def test_access_current_query_id(self):
 
         self.assertIsNone(cursor.query_id)
 
-        cursor.active_op_handle = TOperationHandle(
-            operationId=THandleIdentifier(guid=UUID(operation_id).bytes, secret=0x00),
-            operationType=TOperationType.EXECUTE_STATEMENT,
+        cursor.active_command_id = CommandId.from_thrift_handle(
+            TOperationHandle(
+                operationId=THandleIdentifier(
+                    guid=UUID(operation_id).bytes, secret=0x00
+                ),
+                operationType=TOperationType.EXECUTE_STATEMENT,
+            )
         )
         self.assertEqual(cursor.query_id.upper(), operation_id.upper())
 
@@ -527,88 +538,90 @@ def test_cursor_close_handles_exception(self):
         """Test that Cursor.close() handles exceptions from close_command properly."""
         mock_backend = Mock()
         mock_connection = Mock()
-        mock_op_handle = Mock()
-        
+        mock_command_id = Mock()
+
         mock_backend.close_command.side_effect = Exception("Test error")
 
         cursor = client.Cursor(mock_connection, mock_backend)
-        cursor.active_op_handle = mock_op_handle
+        cursor.active_command_id = mock_command_id
 
         cursor.close()
 
-        mock_backend.close_command.assert_called_once_with(mock_op_handle)
-        
-        self.assertIsNone(cursor.active_op_handle)
-        
+        mock_backend.close_command.assert_called_once_with(mock_command_id)
+
+        self.assertIsNone(cursor.active_command_id)
+
         self.assertFalse(cursor.open)
 
     def test_cursor_context_manager_handles_exit_exception(self):
         """Test that cursor's context manager handles exceptions during __exit__."""
         mock_backend = Mock()
         mock_connection = Mock()
-        
+
         cursor = client.Cursor(mock_connection, mock_backend)
         original_close = cursor.close
         cursor.close = Mock(side_effect=Exception("Test error during close"))
-        
+
         try:
             with cursor:
                 raise ValueError("Test error inside context")
         except ValueError:
             pass
-        
+
         cursor.close.assert_called_once()
 
     def test_connection_close_handles_cursor_close_exception(self):
         """Test that _close handles exceptions from cursor.close() properly."""
         cursors_closed = []
-        
+
         def mock_close_with_exception():
             cursors_closed.append(1)
             raise Exception("Test error during close")
-        
+
         cursor1 = Mock()
         cursor1.close = mock_close_with_exception
-        
+
         def mock_close_normal():
             cursors_closed.append(2)
-        
+
         cursor2 = Mock()
         cursor2.close = mock_close_normal
-        
+
         mock_backend = Mock()
         mock_session_handle = Mock()
-        
+
         try:
             for cursor in [cursor1, cursor2]:
                 try:
                     cursor.close()
                 except Exception:
                     pass
-                    
+
             mock_backend.close_session(mock_session_handle)
         except Exception as e:
             self.fail(f"Connection close should handle exceptions: {e}")
-        
-        self.assertEqual(cursors_closed, [1, 2], "Both cursors should have close called")
+
+        self.assertEqual(
+            cursors_closed, [1, 2], "Both cursors should have close called"
+        )
 
     def test_resultset_close_handles_cursor_already_closed_error(self):
         """Test that ResultSet.close() handles CursorAlreadyClosedError properly."""
         result_set = client.ResultSet.__new__(client.ResultSet)
         result_set.thrift_backend = Mock()
-        result_set.thrift_backend.CLOSED_OP_STATE = 'CLOSED'
+        result_set.thrift_backend.CLOSED_OP_STATE = "CLOSED"
         result_set.connection = Mock()
         result_set.connection.open = True
-        result_set.op_state = 'RUNNING'
+        result_set.op_state = "RUNNING"
         result_set.has_been_closed_server_side = False
         result_set.command_id = Mock()
 
         class MockRequestError(Exception):
             def __init__(self):
                 self.args = ["Error message", CursorAlreadyClosedError()]
-        
+
         result_set.thrift_backend.close_command.side_effect = MockRequestError()
-        
+
         original_close = client.ResultSet.close
         try:
             try:
@@ -624,11 +637,13 @@ def __init__(self):
             finally:
                 result_set.has_been_closed_server_side = True
                 result_set.op_state = result_set.thrift_backend.CLOSED_OP_STATE
-                
-            result_set.thrift_backend.close_command.assert_called_once_with(result_set.command_id)
-            
+
+            result_set.thrift_backend.close_command.assert_called_once_with(
+                result_set.command_id
+            )
+
             assert result_set.has_been_closed_server_side is True
-            
+
             assert result_set.op_state == result_set.thrift_backend.CLOSED_OP_STATE
         finally:
             pass

tests/unit/test_fetches.py

@@ -65,7 +65,7 @@ def make_dummy_result_set_from_batch_list(batch_list):
         batch_index = 0
 
         def fetch_results(
-            op_handle,
+            command_id,
             max_rows,
             max_bytes,
             expected_row_start_offset,