Merge pull request #173 from seqeralabs/fix-resource-creation-error-h…

…andling

FIx handling of resource creation error when emitting JSON output

seqerakit/cli.py

@@ -111,9 +111,9 @@ def __init__(self, sp, list_for_add_method):
         self.sp = sp
         self.list_for_add_method = list_for_add_method
 
-        # Create a separate instance of the Seqera Platform client without JSON output
-        # This is needed because when checking if resources exist during overwrite operations,
-        # we don't want JSON output mixed with the actual resource creation output
+        # Create a separate Seqera Platform client instance without
+        # JSON output to avoid mixing resource checks with creation
+        # output during overwrite operations.
         sp_without_json = seqeraplatform.SeqeraPlatform(
             cli_args=sp.cli_args,
             dryrun=sp.dryrun,

seqerakit/seqeraplatform.py

@@ -134,19 +134,22 @@ def _execute_command(self, full_cmd, to_json=False, print_stdout=True):
         if should_print and not self.json:
             logging.info(f" Command output: {stdout}")
 
-        if "ERROR: " in stdout or process.returncode != 0:
-            self._handle_command_errors(stdout)
-
+        # Try JSON parsing first
         if self.json or to_json:
-            out = json.loads(stdout)
-            if should_print:
-                print(json.dumps(out))
-        else:
-            out = stdout
-            if should_print:
-                print(stdout)
+            try:
+                out = json.loads(stdout)
+                if should_print:
+                    print(json.dumps(out))
+                return out
+            except json.JSONDecodeError:
+                pass
+
+        if process.returncode != 0 and "ERROR: " in stdout:
+            self._handle_command_errors(stdout)
 
-        return out
+        if should_print:
+            print(stdout)
+        return stdout
 
     def _handle_command_errors(self, stdout):
         # Check for specific tw cli error patterns and raise custom exceptions

tests/unit/test_seqeraplatform.py

@@ -1,6 +1,7 @@
 import unittest
 from unittest.mock import MagicMock, patch
 from seqerakit import seqeraplatform
+from seqerakit.seqeraplatform import ResourceCreationError
 import json
 import subprocess
 import os
@@ -382,11 +383,9 @@ def test_print_stdout_override(self, mock_subprocess):
         log_output = self.log_capture.getvalue()
         self.assertIn("Command output: output", log_output)
 
-        # Clear the log capture
         self.log_capture.truncate(0)
         self.log_capture.seek(0)
 
-        # Test with print_stdout=False
         self.sp._execute_command("tw pipelines list", print_stdout=False)
         log_output = self.log_capture.getvalue()
         self.assertNotIn("Command output: output", log_output)
@@ -408,8 +407,53 @@ def test_json_parsing_error(self, mock_subprocess):
         mock_subprocess.return_value = MagicMock(returncode=0)
         mock_subprocess.return_value.communicate.return_value = (b"Invalid JSON", b"")
 
-        with self.assertRaises(json.JSONDecodeError):
-            self.sp._execute_command("tw pipelines list", to_json=True)
+        result = self.sp._execute_command("tw pipelines list", to_json=True)
+
+        self.assertEqual(result, "Invalid JSON")  # Should return raw stdout
+        mock_subprocess.assert_called_once_with(
+            "tw pipelines list",
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+            shell=True,
+        )
+
+    @patch("subprocess.Popen")
+    def test_json_parsing_failure_fallback(self, mock_subprocess):
+        mock_subprocess.return_value = MagicMock(returncode=0)
+        mock_subprocess.return_value.communicate.return_value = (b"Invalid JSON", b"")
+
+        result = self.sp._execute_command("tw pipelines list", to_json=True)
+
+        self.assertEqual(result, "Invalid JSON")
+
+    @patch("subprocess.Popen")
+    def test_error_with_nonzero_return_code(self, mock_subprocess):
+        mock_subprocess.return_value = MagicMock(returncode=1)
+        mock_subprocess.return_value.communicate.return_value = (
+            b"ERROR: Something went wrong",
+            b"",
+        )
+
+        with self.assertRaises(ResourceCreationError):
+            self.sp._execute_command("tw pipelines list")
+
+    @patch("subprocess.Popen")
+    def test_correct_logging(self, mock_subprocess):
+        mock_subprocess.return_value = MagicMock(returncode=0)
+        mock_subprocess.return_value.communicate.return_value = (b"Command output", b"")
+
+        with patch("logging.info") as mock_logging:
+            # Test with JSON enabled
+            self.sp.json = True
+            self.sp._execute_command("tw pipelines list")
+            mock_logging.assert_called_once_with(" Running command: tw pipelines list")
+
+            mock_logging.reset_mock()
+
+            # Test with JSON disabled
+            self.sp.json = False
+            self.sp._execute_command("tw pipelines list")
+            mock_logging.assert_any_call(" Command output: Command output")
 
 
 if __name__ == "__main__":