Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ These are always-on. Deeper guidance (modern EF features, transactions, caching
## Testing

- Unit tests for domain logic and handlers
- **Test structure — AAA with narrative comments.** Every test is structured as **Arrange → Act → Assert** with `// ARRANGE`, `// ACT`, `// ASSERT` markers (all caps, em-dash explanation on the same line). Each phase carries a *story comment*: explain what's being set up and *why it matters*, what's being called, and what each assertion is verifying. A junior dev should be able to read a single test top-to-bottom and understand the contract being checked + the failure mode being guarded against — without having to read the SUT first. When the ASSERT phase verifies multiple invariants, number them and explain why each matters (especially for security boundaries, idempotency guards, and ordering-sensitive operations like cache-after-save). Trivial happy-path tests can be shorter; security/concurrency/idempotency tests get the full story. Reference templates: [ProductAuthorizationTests.cs](tests/CatalogService.Tests.Integration/ProductAuthorizationTests.cs) (IDOR-prevention + rejected-write invariants), [PaymentFailedHandlerTests.cs](tests/OrderService.Tests.Unit/Application/PaymentFailedHandlerTests.cs) (idempotency under at-least-once delivery), [GetShipmentByOrderHandlerTests.cs](tests/ShippingService.Tests.Unit/Application/GetShipmentByOrderHandlerTests.cs) (IDOR-prevention pattern, unit-tier).
- **Test structure — AAA with narrative comments.** Every test is structured as **Arrange → Act → Assert** with `// ARRANGE`, `// ACT`, `// ASSERT` markers (all caps, em-dash explanation on the same line). Each phase carries a *story comment*: explain what's being set up and *why it matters*, what's being called, and what each assertion is verifying. A junior dev should be able to read a single test top-to-bottom and understand the contract being checked + the failure mode being guarded against — without having to read the SUT first. When the ASSERT phase verifies multiple invariants, number them and explain why each matters (especially for security boundaries, idempotency guards, and ordering-sensitive operations like cache-after-save). Trivial happy-path tests can be shorter; security/concurrency/idempotency tests get the full story. Reference templates: [ProductAuthorizationTests.cs](tests/CatalogService.Tests.Integration/ProductAuthorizationTests.cs) (IDOR-prevention + rejected-write invariants, integration-tier), [OrderSagaTests.cs](tests/OrderService.Tests.Integration/OrderSagaTests.cs) (saga consume-side idempotency under at-least-once delivery, integration-tier), [OrderTests.cs](tests/OrderService.Tests.Unit/Domain/OrderTests.cs) (aggregate state-machine invariants, unit-tier — domain-only, no `DbContext`).
- Integration tests with Testcontainers for infrastructure — `tests/{Service}.Tests.Integration`, booting the real API via `WebApplicationFactory<Program>`. Four service slices ship and each runs as its own step in `.github/workflows/ci.yml` so a single-slice failure doesn't mask the rest: **CatalogService** (Postgres + Redis — caching + concurrency token + IDOR), **OrderService** (SQL Server + Wolverine stubbed transport — outbox, saga handlers, `RowVersion` token), **PaymentService** (SQL Server — outbox-in-non-handler wrap, retry/recovery job), **ShippingService** (Postgres — IDOR-by-order saga handlers). Pattern documented in each project's README.
- **Integration tests need Docker.** On macOS, Docker Desktop's socket is at `~/.docker/run/docker.sock`, not `/var/run/docker.sock` — Testcontainers fails fast with `DockerUnavailableException` unless `DOCKER_HOST` points there (or Docker Desktop's "default Docker socket" toggle is on). CI runners have it at the standard path.
- **Fake credentials in test fixtures are suppressed with inline `// gitleaks:allow` markers — there is no project-level gitleaks config.** Some factory fakes have to be protocol-syntax-valid: the fake Azure Service Bus connection string in `OrderApiFactory.cs` / `PaymentApiFactory.cs` / `ShippingApiFactory.cs` is required because `Program.cs` parses `UseAzureServiceBus(GetConnectionString("messaging"))` eagerly at registration time, *before* `DisableAllExternalWolverineTransports()` stubs the transport. The convention: keep the high-entropy base64-encoded **self-labeling** literal (the project's fake `SharedAccessKey` base64-decodes to `fake-shared-key-for-testing-only`) and add `// gitleaks:allow` to the end of the line containing the literal. Gitleaks' default `generic-api-key` rule fires on the entropy of `key=value` strings regardless of value content, so realistic-looking fakes get flagged at PR-scan time. The fix is the **inline marker**, not lowering the fake's entropy below scanner thresholds. **Why no `.gitleaks.toml` lives at repo root:** a path+regex `[[allowlists]]` block was tried and removed — global `[[allowlists]]` requires gitleaks ≥ 8.25.0 (the action ships 8.24.x) and `MatchCondition` defaults to OR not AND, so the block both failed to fire on the pinned scanner version AND would have over-matched if it did. The inline marker is reliable across all 8.x versions, scoped to the exact line, and self-documenting at the call site. Pasting a real key into the same file still trips because the marker only suppresses THAT specific finding line, and pasting the fake literal anywhere without the marker still trips. Do NOT also reproduce the high-entropy literal in CLAUDE.md or any other prose — the scanner walks every diff, including documentation. **The diff-range residue trap:** gitleaks scans every commit in the PR range, not just HEAD, so a fix-in-a-later-commit doesn't suppress findings introduced un-marked in an earlier commit — squash the branch if you hit this. Reference: any of the three factory files for the marker pattern; [.github/workflows/gitleaks.yml](.github/workflows/gitleaks.yml) for the CI wiring.
Expand Down
2 changes: 1 addition & 1 deletion docs/dev-loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ suggestions. Every PR-review pass checks for them.

| Pattern | When required | Why required |
|---|---|---|
| **AAA with narrative comments** | Every test. `// ARRANGE` / `// ACT` / `// ASSERT` all-caps with em-dash explanation; multi-invariant ASSERTs numbered with rationale. | A junior dev reads one test top-to-bottom and understands the contract + failure mode without reading the SUT. Reference templates: [ProductAuthorizationTests.cs](../tests/CatalogService.Tests.Integration/ProductAuthorizationTests.cs), [PaymentFailedHandlerTests.cs](../tests/OrderService.Tests.Unit/Application/PaymentFailedHandlerTests.cs), [GetShipmentByOrderHandlerTests.cs](../tests/ShippingService.Tests.Unit/Application/GetShipmentByOrderHandlerTests.cs). |
| **AAA with narrative comments** | Every test. `// ARRANGE` / `// ACT` / `// ASSERT` all-caps with em-dash explanation; multi-invariant ASSERTs numbered with rationale. | A junior dev reads one test top-to-bottom and understands the contract + failure mode without reading the SUT. Reference templates: [ProductAuthorizationTests.cs](../tests/CatalogService.Tests.Integration/ProductAuthorizationTests.cs), [OrderSagaTests.cs](../tests/OrderService.Tests.Integration/OrderSagaTests.cs), [OrderTests.cs](../tests/OrderService.Tests.Unit/Domain/OrderTests.cs). |
| **IDOR test** | Every new endpoint that returns or mutates a scoped entity | Authenticate as buyer X, request a resource owned by buyer Y, assert **404 (not 403)**. Absence of this test is exactly how the original `GET /api/v1/orders/{id}` IDOR survived undetected for the lifetime of the codebase. |
| **Outbox-in-non-handler test** | Code paths publishing events from outside a Wolverine handler (BackgroundService sweepers, recovery jobs) | Assert a row appears in `wolverine.outgoing_envelopes` in the same transaction as the entity write. The PaymentRecoveryJob outbox bug survived because no test asserted that. |
| **Handler-DI-registration check** | Any integration test using `scope.ServiceProvider.GetRequiredService<MyHandler>()` | Wolverine's handler-discovery does NOT populate `IServiceCollection` — handlers resolved directly in tests must be `AddScoped<MyHandler>()`'d in `AddXInfrastructure`. Failure mode: `InvalidOperationException` on first test run. Catch at PR review, not in CI. |
Expand Down
Loading