🧪 [testing improvement] Add comprehensive tests for ingest_texture

remix_api.py

@@ -351,17 +351,28 @@ def get_material_textures(self, material_prim):
         return None, res.get("error", "Failed to get textures.")
 
     def ingest_texture(self, pbr_type, texture_file_path, project_output_dir_abs):
+        """
+        Asks Stagecraft to ingest a texture file.
+        Returns (success_bool, result_path_or_error_msg).
+        """
+        if not texture_file_path or not pbr_type:
+            return False, "Invalid arguments to ingest_texture"
+
+        # First get default output dir if we don't have one
+        if not project_output_dir_abs:
+            project_output_dir_abs = self.get_project_default_output_dir()
+
         self._log_info(f"Ingesting {pbr_type}: {self.safe_basename(texture_file_path)}")
 
         if not os.path.isfile(texture_file_path):
-             return None, f"File not found: {texture_file_path}"
+             return False, f"File not found: {texture_file_path}"
 
         settings = self.settings_getter()
         output_subfolder = settings.get("remix_output_subfolder", "Textures/PainterConnector_Ingested").strip('/\\')
         target_ingest_dir_abs = os.path.normpath(os.path.join(project_output_dir_abs, output_subfolder))
 
         try: os.makedirs(target_ingest_dir_abs, exist_ok=True)
-        except Exception as e: return None, f"Failed to create directory: {e}"
+        except Exception as e: return False, f"Failed to create directory: {e}"
 
         abs_texture_path = os.path.abspath(texture_file_path).replace(os.sep, '/')
         target_ingest_dir_api = os.path.abspath(target_ingest_dir_abs).replace(os.sep, '/')
@@ -419,7 +430,7 @@ def ingest_texture(self, pbr_type, texture_file_path, project_output_dir_abs):
             retries=1,
             timeout=INGEST_REQUEST_TIMEOUT_SECONDS,
         )
-        if not res["success"]: return None, res.get("error")
+        if not res["success"]: return False, res.get("error")
 
         # Parse response to find output path
         original_base = os.path.splitext(self.safe_basename(texture_file_path))[0]
@@ -499,12 +510,12 @@ def _normalize_ingest_output_stem(dds_path_str: str):
         if not ingested_path and fallback_match:
             ingested_path = fallback_match
 
-        if not ingested_path: return None, "Could not identify output path from API response."
+        if not ingested_path: return False, "Could not identify output path from API response."
 
         final_path = os.path.normpath(ingested_path) if os.path.isabs(ingested_path) else os.path.normpath(os.path.join(target_ingest_dir_api.replace('/', os.sep), ingested_path))
-        if not os.path.isfile(final_path): return None, f"File missing: {final_path}"
+        if not os.path.isfile(final_path): return False, f"File missing: {final_path}"
 
-        return final_path, None
+        return True, final_path
 
     def get_current_edit_target(self):
         self._log_info("Getting current edit target layer from Remix...")

tests/test_remix_api.py

@@ -8,11 +8,11 @@
 # in environments where requests is not installed (e.g. minimal CI images).
 # remix_api treats requests as optional and guards all usage behind _get_session().
 _req_mock = MagicMock()
-_ConnErr = type("ConnectionError", (OSError,), {})
-_Timeout = type("Timeout", (OSError,), {})
+_ReqException = type("RequestException", (OSError,), {}); _ConnErr = type("ConnectionError", (_ReqException,), {})
+_Timeout = type("Timeout", (_ReqException,), {})
 _req_mock.exceptions.ConnectionError = _ConnErr
 _req_mock.exceptions.Timeout = _Timeout
-_req_mock.exceptions.RequestException = type("RequestException", (OSError,), {})
+_req_mock.exceptions.RequestException = _ReqException
 sys.modules.setdefault("requests", _req_mock)
 
 sys.path.insert(0, os.path.dirname(os.path.dirname(__file__)))
@@ -159,3 +159,142 @@ def test_ping_failure(self):
 
 if __name__ == "__main__":
     unittest.main()
