From e9ff6cde779883155e4ab4fe1f84bb0905338516 Mon Sep 17 00:00:00 2001
From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com>
Date: Fri, 5 Jun 2026 14:04:10 +0100
Subject: [PATCH] docs(tests): document helper conventions

Documentation-only PR continuing #2523. Adds tests/README.md to document helper conventions, validation expectations, and the next test-suite refactor phase.
---
 tests/README.md | 106 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 tests/README.md

diff --git a/tests/README.md b/tests/README.md
new file mode 100644
index 000000000..03633ae98
--- /dev/null
+++ b/tests/README.md
@@ -0,0 +1,106 @@
+# Test Suite Notes
+
+## Purpose
+
+This file documents the shared test helpers and the review expectations that go
+with them. The suite is being refactored incrementally, so this is a working
+reference for that effort — not a claim that the suite is already fully
+organized. Read it before adding a new helper or before reviewing a PR that
+touches `tests/helpers/`.
+
+## Core principles
+
+- Keep PRs small and homogeneous: one kind of change per PR.
+- Prefer explicit local setup over hidden global fixtures.
+- Avoid expanding the root `conftest.py` unless absolutely necessary.
+- Do not mix file moves with logic changes in the same PR.
+- Do not weaken tests with `skip`/`xfail` just to make CI pass.
+- Validate the focused files you changed, plus any neighboring or
+  order-sensitive groups they interact with.
+
+## Helper conventions
+
+The helpers below live under `tests/helpers/`. They exist to remove repeated
+boilerplate that already appeared across multiple tests. Reach for one only when
+your test matches its intended use; do not stretch a helper to cover a new case.
+
+### `tests.helpers.cli_loader.load_script`
+
+Use when a test needs to import a script under `scripts/` without repeating
+`SourceFileLoader` / `importlib.util` boilerplate.
+
+- Intended for script/CLI tests that load a single file from `scripts/`.
+- Not for arbitrary package imports — use a normal `import` for those.
+- When migrating an existing test to it, keep the existing stubs and assertions
+  unchanged. Any `sys.modules` stubs the script needs at import time must still
+  be injected (e.g. via `monkeypatch`) before calling `load_script`.
+
+### `tests.helpers.import_state.clear_module`
+
+Use when a test must drop one cached module and its parent-package attribute
+before a fresh import.
+
+- Clears `sys.modules[name]`.
+- Clears the parent-package attribute when present.
+- Good replacement for local `sys.modules.pop(...)` + `delattr(parent, child)`
+  blocks.
+
+### `tests.helpers.import_state.preserve_import_state`
+
+Use when a test temporarily installs stubs into `sys.modules` and needs
+deterministic cleanup afterward.
+
+- Context manager: restores both `sys.modules` entries and parent-package
+  attributes on exit (normal or exception).
+- Useful around module-level stubs or temporary imports.
+- Prefer narrow, explicit module names over broad ones.
+
+### `tests.helpers.import_state.clear_fake_database_modules`
+
+Use only for the guarded fake/stub database cleanup pattern.
+
+- Preserves a real-looking `core.database` (one with a string `__file__`).
+- Removes a fake/stub `core.database` and the related `src.database` state.
+- Do not use as a general database reset fixture.
+
+### `tests.helpers.import_state.clear_fake_endpoint_resolver_modules`
+
+Use only for the guarded fake/stub `src.endpoint_resolver` cleanup pattern.
+
+- Preserves real resolver modules (those with a truthy `__file__`).
+- Evicts fake/stub resolver modules and the dependent route modules that were
+  cached against them.
+- Accepts explicit extra dependent module names to evict alongside the defaults.
+
+## What not to abstract yet
+
+Some remaining patterns should stay as-is for now rather than being forced into
+helpers:
+
+- Large mixed files such as security/review regression files.
+- Setup-oriented `sys.modules` stub installers.
+- One-off custom module patching.
+- DB/session/route setup, until it has been audited separately.
+
+## Validation expectations
+
+Run validation locally before opening or approving a PR. Practical checks:
+
+- `git diff --check` — catch whitespace and conflict-marker errors.
+- `python3 -m py_compile <changed files>` — confirm changed files compile.
+- Focused `pytest` on the changed test files.
+- `pytest` on neighboring or order-sensitive test groups that share import
+  state with the changed files.
+- `grep` for the old boilerplate when replacing it, to confirm no stragglers
+  remain.
+- A fresh audit worktree when changing the helpers themselves, so stale
+  `__pycache__` or import state cannot mask a regression.
+
+## Current roadmap
+
+1. Import-state cleanup — complete.
+2. Document helper conventions (this file).
+3. Audit fake DB / `SessionLocal` / route setup duplication.
+4. Add tiny helpers only when the repeated semantics are clear.
+5. Start low-risk file moves only after helper conventions are documented.
+6. Avoid moving high-risk security/route regression files first.