From 01e512ac7f56fbed353e5a90ed960906a6e8411a Mon Sep 17 00:00:00 2001
From: pkoprda <47797196+pkoprda@users.noreply.github.com>
Date: Thu, 14 Nov 2024 01:55:23 +0100
Subject: [PATCH] feat(client): Improve test coverage of crypto.py (#4268)

* Card ID: CCT-967

While existing tests catch basic invalid playbook scenarios, this
update aims to create more tests that cover specific verification
failure points. Adding new unit tests helps "freeze" expected
verification behavior, making it easier to detect unintended changes in
the future.

Signed-off-by: pkoprda <pkoprda@redhat.com>
---
 insights/client/crypto.py            |  1 +
 insights/tests/client/test_crypto.py | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/insights/client/crypto.py b/insights/client/crypto.py
index 4b7e8d727a..0e5302d9d6 100644
--- a/insights/client/crypto.py
+++ b/insights/client/crypto.py
@@ -99,6 +99,7 @@ def _supports_cleanup_socket(self):
         )
         stdout, stderr = version_process.communicate()
         if version_process.returncode != 0:
+            stderr = stderr.decode("utf-8") if isinstance(stderr, bytes) else stderr
             stderr = "\n".join("stderr: {line}".format(line=line) for line in stderr.split("\n") if len(line))
             logger.debug("could not query for gpg version:\n{err}".format(err=stderr))
             return False
diff --git a/insights/tests/client/test_crypto.py b/insights/tests/client/test_crypto.py
index 7c0b2f67af..7de4df2b94 100644
--- a/insights/tests/client/test_crypto.py
+++ b/insights/tests/client/test_crypto.py
@@ -177,10 +177,13 @@ def test_no_file(file):
         key=home + "/key.public.gpg",
     )
 
+    assert not os.path.isfile(home + file)
     assert result.ok is False
     assert result.return_code > 0
     assert "file '{path}' does not exist".format(path=home + file) == result.stderr
 
+    shutil.rmtree(home)
+
 
 @mock.patch("insights.client.crypto.GPGCommand.TEMPORARY_GPG_HOME_PARENT_DIRECTORY", "/tmp/")
 def test_invalid_public_key():
@@ -277,3 +280,26 @@ def communicate(self, *args, **kwargs):
         result = command._supports_cleanup_socket()
 
     assert result == supports
+
+
+@mock.patch("insights.client.crypto.GPGCommand.TEMPORARY_GPG_HOME_PARENT_DIRECTORY", "/tmp/")
+@mock.patch("subprocess.Popen")
+@mock.patch.object(crypto.GPGCommand, "_cleanup", return_value=None)
+def test_invalid_gpg_setup(mock_cleanup, mock_popen):
+    """An invalid GPG setup can be detected."""
+    gpg_command = crypto.GPGCommand(command=[], key=os.path.join("/dummy", "key"))
+
+    # Mock the process
+    mock_process = mock.Mock()
+    mock_process.communicate.return_value = (b"", b"gpg setup failed")
+    mock_process.returncode = 1
+    mock_popen.return_value = mock_process
+
+    # Run the test
+    result = gpg_command.evaluate()  # type: crypto.GPGCommandResult
+
+    # Verify the results
+    assert not result.ok
+    assert "" == result.stdout
+    assert "gpg setup failed" in result.stderr
+    assert 1 == result.return_code