tests: improve tests for client and config

Author: taru-garg-2000

tests/units/test_client.py

@@ -1 +1,103 @@
+import pytest
+from unittest.mock import Mock, patch
 
+from tfe import client, config
+
+
+@pytest.fixture
+def test_config():
+    return config.Config(
+        address="https://app.terraform.io",
+        token="test-token"
+    )
+
+
+@pytest.fixture
+def mock_response():
+    response = Mock()
+    response.headers = {
+        "TFP-API-Version": "2.5.0",
+        "X-TFE-Version": "v202308-1", 
+        "TFP-AppName": "HCP Terraform"
+    }
+    response.raise_for_status.return_value = None
+    return response
+
+
+class TestClient:
+    @patch('requests.Session.get')
+    def test_client_initialization(self, mock_get, test_config, mock_response):
+        """Test basic client setup works."""
+        mock_get.return_value = mock_response
+        
+        client_instance = client.Client(config=test_config)
+        
+        assert client_instance.config.address == "https://app.terraform.io"
+        assert client_instance.config.token == "test-token"
+        assert client_instance.base_url == "https://app.terraform.io/api/v2/"
+        assert client_instance.registry_base_url == "https://app.terraform.io/api/registry/"
+
+    @patch('requests.Session.get')
+    def test_url_normalization(self, mock_get, mock_response):
+        """Test that paths get normalized with trailing slashes."""
+        mock_get.return_value = mock_response
+        
+        cfg = config.Config(
+            address="https://example.com",
+            token="test",
+            base_path="/custom/api",  # no trailing slash
+            registry_base_path="/registry"  # no trailing slash
+        )
+        
+        client_instance = client.Client(config=cfg)
+        
+        assert client_instance.base_url == "https://example.com/custom/api/"
+        assert client_instance.registry_base_url == "https://example.com/registry/"
+
+    @patch('requests.Session.get')
+    def test_api_metadata_extraction(self, mock_get, test_config, mock_response):
+        """Test that API metadata gets extracted from response headers."""
+        mock_get.return_value = mock_response
+        
+        client_instance = client.Client(config=test_config)
+        
+        assert client_instance.remote_api_version == "2.5.0"
+        assert client_instance.remote_tfe_version == "v202308-1"
+        assert client_instance.app_name == "HCP Terraform"
+
+    @patch('requests.Session.get')
+    def test_cloud_vs_enterprise_detection(self, mock_get, test_config):
+        """Test detection between cloud and enterprise instances."""
+        # Test HCP Terraform (cloud)
+        cloud_response = Mock()
+        cloud_response.headers = {"TFP-AppName": "HCP Terraform"}
+        cloud_response.raise_for_status.return_value = None
+        mock_get.return_value = cloud_response
+        
+        cloud_client = client.Client(config=test_config)
+        assert cloud_client.is_cloud() is True
+        assert cloud_client.is_enterprise() is False
+        
+        # Test Terraform Enterprise
+        enterprise_response = Mock()
+        enterprise_response.headers = {"TFP-AppName": "Terraform Enterprise"}
+        enterprise_response.raise_for_status.return_value = None
+        mock_get.return_value = enterprise_response
+        
+        enterprise_client = client.Client(config=test_config)
+        assert enterprise_client.is_cloud() is False
+        assert enterprise_client.is_enterprise() is True
+
+    @patch('requests.Session.get')
+    def test_fake_api_version_for_testing(self, mock_get, test_config, mock_response):
+        """Test the fake API version setter for testing scenarios."""
+        mock_get.return_value = mock_response
+        
+        client_instance = client.Client(config=test_config)
+        
+        # Original version from mock
+        assert client_instance.remote_api_version == "2.5.0"
+        
+        # Set fake version
+        client_instance.set_fake_remote_api_version("3.0.0")
+        assert client_instance.remote_api_version == "3.0.0"

tests/units/test_config.py

