Merge pull request #4172 from BrighterCommand/test_naming

refactor(spec-0031): align BoxProvisioning test naming to convention

Author: iancooper

.gitignore

@@ -252,6 +252,9 @@ ServiceFabricBackup/
 
 # SQLite files
 *.db
+*.db-shm
+*.db-wal
+*.db-journal
 *.sqlite
 *.sqlite3
 

PROMPT.md

@@ -1,157 +1,42 @@
-# Resume State — Spec 0030 primitive_obsession
+# Resume State — Spec 0031 test_naming_conventions
 
-**Last updated:** 2026-06-08
-**Branch:** `primitive_obsession`
-**Spec dir:** `specs/0030-primitive_obsession/`  ·  `specs/.current-spec` = `0030-primitive_obsession`
-**Issue:** #4164  ·  ADR: `docs/adr/0061-box-provisioning-value-types.md`
-**HEAD:** `7f451cd96`
+**Last updated:** 2026-06-09
+**Branch:** `test_naming`  ·  **Spec dir:** `specs/0031-test_naming_conventions/`  ·  **Issue:** #4157
 
-## Where we are in the workflow
+## ✅ STATUS: IMPLEMENTATION + VERIFICATION COMPLETE — nothing pushed yet
 
-Issue → **Requirements ✅** → **Design (ADR 0061) ✅** → **Tasks ✅** → **Tests/Code ✅** → **Code Review ✅ (all findings resolved)**
+Spec 0031 was a **pure rename refactor** (no behaviour/assertion/logic/count changes; `/test-first`
+did not apply). All 7 tasks done & committed on `test_naming`:
 
-## Code Review — COMPLETE ✅
-
-Findings from `specs/0030-primitive_obsession/review-code.md` all resolved in `7f451cd96`:
-- **Finding 2 (Score 72)** — Spanner provisioners now forward `_configuration.SchemaName` to migration activity
-- **Finding 1 fix-forward** — ServiceActivator .Value call-site completions for string?-widened operators
-- **Finding 3 (Score 64)** — 4 characterisation tests added for mapper null paths
-- **Phase 2 miss** — 3 BoxProvisioning test doubles updated to `BoxTableName` type
-- **Finding 4 (Score 38)** / **Finding 5 (Score 22)** — low severity, no action required
-
-## Phase 1 — COMPLETE ✅  (111/111 tests, net9.0)
-
-All six value types implemented and committed:
-
-| Commit | What |
+| Commit | Task |
 |---|---|
