|
| 1 | +# Agent Guidelines for buildkite-test-collector |
| 2 | + |
| 3 | +Coding guidelines for the buildkite-test-collector Python project - a pytest plugin that collects test execution data and sends it to Buildkite Test Engine. |
| 4 | + |
| 5 | +**Tech Stack**: Python >=3.9, pytest >=7, uv for dependency management |
| 6 | + |
| 7 | +## Build/Lint/Test Commands |
| 8 | + |
| 9 | +### Setup |
| 10 | +```bash |
| 11 | +curl -LsSf https://astral.sh/uv/install.sh | sh # Install uv |
| 12 | +uv sync --all-extras # Install dependencies |
| 13 | +``` |
| 14 | + |
| 15 | +### Testing |
| 16 | +```bash |
| 17 | +uv run pytest # Run all tests |
| 18 | +uv run pytest tests/buildkite_test_collector/collector/test_api.py # Single file |
| 19 | +uv run pytest tests/.../test_api.py::test_submit_local_returns_none # Single test |
| 20 | +uv run pytest -v # Verbose output |
| 21 | +uv run pytest -s # Show print statements |
| 22 | +``` |
| 23 | + |
| 24 | +### Linting |
| 25 | +```bash |
| 26 | +uv run pylint src/ # Lint all (required before PR merge) |
| 27 | +uv run pylint src/buildkite_test_collector/collector/api.py # Single file |
| 28 | +``` |
| 29 | + |
| 30 | +### Building |
| 31 | +```bash |
| 32 | +uv build # Build distribution packages |
| 33 | +``` |
| 34 | + |
| 35 | +## Code Style Guidelines |
| 36 | + |
| 37 | +### Imports |
| 38 | +- **Order**: Standard library → Third-party → Local imports (blank line between groups) |
| 39 | +- **Style**: Absolute imports preferred; relative imports (`..`) acceptable within package |
| 40 | + |
| 41 | +### Formatting |
| 42 | +- **Indentation**: 4 spaces (never tabs); **Line endings**: LF; **Quotes**: Double (`"`) |
| 43 | +- **Trailing whitespace**: Remove all; **Final newline**: Always include |
| 44 | + |
| 45 | +### Type Hints |
| 46 | +- **Required**: All new code must include type hints |
| 47 | +- **Common types**: `Optional`, `Dict`, `List`, `Tuple`, `Literal`, `Mapping`, `Generator` |
| 48 | +- **Type aliases**: Use for complex types (e.g., `JsonValue`, `JsonDict`) |
| 49 | + |
| 50 | +### Naming Conventions |
| 51 | +- **Classes**: `PascalCase` (e.g., `BuildkitePlugin`, `TestData`) |
| 52 | +- **Functions/methods**: `snake_case` (e.g., `pytest_runtest_logstart`) |
| 53 | +- **Constants**: `UPPER_SNAKE_CASE` (e.g., `ENV_TOKEN`, `DEFAULT_API_URL`) |
| 54 | +- **Private methods**: `_single_underscore` prefix; **Special**: `__double_underscore__` only |
| 55 | + |
| 56 | +### Docstrings |
| 57 | +- **Required**: All modules, classes, and public methods |
| 58 | +- **Style**: Imperative mood (e.g., "Submit a payload" not "Submits a payload") |
| 59 | +- **Format**: Triple quotes, concise one-liner preferred |
| 60 | + |
| 61 | +### Dataclasses - Critical Pattern |
| 62 | +- **Default**: Use `@dataclass(frozen=True)` for immutability |
| 63 | +- **Import**: `from dataclasses import dataclass, replace, field` |
| 64 | +- **Modification**: Use `replace()` to create modified copies |
| 65 | +- **Validation**: Use `__post_init__` for input validation |
| 66 | +```python |
| 67 | +@dataclass(frozen=True) |
| 68 | +class TestData: |
| 69 | + id: UUID |
| 70 | + scope: str |
| 71 | + name: str |
| 72 | + |
| 73 | + def passed(self): |
| 74 | + """Return new instance with passed status""" |
| 75 | + return replace(self, result=TestResultPassed()) |
| 76 | +``` |
| 77 | + |
| 78 | +### Error Handling |
| 79 | +- **Specific exceptions**: Catch specific exceptions when possible |
| 80 | +- **Broad exceptions**: Use `except Exception:` with `# pylint: disable=broad-except` comment |
| 81 | +- **Logging**: Always log warnings/errors with `logger.warning()` |
| 82 | +- **Example**: |
| 83 | +```python |
| 84 | +try: |
| 85 | + response = post(url, json=data, timeout=60) |
| 86 | + response.raise_for_status() |
| 87 | +except HTTPError as err: |
| 88 | + logger.warning("Failed to upload test results to buildkite") |
| 89 | + logger.warning(err) |
| 90 | +except Exception: # pylint: disable=broad-except |
| 91 | + logger.warning(traceback.format_exc()) |
| 92 | +``` |
| 93 | + |
| 94 | +### Testing Conventions |
| 95 | +- **Test files**: `test_*.py` prefix (e.g., `test_api.py`) |
| 96 | +- **Test functions**: `test_*` prefix (e.g., `test_submit_local_returns_none`) |
| 97 | +- **Fixtures**: Define in `conftest.py`, use extensively |
| 98 | +- **Assertions**: Simple `assert` statements |
| 99 | +- **HTTP mocking**: Use `@responses.activate` decorator with `responses` library |
| 100 | +- **Conditional tests**: `@pytest.mark.skipif` for version-specific tests |
| 101 | + |
| 102 | +### Pylint Directives |
| 103 | +Use inline disables sparingly with specific error codes: |
| 104 | +- `# pylint: disable=too-few-public-methods` - simple data classes |
| 105 | +- `# pylint: disable=broad-except` - generic exception catching |
| 106 | +- `# pylint: disable=unused-argument` - required but unused pytest hook parameters |
| 107 | +- Place at end of line or on line before the violation |
| 108 | + |
| 109 | +## Project-Specific Patterns |
| 110 | + |
| 111 | +### Immutable State Management |
| 112 | +TestData and related objects are frozen dataclasses. Always reassign after modifications: |
| 113 | +```python |
| 114 | +test_data = self.in_flight[nodeid] |
| 115 | +test_data = test_data.passed() # Returns NEW instance |
| 116 | +self.in_flight[nodeid] = test_data # MUST reassign |
| 117 | +``` |
| 118 | + |
| 119 | +### Pytest Hook Callbacks |
| 120 | +Follow pytest naming strictly. Document which hook you're implementing: |
| 121 | +```python |
| 122 | +def pytest_runtest_logreport(self, report): |
| 123 | + """pytest_runtest_logreport hook callback to get test outcome after test call""" |
| 124 | + # Implementation |
| 125 | +``` |
| 126 | + |
| 127 | +Key hooks: `pytest_configure`, `pytest_runtest_logstart`, `pytest_runtest_logreport`, `pytest_runtest_makereport` |
| 128 | + |
| 129 | +### Environment Variables |
| 130 | +Define as class constants and access safely: |
| 131 | +```python |
| 132 | +class API: |
| 133 | + ENV_TOKEN = "BUILDKITE_ANALYTICS_TOKEN" |
| 134 | + DEFAULT_API_URL = "https://analytics-api.buildkite.com/v1" |
| 135 | + |
| 136 | + def __init__(self, env: Mapping[str, Optional[str]]): |
| 137 | + self.token = env.get(self.ENV_TOKEN) |
| 138 | + self.api_url = env.get(self.ENV_API_URL) or self.DEFAULT_API_URL |
| 139 | +``` |
| 140 | + |
| 141 | +## Common Pitfalls |
| 142 | + |
| 143 | +1. **Forgetting to reassign frozen dataclasses**: `test_data.passed()` returns new instance - must reassign |
| 144 | +2. **Missing type hints**: All new code requires type hints |
| 145 | +3. **Wrong import order**: Standard library → Third-party → Local (with blank lines) |
| 146 | +4. **Skipping docstrings**: All public modules/classes/methods need docstrings |
| 147 | + |
| 148 | +## Release Process |
| 149 | + |
| 150 | +1. Update version in `pyproject.toml` |
| 151 | +2. Create PR with `[release]` in title |
| 152 | +3. Merge triggers automated PyPI release |
| 153 | +4. Create GitHub release tag manually |
0 commit comments