@@ -10,6 +10,7 @@ def reset_environment(monkeypatch):
     monkeypatch.delenv("TFE_ADDRESS", raising=False)
     monkeypatch.delenv("TFE_TOKEN", raising=False)
     monkeypatch.delenv("TFE_HOST", raising=False)
+    monkeypatch.setenv("TFE_TOKEN", "abc123")
     yield
 
 
@@ -34,17 +35,14 @@ def test_default_config(self, cfg):
         assert cfg.address == config.DEFAULT_ADDRESS
         assert cfg.base_path == config.DEFAULT_BASE_PATH
         assert cfg.registry_base_path == config.DEFAULT_REGISTRY_PATH
-        assert cfg.token == ""
         assert isinstance(cfg.http_client, requests.Session)
         assert "User-Agent" in cfg.http_client.headers
-        assert "Authorization" not in cfg.http_client.headers
         assert cfg.retry_log_hook is None
         assert cfg.retry_server_errors is False
 
     def test_env_address_and_token(self, monkeypatch):
         """Test that environment variables TFE_ADDRESS and TFE_TOKEN are read correctly."""
         monkeypatch.setenv("TFE_ADDRESS", "https://custom.tfe")
-        monkeypatch.setenv("TFE_TOKEN", "abc123")
         cfg = config.Config()
         assert cfg.address == "https://custom.tfe"
         assert cfg.token == "abc123"
@@ -86,3 +84,14 @@ def test_custom_session(self, test_session):
         assert "User-Agent" in cfg.http_client.headers
         assert cfg.http_client.headers["User-Agent"] == "test"
         assert cfg.http_client.headers["Authorization"] == "Bearer test"
+
+    def test_validate_config(self, monkeypatch):
+        """Test that configuration validation works as expected."""
+        with pytest.raises(ValueError, match="API token is required") as _:
+            monkeypatch.setenv("TFE_TOKEN", "")
+            cfg = config.Config(token="")
+
+        with pytest.raises(ValueError, match="Address must include protocol") as _:
+            monkeypatch.setenv("TFE_TOKEN", "test-token")
+            monkeypatch.setenv("TFE_ADDRESS", "test.foo.bar")
+            cfg = config.Config()

tfe/client.py

@@ -3,7 +3,7 @@
 """
 
 import logging
-from urllib.parse import urljoin, urlparse
+from urllib.parse import urljoin
 
 from tfe.config import Config
 
@@ -23,82 +23,33 @@ class Client:
     """
 
     def __init__(self, config: Config | None = None) -> None:
-        self.config = Config()
-
-        if config is not None:
-            self._merge_config(config)
-
-        self._validate_config()
+        self.config = config or Config()
         self._setup_urls()
 
         self._api_version = ""
         self._tfe_version = ""
         self._app_name = ""
         self._fetch_api_metadata()
 
-    def _merge_config(self, config: Config) -> None:
-        """Merge provided config with defaults, preserving non-empty values."""
-        if config.address:
-            self.config.address = config.address
-        if config.base_path:
-            self.config.base_path = config.base_path
-        if config.registry_base_path:
-            self.config.registry_base_path = config.registry_base_path
-        if config.token:
-            self.config.token = config.token
-        if config.headers:
-            if self.config.headers is None:
-                self.config.headers = {}
-            self.config.headers.update(config.headers)
-        if config.http_client:
-            self.config.http_client = config.http_client
-        if config.retry_log_hook:
-            self.config.retry_log_hook = config.retry_log_hook
-        self.config.retry_server_errors = config.retry_server_errors
-
-    def _validate_config(self) -> None:
-        """Validate required configuration."""
-        if not self.config.token:
-            raise TFEClientError("API token is required")
-
-        if not self.config.address:
-            raise TFEClientError("API address is required")
-
-        # Type narrowing: after validation, we know these are not None
-        assert self.config.address is not None
-        assert self.config.token is not None
-
     def _setup_urls(self) -> None:
         """Parse and setup base URLs."""