-| `6262cc15d` | fix: resolve CS8604/CS8601 call-site nullability in `Paramore.Brighter` (boy-scout) |
-| `56f77029a` | feat(spec-0030): `BoxTableName` round-trip + value equality (4 tests) |
-| `056204fc7` | feat(spec-0030): `BoxTableName.IsNullOrEmpty` (3 tests) |
-| `30e7ea169` | feat(spec-0030): `SchemaName` round-trip + nullable + `IsNullOrEmpty` (8 tests) |
-| `2efa24466` | feat(spec-0030): `MigrationDescription` round-trip + `IsNullOrEmpty` (7 tests) |
-| `e8f854397` | feat(spec-0030): `SqlScript` round-trip + nullable + no-validation + `IsNullOrEmpty` (9 tests) |
-| `a92ffc70c` | feat(spec-0030): `SourceReference` round-trip + nullable + `IsNullOrEmpty` (8 tests) |
-| `4cb46c41d` | feat(spec-0030): `MigrationVersion` round-trip + int arithmetic + `IComparable` (7 tests) |
-
-New type files (all in `src/Paramore.Brighter.BoxProvisioning/`):
-- `BoxTableName.cs`, `SchemaName.cs`, `MigrationDescription.cs`
-- `SqlScript.cs`, `SourceReference.cs`, `MigrationVersion.cs`
-
----
-
-## Phase 2 — COMPLETE ✅  (111/111 tests net9.0+net10.0, all TFMs build clean)
-
-Commit: `2d0aaa80c` — retype BoxProvisioning contracts to value types.
-
-All Phase 3 and Phase 4 verification tasks also done:
-- All 4 relational backends + Spanner compile with 0 warnings/errors
-- All 4 TFMs (netstandard2.0/net8/net9/net10) build clean
-- Identifier-validation regression: 11/11 tests pass
-- Monotonicity regression: pass
-- Full suite: 111/111 on net9.0 and net10.0
-- No types leaked outside Paramore.Brighter.BoxProvisioning
-- Core Paramore.Brighter assembly unmodified
-
-## Current phase: CODE REVIEW 🔄
-
-All implementation tasks complete. Ready for `/spec:review` or PR creation.
-
----
-
-### Archived: Phase 2 contract-retyping plan (for reference)
-
-### Task 2a — Retype `IAmABoxMigration` and `BoxMigration`
-
-Files to edit:
-- `src/Paramore.Brighter.BoxProvisioning/IAmABoxMigration.cs`
-- `src/Paramore.Brighter.BoxProvisioning/BoxMigration.cs`
-
-Changes:
-- `int Version` → `MigrationVersion`
-- `string Description` → `MigrationDescription`
-- `string UpScript` → `SqlScript`
-- `string? SourceReference` → `SourceReference?`
-- `string? IdempotencyCheckSql` → `SqlScript?`
-- `LogicalColumns` stays `IReadOnlyCollection<string>` — do NOT change
-
-Verify existing `new BoxMigration(1, "Add Source", "ALTER TABLE …", new[] { "Source" })` call sites in the four relational catalog assemblies compile unchanged via implicit conversions (no argument changes needed).
-
-### Task 2b — Retype `IAmABoxMigrationRunner.MigrateAsync` + fix D4 ternary
-
-Files to edit:
-- `src/Paramore.Brighter.BoxProvisioning/IAmABoxMigrationRunner.cs`
-- `src/Paramore.Brighter.BoxProvisioning/SqlBoxMigrationRunner.cs`
-
-Changes:
-- `MigrateAsync(string tableName, string? schemaName, …)` → `MigrateAsync(BoxTableName tableName, SchemaName? schemaName, …)`
-- **D4 ternary** at `SqlBoxMigrationRunner.cs:285`: change to `migrations.Count == 0 ? (MigrationVersion)0 : migrations[migrations.Count - 1].Version` (resolves CS0172 bidirectional-implicit ambiguity)
-- `Identifiers.AssertSafe(tableName, …)` and `AssertSafe(schemaName, …)` lines ~191/194 pass `string?` via implicit conversion — resolve any CS8604 with `.Value` or `!`, do NOT revert operator return type
-- `SqlBoxProvisioner` call `_migrationRunner.MigrateAsync(BoxTableName, _configuration.SchemaName, …)` compiles via implicit `string? → SchemaName?`
-
-### Task 2c — Retype `IAmABoxProvisioner.BoxTableName`
-
-Files to edit:
-- `src/Paramore.Brighter.BoxProvisioning/IAmABoxProvisioner.cs`
-- `src/Paramore.Brighter.BoxProvisioning/SqlBoxProvisioner.cs` (line ~105)
-
-Changes:
-- `string BoxTableName` property → `BoxTableName`
-- `SqlBoxProvisioner` derives property from `_configuration.OutBoxTableName`/`InBoxTableName` (core `string`) — compiles via implicit conversion, no core changes
-- `Identifiers.AssertSafe(BoxTableName, …)` at line ~105 still compiles via implicit `→ string?`; resolve CS8604 with `.Value` or `!`
-
-### Task 2d — Update test doubles to new types (mechanical, no behaviour)
-
-Files to edit (exhaustive list):
-
-**`IAmABoxMigration` implementers** (update property declarations only):
-- `tests/.../When_relational_box_migration_runner_base_migrate_receives_non_monotonic_migrations_it_should_throw_before_opening_connection.cs` (`StubBoxMigration`)
-- `tests/.../When_relational_box_migration_runner_base_migrate_receives_unsafe_identifier_it_should_throw_before_opening_connection.cs` (`StubBoxMigration`)
-
-**`IAmABoxMigrationRunner` direct implementers** (update `MigrateAsync` signature only):
-- `tests/.../When_sql_box_provisioner_detect_table_state_inlines_negative_version_clamp.cs` (`VersionCapturingMigrationRunner`)
-- `tests/.../When_sql_box_provisioner_effective_schema_name_is_overridden_it_should_propagate_to_detection_and_payload_calls_only.cs` (`SchemaCapturingMigrationRunner`)
-- `tests/.../When_sql_box_provisioner_provision_async_receives_unsafe_identifier_it_should_throw_before_opening_connection.cs` (`ThrowingMigrationRunner`)
-- `tests/.../When_sql_box_provisioner_provision_async_runs_successfully_it_should_invoke_hooks_in_documented_order.cs` (`RecordingMigrationRunner`)
-
-**`IAmABoxProvisioner` implementer**:
-- `tests/.../TestDoubles/StubBoxProvisioner.cs` (`BoxTableName` property type)
-
-Do NOT modify: `StubBoxDetectionHelper.cs` (detection-helper, out of scope).
-
----
-
-## Phase 3 — Backend compilability (after Phase 2)
-
-Build all four relational backends (MsSql, MySql, PostgreSql, Sqlite) + Spanner against the retyped interfaces. Confirm compilation via implicit conversions — no argument changes expected. Also confirm null-path behaviour: SQLite null `SchemaName` and V2+ null `IdempotencyCheckSql` produce no NRE.
-
----
-
-## Phase 4 — Cross-cutting verification (after Phase 3)
-
-1. All TFMs compile (`netstandard2.0;net8.0;net9.0;net10.0`) — confirm no `netstandard2.0`-unavailable APIs
-2. Identifier-validation regression: `1Outbox` → `ConfigurationException`; SQLite null schema → succeeds
-3. Monotonicity regression: `[1,2,3]` passes; `[1,3]` → `ConfigurationException` with `V1 followed by V3 (expected V2)`
-4. Full suite parity + telemetry/log string content unchanged
-5. Scope guard: no new types leaked outside `Paramore.Brighter.BoxProvisioning`; core assembly unmodified
-
----
-
-## Key implementation notes
-
-- **No behaviour changes in Phase 2** — only type signatures change; call sites compile via implicit conversions
-- **D4 ternary** is the one required explicit cast; it is pre-identified at `SqlBoxMigrationRunner.cs:285`
-- **`AssertSafe` stays at call sites** — do NOT move validation into constructors (D3)
-- **`LogicalColumns` unchanged** — stays `IReadOnlyCollection<string>` throughout (D5)
-- **CS8604 pattern**: all string-backed operators return `string?`; when passing into non-nullable `AssertSafe(string, …)`, resolve with `.Value` or `!` at the call site (established codebase pattern)
-- Phase 2 tasks 2a–2c should each be committed separately; 2d (test doubles) committed after 2c
-
-## Test run command
-
+| `a3b2bc33d` | T2 — BoxProvisioning.Tests (2) |
+| `53520bb84` | T3 — Sqlite (22) |
+| `d9947c90b` | T4 — MSSQL (33) |
+| `e9358acb3` | T5 — MySQL (27) |
+| `24c4f168d` | T6 — PostgreSQL (34) |
+| `299f18d5c` | spec docs + T1 baseline + T7 sign-off |
+
+**118 non-conforming files renamed** to the convention in `.agent_instructions/testing.md`
+(class `[Behavior]Tests`, method `When_..._should_...`). 0 WRONG + 0 MIXED remain. All AC-1..10 PASS;
+all 5 projects build clean; count + green/red parity vs baseline (MSSQL keeps its 13 pre-existing
+failures — 12 = DateTimeOffset/BST #4161, 1 now under a renamed FQN). 0 file renames needed (names
+already conformed), 0 `src/`/analyzer/CI/agent-instruction changes, 0 `Assert.` lines touched.
+Full evidence: `specs/0031-test_naming_conventions/.scratch/T7-signoff.md`.
+
+## ▶️ FIRST THING NEXT SESSION — ask before reviewing
+
+This change is **style only** (identifier + comment renames, mechanically verified by build + suite
+parity). Before doing more work, **ASK the user whether a `/spec:review code` / code review is even
+warranted here**, or whether we should skip straight to opening the PR / merging. A full adversarial
+code review is likely overkill for a rename-only diff — surface that and let the user decide.
+
+## Remaining options (pending the user's answer)
+- `/spec:review code` (if they want it) → then PR / merge, OR
+- open PR for branch `test_naming` directly, OR
+- merge to `master`.
+
+## Verify (per project)
 ```bash
-dotnet test tests/Paramore.Brighter.BoxProvisioning.Tests/ --framework net9.0 --no-build -q
+dotnet test tests/Paramore.Brighter.<Project>.Tests --framework net9.0 -q
 ```
