chore: add tests for auto-configure and remove unused test code

tests/test_cli.py

@@ -4,7 +4,15 @@
 import json
 import os
 import textwrap
+from pathlib import Path
 from tempfile import NamedTemporaryFile
+from unittest.mock import Mock, patch
+
+import pytest
+import requests
+from click.testing import CliRunner
+
+from stacklet.client.platform.cli import auto_configure
 
 from .utils import BaseCliTest
 
@@ -67,3 +75,320 @@ def test_admin_save_config_and_show(self):
         )
 
         os.unlink(file_location.name)
+
+
+@pytest.fixture
+def mock_config_response():
+    return {
+        "cognito_install": "auth.console.myorg.stacklet.io",
+        "cognito_user_pool_region": "us-east-1",
+        "cognito_user_pool_id": "us-east-1_ABC123",
+        "cognito_user_pool_client_id": "client123",
+        "cubejs_domain": "cubejs.myorg.stacklet.io",
+        "saml_providers": [
+            {"name": "okta", "idp_id": "okta_123"},
+            {"name": "azure", "idp_id": "azure_456"},
+        ],
+    }
+
+
+@pytest.fixture
+def expected_formatted_config():
+    return {
+        "api": "https://api.myorg.stacklet.io",
+        "auth_url": "https://auth.console.myorg.stacklet.io",
+        "cognito_client_id": "client123",
+        "cognito_user_pool_id": "us-east-1_ABC123",
+        "cubejs": "https://cubejs.myorg.stacklet.io",
+        "idp_id": "okta_123",
+        "region": "us-east-1",
+    }
+
+
+def test_auto_configure_comprehensive(mock_config_response, expected_formatted_config):
+    """Comprehensive test covering all major functionality"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        # Setup mocks
+        mock_response = Mock()
+        mock_response.json.return_value = mock_config_response
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_parent = Mock()
+        mock_path.parent = mock_parent
+        mock_path.exists.return_value = False
+        mock_expanduser.return_value = mock_path
+
+        # Test with prefix URL and specific IDP
+        result = runner.invoke(
+            auto_configure, ["--url", "myorg", "--idp", "okta", "--location", "/test/config.json"]
+        )
+
+        assert result.exit_code == 0
+        assert "Saved config to" in result.output
+
+        # Verify requests were made to correct URLs
+        assert mock_get.call_count == 2
+        calls = mock_get.call_args_list
+        assert "https://console.myorg.stacklet.io/config/cognito.json" in calls[0][0][0]
+        assert "https://console.myorg.stacklet.io/config/cubejs.json" in calls[1][0][0]
+
+        # Verify file operations
+        mock_parent.mkdir.assert_called_once_with(parents=True, exist_ok=True)
+        mock_path.write_text.assert_called_once()
+        written_config = json.loads(mock_path.write_text.call_args[0][0])
+        assert written_config == expected_formatted_config
+
+
+def test_url_parsing_variations(mock_config_response):
+    """Test different URL input formats"""
+    runner = CliRunner()
+
+    test_cases = [
+        ("myorg", "console.myorg.stacklet.io"),
+        ("myorg.stacklet.io", "console.myorg.stacklet.io"),
+        ("https://console.myorg.stacklet.io", "console.myorg.stacklet.io"),
+        ("console.myorg.stacklet.io", "console.myorg.stacklet.io"),
+    ]
+
+    for input_url, expected_host in test_cases:
+        with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+            mock_response = Mock()
+            mock_response.json.return_value = {
+                **mock_config_response,
+                "saml_providers": [{"name": "single", "idp_id": "single_123"}],
+            }
+            mock_response.raise_for_status.return_value = None
+            mock_get.return_value = mock_response
+
+            mock_path = Mock(spec=Path)
+            mock_path.exists.return_value = True
+            mock_expanduser.return_value = mock_path
+
+            result = runner.invoke(auto_configure, ["--url", input_url])
+
+            assert result.exit_code == 0
+            # Verify the host was parsed correctly
+            called_url = mock_get.call_args_list[0][0][0]
+            assert expected_host in called_url
+
+
+def test_single_idp_auto_selection(mock_config_response):
+    """Test automatic IDP selection when only one is available"""
+    runner = CliRunner()
+    single_idp_config = {
+        **mock_config_response,
+        "saml_providers": [{"name": "single", "idp_id": "single_123"}],
+    }
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = single_idp_config
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_path.exists.return_value = True
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        written_config = json.loads(mock_path.write_text.call_args[0][0])
+        assert written_config["idp_id"] == "single_123"
+
+
+def test_legacy_saml_config_format(mock_config_response):
+    """Test handling of legacy SAML configuration format"""
+    runner = CliRunner()
+    legacy_config = {**mock_config_response}
+    del legacy_config["saml_providers"]
+    legacy_config["saml"] = {"okta_123": "okta", "azure_456": "azure"}
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = legacy_config
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_path.exists.return_value = True
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg", "--idp", "okta"])
+
+        assert result.exit_code == 0
+        written_config = json.loads(mock_path.write_text.call_args[0][0])
+        assert written_config["idp_id"] == "okta_123"
+
+
+def test_multiple_idp_no_selection(mock_config_response):
+    """Test error when multiple IDPs available but none specified"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = mock_config_response
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_path.exists.return_value = True
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        assert "Multiple identity providers available" in result.output
+        assert "azure, okta" in result.output
+
+
+def test_unknown_idp_error(mock_config_response):
+    """Test error when specified IDP is not found"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = mock_config_response
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_path.exists.return_value = True
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg", "--idp", "unknown"])
+
+        assert result.exit_code == 0
+        assert "Unknown identity provider 'unknown'" in result.output
+        assert "known names: azure, okta" in result.output
+
+
+def test_connection_error():
+    """Test handling of connection errors"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get:
+        mock_get.side_effect = requests.exceptions.ConnectionError("Connection failed")
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        assert "Unable to connect to" in result.output
+        assert "Connection failed" in result.output
+
+
+def test_http_error():
+    """Test handling of HTTP errors"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get:
+        mock_response = Mock()
+        mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError("404 Not Found")
+        mock_get.return_value = mock_response
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        assert "Unable to retrieve configuration details" in result.output
+        assert "404 Not Found" in result.output
+
+
+def test_json_decode_error():
+    """Test handling of JSON decode errors"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get:
+        mock_response = Mock()
+        mock_response.raise_for_status.return_value = None
+        mock_response.json.side_effect = requests.exceptions.JSONDecodeError("Invalid JSON", "", 0)
+        mock_get.return_value = mock_response
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        assert "Unable to parse configuration details" in result.output
+        assert "Invalid JSON" in result.output
+
+
+def test_missing_required_key_error():
+    """Test handling of missing required configuration keys"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = {"incomplete": "config"}  # Missing required keys
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_path.exists.return_value = True
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        assert "missing a required key" in result.output
+
+
+def test_config_file_creation(mock_config_response):
+    """Test config file and directory creation"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = {
+            **mock_config_response,
+            "saml_providers": [{"name": "single", "idp_id": "single_123"}],
+        }
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        # Create mock path instance with all methods we need
+        mock_path = Mock(spec=Path)
+        mock_parent = Mock()
+        mock_path.parent = mock_parent
+        mock_path.exists.return_value = False
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        mock_parent.mkdir.assert_called_once_with(parents=True, exist_ok=True)
+        mock_path.write_text.assert_called_once()
+
+        # Verify JSON formatting
+        written_json = mock_path.write_text.call_args[0][0]
+        parsed_config = json.loads(written_json)
+        assert isinstance(parsed_config, dict)
+        # Verify it's properly formatted (indented and sorted)
+        assert written_json == json.dumps(parsed_config, indent=4, sort_keys=True)
+
+
+def test_existing_config_file(mock_config_response):
+    """Test when config file already exists (no directory creation needed)"""
+    runner = CliRunner()
+
+    with patch("requests.get") as mock_get, patch("pathlib.Path.expanduser") as mock_expanduser:
+        mock_response = Mock()
+        mock_response.json.return_value = {
+            **mock_config_response,
+            "saml_providers": [{"name": "single", "idp_id": "single_123"}],
+        }
+        mock_response.raise_for_status.return_value = None
+        mock_get.return_value = mock_response
+
+        mock_path = Mock(spec=Path)
+        mock_parent = Mock()
+        mock_path.parent = mock_parent
+        mock_path.exists.return_value = True  # File already exists
+        mock_expanduser.return_value = mock_path
+
+        result = runner.invoke(auto_configure, ["--url", "myorg"])
+
+        assert result.exit_code == 0
+        mock_parent.mkdir.assert_not_called()  # Should not create directory
+        mock_path.write_text.assert_called_once()

tests/utils.py

@@ -4,7 +4,7 @@
 import json
 from typing import Any
 from unittest import TestCase
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import requests_mock
 from click.testing import CliRunner, Result
@@ -16,22 +16,6 @@
 JSONDict = dict[str, Any]
 
 
-def get_mock_context():
-    mock_ctx = MagicMock()
-    mock_ctx.config = {
-        "config": None,
-        "output": "yaml",
-        "page_variables": {"first": 20, "last": 20, "before": "", "after": ""},
-        "raw_config": {
-            "cognito_user_pool_id": "foo",
-            "cognito_client_id": "bar",
-            "region": "us-east-1",
-            "api": "https://stacklet.acme.org/api",
-        },
-    }
-    return mock_ctx
-
-
 def get_executor_adapter():
     context = StackletContext(
         raw_config={