diff --git a/.agents/skills/pytest/SKILL.md b/.agents/skills/pytest/SKILL.md new file mode 100644 index 0000000..0e6241a --- /dev/null +++ b/.agents/skills/pytest/SKILL.md @@ -0,0 +1,249 @@ +# pytest Skill Reference + +## Project Convention: Top-Level Test Functions + +**RULE**: Always write pytest tests as top-level functions, NOT grouped in test classes. + +### Correct Pattern +```python +def test_something() -> None: + assert True + +def test_another_thing(monkeypatch: pytest.MonkeyPatch) -> None: + assert True +``` + +### Wrong Pattern (Do NOT use) +```python +class TestSomething: + def test_something(self) -> None: + assert True +``` + +--- + +## Type Hints in Tests + +Always add type hints to test function parameters: + +```python +def test_with_fixtures(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Test using fixtures with proper type hints.""" + pass +``` + +### Common Fixture Type Hints +- `tmp_path: Path` - Temporary directory for file operations +- `monkeypatch: pytest.MonkeyPatch` - Environment variable and attribute mocking +- `fixture_name: FixtureType` - Custom fixtures from conftest.py + +--- + +## Fixtures + +### Using tmp_path for Temporary Files + +**PREFERRED**: Use pytest's built-in `tmp_path` fixture for automatic cleanup. + +```python +def test_with_temp_files(tmp_path: Path) -> None: + """Create temporary files that are automatically cleaned up.""" + config_file = tmp_path / "config.yaml" + config_file.write_text("key: value") + + # Read and test + assert config_file.read_text() == "key: value" + # Automatic cleanup after test +``` + +**NOT PREFERRED**: Manual tempfile operations (requires cleanup). + +```python +# Avoid this pattern +import tempfile +import os + +with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: + temp_path = f.name + +try: + # test code + pass +finally: + os.unlink(temp_path) # Manual cleanup needed +``` + +### Custom Fixtures with tmp_path + +```python +@pytest.fixture +def config_file(tmp_path: Path) -> Path: + """Create a config file for testing.""" + config_data = {"key": "value"} + + config_file = tmp_path / "config.yaml" + with open(config_file, "w") as f: + yaml.dump(config_data, f) + + return config_file # No manual cleanup needed +``` + +--- + +## Environment Variables with monkeypatch + +Use `monkeypatch` for setting/clearing environment variables with automatic rollback: + +```python +def test_env_var_override(monkeypatch: pytest.MonkeyPatch) -> None: + """Test with environment variable override.""" + monkeypatch.setenv("MY_VAR", "test-value") + + # MY_VAR is set in this test + assert os.getenv("MY_VAR") == "test-value" + + # Automatically reset after test + + +def test_env_var_not_set(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that ensures environment variable is NOT set.""" + monkeypatch.delenv("MY_VAR", raising=False) + + # MY_VAR is not set + assert os.getenv("MY_VAR") is None +``` + +--- + +## Test Organization + +Organize tests with comment sections: + +```python +# Basic functionality tests + + +def test_basic_operation() -> None: + pass + + +def test_another_basic() -> None: + pass + + +# Edge case tests + + +def test_edge_case_empty() -> None: + pass + + +def test_edge_case_special_chars() -> None: + pass + + +# Integration tests + + +def test_full_workflow() -> None: + pass +``` + +--- + +## Test Style Guidelines + +### Function Naming +- Start with `test_` prefix +- Use descriptive names: `test_override_list_with_multi_value` ✓ +- Avoid generic names: `test_something` ✗ + +### Assertions +- Use simple `assert` statements +- Add comments explaining complex assertions + +```python +def test_config_override() -> None: + config = load_config(config_file) + + # Value should be overridden by environment variable + assert config["key"] == "env-value" +``` + +### Docstrings +- Always add docstrings explaining what the test validates + +```python +def test_override_nested_key(temp_config_file: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that nested config keys can be overridden via environment variables.""" + pass +``` + +--- + +## Best Practices + +1. **One concept per test**: Each test should verify one specific behavior +2. **Clear test names**: Names should explain what is being tested +3. **Use fixtures**: Reduce code duplication with fixtures +4. **Type hints everywhere**: All parameters and return types +5. **Automatic cleanup**: Use `tmp_path` and `monkeypatch` instead of manual cleanup +6. **Top-level functions only**: Never use test classes +7. **Group related tests**: Use comment sections to organize logically related tests + +--- + +## Running Tests + +```bash +# Run all tests +uv run pytest + +# Run specific test file +uv run pytest tests/test_config.py -v + +# Run specific test +uv run pytest tests/test_config.py::test_override_scalar_value -v + +# Run with coverage +uv run pytest --cov=src + +# Stop on first failure +uv run pytest -x +``` + +--- + +## Common Patterns in This Project + +### Configuration Testing with YAML + Environment Variables + +```python +@pytest.fixture +def config_file(tmp_path: Path) -> Path: + """Create a test config file.""" + config_data = {"api-key": "default", "cors": {"allow-origins": ["http://localhost"]}} + config_file = tmp_path / "config.yaml" + with open(config_file, "w") as f: + yaml.dump(config_data, f) + return config_file + + +def test_override_scalar_value( + config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test overriding scalar config values via env vars.""" + monkeypatch.setenv("LLMOCK_API_KEY", "env-override") + config = load_config(config_file) + assert config["api-key"] == "env-override" + + +def test_override_list_value( + config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test overriding list values with semicolon-separated env vars.""" + monkeypatch.setenv("LLMOCK_CORS_ALLOW_ORIGINS", "http://prod.com;http://api.com") + config = load_config(config_file) + assert config["cors"]["allow-origins"] == ["http://prod.com", "http://api.com"] +``` + diff --git a/AGENTS.md b/AGENTS.md index 88901a2..1e4d2fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,11 +33,18 @@ This project is designed for AI-first development. All agents MUST follow these 2. Update `docs/learnings/INSTRUCTION_UPDATES.md` with a new rule to prevent recurrence. 3. Append relevant rules to `AGENTS.md` if they are project-wide. -### 4. Test-Driven Completion -- **RULE**: A task is NOT done until tests pass. +### 4. Test-Driven Completion (MANDATORY) +- **RULE**: A task is NOT done until tests pass. ALWAYS write tests for new functionality. - **PROCESS**: - 1. Every work package MUST include tests. - 2. Tests MUST pass before the task is marked `completed` in the todo list. + 1. Every new feature/function MUST have corresponding tests. + 2. Create tests BEFORE marking the task as complete. + 3. Organize tests in appropriate test files (e.g., `tests/test_.py`). + 4. Tests MUST pass before the task is marked `completed` in the todo list. + 5. Run full test suite to ensure no regressions: `uv run pytest -v` +- **COMMON MISTAKES TO AVOID**: + - Adding new functionality without tests + - Forgetting to test edge cases (empty values, special characters, nested structures) + - Not testing error conditions and validation ### 5. Code Quality Gate (MANDATORY) - **RULE**: ALWAYS run linting and formatting before completing any code task. diff --git a/README.md b/README.md index 0ba5edb..35b0fe7 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,25 @@ models: - id: "gpt-4o-mini" created: 1721172741 owned_by: "openai" + +### Environment Variable Overrides + +You can override values from `config.yaml` using environment variables with the `LLMOCK_` prefix. +Nested keys are joined with underscores, and dashes are converted to underscores. + +Examples: + +```bash +# Scalar override +export LLMOCK_API_KEY=your-secret-api-key + +# Nested override: cors.allow-origins +export LLMOCK_CORS_ALLOW_ORIGINS="http://localhost:8000;http://localhost:5173" +``` + +Notes: +- Lists are parsed from semicolon-separated values. +- Only keys that exist in `config.yaml` are overridden. ``` ## Development diff --git a/src/llmock/config.py b/src/llmock/config.py index 46ab8ef..b2e9436 100644 --- a/src/llmock/config.py +++ b/src/llmock/config.py @@ -1,6 +1,7 @@ -"""Configuration management - loads YAML as a raw dict.""" +"""Configuration management - loads YAML with environment variable overrides.""" from functools import lru_cache +import os from pathlib import Path from typing import Any @@ -9,6 +10,9 @@ # Type alias for the config dict Config = dict[str, Any] +# Environment variable prefix for config overrides +ENV_PREFIX = "LLMOCK_" + def load_config(config_path: Path = Path("config.yaml")) -> Config: """Load configuration from YAML file. @@ -26,7 +30,47 @@ def load_config(config_path: Path = Path("config.yaml")) -> Config: raise FileNotFoundError(f"Config file not found: {config_path}") with open(config_path) as f: - return yaml.safe_load(f) or {} + config = yaml.safe_load(f) or {} + + # Apply environment variable overrides + _apply_env_overrides(config) + return config + + +def _apply_env_overrides( + config: Config, prefix: str = ENV_PREFIX, path: str = "" +) -> None: + """Recursively traverse config dict and apply environment variable overrides. + + Environment variables use the format: LLMOCK_SECTION_KEY=value + For lists, use semicolon-separated values: LLMOCK_CORS_ALLOW_ORIGINS=http://localhost:8000;http://localhost:5173 + + Args: + config: Configuration dict to modify in-place. + prefix: Current environment variable prefix (includes parent keys). + path: Current path in the config tree (for debugging). + """ + for key, value in list(config.items()): + # Build the environment variable name + env_key = f"{prefix}{key.upper().replace('-', '_')}" + + # Check if env var exists + env_value = os.getenv(env_key) + + if env_value is not None: + # Apply the override + if isinstance(value, list): + # For lists, split by semicolon + config[key] = env_value.split(";") + elif isinstance(value, dict): + # For dicts, can't override directly from env, but traverse it + _apply_env_overrides(value, f"{env_key}_", f"{path}{key}.") + else: + # For scalars, use the value directly + config[key] = env_value + elif isinstance(value, dict): + # Recursively process nested dicts + _apply_env_overrides(value, f"{env_key}_", f"{path}{key}.") @lru_cache diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..a4514dd --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,278 @@ +"""Tests for configuration management with environment variable overrides.""" + +from pathlib import Path + +import pytest +import yaml + +from llmock.config import ( + ENV_PREFIX, + _apply_env_overrides, + load_config, +) + + +@pytest.fixture +def temp_config_file(tmp_path: Path) -> Path: + """Create a temporary config file for testing.""" + config_data = { + "api-key": "default-key", + "cors": { + "allow-origins": ["http://localhost:8000"], + }, + "models": [ + {"id": "gpt-4", "created": 1700000000, "owned_by": "openai"}, + {"id": "gpt-3.5-turbo", "created": 1600000000, "owned_by": "openai"}, + ], + } + + config_file = tmp_path / "config.yaml" + with open(config_file, "w") as f: + yaml.dump(config_data, f) + + return config_file + + +# Basic config loading tests + + +def test_load_config_reads_yaml_file(temp_config_file: Path) -> None: + """Test that load_config reads YAML file correctly.""" + config = load_config(temp_config_file) + + assert config["api-key"] == "default-key" + assert config["cors"]["allow-origins"] == ["http://localhost:8000"] + assert len(config["models"]) == 2 + + +def test_load_config_missing_file_raises_error() -> None: + """Test that missing config file raises FileNotFoundError.""" + with pytest.raises(FileNotFoundError, match="Config file not found"): + load_config(Path("/nonexistent/path/config.yaml")) + + +def test_load_config_empty_file_returns_empty_dict(tmp_path: Path) -> None: + """Test that empty YAML file returns empty dict.""" + empty_file = tmp_path / "empty.yaml" + empty_file.write_text("") + + config = load_config(empty_file) + assert config == {} + + +# Environment variable override tests + + +def test_override_scalar_value( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test overriding a scalar config value via env var.""" + monkeypatch.setenv(f"{ENV_PREFIX}API_KEY", "env-override-key") + + config = load_config(temp_config_file) + + assert config["api-key"] == "env-override-key" + + +def test_override_list_value_with_semicolon_separated( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test overriding a list value with semicolon-separated env var.""" + origins = "http://localhost:3000;http://example.com;http://dev.local" + monkeypatch.setenv(f"{ENV_PREFIX}CORS_ALLOW_ORIGINS", origins) + + config = load_config(temp_config_file) + + assert config["cors"]["allow-origins"] == [ + "http://localhost:3000", + "http://example.com", + "http://dev.local", + ] + + +def test_override_nested_key( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test overriding a nested config key.""" + monkeypatch.setenv(f"{ENV_PREFIX}CORS_ALLOW_ORIGINS", "http://localhost:5173") + + config = load_config(temp_config_file) + + assert config["cors"]["allow-origins"] == ["http://localhost:5173"] + + +def test_override_with_dashes_in_key_name( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test that dashes in key names are converted to underscores in env vars.""" + monkeypatch.setenv(f"{ENV_PREFIX}API_KEY", "new-api-key") + + config = load_config(temp_config_file) + + # The key in YAML still has dash, but value should be overridden + assert config["api-key"] == "new-api-key" + + +def test_default_value_used_when_env_var_not_set( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test that default YAML values are used when env var is not set.""" + # Make sure env var is not set + monkeypatch.delenv(f"{ENV_PREFIX}API_KEY", raising=False) + + config = load_config(temp_config_file) + + assert config["api-key"] == "default-key" + assert config["cors"]["allow-origins"] == ["http://localhost:8000"] + + +def test_multiple_overrides( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test multiple environment variable overrides at once.""" + monkeypatch.setenv(f"{ENV_PREFIX}API_KEY", "override-key") + monkeypatch.setenv(f"{ENV_PREFIX}CORS_ALLOW_ORIGINS", "http://prod.example.com") + + config = load_config(temp_config_file) + + assert config["api-key"] == "override-key" + assert config["cors"]["allow-origins"] == ["http://prod.example.com"] + + +def test_override_empty_string_value( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test that empty string env var can override default.""" + monkeypatch.setenv(f"{ENV_PREFIX}API_KEY", "") + + config = load_config(temp_config_file) + + # Empty string should still override + assert config["api-key"] == "" + + +def test_override_list_with_single_value( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test overriding a list with a single value (no semicolon).""" + monkeypatch.setenv(f"{ENV_PREFIX}CORS_ALLOW_ORIGINS", "http://localhost:3000") + + config = load_config(temp_config_file) + + # Single value should still be in a list + assert config["cors"]["allow-origins"] == ["http://localhost:3000"] + + +def test_override_list_with_empty_values( + temp_config_file: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test that empty segments in semicolon list are preserved.""" + monkeypatch.setenv( + f"{ENV_PREFIX}CORS_ALLOW_ORIGINS", + "http://localhost:8000;;http://localhost:3000", + ) + + config = load_config(temp_config_file) + + # Empty string in the middle should be preserved as part of split + assert config["cors"]["allow-origins"] == [ + "http://localhost:8000", + "", + "http://localhost:3000", + ] + + +# Direct _apply_env_overrides function tests + + +def test_apply_env_overrides_modifies_dict_in_place( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test that _apply_env_overrides modifies the dict in-place.""" + config = {"api-key": "original", "nested": {"value": "default"}} + monkeypatch.setenv(f"{ENV_PREFIX}API_KEY", "modified") + + _apply_env_overrides(config) + + assert config["api-key"] == "modified" + + +def test_apply_env_overrides_with_custom_prefix( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test _apply_env_overrides with a custom prefix.""" + config = {"key": "original"} + monkeypatch.setenv("CUSTOM_KEY", "modified") + + _apply_env_overrides(config, prefix="CUSTOM_") + + assert config["key"] == "modified" + + +def test_deeply_nested_override(monkeypatch: pytest.MonkeyPatch) -> None: + """Test overriding deeply nested values.""" + config = { + "level1": { + "level2": { + "level3": "original", + } + } + } + monkeypatch.setenv(f"{ENV_PREFIX}LEVEL1_LEVEL2_LEVEL3", "modified") + + _apply_env_overrides(config) + + assert config["level1"]["level2"]["level3"] == "modified" + + +def test_complex_nested_structure(monkeypatch: pytest.MonkeyPatch) -> None: + """Test override in a complex nested structure.""" + config = { + "service": { + "database": { + "host": "localhost", + "port": "5432", + }, + "cache": { + "ttl": "3600", + }, + } + } + monkeypatch.setenv(f"{ENV_PREFIX}SERVICE_DATABASE_HOST", "prod.db.com") + monkeypatch.setenv(f"{ENV_PREFIX}SERVICE_CACHE_TTL", "7200") + + _apply_env_overrides(config) + + assert config["service"]["database"]["host"] == "prod.db.com" + assert config["service"]["database"]["port"] == "5432" # unchanged + assert config["service"]["cache"]["ttl"] == "7200" + + +# Integration tests + + +def test_full_workflow_yaml_with_env_overrides( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test complete workflow: load YAML and apply env overrides.""" + config_data = { + "api-key": "yaml-key", + "cors": { + "allow-origins": ["http://localhost:8000"], + }, + "max-connections": "10", + } + + config_file = tmp_path / "config.yaml" + with open(config_file, "w") as f: + yaml.dump(config_data, f) + + monkeypatch.setenv(f"{ENV_PREFIX}API_KEY", "env-key") + monkeypatch.setenv(f"{ENV_PREFIX}CORS_ALLOW_ORIGINS", "http://prod.com") + monkeypatch.setenv(f"{ENV_PREFIX}MAX_CONNECTIONS", "50") + + config = load_config(config_file) + + assert config["api-key"] == "env-key" + assert config["cors"]["allow-origins"] == ["http://prod.com"] + assert config["max-connections"] == "50"