|
| 1 | +# Testing Architecture |
| 2 | + |
| 3 | +This document describes the structure and conventions of the |
| 4 | +Whitelist Manager test suite. |
| 5 | + |
| 6 | +It exists because Ring 0 of the test-coverage hardening track |
| 7 | +(2026-05-07) revealed that without an architecture document, the |
| 8 | +test suite drifts: zombies accumulate (94 dead tests for 5+ weeks |
| 9 | +in our case), shallow contract tests spread, and developers can't |
| 10 | +tell which layer to add a new test to. This file is the |
| 11 | +authoritative answer to "where does this test go?" and "what does |
| 12 | +it need to assert?" |
| 13 | + |
| 14 | +## Test layers |
| 15 | + |
| 16 | +``` |
| 17 | +┌──────────────────────────────────────────────────────────────┐ |
| 18 | +│ tests/e2e/ — Playwright .cjs, full browser flows │ |
| 19 | +│ tests/integration/ — Python, real Splunk container │ |
| 20 | +│ tests/unit/ — Python, pure helpers + Splunk stubs │ |
| 21 | +└──────────────────────────────────────────────────────────────┘ |
| 22 | +``` |
| 23 | + |
| 24 | +### `tests/unit/` — pure-helper tests |
| 25 | + |
| 26 | +Tests for library code that has NO dependency on a running Splunk |
| 27 | +instance. Imports from `bin/` go through `tests/conftest.py`, |
| 28 | +which adds `tests/stubs/` to the path so `splunk.rest` and |
| 29 | +`splunk.persistconn.application` resolve to the no-op stubs. |
| 30 | + |
| 31 | +What lives here: |
| 32 | + |
| 33 | +- Validators (`wl_validation.py`, `wl_rbac.py`) |
| 34 | +- Pure transformations (`wl_csv.py` diff/parse, `wl_versions.py` |
| 35 | + manifest helpers, `wl_approval.project_pending_info`) |
| 36 | +- HMAC primitives, file lock helpers, audit event builders |
| 37 | +- Anything callable as a function with no side effects on Splunk |
| 38 | + state |
| 39 | + |
| 40 | +What does NOT live here: |
| 41 | + |
| 42 | +- Tests that need to call a REST endpoint |
| 43 | +- Tests that need to verify KV collection state |
| 44 | +- Tests that need to interact with a running handler instance |
| 45 | + |
| 46 | +Speed: <1 second total. Run on every save (`pytest tests/unit/`). |
| 47 | + |
| 48 | +### `tests/integration/` — handler-with-container tests |
| 49 | + |
| 50 | +Tests that exercise the handler end-to-end against the real |
| 51 | +`wl_manager_test` Docker container. These are the slow but |
| 52 | +authoritative tests — they catch projection drift, dispatch |
| 53 | +table bugs, RBAC mismatches, and audit emission gaps. |
| 54 | + |
| 55 | +Two kinds: |
| 56 | + |
| 57 | +1. **Container-state tests** (`@pytest.mark.docker`) — call REST |
| 58 | + endpoints via `_docker_curl()`, assert response shapes, |
| 59 | + inspect KV state, verify audit events. State-mutating tests |
| 60 | + MUST use the `container_state` fixture (see below). |
| 61 | + |
| 62 | +2. **In-process handler tests** (no `@pytest.mark.docker`, default |
| 63 | + collection) — instantiate `WhitelistHandler()` directly, |
| 64 | + exercise method-level contracts. Faster than container tests, |
| 65 | + slower than unit tests. Useful when the goal is to verify the |
| 66 | + Python code path without the network round-trip. |
| 67 | + |
| 68 | +Speed: ~10-30 seconds for a full integration run. Run on every |
| 69 | +PR. |
| 70 | + |
| 71 | +### `tests/e2e/` — Playwright browser flows |
| 72 | + |
| 73 | +Full-stack tests that drive the Splunk Web UI via Playwright. |
| 74 | +Used for user-flow validation (click X, see Y), cross-browser |
| 75 | +checks, visual regression, and accessibility. |
| 76 | + |
| 77 | +These complement the integration tests — integration covers the |
| 78 | +backend contract, E2E covers the frontend wiring. |
| 79 | + |
| 80 | +Speed: minutes per file. Run on every PR but in a separate CI |
| 81 | +job from the Python suite so a flaky test in one doesn't block |
| 82 | +the other. |
| 83 | + |
| 84 | +## Container-state isolation (`container_state` fixture) |
| 85 | + |
| 86 | +State-mutating integration tests use the `container_state` pytest |
| 87 | +fixture defined in `tests/integration/conftest.py`. The fixture: |
| 88 | + |
| 89 | +1. **Snapshots** the relevant container state before the test: |
| 90 | + - `lookups/` directory (all approval queues, version |
| 91 | + snapshots, FIM baselines, daily limits, notifications, |
| 92 | + trash, KV mirror files) |
| 93 | + - All KV collections that the handler maintains |
| 94 | + (`wl_cooldowns`, `wl_fim_baseline`, `wl_presence`, |
| 95 | + `wl_lockdown`) |
| 96 | +2. **Yields** to the test |
| 97 | +3. **Restores** the snapshot — replaces files, repopulates KV |
| 98 | + |
| 99 | +Cost: ~2-4 seconds per test (snapshot + restore + tar I/O). This |
| 100 | +is the price we pay for accuracy. The user's explicit decision |
| 101 | +during Ring 0 was "container tests for accuracy" over speed. |
| 102 | + |
| 103 | +Tests that DON'T mutate state can opt out by NOT requesting the |
| 104 | +`container_state` fixture — they run faster and don't pay the |
| 105 | +restore cost. |
| 106 | + |
| 107 | +### When to use `container_state` |
| 108 | + |
| 109 | +Required: |
| 110 | + |
| 111 | +- Any test that issues a POST that writes to disk or KV |
| 112 | +- Any test that triggers a notification or audit event |
| 113 | +- Any test that creates/modifies/deletes a CSV, rule, or trash |
| 114 | + entry |
| 115 | +- Any test that approves, rejects, or expires queue entries |
| 116 | + |
| 117 | +Not required: |
| 118 | + |
| 119 | +- Pure GET tests that only read state (`get_csv_content`, |
| 120 | + `get_pending_approvals`, `list_trash`) |
| 121 | +- Tests that only verify dispatch table integrity |
| 122 | +- Tests that only check method existence / signatures |
| 123 | + |
| 124 | +### When `container_state` is NOT enough |
| 125 | + |
| 126 | +The fixture restores the application's state (lookups + KV). It |
| 127 | +does NOT restore: |
| 128 | + |
| 129 | +- Splunk's internal indexes (`wl_audit`, `_internal`, |
| 130 | + `_introspection`). Audit events emitted by the test will |
| 131 | + remain in the index. Tests that need to verify audit emission |
| 132 | + should use the `audit_query` fixture (Ring 1 Day 4) which |
| 133 | + queries by a unique marker the test embeds in the event. |
| 134 | +- Splunk's runtime in-memory state (RBAC cache, session keys). |
| 135 | + These survive across tests but are reset on container restart. |
| 136 | +- Files outside `lookups/` (e.g., `default/`, `bin/`, |
| 137 | + `appserver/`). Tests should never mutate these. |
| 138 | + |
| 139 | +If a test needs broader state reset, use the |
| 140 | +`@pytest.mark.docker_restart` marker which restarts Splunk |
| 141 | +between tests. This is even slower (~30 seconds per test) and |
| 142 | +should be used sparingly — only for tests of in-memory cached |
| 143 | +state (e.g., HMAC key derivation, lockdown TTL). |
| 144 | + |
| 145 | +## What "complete" coverage looks like (Ring 1 standards) |
| 146 | + |
| 147 | +Every endpoint test must: |
| 148 | + |
| 149 | +1. Run against the real container (per user's accuracy |
| 150 | + preference) |
| 151 | +2. **Inspect the FULL response shape** — every documented field, |
| 152 | + not just top-level keys. This is the build-641 / R0-F5 |
| 153 | + class. Shallow tests pass while real bugs ship. |
| 154 | +3. Carry a corresponding mutation gate — sabotage the handler in |
| 155 | + 1-2 ways the test should catch, confirm failure, restore. |
| 156 | + Mutation kill rate ≥80% per ring. |
| 157 | +4. Use `container_state` if it mutates state. |
| 158 | +5. Have a clear docstring stating: what it pins, what bug class |
| 159 | + it catches, what failure mode it would surface. |
| 160 | + |
| 161 | +### Anti-pattern: shallow contract tests |
| 162 | + |
| 163 | +```python |
| 164 | +# BAD — what build-641 slipped past |
| 165 | +def test_get_pending_approvals_response_shape(self, docker_available): |
| 166 | + code, body = _docker_curl("get_pending_approvals") |
| 167 | + assert code == 200 |
| 168 | + assert "pending" in body or "pending_approvals" in body |
| 169 | +``` |
| 170 | + |
| 171 | +This test asserts the response has a top-level array key and |
| 172 | +stops. A projection that strips every field except `request_id` |
| 173 | +from each entry passes. **It exists in name only.** |
| 174 | + |
| 175 | +### Pattern: deep contract tests |
| 176 | + |
| 177 | +```python |
| 178 | +# GOOD — pins the full contract per entry |
| 179 | +PENDING_INFO_FIELDS = { |
| 180 | + "request_id", "action_type", "description", "comment", |
| 181 | + "analyst", "timestamp", "pending_highlight", "payload", |
| 182 | +} |
| 183 | + |
| 184 | +def test_pending_approvals_entry_carries_full_shape( |
| 185 | + self, docker_available, container_state, seeded_pending_request): |
| 186 | + """get_pending_approvals must return entries with all 8 fields. |
| 187 | +
|
| 188 | + Pins: build-641 projection contract. |
| 189 | + Catches: any projection that drops a field on the way to the |
| 190 | + frontend (the build-641 bug class). Both endpoints that |
| 191 | + return pending_info shapes (_get_csv_content and |
| 192 | + _action_get_pending_approvals) share this contract. |
| 193 | + """ |
| 194 | + code, body = _docker_curl("get_pending_approvals") |
| 195 | + assert code == 200 |
| 196 | + assert body["pending_approvals"], "fixture failed: no pending" |
| 197 | + for entry in body["pending_approvals"]: |
| 198 | + assert set(entry.keys()) == PENDING_INFO_FIELDS, \ |
| 199 | + f"projection drift: {entry.keys() ^ PENDING_INFO_FIELDS}" |
| 200 | +``` |
| 201 | + |
| 202 | +Every Ring 1 contract test follows the second pattern. |
| 203 | + |
| 204 | +## Markers |
| 205 | + |
| 206 | +Defined in `tests/pytest.ini`: |
| 207 | + |
| 208 | +| Marker | Meaning | |
| 209 | +|--------|---------| |
| 210 | +| `unit` | Pure-helper test, no Splunk dependency | |
| 211 | +| `integration` | Handler-level test, may use Splunk stubs | |
| 212 | +| `docker` | Requires the `wl_manager_test` container to be running | |
| 213 | +| `docker_restart` | Requires Splunk to be restarted between tests (use sparingly) | |
| 214 | +| `slow` | Test takes >5 seconds — concurrent, fuzz, stress, E2E | |
| 215 | +| `crud` | Core CRUD workflow (add/edit/remove/revert) | |
| 216 | +| `approval` | Approval workflow (submit/approve/reject) | |
| 217 | +| `revert` | Version revert | |
| 218 | +| `admin` | Admin panel actions | |
| 219 | +| `stress` | Wide CSV / large row count | |
| 220 | +| `security` | Security/attack/injection | |
| 221 | + |
| 222 | +When adding a new test, pick the most specific applicable marker. |
| 223 | +Tests can have multiple markers (e.g., `@pytest.mark.docker |
| 224 | +@pytest.mark.approval`). |
| 225 | + |
| 226 | +## Running the suite |
| 227 | + |
| 228 | +```bash |
| 229 | +# Fastest — unit tests only, ~1 second |
| 230 | +pytest tests/unit/ |
| 231 | + |
| 232 | +# Standard PR check — unit + non-docker integration, ~15 seconds |
| 233 | +pytest tests/unit tests/integration -m "not docker and not slow" |
| 234 | + |
| 235 | +# Full suite including docker, ~1 minute |
| 236 | +pytest tests/unit tests/integration |
| 237 | + |
| 238 | +# Just the docker-marked tests, ~15 seconds (after container is up) |
| 239 | +pytest tests/integration -m docker |
| 240 | + |
| 241 | +# Full suite + slow + E2E, ~10 minutes |
| 242 | +pytest tests/ |
| 243 | + |
| 244 | +# Specific marker |
| 245 | +pytest -m approval |
| 246 | +pytest -m "docker and approval" |
| 247 | +``` |
| 248 | + |
| 249 | +## Adding a new test |
| 250 | + |
| 251 | +1. **Pick a layer** — unit if it's a pure function, integration |
| 252 | + if it needs the handler, e2e if it needs the browser |
| 253 | +2. **Pick markers** — at minimum the layer marker, plus any |
| 254 | + workflow markers that apply |
| 255 | +3. **Use the right isolation** — `container_state` for any |
| 256 | + integration test that mutates state |
| 257 | +4. **Write a deep contract assertion** — full response shape, not |
| 258 | + just top-level |
| 259 | +5. **Add a docstring** that names the bug class the test catches |
| 260 | +6. **Run the mutation gate** — pick a way the production code |
| 261 | + could be broken that the test SHOULD catch, sabotage, run, |
| 262 | + confirm failure, restore |
| 263 | + |
| 264 | +## Why Ring 0 happened |
| 265 | + |
| 266 | +Five weeks of fictional safety net (94 zombie tests) shipped |
| 267 | +because: |
| 268 | + |
| 269 | +- The test files used `try/except ImportError` which silently |
| 270 | + set the handler to None and skipped every test |
| 271 | +- pytest's "skipped" output was indistinguishable from |
| 272 | + legitimate "needs docker, ok skipped" cases |
| 273 | +- No test architecture document existed to explain what each |
| 274 | + layer should look like |
| 275 | + |
| 276 | +Ring 0 fixed the immediate damage. This document is the lesson |
| 277 | +written down so the next contributor — or the next Claude |
| 278 | +session — has the architecture in front of them before they add |
| 279 | +a test that drifts the same way. |
| 280 | + |
| 281 | +When in doubt: **deep contract over shallow shape, container over |
| 282 | +mock, mutation-gated over feeling-confident.** |
0 commit comments