SK-2842: improve coverage and revert details in skyflow error class

Author: saileshwar-skyflow

skyflow/error/_skyflow_error.py

@@ -12,6 +12,6 @@ def __init__(self,
         self.http_code = http_code
         self.grpc_code = grpc_code
         self.http_status = http_status if http_status else SkyflowMessages.HttpStatus.BAD_REQUEST.value
-        self.details = details if details else None
+        self.details = details if details else []
         self.request_id = request_id
         super().__init__(message)

tests/vault/connection/__init__.py

@@ -0,0 +1,26 @@
+import unittest
+from skyflow.vault.connection._invoke_connection_response import InvokeConnectionResponse
+
+
+class TestInvokeConnectionResponse(unittest.TestCase):
+    def test_repr(self):
+        r = InvokeConnectionResponse(data={"key": "val"}, metadata={"m": 1}, errors=None)
+        self.assertIn("ConnectionResponse", repr(r))
+
+    def test_str(self):
+        r = InvokeConnectionResponse(data={"key": "val"})
+        self.assertEqual(str(r), repr(r))
+
+    def test_defaults(self):
+        r = InvokeConnectionResponse()
+        self.assertIsNone(r.data)
+        self.assertEqual(r.metadata, {})
+        self.assertIsNone(r.errors)
+
+    def test_metadata_defaults_to_empty_dict_when_falsy(self):
+        r = InvokeConnectionResponse(metadata=None)
+        self.assertEqual(r.metadata, {})
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/vault/connection/test_responses.py

@@ -0,0 +1,27 @@
+import unittest
+from skyflow.vault.controller._audit import Audit
+from skyflow.vault.controller._bin_look_up import BinLookUp
+
+
+class TestAudit(unittest.TestCase):
+    def test_instantiation(self):
+        a = Audit()
+        self.assertIsNotNone(a)
+
+    def test_list_returns_none(self):
+        a = Audit()
+        self.assertIsNone(a.list())
+
+
+class TestBinLookUp(unittest.TestCase):
+    def test_instantiation(self):
+        b = BinLookUp()
+        self.assertIsNotNone(b)
+
+    def test_get_returns_none(self):
+        b = BinLookUp()
+        self.assertIsNone(b.get())
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/vault/controller/test__audit_binlookup.py

@@ -781,6 +781,48 @@ def test_save_output_write_exception(self):
                     response, tmp_dir, "file.txt", "file"
                 )
 
+    def test_save_output_no_file_extension_uses_original_name(self):
+        """Branches 113->117 and 119->124: processed_file_extension is falsy — safe_ext stays None."""
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            output = Mock()
+            output.processedFile = base64.b64encode(b"data").decode()
+            output.processedFileType = "redacted_file"
+            output.processedFileExtension = None
+            output.processed_file_extension = None
+            response = Mock()
+            response.output = [output]
+            self.detect._Detect__save_deidentify_file_response_output(
+                response, tmp_dir, "original.txt", "original"
+            )
+
+    @patch("skyflow.vault.controller._detect.time.sleep", return_value=None)
+    def test_poll_unknown_status_then_success(self, mock_sleep):
+        """Branch 80->65: status is unknown, loops back, then returns SUCCESS."""
+        files_api = Mock()
+        files_api.with_raw_response = files_api
+        self.vault_client.get_detect_file_api.return_value = files_api
+
+        call_count = {"n": 0}
+
+        def side_effect(*args, **kwargs):
+            call_count["n"] += 1
+            r = Mock()
+            if call_count["n"] == 1:
+                r.status = "UNKNOWN_STATUS"
+            else:
+                r.status = "SUCCESS"
+            return Mock(data=r)
+
+        files_api.get_run.side_effect = side_effect
+        result = self.detect._Detect__poll_for_processed_file("runid", max_wait_time=10)
+        self.assertEqual(result.status, "SUCCESS")
+
+    def test_get_file_from_request_no_file_no_path_returns_none(self):
+        """Branch 285->exit: file_input has neither file nor file_path set."""
+        req = DeidentifyFileRequest(file=FileInput(file=None, file_path=None))
+        result = self.detect._Detect__get_file_from_request(req)
+        self.assertIsNone(result)
+
     @patch("skyflow.vault.controller._detect.validate_deidentify_file_request")
     @patch("skyflow.vault.controller._detect.base64")
     def test_deidentify_file_api_error_inside_try(self, mock_base64, mock_validate):

tests/vault/controller/test__detect.py

@@ -742,6 +742,56 @@ def test_upload_file_without_skyflow_id_successful(self, mock_validate):
         self.assertEqual(result.skyflow_id, "generated-id-123")
         self.assertIsNone(result.errors)
 
