DEVOPS-735 fix: sf cli not accessible in windows for clariti commands (#9)

* DEVOPS-735 refactor: replace subprocess calls with sfdx function for org checkout and alias setting

* DEVOPS-735 refactor: replace subprocess calls with sfdx function in test_utils_clariti.py

* DEVOPS-735 chore: improve stdout and stderr handling in checkout_org_from_pool and set_sf_alias functions

* DEVOPS-735 chore: streamline stdout and stderr handling in checkout_org_from_pool and set_sf_alias functions

Author: dipakparmar

cumulusci/tests/test_utils_clariti.py

@@ -1,3 +1,4 @@
+import io
 import json
 from types import SimpleNamespace
 
@@ -13,6 +14,14 @@
 )
 
 
+def _make_proc(*, stdout: str = "", stderr: str = "", returncode: int = 0):
+    return SimpleNamespace(
+        returncode=returncode,
+        stdout_text=io.StringIO(stdout),
+        stderr_text=io.StringIO(stderr),
+    )
+
+
 def test_resolve_pool_id_prefers_explicit():
     assert resolve_pool_id("Pool42", None) == "Pool42"
 
@@ -43,12 +52,15 @@ def test_checkout_org_from_pool_parses_username(monkeypatch):
         "poolId": "Pool42",
     }
 
-    def fake_run(command, check, text, capture_output, env):
-        assert command[:4] == ["sf", "clariti", "org", "checkout"]
-        assert "--json" in command
-        return SimpleNamespace(returncode=0, stdout=json.dumps(payload), stderr="")
+    def fake_sfdx(command, **kwargs):
+        assert command == "clariti org checkout"
+        args = kwargs["args"]
+        assert args[0] == "--json"
+        assert "--pool-id" in args and "Pool42" in args
+        assert "--alias" in args and "MyAlias" in args
+        return _make_proc(stdout=json.dumps(payload))
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     result = checkout_org_from_pool("Pool42", alias="MyAlias")
 
@@ -68,11 +80,11 @@ def test_checkout_org_from_pool_reads_nested_username(monkeypatch):
         },
     }
 
-    def fake_run(command, check, text, capture_output, env):
-        assert "--pool-id" in command
-        return SimpleNamespace(returncode=0, stdout=json.dumps(payload), stderr="")
+    def fake_sfdx(command, **kwargs):
+        assert "--pool-id" in kwargs["args"]
+        return _make_proc(stdout=json.dumps(payload))
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     result = checkout_org_from_pool("PoolNested")
 
@@ -86,22 +98,22 @@ def test_checkout_org_from_pool_without_pool_id(monkeypatch):
         "poolId": "Pool-From-Config",
     }
 
-    def fake_run(command, check, text, capture_output, env):
-        assert "--pool-id" not in command
-        return SimpleNamespace(returncode=0, stdout=json.dumps(payload), stderr="")
+    def fake_sfdx(command, **kwargs):
+        assert "--pool-id" not in kwargs["args"]
+        return _make_proc(stdout=json.dumps(payload))
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     result = checkout_org_from_pool(None)
 
     assert result.username == "user@example.com"
 
 
 def test_checkout_org_from_pool_handles_failure(monkeypatch):
-    def fake_run(command, check, text, capture_output, env):
-        return SimpleNamespace(returncode=1, stdout="", stderr="No orgs available")
+    def fake_sfdx(command, **kwargs):
+        return _make_proc(stdout="", stderr="No orgs available", returncode=1)
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     with pytest.raises(ClaritiError) as exc:
         checkout_org_from_pool("EmptyPool")
@@ -117,12 +129,10 @@ def test_checkout_org_from_pool_formats_json_error(monkeypatch):
         "message": "Failed to get org from pool: No healthy orgs available in this pool",
     }
 
-    def fake_run(command, check, text, capture_output, env):
-        return SimpleNamespace(
-            returncode=1, stdout=json.dumps(payload), stderr=""
-        )
+    def fake_sfdx(command, **kwargs):
+        return _make_proc(stdout=json.dumps(payload), stderr="", returncode=1)
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     with pytest.raises(ClaritiError) as exc:
         checkout_org_from_pool("Pool42")
@@ -139,12 +149,10 @@ def test_checkout_org_from_pool_formats_json_error_debug(monkeypatch):
         "message": "Failed", "extra": "details",
     }
 
-    def fake_run(command, check, text, capture_output, env):
-        return SimpleNamespace(
-            returncode=1, stdout=json.dumps(payload), stderr=""
-        )
+    def fake_sfdx(command, **kwargs):
+        return _make_proc(stdout=json.dumps(payload), stderr="", returncode=1)
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     from cumulusci.core.debug import set_debug_mode
 
@@ -159,11 +167,12 @@ def fake_run(command, check, text, capture_output, env):
 
 
 def test_set_sf_alias_success(monkeypatch):
-    def fake_run(command, check, text, capture_output, env):
-        assert command == ["sf", "alias", "set", "target=user@example.com"]
-        return SimpleNamespace(returncode=0, stdout="", stderr="")
+    def fake_sfdx(command, **kwargs):
+        assert command == "alias set"
+        assert kwargs["args"] == ["target=user@example.com"]
+        return _make_proc()
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     success, message = set_sf_alias("target", "user@example.com")
 
@@ -172,10 +181,10 @@ def fake_run(command, check, text, capture_output, env):
 
 
 def test_set_sf_alias_failure(monkeypatch):
-    def fake_run(command, check, text, capture_output, env):
-        return SimpleNamespace(returncode=1, stdout="", stderr="Alias failure")
+    def fake_sfdx(command, **kwargs):
+        return _make_proc(stdout="", stderr="Alias failure", returncode=1)
 