-
-## Spec 0030 about
-
-Replace bare `string`/`int` primitives in Box Provisioning public interfaces with six dedicated value-type `record`s modelled on `src/Paramore.Brighter/Id.cs`. Six types: `BoxTableName`, `SchemaName`, `MigrationDescription`, `SqlScript`, `SourceReference`, `MigrationVersion`. Implicit conversions preserve full source compatibility at every existing call site.

specs/.current-spec

@@ -1 +1 @@
-0030-primitive_obsession
+0031-test_naming_conventions

specs/0031-test_naming_conventions/.design-approved

@@ -0,0 +1 @@
+design N/A: correction to an already-documented convention (.agent_instructions/testing.md), not a design decision. Approved 2026-06-09.

specs/0031-test_naming_conventions/.issue-number

@@ -0,0 +1 @@
+4157

specs/0031-test_naming_conventions/.requirements-approved

@@ -0,0 +1,36 @@
+# T7 — Whole-spec sign-off (Spec 0031) — 2026-06-09
+
+Pre-work base commit: 51c2d9f29 (a3b2bc33d^). Spec commits: T2 a3b2bc33d · T3 53520bb84 · T4 d9947c90b · T5 e9358acb3 · T6 24c4f168d.
+
+## Acceptance criteria
+- **AC-9 (0 WRONG + 0 MIXED)**: all 5 folders — 0 `class When_`, 0 `Should_` test methods. PASS.
+- **AC-2 (no Should_ method remains)**: 0 across scope. PASS.
+- **AC-1/AC-4 (classes → [Behavior]Tests; multi-case files structured)**: every test class ends in `Tests`
+  (sole non-Tests public class = `BoxProvisioningObservabilityCollection`, an xUnit `[CollectionDefinition]`
+  fixture — not a test class, untouched). Every `[Fact]`/`[Theory]` method begins with `When_`. PASS.
+- **AC-3 (file renames via git mv, history preserved)**: 0 file renames were required — every in-scope
+  test file name already conformed to `When_..._should_...`, so files were modified in place (118 × M,
+  0 × R). git history is intact (no moves). PASS (vacuously — no rename needed).
+- **AC-5 / FR-5 / NFR-1 (CORRECT files untouched)**: exactly 118 files changed (2+22+33+27+34), matching the
+  non-conforming set; all 77 CORRECT test files + helpers show no diff. PASS.
+- **AC-10 (scope containment)**: (a) 0 files changed outside the 5 in-scope folders; (b) diff contains
+  ONLY class/method/constructor declaration identifiers + comment-reference updates — 0 `Assert.` lines
+  changed, no arrange/act/assertion logic touched; (c) 0 analyzer / .github CI / .editorconfig /
+  agent-instruction / hook / testing.md changes; (d) 0 production (src/) files changed. PASS.
+- **AC-6 (compiles, references resolve)**: all 5 projects `dotnet build` net9.0 = 0 Error(s). PASS.
+- **AC-8 (count parity)** & **AC-7 (green/red parity)** vs T1 baseline — all containers available, every suite run:
+
+| Project | Baseline Total | After Total | Baseline P/F | After P/F | Verdict |
+|---|---|---|---|---|---|
+| BoxProvisioning.Tests | 111 | 111 | 111/0 | 111/0 | parity |
+| Sqlite.Tests | 127 | 127 | 127/0 | 127/0 | parity |
+| MSSQL.Tests | 198 | 198 | 185/13 | 185/13 | parity (13 pre-existing fails preserved; 1 in-scope under renamed FQN `MsSqlRunnerLockResourceSchemaQualificationTests`, 12 = DateTimeOffset/BST #4161) |
+| MySQL.Tests | 160 | 160 | 160/0 | 160/0 | parity |
+| PostgresSQL.Tests | 191 | 191 | 191/0 | 191/0 | parity |
+
+No "infra unavailable" gaps — MSSQL/Postgres/MySQL Docker all up; Spanner not in scope (no Spanner BoxProvisioning rename folder).
+
+## Conclusion
+All FR-1..6, NFR-1..6, AC-1..10 satisfied. Pure rename refactor complete; behaviour, assertions, and test
+counts preserved exactly. Cross-file comment references (4 total: 1 Sqlite, 1 MSSQL, 1 MySQL, 2 Postgres-wait=2)
+updated to new class names; file-name comments (referencing unchanged .cs files) left intact.

specs/0031-test_naming_conventions/.scratch/T7-signoff.md

@@ -0,0 +1,13 @@
+Paramore.Brighter.MSSQL.Tests.BoxProvisioning.When_mssql_runner_acquires_lock_resource_should_be_qualified_by_schema.Should_qualify_lock_resource_with_effective_schema(configuredSchema: null, expectedSchemaInLockResource: "dbo")
+Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Async.WhenAddingAMessageItShouldBeStoredWithAllPropertiesAsync.When_Adding_A_Message_It_Should_Be_Stored_With_All_Properties_Async
+Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Async.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommitAsync.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit_Async
+Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Async.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessageAsync.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message_Async
+Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenAddingAMessageItShouldBeStoredWithAllProperties.When_Adding_A_Message_It_Should_Be_Stored_With_All_Properties
+Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommit.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit
+Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessage.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message
+Paramore.Brighter.MSSQL.Tests.Outbox.Text.Async.WhenAddingAMessageItShouldBeStoredWithAllPropertiesAsync.When_Adding_A_Message_It_Should_Be_Stored_With_All_Properties_Async
+Paramore.Brighter.MSSQL.Tests.Outbox.Text.Async.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommitAsync.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit_Async
+Paramore.Brighter.MSSQL.Tests.Outbox.Text.Async.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessageAsync.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message_Async
+Paramore.Brighter.MSSQL.Tests.Outbox.Text.Sync.WhenAddingAMessageItShouldBeStoredWithAllProperties.When_Adding_A_Message_It_Should_Be_Stored_With_All_Properties
+Paramore.Brighter.MSSQL.Tests.Outbox.Text.Sync.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommit.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit
+Paramore.Brighter.MSSQL.Tests.Outbox.Text.Sync.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessage.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message

specs/0031-test_naming_conventions/.scratch/baseline-mssql-failures.txt

@@ -0,0 +1 @@
+Passed!  - Failed:     0, Passed:   111, Skipped:     0, Total:   111, Duration: 148 ms - Paramore.Brighter.BoxProvisioning.Tests.dll (net9.0)

specs/0031-test_naming_conventions/.scratch/baseline-suite-boxprov.txt

@@ -0,0 +1,25 @@
+  Stack Trace:
+     at Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenAddingAMessageItShouldBeStoredWithAllProperties.When_Adding_A_Message_It_Should_Be_Stored_With_All_Properties() in /Users/ian_hammond_cooper/CSharpProjects/github/BrighterCommand/Brighter/tests/Paramore.Brighter.MSSQL.Tests/Outbox/Binary/Generated/Sync/When_Adding_A_Message_It_Should_Be_Stored_With_All_Properties.cs:line 72
+   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
+   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
+[xUnit.net 00:00:13.97]     Paramore.Brighter.MSSQL.Tests.Outbox.Text.Async.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommitAsync.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit_Async [FAIL]
+  Failed Paramore.Brighter.MSSQL.Tests.Outbox.Text.Async.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommitAsync.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit_Async [11 ms]
+  Error Message:
+   Assert.Equal() Failure: Values differ
+Expected: 2026-06-09T09:01:20.8916580+00:00
+Actual:   2026-06-09T09:01:20.8900000+01:00 (difference 01:00:00.0016580 is larger than 00:00:01)
+  Stack Trace:
+     at Paramore.Brighter.MSSQL.Tests.Outbox.Text.Async.WhenAddingAMessageWithinTransactionItShouldBeStoredAfterCommitAsync.When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit_Async() in /Users/ian_hammond_cooper/CSharpProjects/github/BrighterCommand/Brighter/tests/Paramore.Brighter.MSSQL.Tests/Outbox/Text/Generated/Async/When_Adding_A_Message_Within_Transaction_It_Should_Be_Stored_After_Commit_Async.cs:line 62
+--- End of stack trace from previous location ---
+[xUnit.net 00:00:14.31]     Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessage.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message [FAIL]
+  Failed Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessage.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message [27 ms]
+  Error Message:
+   Assert.Equal() Failure: Values differ
+Expected: 2026-06-09T09:01:21.2130000+01:00
+Actual:   2026-06-09T09:01:21.2145240+00:00 (difference 01:00:00.0015240 is larger than 00:00:01)
+  Stack Trace:
+     at Paramore.Brighter.MSSQL.Tests.Outbox.Binary.Sync.WhenRetrievingAMessageByIdItShouldReturnTheCorrectMessage.When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message() in /Users/ian_hammond_cooper/CSharpProjects/github/BrighterCommand/Brighter/tests/Paramore.Brighter.MSSQL.Tests/Outbox/Binary/Generated/Sync/When_Retrieving_A_Message_By_Id_It_Should_Return_The_Correct_Message.cs:line 79
+   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
+   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
+
+Failed!  - Failed:    13, Passed:   185, Skipped:     0, Total:   198, Duration: 14 s - Paramore.Brighter.MSSQL.Tests.dll (net9.0)