-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: consolidate test infrastructure and document conventions #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,237 @@ | ||
| # Testing Guide | ||
|
|
||
| How to run, write, and maintain tests in the socratic-study-mentor workspace. | ||
|
|
||
| ## Running Tests | ||
|
|
||
| The workspace has two packages with independent test suites: | ||
|
|
||
| | Package | Tests | Path | | ||
| |---------|-------|------| | ||
| | studyctl | 293 | `packages/studyctl/tests/` | | ||
| | agent-session-tools | 357 | `packages/agent-session-tools/tests/` | | ||
|
|
||
| ### Prerequisites | ||
|
|
||
| Before running tests, install all workspace packages: | ||
|
|
||
| ```bash | ||
| uv sync --all-packages | ||
| ``` | ||
|
|
||
| Plain `uv sync` only installs root deps — you will get `ModuleNotFoundError` on `click`, `typer`, etc. if you skip this. | ||
|
|
||
| ### Invocation Paths | ||
|
|
||
| **From workspace root (recommended):** | ||
|
|
||
| ```bash | ||
| uv run pytest # both packages, 650 tests | ||
| uv run pytest packages/studyctl/tests/ # studyctl only | ||
| uv run pytest packages/agent-session-tools/tests/ # agent-session-tools only | ||
| ``` | ||
|
|
||
| The root `pyproject.toml` sets `--import-mode=importlib` which is **load-bearing** — it prevents namespace conflicts between the two packages. Do not remove or override this. | ||
|
|
||
| **From inside a package directory:** | ||
|
|
||
| ```bash | ||
| cd packages/studyctl | ||
| uv run pytest # uses studyctl's own pyproject.toml config | ||
| ``` | ||
|
|
||
| Note: when running from a member directory, the root config's `--import-mode=importlib` is NOT applied (the member's own `addopts` take precedence). This is fine for focused development but the workspace root is the authoritative invocation path. | ||
|
|
||
| ### Useful Options | ||
|
|
||
| ```bash | ||
| uv run pytest -x # stop on first failure | ||
| uv run pytest -k "test_review" # run tests matching a pattern | ||
| uv run pytest -m "not integration" # skip integration tests | ||
| uv run pytest --tb=long # verbose tracebacks | ||
| uv run pytest -v # show individual test names | ||
| ``` | ||
|
|
||
| ## Why No conftest.py in studyctl | ||
|
|
||
| This is the single most important architectural constraint in the test suite. | ||
|
|
||
| **The problem:** When pytest collects both packages from the workspace root, it loads conftest files from both `packages/agent-session-tools/tests/conftest.py` and (if it existed) `packages/studyctl/tests/conftest.py`. These conftest files register as pytest plugins via pluggy. With `--import-mode=importlib`, the two conftest modules would occupy the same plugin namespace, causing a registration conflict. | ||
|
|
||
| **The solution:** agent-session-tools has a conftest (it was here first and holds shared fixtures like `temp_db`, `migrated_db`, `populated_db`). studyctl does NOT have a conftest. All studyctl fixtures are either: | ||
|
|
||
| 1. **Inlined** in each test file (the current pattern for most tests) | ||
| 2. **Imported from `_helpers.py`** — a plain Python module with factory functions (not pytest fixtures) | ||
|
|
||
| **Never create `packages/studyctl/tests/conftest.py`.** If you need shared utilities for studyctl tests, add them to `packages/studyctl/tests/_helpers.py`. | ||
|
|
||
| ## Fixture Patterns | ||
|
|
||
| ### agent-session-tools: Use conftest | ||
|
|
||
| Shared fixtures live in `packages/agent-session-tools/tests/conftest.py`: | ||
|
|
||
| - `temp_db` — temporary SQLite DB with base schema, yields `(conn, db_path)` | ||
| - `migrated_db` — same as `temp_db` but with all migrations applied | ||
| - `populated_db` — `migrated_db` with a sample session and message inserted | ||
| - `temp_config_dir` — temporary config directory structure | ||
| - `sample_session_data` — dict with canonical session fields | ||
| - `sample_message_data` — dict with canonical message fields | ||
|
|
||
| Use these directly in your test function signatures: | ||
|
|
||
| ```python | ||
| def test_export_creates_session(migrated_db, tmp_path): | ||
| conn, db_path = migrated_db | ||
| # conn has all migrations applied, ready for exporter testing | ||
| ``` | ||
|
|
||
| ### studyctl: Use _helpers.py or Inline | ||
|
|
||
| Import factory functions from `_helpers.py` and wrap them in `@pytest.fixture`: | ||
|
|
||
| ```python | ||
| import pytest | ||
| from _helpers import make_review_db, make_isolated_config | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def review_db(tmp_path): | ||
| return make_review_db(tmp_path) | ||
|
|
||
|
Comment on lines
+91
to
+101
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def isolated_config(tmp_path, monkeypatch): | ||
| return make_isolated_config(tmp_path, monkeypatch) | ||
|
|
||
|
|
||
| def test_something(review_db): | ||
| # review_db is a Path to a temp SQLite file with review tables | ||
| ... | ||
| ``` | ||
|
|
||
| **Available helpers:** | ||
|
|
||
| | Function | Returns | What it does | | ||
| |----------|---------|--------------| | ||
| | `make_review_db(tmp_path)` | `Path` | Creates a temp SQLite DB with review schema (WAL mode) | | ||
| | `make_isolated_config(tmp_path, monkeypatch)` | `Path` | Redirects `CONFIG_DIR` and `_CONFIG_PATH` to temp dir | | ||
|
|
||
| For simple, one-off fixtures, just inline them in the test file. Only use `_helpers.py` when the pattern recurs across multiple files. | ||
|
|
||
| ## Mock Conventions | ||
|
|
||
| Two tools, each for a specific purpose: | ||
|
|
||
| ### `monkeypatch` — for attributes and environment | ||
|
|
||
| Use `monkeypatch.setattr` to replace module-level constants and object attributes. Use `monkeypatch.setenv` for environment variables. These auto-revert when the test ends. | ||
|
|
||
| ```python | ||
| def test_custom_config_dir(monkeypatch, tmp_path): | ||
| monkeypatch.setattr("studyctl.settings.CONFIG_DIR", tmp_path) | ||
| monkeypatch.setenv("STUDYCTL_CONFIG", str(tmp_path / "config.yaml")) | ||
| # settings module now reads from tmp_path | ||
| ``` | ||
|
|
||
| ### `unittest.mock.patch` — for callables | ||
|
|
||
| Use `patch` when you need to replace a function AND verify it was called. The `MagicMock`/`AsyncMock` objects provide `.assert_called_once_with()`, `.call_args`, etc. | ||
|
|
||
| ```python | ||
| from unittest.mock import patch | ||
|
|
||
| def test_pypi_check_offline(tmp_path): | ||
| with patch("studyctl.doctor.updates._fetch_pypi_version", return_value="2.1.0"): | ||
| result = check_update_available() | ||
| assert result.status == "pass" | ||
| ``` | ||
|
|
||
| For async code, use `AsyncMock`: | ||
|
|
||
| ```python | ||
| from unittest.mock import AsyncMock, patch | ||
|
|
||
| @patch("studyctl.content.notebooklm_client.asyncio.sleep", new_callable=AsyncMock) | ||
| async def test_rate_limit(mock_sleep): | ||
| await generate_with_rate_limit() | ||
| mock_sleep.assert_called() | ||
| ``` | ||
|
|
||
| ### When to use which | ||
|
|
||
| | Scenario | Tool | | ||
| |----------|------| | ||
| | Replace a module constant (`CONFIG_DIR`, `DEFAULT_DB`) | `monkeypatch.setattr` | | ||
| | Set/override an env var | `monkeypatch.setenv` | | ||
| | Replace a function to control its return value | `patch` (context manager) | | ||
| | Replace a function AND verify call arguments | `patch` (decorator or context manager) | | ||
| | Replace an async function | `patch` with `new_callable=AsyncMock` | | ||
| | Replace `shutil.which` for tool detection | `patch` | | ||
| | Replace `subprocess.run` for command isolation | `patch` | | ||
|
|
||
| ## Test Markers | ||
|
|
||
| ### `@pytest.mark.integration` | ||
|
|
||
| For tests that require external infrastructure (tmux, network, real databases). These are excluded from fast local runs: | ||
|
|
||
| ```python | ||
| import pytest | ||
|
|
||
| pytestmark = pytest.mark.integration # marks entire module | ||
|
|
||
| # Or per-test: | ||
| @pytest.mark.integration | ||
| def test_tmux_session_lifecycle(): | ||
| ... | ||
| ``` | ||
|
|
||
| Run integration tests explicitly: | ||
|
|
||
| ```bash | ||
| uv run pytest -m integration # only integration tests | ||
| uv run pytest -m "not integration" # skip integration tests | ||
| ``` | ||
|
|
||
| ### `pytest.importorskip` — for optional dependencies | ||
|
|
||
| Tests that require optional packages use `importorskip` at module level. The test file is collected normally but all tests in it are **skipped** at runtime if the dependency is missing: | ||
|
|
||
| ```python | ||
| # At the top of the file, before any other imports from the optional package: | ||
| pytest.importorskip("pymupdf") | ||
| pytest.importorskip("fastapi") | ||
| ``` | ||
|
|
||
| Currently skipped groups: `pymupdf` (content splitter), `notebooklm` (notebooklm client), `fastapi` (web app/artefacts), `mcp` (MCP tools). | ||
|
|
||
| ## Adding a New Test | ||
|
|
||
| ### For studyctl | ||
|
|
||
| 1. Create `packages/studyctl/tests/test_<module>.py` | ||
| 2. Import helpers if needed: `from _helpers import make_review_db` | ||
| 3. Define fixtures inline or wrap helpers in `@pytest.fixture` | ||
| 4. If testing optional-dep code, add `pytest.importorskip("package")` at module level | ||
| 5. Do NOT create a conftest.py | ||
|
|
||
| ### For agent-session-tools | ||
|
|
||
| 1. Create `packages/agent-session-tools/tests/test_<module>.py` | ||
| 2. Use conftest fixtures (`temp_db`, `migrated_db`, etc.) directly in test signatures | ||
| 3. For exporter tests, use `migrated_db` — it has all migrations applied | ||
| 4. Add new shared fixtures to conftest.py if they'll be used by 3+ test files | ||
|
|
||
| ### Naming | ||
|
|
||
| - Test files: `test_<module_name>.py` | ||
| - Test classes: `Test<Feature>` (optional — flat functions are fine) | ||
| - Test functions: `test_<behaviour_under_test>` | ||
| - Fixtures: descriptive nouns (`review_db`, `isolated_config`, `projects_dir`) | ||
|
|
||
| ## Known TODOs | ||
|
|
||
| - **Coverage configuration** — `pytest-cov` is in dev deps but studyctl has no `[tool.coverage.run]` config. Agent-session-tools does. | ||
| - **`addopts` split** — running from workspace root uses `--import-mode=importlib` only. Running from a member directory uses that member's `addopts`. The root path is authoritative. | ||
| - **Worktree tests** — 8 test files in `.claude/worktrees/feat+live-session-dashboard/` need merging (blocked on 5 source modules landing in main first). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # Test Consolidation & TDD Foundation | ||
|
|
||
| **Date**: 2026-04-01 | ||
| **Status**: Decided | ||
| **Next**: `/ce:plan` for cleanup work; TDD learning via fresh project | ||
|
|
||
| --- | ||
|
|
||
| ## What We're Building | ||
|
|
||
| A consolidated, well-documented test suite for socratic-study-mentor, plus a learning path for TDD/pytest skills. | ||
|
|
||
| **NOT a framework from scratch** — extracting and documenting patterns that already exist in 650 tests across two packages. | ||
|
|
||
| ## Why This Approach | ||
|
|
||
| ### The Problem | ||
|
|
||
| - **650 tests exist** (293 studyctl + 357 agent-session-tools) but the user can't maintain them — they were AI-generated | ||
| - **Near-zero pytest experience** — fixtures, parametrize, markers, conftest patterns are unfamiliar territory | ||
| - **Inconsistent patterns**: 3 different mocking styles, 7 copy-pasted `migrated_db` fixtures, no pytest config for studyctl | ||
| - **AuDHD users waiting** for the app — can't pause features for a learning sabbatical | ||
| - **conftest prohibition** in studyctl/tests (pluggy conflict with agent-session-tools) forces inline fixtures | ||
|
|
||
| ### Rejected Approaches | ||
|
|
||
| 1. **"Build comprehensive test framework first"** — Rejected. This is Big Design Up Front for tests. YAGNI applies to test infrastructure. You can't design fixtures for features that don't exist yet. | ||
|
|
||
| 2. **"TDD bootcamp on this codebase"** — Rejected for now. Retrofitting TDD onto 650 AI-generated tests is the hardest way to learn. Better to learn TDD on a fresh project where every decision is yours. | ||
|
|
||
| 3. **"Document conventions, then apply"** — Rejected as primary approach. Risks the same "framework first" trap. Conventions emerge from practice. | ||
|
|
||
| ### Chosen Approach: Three-Part Split | ||
|
|
||
| 1. **Cleanup (this session)**: Mechanical consolidation — dedup fixtures, merge worktree tests, add pytest config, write TESTING.md. Claude does the work with heavy teaching commentary. | ||
|
|
||
| 2. **Feature work continues**: New features get properly tested by Claude, with inline teaching moments explaining every test pattern used. User learns by active observation. | ||
|
|
||
| 3. **TDD from scratch (future)**: User starts a new project using TDD from day one. Fresh codebase, full ownership, concepts grounded by patterns observed here. | ||
|
|
||
| ## Key Decisions | ||
|
|
||
| | Decision | Choice | Rationale | | ||
| |----------|--------|-----------| | ||
| | Learn TDD on this codebase? | No — fresh project later | Retrofitting is the hardest way to learn | | ||
| | Build test framework first? | No — extract from existing | YAGNI; emergent architecture over BDUF | | ||
| | Include worktree tests? | Yes | 7 new files with useful patterns (tmux integration, session state, parking lot) | | ||
| | Who does the cleanup? | Claude, with teaching commentary | User learns by watching, ships features in parallel | | ||
| | conftest for studyctl? | Still no — use test helpers module instead | Pluggy conflict is real; a `tests/_helpers.py` can share fixtures without conftest | | ||
| | Mock convention? | `monkeypatch` for attrs/env, `patch` for functions | Document when to use which; standardise across both packages | | ||
|
|
||
| ## Current State Audit | ||
|
|
||
| ### What works | ||
| - 650 tests, 0 collection errors (after `uv sync --all-packages`) | ||
| - agent-session-tools has clean conftest with 5 shared fixtures | ||
| - `--import-mode=importlib` at root prevents pluggy conflicts | ||
| - `pytest.importorskip()` pattern for optional deps is clean | ||
| - Exporter tests use real fake filesystems (good integration pattern) | ||
|
|
||
| ### What needs fixing | ||
| - 7 identical `migrated_db` fixtures across exporter tests → move to conftest | ||
| - studyctl has no `[tool.pytest.ini_options]` in its pyproject.toml | ||
| - `pytest.mark.live` declared but never used → either use it or remove it | ||
| - 3 mocking styles with no convention for when to use which | ||
| - 7 worktree test files need merging with conventions applied | ||
| - No TESTING.md documenting patterns for contributors | ||
|
|
||
| ### What's intentional (don't change) | ||
| - No conftest.py in studyctl/tests (pluggy conflict) | ||
| - Inline fixtures in studyctl tests (consequence of above) | ||
| - `pytest.importorskip()` at module level for optional deps | ||
| - `--import-mode=importlib` (load-bearing for workspace) | ||
|
|
||
| ## Cleanup Scope | ||
|
|
||
| ### Phase 1: Mechanical (Claude does this) | ||
| - [ ] Deduplicate `migrated_db` fixture into agent-session-tools conftest | ||
| - [ ] Add `[tool.pytest.ini_options]` to studyctl's pyproject.toml | ||
| - [ ] Resolve `pytest.mark.live` — use it or remove it | ||
| - [ ] Merge 7 worktree test files into main tree | ||
| - [ ] Create `packages/studyctl/tests/_helpers.py` for shared fixture functions (not conftest) | ||
| - [ ] Write `docs/TESTING.md` with conventions | ||
|
|
||
| ### Phase 2: Ongoing (with features) | ||
| - [ ] Every new feature gets TDD-style tests with teaching commentary | ||
| - [ ] Teaching moments saved to Obsidian for reference | ||
|
|
||
| ### Phase 3: User's learning path (separate) | ||
| - [ ] Start fresh project using TDD from scratch | ||
| - [ ] Study-Mentor session on pytest/TDD fundamentals | ||
|
|
||
| ## Open Questions | ||
|
|
||
| *None — all resolved during brainstorming dialogue.* | ||
|
|
||
| ## Success Criteria | ||
|
|
||
| 1. All 650+ tests pass from workspace root with `uv run pytest` | ||
| 2. Zero duplicated fixtures | ||
| 3. TESTING.md exists with clear conventions for: fixture patterns, mock conventions, test tiers, how to run tests | ||
| 4. User can read any test file and understand what it does (teaching moments bridge the gap) | ||
| 5. New features get properly tested with patterns explained inline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guide hard-codes exact test counts (293/357/650). Those numbers will drift as tests are added/removed, which can quickly make the doc misleading. Consider removing the counts or phrasing them as approximate (or pointing readers to
pytest --collect-only/pytest -qto see current totals).