+
+class TestIngestTexture(unittest.TestCase):
+    def setUp(self):
+        self.client = _make_client()
+
+    def test_invalid_arguments(self):
+        success, err = self.client.ingest_texture(None, "/fake/file.png", "/out")
+        self.assertFalse(success)
+        self.assertIn("Invalid arguments", err)
+
+        success, err = self.client.ingest_texture("albedo", None, "/out")
+        self.assertFalse(success)
+        self.assertIn("Invalid arguments", err)
+
+    @patch("os.path.isfile")
+    def test_fallback_output_dir(self, mock_isfile):
+        mock_isfile.return_value = False
+        with patch.object(self.client, "get_project_default_output_dir", return_value="/default/out"):
+            success, err = self.client.ingest_texture("albedo", "/fake/file.png", None)
+            self.assertFalse(success)
+            self.assertIn("File not found", err)
+
+    @patch("os.path.isfile")
+    def test_file_not_found(self, mock_isfile):
+        mock_isfile.return_value = False
+        success, err = self.client.ingest_texture("albedo", "/fake/file.png", "/out")
+        self.assertFalse(success)
+        self.assertIn("File not found", err)
+
+    @patch("os.path.isfile")
+    @patch("os.makedirs")
+    def test_mkdir_fails(self, mock_makedirs, mock_isfile):
+        mock_isfile.return_value = True
+        mock_makedirs.side_effect = Exception("access denied")
+        success, err = self.client.ingest_texture("albedo", "/fake/file.png", "/out")
+        self.assertFalse(success)
+        self.assertIn("Failed to create directory", err)
+
+    @patch("os.path.isfile")
+    @patch("os.makedirs")
+    def test_api_failure(self, mock_makedirs, mock_isfile):
+        mock_isfile.return_value = True
+        mock_makedirs.return_value = None
+
+        with patch.object(self.client, "make_request", return_value={"success": False, "error": "api down"}):
+            success, err = self.client.ingest_texture("albedo", "/fake/file.png", "/out")
+            self.assertFalse(success)
+            self.assertEqual(err, "api down")
+
+    @patch("os.path.isfile")
+    @patch("os.makedirs")
+    def test_success_completed_schemas(self, mock_makedirs, mock_isfile):
+        # We need os.path.isfile to be True for the initial check, and True for the final check.
+        # final check calls os.path.isfile(final_path)
+        mock_isfile.return_value = True
+        mock_makedirs.return_value = None
+
+        api_resp = {
+            "success": True,
+            "data": {
+                "completed_schemas": [
+                    {
+                        "check_plugins": [
+                            {
+                                "data": {
+                                    "data_flows": [
+                                        {"channel": "ingestion_output", "output_data": ["file.a.rtex.dds"]}
+                                    ]
+                                }
+                            }
+                        ]
+                    }
+                ]
+            }
+        }
+        with patch.object(self.client, "make_request", return_value=api_resp):
+            # pbr_type "albedo" expects suffix "a"
+            success, path = self.client.ingest_texture("albedo", "/fake/file.png", "/out")
+            self.assertTrue(success)
+            self.assertTrue(path.endswith("file.a.rtex.dds"))
+
+    @patch("os.path.isfile")
+    @patch("os.makedirs")
+    def test_success_content_fallback(self, mock_makedirs, mock_isfile):
+        mock_isfile.return_value = True
+        mock_makedirs.return_value = None
+
+        api_resp = {
+            "success": True,
+            "data": {
+                "content": ["file.n.rtex.dds"]
+            }
+        }
+        with patch.object(self.client, "make_request", return_value=api_resp):
+            # normal map -> expects suffix "n"
+            success, path = self.client.ingest_texture("normal", "/fake/file.png", "/out")
+            self.assertTrue(success)
+            self.assertTrue(path.endswith("file.n.rtex.dds"))
+
+    @patch("os.path.isfile")
+    @patch("os.makedirs")
+    def test_output_path_not_identified(self, mock_makedirs, mock_isfile):
+        mock_isfile.return_value = True
+        mock_makedirs.return_value = None
+
+        # Missing paths entirely
+        api_resp = {
+            "success": True,
+            "data": {}
+        }
+        with patch.object(self.client, "make_request", return_value=api_resp):
+            success, err = self.client.ingest_texture("albedo", "/fake/file.png", "/out")
+            self.assertFalse(success)
+            self.assertIn("Could not identify output path", err)
+
+    @patch("os.path.isfile")
+    @patch("os.makedirs")
+    def test_file_missing_after_ingest(self, mock_makedirs, mock_isfile):
+        # Return True for the initial check, False for the final check.
+        # We can use side_effect with a list or a small function.
+        def isfile_side_effect(p):
+            # the initial file check
+            if p == "/fake/file.png": return True
+            # the final file check
+            return False
+
+        mock_isfile.side_effect = isfile_side_effect
+        mock_makedirs.return_value = None
+
+        api_resp = {
+            "success": True,
+            "data": {
+                "content": ["file.a.rtex.dds"]
+            }
+        }
+        with patch.object(self.client, "make_request", return_value=api_resp):
+            success, err = self.client.ingest_texture("albedo", "/fake/file.png", "/out")
+            self.assertFalse(success)
+            self.assertIn("File missing", err)