-    monkeypatch.setattr("cumulusci.utils.clariti.subprocess.run", fake_run)
+    monkeypatch.setattr("cumulusci.utils.clariti.sfdx", fake_sfdx)
 
     success, message = set_sf_alias("target", "username")
 

cumulusci/utils/clariti.py

@@ -8,7 +8,7 @@
 from pathlib import Path
 from typing import Any, Dict, Optional, Sequence, Tuple, cast
 
-import subprocess
+from cumulusci.core.sfdx import sfdx
 
 from cumulusci.core.debug import get_debug_mode
 
@@ -105,38 +105,33 @@ def checkout_org_from_pool(
         data.
     """
 
-    command = [
-        "sf",
-        "clariti",
-        "org",
-        "checkout",
-        "--json",
-    ]
+    command_args = ["--json"]
     if pool_id:
-        command.extend(["--pool-id", pool_id])
+        command_args.extend(["--pool-id", pool_id])
     if alias:
-        command.extend(["--alias", alias])
+        command_args.extend(["--alias", alias])
 
     try:
-        proc = subprocess.run(
-            command,
-            check=False,
-            text=True,
-            capture_output=True,
+        proc = sfdx(
+            "clariti org checkout",
+            args=command_args,
             env=env,
+            capture_output=True,
+            check_return=False,
         )
     except FileNotFoundError as err:
         raise ClaritiError("Salesforce CLI 'sf' was not found on PATH.") from err
+    except OSError as err:
+        raise ClaritiError("Failed to execute Salesforce CLI 'sf'.") from err
+
+    stdout = _read_process_stream(proc, "stdout_text", "stdout")
+    stderr = _read_process_stream(proc, "stderr_text", "stderr")
 
-    stdout = (proc.stdout or "").strip()
-    stderr = (proc.stderr or "").strip()
     if proc.returncode:
         summary, raw_output = _summarize_error_output(stdout, stderr, proc.returncode)
         summary = f"Clariti checkout failed: {summary}"
         if get_debug_mode():
-            raise ClaritiError(
-                f"{summary}\nClariti raw response:\n{raw_output}"
-            )
+            raise ClaritiError(f"{summary}\nClariti raw response:\n{raw_output}")
         raise ClaritiError(summary)
 
     if not stdout:
@@ -152,18 +147,12 @@ def checkout_org_from_pool(
 
     username = cast(str, _extract_string(payload, _USERNAME_PATHS))
     alias_value = _extract_string(payload, _ALIAS_PATHS, allow_missing=True)
-    org_id_value = _extract_string(
-        payload, (("orgId",),), allow_missing=True
-    )
+    org_id_value = _extract_string(payload, (("orgId",),), allow_missing=True)
     instance_url_value = _extract_string(
         payload, (("instanceUrl",),), allow_missing=True
     )
-    org_type_value = _extract_string(
-        payload, (("orgType",),), allow_missing=True
-    )
-    pool_id_value = _extract_string(
-        payload, (("poolId",),), allow_missing=True
-    )
+    org_type_value = _extract_string(payload, (("orgType",),), allow_missing=True)
+    pool_id_value = _extract_string(payload, (("poolId",),), allow_missing=True)
 
     return ClaritiCheckoutResult(
         username=username,
@@ -190,20 +179,21 @@ def set_sf_alias(
     if not alias or not username:
         return False, "Alias and username are required."
 
-    command = ["sf", "alias", "set", f"{alias}={username}"]
     try:
-        proc = subprocess.run(
-            command,
-            check=False,
-            text=True,
-            capture_output=True,
+        proc = sfdx(
+            "alias set",
+            args=[f"{alias}={username}"],
             env=env,
+            capture_output=True,
+            check_return=False,
         )
     except FileNotFoundError:
         return False, "Salesforce CLI 'sf' was not found on PATH."
+    except OSError:
+        return False, "Failed to execute Salesforce CLI 'sf'."
 
-    stdout = (proc.stdout or "").strip()
-    stderr = (proc.stderr or "").strip()
+    stdout = _read_process_stream(proc, "stdout_text", "stdout")
+    stderr = _read_process_stream(proc, "stderr_text", "stderr")
     if proc.returncode:
         summary, raw_output = _summarize_error_output(stdout, stderr, proc.returncode)
         summary = f"Failed to set SF alias: {summary}"
@@ -214,6 +204,47 @@ def set_sf_alias(
     return True, None
 
 
+def _read_process_stream(proc: Any, text_attr: str, raw_attr: str) -> str:
+    """Normalize subprocess output attributes into plain text."""
+
+    value = getattr(proc, text_attr, None)
+    text = _coerce_stream_value(value)
+    if not text:
+        text = _coerce_stream_value(getattr(proc, raw_attr, None))
+    return text.strip()
+
+
+def _coerce_stream_value(value: Any) -> str:
+    """Return a string representation from subprocess output containers."""
+
+    if value is None:
+        return ""
+    if isinstance(value, str):
+        return value
+    if isinstance(value, (bytes, bytearray)):
+        return value.decode()
+    getvalue = getattr(value, "getvalue", None)
+    if callable(getvalue):
+        result = getvalue()
+        if isinstance(result, str):
+            return result
+        if isinstance(result, (bytes, bytearray)):
+            return result.decode()
+        return str(result)
+    read = getattr(value, "read", None)
+    if callable(read):
+        position = value.tell() if hasattr(value, "tell") else None
+        result = read()
+        if position is not None and hasattr(value, "seek"):
+            value.seek(position)
+        if isinstance(result, str):
+            return result
+        if isinstance(result, (bytes, bytearray)):
+            return result.decode()
+        return str(result) if result is not None else ""
+    return str(value)
+
+
 def _extract_string(
     payload: Dict[str, Any],
     paths: Sequence[Sequence[str]],