copilot review

Author: BartoszBlizniak

cloudsmith_cli/cli/tests/test_webserver.py

@@ -1,5 +1,6 @@
 """Tests for the webserver module."""
 
+from http.server import HTTPServer
 from unittest.mock import MagicMock, PropertyMock, patch
 
 import pytest
@@ -12,9 +13,12 @@ class TestAuthenticationWebServer:
 
     def test_sso_access_token_initialized_to_none(self):
         """Verify sso_access_token is initialized to None."""
-        with patch.object(AuthenticationWebServer, "__init__", lambda *a, **kw: None):
-            server = AuthenticationWebServer.__new__(AuthenticationWebServer)
-            server.sso_access_token = None
+
+        def _fake_http_server_init(self, *args, **kwargs):
+            self.socket = MagicMock()
+
+        with patch.object(HTTPServer, "__init__", _fake_http_server_init):
+            server = AuthenticationWebServer(("localhost", 0), MagicMock())
             assert server.sso_access_token is None
 
     def test_refresh_api_config_passes_sso_token(self):

cloudsmith_cli/core/keyring.py

@@ -10,7 +10,7 @@
 
 def should_use_keyring():
     """Check if keyring should be used based on CLOUDSMITH_NO_KEYRING env var."""
-    env_value = os.environ.get("CLOUDSMITH_NO_KEYRING", "").lower()
+    env_value = os.environ.get("CLOUDSMITH_NO_KEYRING", "").strip().lower()
     if env_value in ("1", "true", "yes"):
         return False
     return True

cloudsmith_cli/core/tests/test_keyring.py

@@ -1,7 +1,7 @@
 import getpass
 import os
 from datetime import datetime, timedelta, timezone
-from unittest.mock import patch
+from unittest.mock import ANY, patch
 
 import keyring
 import pytest
@@ -183,8 +183,6 @@ def test_get_refresh_token_when_error_raised(
 
     @freeze_time("2024-06-01 10:00:00")
     def test_store_sso_tokens(self, mock_get_user, mock_set_password):
-        refresh_attempted_at = datetime.utcnow().isoformat()
-
         # Ensure keyring is enabled
         env = os.environ.copy()
         env.pop("CLOUDSMITH_NO_KEYRING", None)
@@ -201,7 +199,7 @@ def test_store_sso_tokens(self, mock_get_user, mock_set_password):
         mock_set_password.assert_any_call(
             "cloudsmith_cli-access_token_refresh_attempted_at-https://example.com",
             "test_user",
-            refresh_attempted_at,
+            ANY,
         )
         mock_set_password.assert_any_call(
             "cloudsmith_cli-refresh_token-https://example.com",
@@ -243,10 +241,3 @@ def test_returns_true_when_env_var_is_falsy(self, env_value):
         """When CLOUDSMITH_NO_KEYRING is set to a falsy value, keyring should be used."""
         with patch.dict(os.environ, {"CLOUDSMITH_NO_KEYRING": env_value}):
             assert should_use_keyring() is True
-
-    def test_returns_true_when_env_var_not_set(self):
-        """When CLOUDSMITH_NO_KEYRING is not set at all, keyring should be used."""
-        env = os.environ.copy()
-        env.pop("CLOUDSMITH_NO_KEYRING", None)
-        with patch.dict(os.environ, env, clear=True):
-            assert should_use_keyring() is True