+    @patch("skyflow.vault.controller._vault.validate_file_upload_request")
+    @patch("skyflow.vault.controller._vault.open", mock_open(read_data=b"file_content"), create=True)
+    def test_upload_file_file_path_with_existing_file_name(self, mock_validate):
+        """Branch 73->76: file_name already set when file_path is present — skips basename call."""
+        request = FileUploadRequest(
+            table=TABLE_NAME,
+            column_name="file_col",
+            file_path="/path/to/file.txt",
+            file_name="already_set.txt"
+        )
+        mock_api = self.vault_client.get_records_api.return_value.with_raw_response
+        mock_response = Mock()
+        mock_response.data.skyflow_id = "sky123"
+        mock_api.upload_file_v_2.return_value = mock_response
+
+        self.vault.upload_file(request)
+        mock_api.upload_file_v_2.assert_called_once()
+
+    @patch("skyflow.vault.controller._vault.validate_file_upload_request")
+    def test_upload_file_file_object_without_name_attr(self, mock_validate):
+        """Branch 84->89: file_object has no 'name' attr — __get_file_for_file_upload returns None."""
+        file_obj = Mock(spec=[])
+        request = FileUploadRequest(
+            table=TABLE_NAME,
+            column_name="file_col",
+            file_object=file_obj
+        )
+        mock_api = self.vault_client.get_records_api.return_value.with_raw_response
+        mock_response = Mock()
+        mock_response.data.skyflow_id = "sky123"
+        mock_api.upload_file_v_2.return_value = mock_response
+
+        self.vault.upload_file(request)
+        mock_api.upload_file_v_2.assert_called_once()
+
+    @patch("skyflow.vault.controller._vault.validate_file_upload_request")
+    def test_upload_file_no_file_source_returns_none_file(self, mock_validate):
+        """Branch 84->89 (elif False): all file sources None — __get_file_for_file_upload returns None."""
+        request = FileUploadRequest(
+            table=TABLE_NAME,
+            column_name="file_col"
+        )
+        mock_api = self.vault_client.get_records_api.return_value.with_raw_response
+        mock_response = Mock()
+        mock_response.data.skyflow_id = "sky123"
+        mock_api.upload_file_v_2.return_value = mock_response
+
+        self.vault.upload_file(request)
+        mock_api.upload_file_v_2.assert_called_once()
+
 class TestFileUploadValidation(unittest.TestCase):
     def setUp(self):
         self.logger = Mock()

tests/vault/controller/test__vault.py

@@ -0,0 +1,108 @@
+import unittest
+from skyflow.vault.data._delete_response import DeleteResponse
+from skyflow.vault.data._file_upload_response import FileUploadResponse
+from skyflow.vault.data._get_response import GetResponse
+from skyflow.vault.data._insert_response import InsertResponse
+from skyflow.vault.data._query_response import QueryResponse
+from skyflow.vault.data._update_response import UpdateResponse
+from skyflow.vault.data._upload_file_request import UploadFileRequest
+
+
+class TestDeleteResponse(unittest.TestCase):
+    def test_repr(self):
+        r = DeleteResponse(deleted_ids=["id1"], errors=None)
+        self.assertIn("DeleteResponse", repr(r))
+        self.assertIn("id1", repr(r))
+
+    def test_str(self):
+        r = DeleteResponse(deleted_ids=["id1"], errors=None)
+        self.assertEqual(str(r), repr(r))
+
+    def test_defaults(self):
+        r = DeleteResponse()
+        self.assertIsNone(r.deleted_ids)
+        self.assertIsNone(r.errors)
+
+
+class TestFileUploadResponse(unittest.TestCase):
+    def test_repr(self):
+        r = FileUploadResponse(skyflow_id="sky123", errors=None)
+        self.assertIn("FileUploadResponse", repr(r))
+        self.assertIn("sky123", repr(r))
+
+    def test_str(self):
+        r = FileUploadResponse(skyflow_id="sky123", errors=None)
+        self.assertEqual(str(r), repr(r))
+
+
+class TestGetResponse(unittest.TestCase):
+    def test_repr(self):
+        r = GetResponse(data=[{"field": "val"}], errors=None)
+        self.assertIn("GetResponse", repr(r))
+
+    def test_str(self):
+        r = GetResponse(data=[{"field": "val"}], errors=None)
+        self.assertEqual(str(r), repr(r))
+
+    def test_none_data_defaults_to_empty_list(self):
+        r = GetResponse(data=None)
+        self.assertEqual(r.data, [])
+
+    def test_empty_data_not_replaced(self):
+        r = GetResponse(data={})
+        self.assertEqual(r.data, {})
+
+
+class TestInsertResponse(unittest.TestCase):
+    def test_repr(self):
+        r = InsertResponse(inserted_fields=[{"skyflow_id": "id1"}], errors=None)
+        self.assertIn("InsertResponse", repr(r))
+
+    def test_str(self):
+        r = InsertResponse(inserted_fields=[{"skyflow_id": "id1"}])
+        self.assertEqual(str(r), repr(r))
+
+    def test_defaults(self):
+        r = InsertResponse()
+        self.assertIsNone(r.inserted_fields)
+        self.assertIsNone(r.errors)
+
+
+class TestQueryResponse(unittest.TestCase):
+    def test_repr(self):
+        r = QueryResponse()
+        self.assertIn("QueryResponse", repr(r))
+
+    def test_str(self):
+        r = QueryResponse()
+        self.assertEqual(str(r), repr(r))
+
+    def test_defaults(self):
+        r = QueryResponse()
+        self.assertEqual(r.fields, [])
+        self.assertIsNone(r.errors)
+
+
+class TestUpdateResponse(unittest.TestCase):
+    def test_repr(self):
+        r = UpdateResponse(updated_field={"skyflow_id": "id1"}, errors=None)
+        self.assertIn("UpdateResponse", repr(r))
+
+    def test_str(self):
+        r = UpdateResponse(updated_field={"skyflow_id": "id1"})
+        self.assertEqual(str(r), repr(r))
+
+    def test_defaults(self):
+        r = UpdateResponse()
+        self.assertIsNone(r.updated_field)
+        self.assertIsNone(r.errors)
+
+
+class TestUploadFileRequest(unittest.TestCase):
+    def test_instantiation(self):
+        r = UploadFileRequest()
+        self.assertIsNotNone(r)
+
+
+if __name__ == "__main__":
+    unittest.main()