distill: 48 cross-project best-practices from 2026-07 reflection sweep
Promotions from reflecting 21 projects' session logs (incl. agent-runtimes 122-log drain). Adds coverage across networking (eBPF VIP/VPN SNAT/VLAN bridge/forward-auth preflight/ingress TLS), kubernetes (CSI hotplug/PodSecurity debug/self-managed GitOps/runtime annotations), CI (dispatch tokens/runner death/base image), git (CI-rebase/shallow reset/PR governance), python (async session pool/httpx redirects/logging), TDD (AsyncMock/xfail lifecycle), api-integration (SDK parse/token-scope 404/schema probing), plus docker, scripting, debugging, security-architecture, secrets, react, octopus. State: .distill-state.json refreshed with current HEADs + 5 newly-tracked projects.
This commit is contained in:
@@ -423,6 +423,14 @@ def test_regression_crlf_corruption():
|
||||
|
||||
These are the highest-value tests because they catch proven failure modes.
|
||||
|
||||
### `xfail(strict=True)` Is the Right Red Primitive for Specs Ahead of Implementation
|
||||
|
||||
When a spec requirement has no implementation yet, write a test that asserts the not-yet-existing import/attribute/behaviour and mark it `@pytest.mark.xfail(strict=True)`. It keeps CI green while red; when the impl lands and the test passes, strict mode flips it to XPASS (a visible CI failure) that signals "remove the marker." Better than `@pytest.mark.skip` (never runs, false-green) or no marker (breaks CI immediately).
|
||||
|
||||
Two traps: a module-level `pytest.skip(allow_module_level=True)` on a failed import swallows every xfail-strict marker in the file (reports `skipped`, not `xfailed`) — put spec-ahead tests in a file with no module-level skip and lazy-import inside each test body. And `pytest.importorskip("mod.foo")` on the very module being implemented produces a false-green SKIP that lets an agent claim success without writing code — require a `pytest --collect-only` ImportError gate instead.
|
||||
|
||||
**Retire markers once the feature ships.** After each milestone, grep for `xfail(reason=` / stale `xfail` markers and remove the now-obsolete ones — leftover xfail state contributes coverage noise and can hold total coverage below the gate even though the code paths execute. Set `xfail_strict = true` globally so xpassing tests fail loudly and force the cleanup rather than silently rotting.
|
||||
|
||||
## Test Quality Metrics
|
||||
|
||||
### What to Measure
|
||||
@@ -502,6 +510,10 @@ Patching an entire module (e.g., `patch("mod.kubernetes.config")`) replaces exce
|
||||
|
||||
When migrating a codebase from sync to async, helper functions get converted but test functions are often left as sync `def`. Every test that calls an async function needs `async def` + `@pytest.mark.asyncio` + `await`. After any async migration, run tests and grep for `RuntimeWarning: coroutine '...' was never awaited` to find remaining sync-to-async gaps.
|
||||
|
||||
### `AsyncMock.side_effect` on a Sync Call Is Silently Dead
|
||||
|
||||
`mock = AsyncMock(side_effect=SomeError)`; a *synchronous* `mock()` call returns an un-awaited coroutine and never raises — the `side_effect` is discarded and the test passes regardless of implementation (a `pytest.fail` line placed after the call executes, masking the real assertion). Fix: make the test `async def` + `@pytest.mark.asyncio` and `await` the call, or use a plain `MagicMock(side_effect=...)` when the production code is sync. Sibling trap: `getattr(MagicMock(), "attr", None)` returns a *new MagicMock*, not `None` — so `mock.account_id == "x"` is always False. Explicitly set every attribute the code reads (`m.account_id = "x"`) on test MagicMocks.
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### Tests that mirror implementation
|
||||
|
||||
Reference in New Issue
Block a user