-        try:
-            # After validation, we know address is not None
-            parsed_url = urlparse(self.config.address)
-            if not parsed_url.scheme:
-                raise ValueError("Address must include protocol (http/https)")
+        # Ensure base path ends with /
+        base_path = self.config.base_path
+        if not base_path.endswith("/"):
+            base_path += "/"
 
-            # Ensure base path ends with /
-            base_path = self.config.base_path
-            if not base_path.endswith("/"):
-                base_path += "/"
+        registry_path = self.config.registry_base_path
+        if not registry_path.endswith("/"):
+            registry_path += "/"
 
-            registry_path = self.config.registry_base_path
-            if not registry_path.endswith("/"):
-                registry_path += "/"
-
-            self.base_url = urljoin(self.config.address, base_path)
-            self.registry_base_url = urljoin(self.config.address, registry_path)
-
-        except Exception as e:
-            raise TFEClientError(f"Invalid address '{self.config.address}': {e}") from e
+        self.base_url = urljoin(self.config.address, base_path)
+        self.registry_base_url = urljoin(self.config.address, registry_path)
 
     def _fetch_api_metadata(self) -> None:
         """Fetch API metadata from the server."""
         ping_url = urljoin(self.base_url, "ping")
-
-        # After validation, we know token is not None
         headers = {
             "Accept": "application/vnd.api+json",
-            "Authorization": f"Bearer {self.config.token}",
         }
         if self.config.headers:
             headers.update(self.config.headers)

tfe/config.py

@@ -2,6 +2,7 @@
 import os
 from collections.abc import Callable
 from dataclasses import dataclass, field
+from urllib.parse import urlparse
 
 import requests
 
@@ -28,7 +29,7 @@ class Config:
 
     # Headers to include in API requests
     # TODO: Do we need headers ? we can pass them directly to http_client, but this will differ from the go-tfe module
-    headers: dict[str, str] | None = None
+    headers: dict[str, str] = field(default_factory=dict)
 
     # Custom request session which needs to be used
     http_client: requests.Session = field(default_factory=requests.Session)
@@ -39,7 +40,7 @@ class Config:
     # Enable/Disable retry logic
     retry_server_errors: bool = False
 
-    def __post_init__(self) -> None:
+    def _set_address(self) -> None:
         tfe_address = os.getenv("TFE_ADDRESS", "")
         if tfe_address:
             self.address = tfe_address
@@ -50,23 +51,36 @@ def __post_init__(self) -> None:
             else:
                 self.address = DEFAULT_ADDRESS
 
+    def _set_token(self) -> None:
         if not self.token:
             self.token = os.getenv("TFE_TOKEN", "")
 
-        if self.headers is None:
-            self.headers = {}
+        if (
+            self.token
+            and "Authorization" not in self.http_client.headers
+            and "Authorization" not in self.headers
+        ):
+            self.headers["Authorization"] = f"Bearer {self.token}"
 
+    def _set_user_agent(self) -> None:
         if (
             "User-Agent" not in self.http_client.headers
             and "User-Agent" not in self.headers
         ):
             self.headers["User-Agent"] = "python-tfe"
 
-        if (
-            self.token
-            and "Authorization" not in self.http_client.headers
-            and "Authorization" not in self.headers
-        ):
-            self.headers["Authorization"] = f"Bearer {self.token}"
+    def _validate_config(self) -> None:
+        if not self.http_client.headers.get("Authorization"):
+            raise ValueError(
+                "API token is required, please set the TFE_TOKEN environment variable or the token field in the configuration."
+            )
+        parsed_url = urlparse(self.address)
+        if not parsed_url.scheme:
+            raise ValueError("Address must include protocol (http/https)")
 
+    def __post_init__(self) -> None:
+        self._set_address()
+        self._set_token()
+        self._set_user_agent()
         self.http_client.headers.update(self.headers)
+        self._validate_config()