From fa5102544a9b5343f9f9a786240e1ee3b1e276ad Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:11:14 +0300 Subject: [PATCH 01/14] docs(gsd): reconcile .planning with merged T7 (#29); freeze FIX-PLAN.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GSD is now the source of truth for project tracking. Reconcile .planning to the real git state and retire FIX-PLAN.md as the working doc. - Mark ENG-01/T7 complete across REQUIREMENTS/ROADMAP/STATE/PROJECT (shipped pre-GSD via #29 — double-checked locking + same/distinct-interface stress tests) - Clear the stale "T7 race open" concern in STATE; log the generator-serialization decision (one gate over all generation; not Lazy-per-type) - Phase 2 now: ENG-01 done; COLL-01 (C1, deferred) + TEST-01/02/03 remain - Freeze FIX-PLAN.md with a banner pointing at .planning/ (kept for its per-item file:line detail, mined by each phase's CONTEXT/PLAN) Local branch only (no push) — rides the next work branch to avoid a doc-only master alpha. SESSION-HANDOFF.md left uncommitted (living handoff). --- .planning/PROJECT.md | 14 ++++++++------ .planning/REQUIREMENTS.md | 8 ++++---- .planning/ROADMAP.md | 9 +++++---- .planning/STATE.md | 7 ++++--- FIX-PLAN.md | 14 ++++++++++++-- 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/.planning/PROJECT.md b/.planning/PROJECT.md index 9c6ef89..d2c06eb 100644 --- a/.planning/PROJECT.md +++ b/.planning/PROJECT.md @@ -48,7 +48,7 @@ converters. When tradeoffs arise, binding correctness wins. - [ ] **TEST-01** (T4): `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts - [ ] **TEST-02** (T5): `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths - [ ] **TEST-03** (T6): Converter tests residual — `Uri`/`DateTime` + the `List` doc test tied to C1 -- [ ] **ENG-01** (T7): Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests +- [x] **ENG-01** (T7): Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests *(merged #29 — double-checked locking; one gate over all generation)* - [ ] **API-01** (A5): Make `SettingsHolder`/`ISettingsHolder` internal *(breaking)* - [ ] **PKG-01** (A3): `Core.AspNet` exposes a public type (`Environments` public) or the package is dropped - [ ] **PKG-02** (A4): Float `Microsoft.Extensions.*` floor per-TFM (`8.0.x` for net8) or justify the pin @@ -74,13 +74,14 @@ converters. When tradeoffs arise, binding correctness wins. `v2.0.0-beta`. - **Recently completed & merged:** performance track P0–P5, quick wins Q1–Q5, naming consolidation (A2/D3), the provider-cache decision (C3/P1), the allocation-gated - benchmark harness, S1 secret redaction (#27), and the public exception hierarchy (C2, #28). + benchmark harness, S1 secret redaction (#27), the public exception hierarchy (C2, #28), + and the generator concurrency-race fix (ENG-01/T7, #29). - **Test baseline:** 82 tests on net10 (incl. +5 S1 redaction, +6 C2 hierarchy). TUnit on Microsoft.Testing.Platform; run from `src/`. net8 is build-only locally (net10 runtime installed); CI runs both. -- **Known open concerns (source-verified):** the generator concurrency race (T7), missing - engine tests (T4/T5), `IEnumerable`-only collection support (C1), no AOT/trim - annotations (A1), and the command-line quoted-value bug (A6). *(C2 public exception base — resolved #28.)* +- **Known open concerns (source-verified):** missing engine tests (T4/T5), + `IEnumerable`-only collection support (C1), no AOT/trim annotations (A1), and the + command-line quoted-value bug (A6). *(C2 public exception base — resolved #28; T7 generator race — resolved #29.)* ## Constraints @@ -100,6 +101,7 @@ converters. When tradeoffs arise, binding correctness wins. | Secret-safe exception invariant: `SettingsPropertyValueException` carries no value and chains no inner; value-free "required missing" is a separate `SettingsPropertyNullException`; opt-in restore rejected | Bound values (secrets) reach logs via `Exception.ToString()`; a config flag to restore them is insecure-by-configuration | ✓ Good (merged #27; made structural by C2 #28) | | Public exception hierarchy: all library exceptions derive from `public abstract SimpleSettingsException` in the root namespace (reflection invariant test enforces it); `SettingsTypeNotInterfaceException` replaces the `TypeIsNotInterface` `InvalidOperationException` throws | Consumers need one catchable category; the two unreachable "no converter" guards stay outside the family | ✓ Good (C2 #28; one runtime break → release notes) | | Provider caches the built instance per type (C3 option 2); Core `GetSettings` unchanged; no reload path | Consistent objects between DI singletons and `provider.GetSettings()`; `IOptionsMonitor`-style reload deferred as future "option 3" | ✓ Good (P1, #17) | +| Generator serializes ALL type generation behind one gate (double-checked locking; warm path lock-free); NOT `Lazy`-per-type | `Reflection.Emit` isn't thread-safe — concurrent `DefineType` of distinct interfaces also races the shared `ModuleBuilder`; per-type Lazy would reopen that race | ✓ Good (T7 #29; same/distinct-interface stress tests) | | Benchmark CI gates on allocated bytes, not wall-clock time | Allocations are deterministic; time is noisy/informational | ✓ Good (merged #22) | | Canonical naming `ExistForAll.SimpleSettings`; package renamed from legacy `SimpleConfig` | Consolidate three historical spellings onto one identity before stable | ✓ Good (A2 #15, D3 #11) | | Keep the generated impl type name separate from `GetNormalizeInterfaceName` (section name) | The two serve different purposes (collision-safe impl name vs. config section name); merging would break section resolution | ✓ Good (M1 #21) | @@ -107,4 +109,4 @@ converters. When tradeoffs arise, binding correctness wins. | `Validations/*` (D1) and `EqualityCompererCreator` (D2) are HELD — do NOT delete | Dead today but reserved for coming feature work; D1 reconciles with the `validate-settings` branch | — Pending (owner-driven feature) | --- -*Last updated: 2026-07-14 after Phase 1 (S1 #27, C2 #28) shipped — reconciled from session handoff* +*Last updated: 2026-07-14 — ENG-01/T7 shipped (#29). GSD is now the source of truth; FIX-PLAN.md frozen as a historical reference. Reconciled from session handoff + git.* diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 07f429d..f1aba2d 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -48,7 +48,7 @@ Remaining open work (from `FIX-PLAN.md`), batched toward the first `v2.0.0-beta` - [ ] **TEST-01**: `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts (T4) - [ ] **TEST-02**: `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths (T5) - [ ] **TEST-03**: Converter tests residual — `Uri`/`DateTime` + `List` doc test tied to C1 (T6) -- [ ] **ENG-01**: Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests (T7) +- [x] **ENG-01**: Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests (T7 — shipped pre-GSD via the FIX-PLAN track, merged #29: double-checked locking, one gate over all generation; same- + distinct-interface `Barrier` stress tests) ### Public Surface, Packaging & Binder Cleanup @@ -99,7 +99,7 @@ Deferred / held. Tracked but not in the current roadmap. | TEST-01 | Phase 2 | Pending | | TEST-02 | Phase 2 | Pending | | TEST-03 | Phase 2 | Pending | -| ENG-01 | Phase 2 | Pending | +| ENG-01 | Phase 2 | ✓ Complete (#29) | | API-01 | Phase 3 | Pending | | PKG-01 | Phase 3 | Pending | | PKG-02 | Phase 3 | Pending | @@ -112,9 +112,9 @@ Deferred / held. Tracked but not in the current roadmap. - v1 requirements: 15 total - Mapped to phases: 15 - Unmapped: 0 ✓ -- Complete: 3 (Phase 1 — SEC-01/SEC-02/EXC-01, shipped #27/#28); Pending: 12 +- Complete: 4 (Phase 1 SEC-01/SEC-02/EXC-01 #27/#28 + ENG-01/T7 #29); Pending: 11 - Validated (shipped, no phase): 13 --- *Requirements defined: 2026-07-13* -*Last updated: 2026-07-14 after Phase 1 (S1 #27, C2 #28) shipped — reconciled from session handoff* +*Last updated: 2026-07-14 — ENG-01/T7 marked complete (#29). GSD is now the source of truth; FIX-PLAN.md frozen as a historical reference. Reconciled from session handoff + git.* diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index e03f083..e3be8ec 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -2,7 +2,7 @@ ## Overview -**Status (2026-07-14):** Phase 1 shipped — S1 (#27) + C2 (#28) merged to `master` @ `13b78dd`. Active phase is now **Phase 2**. +**Status (2026-07-14):** Phase 1 shipped (S1 #27, C2 #28); ENG-01/T7 also merged (#29). `master` @ `10f9275`. Active phase is **Phase 2** — ENG-01 done; COLL-01 (deferred) + engine tests (TEST-01/02/03) remain. The binding engine already ships and works. This milestone is a hardening + pre-stable cleanup pass that batches every remaining breaking change and safety fix before cutting the @@ -21,7 +21,7 @@ value: config → typed settings maps accurately, and never leaks a secret doing - Decimal phases (2.1, 2.2): Urgent insertions (marked with INSERTED) - [x] **Phase 1: Exception Safety & Public Hierarchy** — ✓ COMPLETE (S1 #27, C2 #28 merged 2026-07-14) - No secret leaks; one catchable, structured `SimpleSettingsException` base -- [ ] **Phase 2: Binding Correctness & Engine Test Hardening** - Collections/nullable/converters verified; generator race closed by tests +- [ ] **Phase 2: Binding Correctness & Engine Test Hardening** - Collections/nullable/converters verified; generator race closed by tests *(ENG-01/T7 done #29; COLL-01 + TEST-01/02/03 remain)* - [ ] **Phase 3: Public Surface, Packaging & Binder Cleanup** - Meaningful public surface; per-TFM deps; correct command-line parsing - [ ] **Phase 4: AOT/Trim Honesty & Documentation** - Honest AOT/trim signals; canonically-named docs - [ ] **Phase 5: First v2.0.0-beta Release** - Batched breaking changes ship as an installable pre-release @@ -45,11 +45,12 @@ value: config → typed settings maps accurately, and never leaks a secret doing **Goal**: Binding maps config to typed settings accurately across every supported collection, nullable, and converter shape, with the engine's concurrency and precedence behavior locked by tests. **Depends on**: Phase 1 (engine tests assert the S1/C2 exception contract) **Requirements**: COLL-01, TEST-01, TEST-02, TEST-03, ENG-01 +**Status**: In progress — ENG-01/T7 delivered pre-GSD (merged #29: generator concurrency race closed via double-checked locking + same/distinct-interface stress tests). COLL-01 (C1 — decision deferred) and TEST-01/02/03 (T4/T5/T6) remain. **Success Criteria** (what must be TRUE): 1. A settings interface exposing `List`/`IList`/`ICollection` either binds correctly or fails with a clear, documented error (per the C1 decision), covered by a test. 2. Binder precedence (last binder wins; attribute default applies when none set) is verified by `ValuesPopulator` tests. 3. `TypeConverter` null / nullable / empty-enumerable / `AllowEmpty` / attribute-`ConverterType` paths are verified by tests. - 4. Concurrent first-touch generation of the same interface returns one `ReferenceEquals` implementation with no duplicate-`DefineType` race. + 4. Concurrent first-touch generation of the same interface returns one `ReferenceEquals` implementation with no duplicate-`DefineType` race. ✓ Met by #29 (T7). 5. `Uri`/`DateTime` and collection converters have parity tests passing on net8 and net10. **Plans**: TBD @@ -93,7 +94,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| | 1. Exception Safety & Public Hierarchy | n/a (shipped) | ✓ Complete | 2026-07-14 (#27/#28) | -| 2. Binding Correctness & Engine Test Hardening | 0/TBD | Not started | - | +| 2. Binding Correctness & Engine Test Hardening | ENG-01 ✓ (#29); rest 0/TBD | ◆ In progress | - | | 3. Public Surface, Packaging & Binder Cleanup | 0/TBD | Not started | - | | 4. AOT/Trim Honesty & Documentation | 0/TBD | Not started | - | | 5. First v2.0.0-beta Release | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 3856d01..4fd7509 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -22,8 +22,8 @@ See: .planning/PROJECT.md (updated 2026-07-13) Phase: 2 of 5 (Binding Correctness & Engine Test Hardening) Plan: 0 of TBD in current phase -Status: Ready to plan -Last activity: 2026-07-14 — Phase 1 (S1 #27, C2 #28) merged to master @ 13b78dd; marked complete, roadmap reconciled from session handoff +Status: In progress — ENG-01/T7 done (#29); COLL-01 (C1, deferred) + TEST-01/02/03 remain to plan +Last activity: 2026-07-14 — ENG-01/T7 merged (#29, master @ 10f9275); GSD ownership cutover: .planning reconciled to reality, FIX-PLAN.md frozen as a historical reference (GSD is now source of truth) Progress: [██░░░░░░░░] 20% @@ -53,6 +53,7 @@ Progress: [██░░░░░░░░] 20% Decisions are logged in PROJECT.md Key Decisions table. Recent decisions affecting current work: +- Generator concurrency (T7/ENG-01, merged #29): `SettingsClassGenerator.GenerateType` serializes ALL generation behind one gate (double-checked locking; warm cache-hit path lock-free). NOT `Lazy`-per-type — `Reflection.Emit` isn't thread-safe, so concurrent `DefineType` of *distinct* interfaces also races the shared `ModuleBuilder`; a distinct-interface stress test guards this. - Secret-safe exception invariant (S1, merged #27; made structural by C2): `SettingsPropertyValueException` takes the failure `Type` not the `Exception` — no bound value, no chained inner. `SettingsPropertyNullException` for value-free "required missing"; opt-in restore rejected. - Public exception hierarchy (C2, merged #28): all library exceptions derive from `public abstract SimpleSettingsException` in the root namespace, enforced by a reflection invariant test; `SettingsTypeNotInterfaceException` replaces the `TypeIsNotInterface` throws (one runtime break → release notes). - Breaking changes are free until the first `v2.0.0-beta` and are batched before cutting it. @@ -64,7 +65,7 @@ None yet. ### Blockers/Concerns -- Open concern (Phase 2 target): the `SettingsClassGenerator` check-then-`DefineType` concurrency race (T7/ENG-01) is not yet closed — the highest-value correctness item remaining. +- None blocking. (T7/ENG-01 `SettingsClassGenerator` concurrency race **closed** — shipped pre-GSD via #29: double-checked locking, warm path lock-free, same/distinct-interface `Barrier` stress tests.) ## Deferred Items diff --git a/FIX-PLAN.md b/FIX-PLAN.md index 947c05b..5f5ceba 100644 --- a/FIX-PLAN.md +++ b/FIX-PLAN.md @@ -1,5 +1,15 @@ # SimpleSettings — Fix Plan +> **⚠ FROZEN — HISTORICAL REFERENCE (2026-07-14).** This file is no longer the working source of +> truth. Project tracking has migrated to **GSD** (`.planning/`): +> - **Requirements & status** → `.planning/REQUIREMENTS.md` +> - **Phases, goals & success criteria** → `.planning/ROADMAP.md` +> - **Project overview & key decisions** → `.planning/PROJECT.md` +> - **Current state / session position** → `.planning/STATE.md` +> +> Retained only for its per-item file:line implementation detail, which each GSD phase's +> CONTEXT/PLAN mines as it is planned. **Do not update this file** — record new work in GSD. + _Derived from the 2026-07-10 three-part review (architecture · tests · performance). Every finding below was verified against source with file:line. Work items are self-contained and ordered so they can be implemented one at a time._ ## Progress (2026-07-13) @@ -11,8 +21,8 @@ _Derived from the 2026-07-10 three-part review (architecture · tests · perform - **Perf track P0–P5 = COMPLETE + merged.** `master` @ `498fc81`. - **S1 shipped + merged (#27).** Conversion-failure exceptions no longer carry the bound value **or** chain the value-bearing framework inner; the value-free "required value missing" case split into its own `SettingsPropertyNullException`. `master` @ `5277c60`. Detail in §S1 below. - **C2 merged (#28).** **Public exception hierarchy**: `SimpleSettingsException` base, reparent all 10, promote the 4 escapees to public, flatten 3 to root namespace, leak-safe structured properties, retype the `TypeIsNotInterface` throw. Plan reviewed by `security-auditor` + `dotnet-architect` (both ENDORSE-WITH-CHANGES, both fired cleanly) + perf in-context; code by `/code-review`. Suite **82 net10** (was 76; +6). `master` @ `13b78dd`. Detail in §C2 below. -- **T7 (generator concurrency race) shipped** (branch `test/t7-generator-concurrency`, PR open) — `SettingsClassGenerator.GenerateType` now uses double-checked locking + a single generation gate (Reflection.Emit isn't thread-safe: concurrent `DefineType` races the shared `ModuleBuilder` for both same- and distinct-interface generation). Fast cache-hit path stays lock-free. Plan reviewed by `dotnet-architect` (ENDORSE-WITH-CHANGES — lock-all required, `Lazy`-per-type rejected) + perf/security in-context; code by `/code-review`. +2 concurrency stress tests; suite **84 net10** (was 82; 5× green). -- **In flight:** T7 PR open (branch `test/t7-generator-concurrency`) — carries this fix-plan + handoff refresh. +- **T7 (generator concurrency race) merged (#29).** `SettingsClassGenerator.GenerateType` now uses double-checked locking + a single generation gate (Reflection.Emit isn't thread-safe: concurrent `DefineType` races the shared `ModuleBuilder` for both same- and distinct-interface generation). Fast cache-hit path stays lock-free. **Perf verified same-machine before/after: 0 B allocation delta (16.81 MB cold both, 0 B warm both), no measurable time change.** Plan reviewed by `dotnet-architect` (ENDORSE-WITH-CHANGES — lock-all required, `Lazy`-per-type rejected) + perf/security in-context; code by `/code-review`. +2 concurrency stress tests; suite **84 net10** (was 82; 5× green). `master` @ `10f9275`. +- **In flight:** none. This fix-plan + handoff refresh sit **uncommitted** in the working tree (session wrap) to ride the next work branch — no doc-only `master` push. - **Next:** engine tests T4 `ValuesPopulator` / T5 `TypeConverter` (+ optional T7 leftovers: collection not-found / binder edge cases) **or** continue the pre-stable breaking cleanups (A5 make `SettingsHolder` internal / C1 `List` support / A6 command-line quoting / A3 `Core.AspNet` / A4 dependency floor) · **A1 (HIGH)** AOT/trim story · optional P3b. - **C3 — DECIDED (option 2):** cache in the provider only; Core `SettingsBuilder.GetSettings` unchanged; no reload. See #17. - **Held — do NOT delete (feature work coming):** D1 Validations (reconcile with the `validate-settings` branch) · D2 EqualityCompererCreator. From b6e5515243cbce1d97e272843102ad6bbd542a56 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:24:57 +0300 Subject: [PATCH 02/14] docs(02): synthesize phase context from FIX-PLAN (COLL-01 deferred, ENG-01 verify-only) --- .../02-CONTEXT.md | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-CONTEXT.md diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-CONTEXT.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-CONTEXT.md new file mode 100644 index 0000000..7981a77 --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-CONTEXT.md @@ -0,0 +1,123 @@ +# Phase 2: Binding Correctness & Engine Test Hardening — Context + +**Gathered:** 2026-07-14 +**Status:** Ready for planning (partial — COLL-01 deferred pending the C1 decision) +**Source:** Synthesized from `FIX-PLAN.md` (frozen) + `REQUIREMENTS.md`, reconciled to current git (post-#29) + + +## Phase Boundary + +Harden the binding engine's test coverage and lock its correctness contracts. The engine already +ships; this phase adds the missing unit tests around **value population / binder precedence** +(TEST-01), **type conversion** (TEST-02), and **converter residuals** (TEST-03), and makes the +one open collection-support decision (COLL-01). + +**Already delivered (verify-only):** ENG-01/T7 (generator concurrency race) shipped pre-GSD via +#29 — do NOT re-implement. + +**Deferred this phase:** COLL-01/C1 (the `List`/`IList`/`ICollection` broaden-vs-throw +decision) is deferred by the owner; it and its `List` positive/doc test are excluded from these +plans. + +Not in scope: any new binding features, the held `Validations`/`EqualityCompererCreator` code (D1/D2). + + + +## Implementation Decisions + +### ENG-01 / T7 — DONE (verify-only) +- Merged #29 (double-checked locking; one `_generationGate` over all generation; warm cache-hit + path lock-free). Do NOT re-implement. If a plan references ENG-01 it is **verify-only**: confirm + the fix in `SettingsClassGenerator.cs` and the 2 stress tests in `SettingsClassGeneratorTests.cs` + satisfy success criterion #4 (concurrent same-interface generation → one `ReferenceEquals` impl, + no duplicate-`DefineType`). + +### TEST-01 / T4 — `ValuesPopulator` tests +- New file: `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs` (uses the + internal ctor + fake binders/converters). +- Cases: binder-throws ⇒ `SettingsBindingException`; a later binder overrides an earlier one + (last-writer-wins precedence); no binder sets a value ⇒ the `[SettingsProperty]` attribute default + survives; conversion-throws ⇒ `SettingsPropertyValueException`. +- **⚠ CONTRACT UPDATE (post-S1/C2) — FIX-PLAN's T4 wording is STALE.** FIX-PLAN §T4 says the value + exception "carries type/value/property" with "inner preserved." That is no longer true. + `SettingsPropertyValueException` now takes the failure **`Type`**, carries **no bound value**, and + **chains no inner** (`InnerException == null`). Assert the *current* leak-safe contract: property + name + target type + failure-type name present; the bound value ABSENT from the whole + `ex.ToString()` chain. The value-free "required missing" path is `SettingsPropertyNullException`. + `SettingsBindingException` chains the binder's inner but stores only primitives + (`BinderType`/`Section`/`Key`), never the `BindingContext`. + +### TEST-02 / T5 — `TypeConverter` tests +- New file: `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs`. +- Cases: `null` → value-type default; `null` → `IEnumerable` empty (not null); `Nullable` + strip-and-convert; `AllowEmpty = false` + no value ⇒ throw (`SettingsPropertyNullException`); + attribute `ConverterType` bypasses the collection converter. + +### TEST-03 / T6 — Converter residual (Uri/DateTime) +- P4 (#25) already added `Conversion/CollectionConversionTests.cs` (array/enumerable incl. + int/string/enum/DateTime/Uri elements). **Scope T6 to the RESIDUAL only** — verify existing + coverage first, then add `Uri`/`DateTime` scalar-conversion + edge tests where genuinely missing. + Do NOT duplicate P4's coverage. +- The `List` positive/doc test is **DEFERRED with COLL-01** — not in this phase. + +### COLL-01 / C1 — DEFERRED (open decision, do not plan) +- The broaden-vs-document+throw decision for `List`/`IList`/`ICollection` is deferred by the + owner. Do NOT plan the implementation or the `List` positive/doc test in this phase. +- Background for when it's decided: `TypeExtensions.IsEnumerable` (`TypeExtensions.cs:23-28`) matches + only exact `IEnumerable<>`; other shapes fall through to `DefaultTypeConverter` and throw. + `CollectionTypeConverter` already materializes a `T[]`; a `List` satisfies + `IList`/`ICollection`/`IReadOnlyList`/`IReadOnlyCollection` in one shot. +- This is an explicit gap — the requirements-coverage gate will (correctly) flag COLL-01 as + intentionally deferred, not silently dropped. + +### Claude's Discretion +- Test file layout, fixture interface shapes, `[Arguments]`/`[NotInParallel]` usage per existing TUnit + conventions; whether ENG-01 verify-only warrants its own thin plan or a note inside another plan. + + + +## Canonical References + +**Downstream agents MUST read these before planning or implementing.** + +### Work-item detail (frozen historical — mine, do not update) +- `FIX-PLAN.md` — §T4/T5/T6/C1 per-item detail. ⚠ Test specs predate S1/C2; trust the current + source + the contract note above over FIX-PLAN's exception wording. + +### Source under test +- `src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs` — TEST-01 target (binder precedence, + bind/convert exception wrapping) +- `src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs` — TEST-02 target (convert orchestration) +- `src/Core/ExistForAll.SimpleSettings/Conversion/` — `PropertyConversion.cs` (null/AllowEmpty), + `CollectionTypeConverter.cs`, `Enumerable/Array/Enum/DateTime/Uri/Default` converters — TEST-02/03 +- `src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeExtensions.cs:23-28` — `IsEnumerable` (C1 pivot; deferred) +- Exception contract (post-S1/C2): `SettingsPropertyValueException`, `SettingsPropertyNullException`, + `SettingsBindingException`, `SimpleSettingsException` + +### Test conventions + existing coverage (dedupe against these) +- `.planning/codebase/TESTING.md` — TUnit conventions +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsClassGeneratorTests.cs` — ENG-01 tests (done); pattern reference +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs` — P4 collection coverage (T6 overlap) +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/DefaultTypeConverterTests.cs` — existing converter test (T1 culture) + + + +## Specific Ideas +- Build/test from `src/` (TUnit on Microsoft.Testing.Platform via `global.json`). net10 runtime only + locally → net8 build-only; CI runs both. Run one project: `dotnet test --framework net10.0 --no-build` (build first). +- Match TUnit conventions: `[Test] async Task`, `[Arguments(...)]`, `[NotInParallel("…")]` for + process-global state (e.g. `CultureInfo.CurrentCulture`), `await Assert.That(...).IsEqualTo/.Throws()`, + nested `public interface` fixtures per class, build via `SettingsBuilder.CreateBuilder(x => …)`. +- New test files under `src/Tests/ExistForAll.SimpleSettings.UnitTests/`. + + + +## Deferred Ideas +- **COLL-01 / C1** — `List`/`IList`/`ICollection` support decision (broaden vs document+throw) + and its `List` positive/doc test. Deferred by owner; excluded from this phase's plans. +- **D1 Validations**, **D2 EqualityCompererCreator** — HELD; out of scope. + + +--- +*Phase: 02-binding-correctness-engine-test-hardening* +*Context synthesized 2026-07-14 from FIX-PLAN.md (frozen) — GSD is now the source of truth* From 325259bbd1bf654c2e7d41f38561f6df2588a3b3 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:31:31 +0300 Subject: [PATCH 03/14] docs(02): research binding-correctness engine test hardening --- .../02-RESEARCH.md | 398 ++++++++++++++++++ 1 file changed, 398 insertions(+) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-RESEARCH.md diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-RESEARCH.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-RESEARCH.md new file mode 100644 index 0000000..1753fae --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-RESEARCH.md @@ -0,0 +1,398 @@ +# Phase 2: Binding Correctness & Engine Test Hardening - Research + +**Researched:** 2026-07-14 +**Domain:** .NET unit-test authoring (TUnit / Microsoft.Testing.Platform) against an existing runtime binding + conversion engine — brownfield test hardening, no production code changes +**Confidence:** HIGH (all findings verified against live source in this session) + +## Summary + +This is a **test-only** phase against a shipped engine. The open work is three test files — TEST-01 (`ValuesPopulator`), TEST-02 (`TypeConverter`), TEST-03 (Uri/DateTime scalar residual). ENG-01 is done (verify-only, #29) and COLL-01 is deferred by the owner; neither is implementation work here. + +The single most important research finding: **much of what FIX-PLAN §T4/T5/T6 described as missing is already covered** by `ExceptionHierarchyTests.cs`, `ExceptionRedactionTests.cs`, `CollectionConversionTests.cs`, `SettingsPropertyTests.cs`, and `SettingsBuilderConversionsTests.cs`. The post-S1/C2 exception contract (`SettingsPropertyValueException` = value-free, `InnerException == null`; `SettingsBindingException` = primitives only; `SettingsPropertyNullException` = the required-missing path) is **already locked by live, passing tests** (verified below). Planning must dedupe hard against these — the genuine residual is small and specific. + +**Primary recommendation:** Write three focused test files that add ONLY the uncovered cases — binder last-writer-wins precedence, `Nullable` strip+convert, `null → value-type default`, `ConverterType` overriding the *collection* converter, and scalar `Uri`/`DateTime` positive conversion. Use the existing `SettingsBuilder.CreateBuilder(...)` + `InMemoryBinder`/`InMemoryCollection` integration pattern for most cases, and the internal `TypeConverter`/`PropertyConversion` seam (InternalsVisibleTo is already granted) for the pure converter-orchestration cases. Do NOT re-assert the exception contract that `ExceptionHierarchyTests` already owns. + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions + +**ENG-01 / T7 — DONE (verify-only).** Merged #29 (double-checked locking; one `_generationGate` over all generation; warm cache-hit path lock-free). Do NOT re-implement. If a plan references ENG-01 it is verify-only: confirm the fix in `SettingsClassGenerator.cs` and the 2 stress tests in `SettingsClassGeneratorTests.cs` satisfy success criterion #4 (concurrent same-interface generation → one `ReferenceEquals` impl, no duplicate-`DefineType`). + +**TEST-01 / T4 — `ValuesPopulator` tests.** +- New file: `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs` (uses the internal ctor + fake binders/converters). +- Cases: binder-throws ⇒ `SettingsBindingException`; a later binder overrides an earlier one (last-writer-wins precedence); no binder sets a value ⇒ the `[SettingsProperty]` attribute default survives; conversion-throws ⇒ `SettingsPropertyValueException`. +- ⚠ CONTRACT UPDATE (post-S1/C2) — FIX-PLAN's T4 wording is STALE. `SettingsPropertyValueException` now takes the failure `Type`, carries NO bound value, and chains NO inner (`InnerException == null`). Assert the current leak-safe contract: property name + target type + failure-type name present; the bound value ABSENT from the whole `ex.ToString()` chain. The value-free "required missing" path is `SettingsPropertyNullException`. `SettingsBindingException` chains the binder's inner but stores only primitives (`BinderType`/`Section`/`Key`), never the `BindingContext`. + +**TEST-02 / T5 — `TypeConverter` tests.** +- New file: `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs`. +- Cases: `null` → value-type default; `null` → `IEnumerable` empty (not null); `Nullable` strip-and-convert; `AllowEmpty = false` + no value ⇒ throw (`SettingsPropertyNullException`); attribute `ConverterType` bypasses the collection converter. + +**TEST-03 / T6 — Converter residual (Uri/DateTime).** +- P4 (#25) already added `Conversion/CollectionConversionTests.cs`. Scope T6 to the RESIDUAL only — verify existing coverage first, then add `Uri`/`DateTime` scalar-conversion + edge tests where genuinely missing. Do NOT duplicate P4's coverage. +- The `List` positive/doc test is DEFERRED with COLL-01 — not in this phase. + +**COLL-01 / C1 — DEFERRED (open decision, do not plan).** The broaden-vs-document+throw decision for `List`/`IList`/`ICollection` is deferred by the owner. Do NOT plan the implementation or the `List` positive/doc test. Background: `TypeExtensions.IsEnumerable` matches only exact `IEnumerable<>`; other shapes fall through to `DefaultTypeConverter` and throw. + +### Claude's Discretion + +- Test file layout, fixture interface shapes, `[Arguments]`/`[NotInParallel]` usage per existing TUnit conventions; whether ENG-01 verify-only warrants its own thin plan or a note inside another plan. + +### Deferred Ideas (OUT OF SCOPE) + +- **COLL-01 / C1** — `List`/`IList`/`ICollection` support decision and its `List` positive/doc test. Deferred by owner. +- **D1 Validations**, **D2 EqualityCompererCreator** — HELD; out of scope. +- Any new binding features. + + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|------------------| +| TEST-01 | `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts (T4) | Source read (`ValuesPopulator.cs`); genuine gap = last-writer-wins precedence + default-survives; contract cases already partly locked by `ExceptionHierarchyTests` (see Don't-Duplicate table) | +| TEST-02 | `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths (T5) | Source read (`TypeConverter.cs`, `PropertyConversion.cs`); gaps = null→value-type default, `Nullable` strip+convert, `ConverterType`-over-collection; empty-enumerable + AllowEmpty already covered | +| TEST-03 | Converter residual — `Uri`/`DateTime` scalar (T6; `List` doc test deferred) | Source read (`UriTypeConvertor.cs`, `DateTimeTypeConverter.cs`); array cases fully covered by P4; residual = scalar positive conversion + format-mismatch | +| ENG-01 | Generator concurrency race (T7) | **VERIFY-ONLY** — confirmed present in `SettingsClassGeneratorTests.cs` (2 stress tests, lines 103–166); no new work | +| COLL-01 | Collection support decision (C1) | **DEFERRED** — do not plan; documented for the requirements-coverage gate as intentionally deferred | + + +## Architectural Responsibility Map + +| Capability | Primary Tier | Secondary Tier | Rationale | +|------------|-------------|----------------|-----------| +| Binder precedence (last-writer-wins) | Engine core (`ValuesPopulator`) | — | Populate loop iterates binders in order, overwriting `tempValue` per `context.HasNewValue` | +| Bind-failure wrapping | Engine core (`ValuesPopulator` catch) | Exception types | `try/catch` around `binder.BindPropertySettings` → `SettingsBindingException` | +| Convert-failure wrapping (leak-safe) | Engine core (`ValuesPopulator.ConvertPropertyValue`) | `PropertyConversion` | Redacting `SettingsPropertyValueException` (Type only) | +| Conversion orchestration (null/nullable/converter-select) | `Core.Reflection.TypeConverter` → `PropertyConversion` | Converter chain | `CreateConversion` resolves converter + null-result once per type | +| Scalar type conversion (Uri/DateTime) | Individual `ISettingsTypeConverter`s | `SettingsOptions` (format/delimiter) | `UriTypeConvertor`, `DateTimeTypeConverter` | + +All test targets live in one tier — the engine core assembly `ExistForAll.SimpleSettings`. No multi-tier concerns. + +## Standard Stack + +### Core +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| TUnit | 1.58.0 (pinned in `src/Directory.Packages.props`) | Test framework + fluent assertions | Already the project's only test framework; runs on Microsoft.Testing.Platform per `src/global.json` | +| .NET SDK | 10.0.301 installed (global.json floor 10.0.100, `rollForward: latestFeature`) | Build/run | Local runs net10 only; net8 is build-only; CI runs both TFMs | + +**No new packages are introduced by this phase.** All test doubles are hand-written (project convention: no Moq/NSubstitute). `## Package Legitimacy Audit` is therefore **N/A — no external packages installed.** + +**Verification:** `dotnet --version` → `10.0.301` [VERIFIED: local shell]. TUnit 1.58.0 pinned [VERIFIED: TESTING.md + Directory.Packages.props reference]. InternalsVisibleTo to the test project confirmed in `src/Core/ExistForAll.SimpleSettings/Info.cs:3` [VERIFIED: grep]. + +## Architecture Patterns + +### System Under Test — data flow + +``` +config value (string, from InMemoryBinder/ConfigurationBinder/etc.) + │ + ▼ +ValuesPopulator.PopulateInstanceWithValues(instance, settings, options, binders[]) + │ builds/caches SettingsPlan (one per interface type) + │ + ├─ for each PropertyPlan: + │ tempValue = propertyPlan.DefaultValue ← [SettingsProperty] default + │ for each binder (in order): ← LAST-WRITER-WINS + │ try binder.BindPropertySettings(context) + │ if context.HasNewValue → tempValue = context.NewValue + │ catch → throw SettingsBindingException(binder, context, inner) ← wraps, primitives only + │ + │ propertyValue = ConvertPropertyValue(tempValue) + │ └─ PropertyConversion.Convert(value): + │ value == null && throwOnNull → throw SettingsPropertyNullException(name) + │ value == null → return nullResult (empty[] | default(T) | null) + │ else → converter.Convert(value, strippedType) + │ catch (not SettingsPropertyNullException) → throw SettingsPropertyValueException(Type only) ← leak-safe + │ + └─ property.SetValue(instance, propertyValue) +``` + +`TypeConverter.CreateConversion(propertyInfo, attribute, options)` (called once per property at plan build) decides, up front: `throwOnNull = attribute is { AllowEmpty: false }`; `nullResult` (empty `T[]` for `IEnumerable`, `Activator.CreateInstance` for value types, else null); `strippedType` (unwraps `Nullable<>`); and the chosen converter (`attribute.ConverterType` wins outright, else first `CanConvert` in `options.Converters`). + +### Component Responsibilities + +| File | Responsibility | Test target | +|------|----------------|-------------| +| `src/Core/.../ValuesPopulator.cs` | Binder loop, precedence, bind/convert exception wrapping | TEST-01 | +| `src/Core/.../Core/Reflection/TypeConverter.cs` | Conversion resolution (null-result, nullable strip, converter select) | TEST-02 | +| `src/Core/.../Conversion/PropertyConversion.cs` | Runtime null-check + throwOnNull + converter dispatch | TEST-02 | +| `src/Core/.../Conversion/UriTypeConvertor.cs` | `new Uri((string)value)` | TEST-03 | +| `src/Core/.../Conversion/DateTimeTypeConverter.cs` | `DateTime.ParseExact(value, options.DateTimeFormat, Invariant)` | TEST-03 | + +### Pattern 1: Integration-style build with in-memory binder (dominant existing pattern) +**What:** Drive the whole engine through the public builder, feed values via `InMemoryCollection`/`InMemoryBinder`. +**When to use:** Most TEST-01/02/03 cases — it exercises the real plan/convert path and matches every existing conversion test. +**Example:** +```csharp +// Source: src/Tests/.../Conversion/CollectionConversionTests.cs (verified live) +var collection = new InMemoryCollection(); +collection.Add("SectionName", nameof(IThing.Value), "raw"); +var builder = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection))); +var result = builder.GetSettings(); +await Assert.That(result.Value).IsEqualTo(expected); +``` +Note: default `SectionNameFormatter` strips a leading `I` (`IThing` → section `"Thing"`). `InMemoryBinder`/`InMemoryCollection` are in namespace `ExistForAll.SimpleSettings.Binder` (visible via InternalsVisibleTo). + +### Pattern 2: Direct converter-orchestration seam (for pure TypeConverter cases) +**What:** Construct `SettingsOptions` (public parameterless ctor; its `Converters` are auto-seeded with the default chain) and call the internal `TypeConverter.CreateConversion(...)` directly, then exercise the returned `PropertyConversion.Convert(...)`. +**When to use:** TEST-02 cases that are about resolution logic itself (null→default, nullable strip, ConverterType selection) without needing a generated instance. +**Example:** +```csharp +// Source: verified against TypeConverter.cs + SettingsOptions.cs + PropertyConversion.cs +var options = new SettingsOptions(); // Converters seeded: DateTime,Uri,Array,Enumerable,Enum,Default +var prop = typeof(ISample).GetProperty(nameof(ISample.Count))!; // e.g. int? or int +var attribute = prop.GetCustomAttribute(inherit: true); +var conversion = new TypeConverter().CreateConversion(prop, attribute, options); +await Assert.That(conversion.Convert(null)).IsEqualTo(0); // null → value-type default +await Assert.That(conversion.Convert("42")).IsEqualTo(42); // Nullable strip+convert +``` +`TypeConverter`, `ITypeConverter`, `PropertyConversion`, and `SettingsOptions.Converters` are all internal but reachable via `[InternalsVisibleTo("ExistForAll.SimpleSettings.UnitTests")]`. + +### Pattern 3: `ValuesPopulator` internal ctor + fake binders (for TEST-01 unit isolation) +**What:** `ValuesPopulator` exposes `internal ValuesPopulator(ITypePropertiesExtractor, ITypeConverter)` plus public `PopulateInstanceWithValues(instance, settings, options, binders)`. +**When to use:** TEST-01 precedence + default-survives, where you want to feed hand-written fake `ISectionBinder`s in a known order and assert on the resulting instance. `instance` can be a hand-written concrete class implementing the test interface (property `SetValue` targets the interface `PropertyInfo`, so any implementer works); the simplest route is still `SettingsBuilder` with two ordered `InMemoryBinder`s. Discretion per CONTEXT. +**Example (fake binder shape, from the live `ThrowingBinder`):** +```csharp +// Source: src/Tests/.../SimpleSettings/ExceptionHierarchyTests.cs (verified live) +private class ThrowingBinder : ISectionBinder +{ + public void BindPropertySettings(BindingContext context) + => throw new InvalidOperationException("binder failed"); +} +// A setting binder: call context.SetNewValue(x) to contribute a value. +``` + +### Anti-Patterns to Avoid +- **Duplicating the exception contract.** `ExceptionHierarchyTests` + `ExceptionRedactionTests` already lock `SettingsPropertyValueException` (value-free, `InnerException == null`, safe metadata), `SettingsBindingException` (BinderType/Section/Key), and secret-absence in `ToString()` for int/enum/DateTime/Uri/custom. Do NOT re-assert these; add only the uncovered precedence/orchestration behaviors. +- **Trusting FIX-PLAN's exception wording.** FIX-PLAN §T4 says the value exception "carries value/inner." That is STALE — the live type takes a `Type`, never chains an inner. Trust the source. +- **Using a mock framework.** Project has none; hand-write in-memory fakes. +- **Culture-sensitive assertions without isolation.** Any test touching `CultureInfo.CurrentCulture` must use `[NotInParallel]` and restore in `finally` (see `DefaultTypeConverterTests`). + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Feeding config values | A custom `ISectionBinder` per test | `InMemoryCollection` + `InMemoryBinder` (`ExistForAll.SimpleSettings.Binder`) | Already the established, visible test double | +| Env-var isolation | Manual set/unset | `DisposableEnvironmentVariable` (`using (...)`) | Existing RAII helper; not needed here but noted | +| Secret-leak assertions | New redaction helper | The sentinel + `ToString().Contains(Secret)` pattern | Already in `ExceptionRedactionTests` if any leak assertion is truly needed | +| Building a settings instance | Reflection.Emit by hand | `SettingsBuilder.CreateBuilder(...).GetSettings()` | The public path already used everywhere | + +**Key insight:** The test infrastructure for this engine is mature. Every needed primitive (in-memory binder, sentinel redaction assert, culture isolation, `[Arguments]` data-driving) already exists in the test project — the phase is about *coverage*, not tooling. + +## Don't-Duplicate — Existing Coverage Map (dedupe target) + +> This is the load-bearing research output. Verified by reading each test file live. + +| CONTEXT case | Already covered by | New work needed? | +|--------------|--------------------|------------------| +| binder-throws ⇒ `SettingsBindingException` (BinderType/Section/Key) | `ExceptionHierarchyTests.BinderThrows_ExposesBinderContext` | Residual only — a **unit-level** version via `ValuesPopulator` + fake binder (integration contract already locked) | +| conversion-throws ⇒ `SettingsPropertyValueException` (value-free, `InnerException == null`, safe metadata) | `ExceptionHierarchyTests.ConversionFailure_ExposesSafeStructuredMetadata_AndNoChainedInner` + all of `ExceptionRedactionTests` | **NO — do not duplicate.** Contract fully locked | +| secret absent from `ToString()` (int/enum/DateTime/Uri/custom converter) | `ExceptionRedactionTests` (5 tests) | **NO** | +| last-writer-wins precedence (later binder overrides earlier) | **nothing** | **YES — genuine gap** | +| no binder sets value ⇒ `[SettingsProperty]` default survives | `CollectionConversionTests.Convert_DefaultArray_IsPassedThrough` (no-binder case only) | **YES (thin)** — the "binders present but none set" scalar case is uncovered | +| `null` → value-type default | **nothing** (only the enumerable null case exists) | **YES — genuine gap** | +| `null` → `IEnumerable` empty (not null) | `CollectionConversionTests.Convert_UnboundEnumerable_NoDefault_YieldsEmptyArray` | **NO — do not duplicate** | +| `Nullable` strip-and-convert | **nothing** (grep: no `int?`/`Nullable` in any test) | **YES — genuine gap** | +| `AllowEmpty = false` + no value ⇒ `SettingsPropertyNullException` | `SettingsPropertyTests.Build_WhenAllowEmptyIsFalse_ShouldThrowException` | Residual only — optional unit-level `PropertyConversion.Convert(null)` variant | +| attribute `ConverterType` wins (scalar) | `SettingsBuilderConversionsTests.Build_WhenAddLocalConverter_ShouldReturnConverterValue` (Guid) | Scalar covered; **the "bypasses the *collection* converter" case (ConverterType on an `IEnumerable`/array property) is the residual — YES** | +| array of `Uri` / `DateTime` | `CollectionConversionTests.Convert_DelimitedString_ToUriArray_*` / `ToDateTimeArray_*` | **NO — do not duplicate** | +| scalar `Uri` positive parse | **nothing** (only the redaction failure case) | **YES — genuine gap** | +| scalar `DateTime` positive parse with configured `DateTimeFormat` | **nothing** (only array + redaction failure) | **YES — genuine gap** | +| ENG-01 concurrency (same + distinct interface) | `SettingsClassGeneratorTests` lines 103–166 (2 Barrier/Parallel stress tests) | **NO — verify-only, already present** | + +## Runtime State Inventory + +**Omitted — not a rename/refactor/migration phase.** This phase adds test files only; no stored data, service config, OS registration, secrets, or build artifacts are renamed or migrated. Verified: the work is net-new `[Test]` methods in the existing test project. + +## Common Pitfalls + +### Pitfall 1: Section-name formatting trips up in-memory binder keys +**What goes wrong:** Values never bind because the section key doesn't match. +**Why it happens:** The default `SectionNameFormatter` strips a leading `I` — `IIntSetting` → section `"IntSetting"`, and `[SettingsSection]`/interface-name rules apply. `InMemoryCollection.Add(section, key, value)` must use the *formatted* section name. +**How to avoid:** Mirror existing tests — `collection.Add("Thing", nameof(IThing.Value), ...)` for `IThing`, or `typeof(T).Name.Substring(1)`. +**Warning signs:** Property comes back as its default/empty instead of the bound value. + +### Pitfall 2: Asserting the stale exception shape +**What goes wrong:** A test asserts `ex.InnerException` is non-null or that the value appears in the message — fails (or worse, would pass on old code). +**Why it happens:** FIX-PLAN §T4 predates S1/C2. +**How to avoid:** Assert `ex.InnerException` **IsNull**, `ex.ConversionErrorType == typeof(FormatException)` (etc.), and value ABSENT from `ex.ToString()`. See `ExceptionHierarchyTests.ConversionFailure_*` for the exact shape. + +### Pitfall 3: Nullable target types +**What goes wrong:** Assuming `int?` flows through unchanged. +**Why it happens:** `TypeConverter.StripIfNullable` unwraps `Nullable<>` before converter selection, so an `int?` property converts via the `int` path; `null` yields `Activator.CreateInstance(int?)` = `null` (not `0`) because `CreateNullResult` runs on the *original* (nullable) type, which is a value type → `default(int?)` = null. Distinguish the non-nullable `int` case (null → `0`) from `int?` (null → `null`). +**How to avoid:** Test both `int` and `int?` explicitly; assert `int` null→`0` and `int?` "42"→`42`. +**Warning signs:** Unexpected `0` vs `null`. + +### Pitfall 4: Culture / parallelism +**What goes wrong:** Flaky DateTime/decimal parses under non-invariant cultures when tests run in parallel. +**Why it happens:** `CultureInfo.CurrentCulture` is process-global. TUnit runs tests in parallel by default. +**How to avoid:** `[NotInParallel]` on any culture-mutating test and restore in `finally` (pattern in `DefaultTypeConverterTests`). Scalar `Uri`/`DateTime` positive tests using invariant format need no culture mutation but keep `[NotInParallel]` if they set culture. + +### Pitfall 5: DateTime converter requires string input and exact format +**What goes wrong:** Passing a non-string or a differently-formatted date throws. +**Why it happens:** `DateTimeTypeConverter.Convert` does `DateTime.ParseExact((string)value, options.DateTimeFormat, InvariantCulture)`; default format is `"yyyy-MM-dd"`. A mismatch throws `FormatException` → wrapped in `SettingsPropertyValueException`. +**How to avoid:** Positive test uses `"2020-01-02"`; a negative/edge test can assert a mismatched format wraps as `SettingsPropertyValueException` (already implied by redaction test — don't over-duplicate). + +## Code Examples + +### TEST-02: null → value-type default vs `Nullable` (direct seam) +```csharp +// Source: verified against TypeConverter.cs / PropertyConversion.cs / SettingsOptions.cs +var options = new SettingsOptions(); +var conv = new TypeConverter(); + +var intProp = typeof(ISample).GetProperty(nameof(ISample.Count))!; // int +var intConv = conv.CreateConversion(intProp, intProp.GetCustomAttribute(true), options); +await Assert.That(intConv.Convert(null)).IsEqualTo(0); // value-type default + +var nullProp = typeof(ISample).GetProperty(nameof(ISample.Maybe))!; // int? +var nullConv = conv.CreateConversion(nullProp, nullProp.GetCustomAttribute(true), options); +await Assert.That(nullConv.Convert(null)).IsNull(); // nullable → null result +await Assert.That(nullConv.Convert("42")).IsEqualTo(42); // strip + convert +``` + +### TEST-01: last-writer-wins precedence (integration) +```csharp +// Two ordered binders; the SECOND must win. No existing test covers this. +var c1 = new InMemoryCollection(); c1.Add("Sample", nameof(ISample.Name), "first"); +var c2 = new InMemoryCollection(); c2.Add("Sample", nameof(ISample.Name), "second"); +var builder = SettingsBuilder.CreateBuilder(x => +{ + x.AddSectionBinder(new InMemoryBinder(c1)); + x.AddSectionBinder(new InMemoryBinder(c2)); // later → wins +}); +await Assert.That(builder.GetSettings().Name).IsEqualTo("second"); +``` + +### TEST-03: scalar Uri / DateTime positive +```csharp +var c = new InMemoryCollection(); +c.Add("Endpoint", nameof(IEndpoint.Url), "https://a.example/"); +c.Add("Endpoint", nameof(IEndpoint.When), "2020-01-02"); // default format yyyy-MM-dd +var b = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(c))); +var r = b.GetSettings(); +await Assert.That(r.Url).IsEqualTo(new Uri("https://a.example/")); +await Assert.That(r.When).IsEqualTo(new DateTime(2020, 1, 2)); +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| `SettingsPropertyValueException` carries value + chained inner (FIX-PLAN §T4) | Value-free; takes failure `Type`; `InnerException == null` | S1 #27 / C2 #28 | Tests must assert the leak-safe shape; contract already locked by `ExceptionHierarchyTests` | +| VSTest runner | Microsoft.Testing.Platform (TUnit) | pre-GSD | Run from `src/`; `dotnet test` with the platform opt-in in `global.json` | +| `IEnumerable` null → `Enumerable.Empty()` | `Array.CreateInstance(elementType, 0)` (a real `T[]`) | P4 #25 | `null → IEnumerable` yields `int[]`; already tested — do not duplicate | + +**Deprecated/outdated:** FIX-PLAN.md is frozen historical; its T4/T5/T6 test specs predate S1/C2. Mine it for intent, trust the live source for the contract. + +## Assumptions Log + +| # | Claim | Section | Risk if Wrong | +|---|-------|---------|---------------| +| — | (none) | — | All findings verified against live source or the local toolchain this session. | + +**All claims in this research were verified against the codebase or the local environment — no user confirmation needed.** + +## Open Questions + +1. **Should TEST-01 use the internal `ValuesPopulator` ctor with fakes, or the `SettingsBuilder` integration path?** + - What we know: CONTEXT names the internal ctor; the integration path is simpler and dominates the existing suite; both are viable and InternalsVisibleTo is granted. + - What's unclear: nothing blocking — this is explicitly Claude's Discretion. + - Recommendation: Use the integration path for precedence/default (clearest, matches suite); reserve the internal ctor + fakes only if a case genuinely needs binder-order isolation the builder can't express. + +2. **Depth of TEST-03 negative/edge cases (DateTime format mismatch, malformed Uri).** + - What we know: The failure→`SettingsPropertyValueException` wrapping is already locked by redaction tests for both types. + - Recommendation: Add scalar POSITIVE tests (the real gap) plus at most one format-mismatch negative; do not re-prove redaction. + +## Environment Availability + +| Dependency | Required By | Available | Version | Fallback | +|------------|------------|-----------|---------|----------| +| .NET SDK | build + test | ✓ | 10.0.301 | — | +| net10.0 runtime | local test run | ✓ | 10.0 | — | +| net8.0 runtime | local test run | ✗ (build-only locally) | — | CI runs net8; locally target net10 with `--framework net10.0` | +| TUnit / Microsoft.Testing.Platform | test execution | ✓ | 1.58.0 (via restore) | — | + +**Missing with no fallback:** none. +**Missing with fallback:** net8 runtime — run net10 locally (`dotnet test --framework net10.0 --no-build`, build first); CI covers net8 parity. + +## Validation Architecture + +### Test Framework +| Property | Value | +|----------|-------| +| Framework | TUnit 1.58.0 on Microsoft.Testing.Platform | +| Config file | `src/global.json` (`test.runner`), `src/Directory.Packages.props` (version pin) | +| Quick run command | `dotnet test src/Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build` (build first from `src/`) | +| Full suite command | from `src/`: `dotnet build "$SOLUTION" -c Release --no-restore` then `dotnet test "$SOLUTION" -c Release --no-build` (both TFMs in CI) | + +### Phase Requirements → Test Map +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| TEST-01 | Last-writer-wins precedence (later binder overrides) | unit/integration | `dotnet test ...UnitTests --framework net10.0 --no-build --filter "*ValuesPopulator*"` | ❌ Wave 0 (`Core/ValuesPopulatorTests.cs`) | +| TEST-01 | No binder sets value ⇒ `[SettingsProperty]` default survives | unit/integration | same filter | ❌ Wave 0 | +| TEST-01 | binder-throws ⇒ `SettingsBindingException` (unit-level, fake binder) | unit | same filter | ❌ Wave 0 (contract already covered at integration level) | +| TEST-02 | `null` → value-type default | unit | `--filter "*TypeConverter*"` | ❌ Wave 0 (`Core/TypeConverterTests.cs`) | +| TEST-02 | `Nullable` strip + convert | unit | same filter | ❌ Wave 0 | +| TEST-02 | `ConverterType` bypasses the collection converter | unit/integration | same filter | ❌ Wave 0 | +| TEST-03 | scalar `Uri` positive parse | integration | `--filter "*Uri*"` / new class | ❌ Wave 0 (`Conversion/ScalarConversionTests.cs` or extend existing) | +| TEST-03 | scalar `DateTime` positive parse (configured format) | integration | same | ❌ Wave 0 | +| ENG-01 | Concurrent generation → one shared impl | stress (verify-only) | `--filter "*Concurrent*"` | ✅ `SettingsClassGeneratorTests.cs` (lines 103–166) | + +### Sampling Rate +- **Per task commit:** `dotnet test src/Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build` (net10, fast). +- **Per wave merge:** full net10 suite from `src/`. +- **Phase gate:** full suite green on **net8 + net10** (CI parity) before `/gsd-verify-work`. + +### Wave 0 Gaps +- [ ] `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs` — TEST-01 (precedence, default-survives, unit-level bind-throw) +- [ ] `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs` — TEST-02 (null→default, nullable strip, ConverterType-over-collection) +- [ ] TEST-03 scalar `Uri`/`DateTime` — new `Conversion/ScalarConversionTests.cs` OR add to an existing converter test class (discretion) +- [ ] No framework install needed — infrastructure exists; `Core/` test subfolder is new (mirrors source `Core/` namespace). + +## Security Domain + +`security_enforcement: true`, ASVS level 1. This phase adds **no new attack surface** — it is test code. Its security relevance is **reinforcing** the existing SEC-01 secret-redaction invariant. + +### Applicable ASVS Categories +| ASVS Category | Applies | Standard Control | +|---------------|---------|-----------------| +| V5 Input Validation | indirect | Conversion is validation; tests confirm typed conversion + null/AllowEmpty handling | +| V7/V8 Error Handling & Logging (secret leakage) | yes (verify) | Existing `SettingsPropertyValueException` value-free contract + `ExceptionRedactionTests` — this phase must NOT weaken it; any new failure-path test must keep the value-absent assertion if it touches `ToString()` | +| V6 Cryptography | no | — | +| V2/V3/V4 Auth/Session/Access | no | Library concern, not this phase | + +### Known Threat Patterns for this stack +| Pattern | STRIDE | Standard Mitigation | +|---------|--------|---------------------| +| Bound secret leaking via exception message / chained inner / logs | Information Disclosure | Value-free `SettingsPropertyValueException` (takes `Type`, no inner); locked by `ExceptionHierarchyTests` + `ExceptionRedactionTests`. New tests must not introduce assertions that print bound values. | + +**Guidance for the planner:** Any new negative/failure test that inspects an exception must reuse the sentinel-absence pattern (`Assert.That(ex.ToString().Contains(Secret)).IsFalse()`) rather than asserting on the raw value. No security blocker for this phase. + +## Sources + +### Primary (HIGH confidence — live source read this session) +- `src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs` — binder loop, precedence, exception wrapping +- `src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs` + `Conversion/PropertyConversion.cs` — conversion resolution +- `src/Core/ExistForAll.SimpleSettings/SettingsPropertyValueException.cs`, `SettingsPropertyNullException.cs`, `SettingsBindingException.cs`, `SimpleSettingsException.cs` — post-S1/C2 contract +- `src/Core/ExistForAll.SimpleSettings/Conversion/{DateTimeTypeConverter,UriTypeConvertor,DefaultTypeConverter,TypeConvertersCollections}.cs` +- `src/Core/ExistForAll.SimpleSettings/{SettingsOptions,BindingContext,ISectionBinder,IValuesPopulator}.cs`, `Core/Reflection/{ITypeConverter,ITypePropertiesExtractor,TypeExtensions}.cs`, `Binder/{InMemoryBinder,InMemoryCollection}.cs`, `Info.cs` (InternalsVisibleTo) +- Test files: `Conversion/{CollectionConversionTests,DefaultTypeConverterTests,ExceptionRedactionTests}.cs`, `SimpleSettings/{ExceptionHierarchyTests,SettingsPropertyTests,SettingsBuilderConversionsTests,SettingsClassGeneratorTests}.cs` +- `.planning/codebase/TESTING.md`; `.planning/config.json`; `src/global.json` +- `dotnet --version` → 10.0.301 + +### Secondary / Tertiary +- None — no web research required; the phase is fully internal. + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH — versions verified from installed SDK + pinned package + global.json. +- Architecture / data flow: HIGH — read every file in the target path. +- Existing-coverage dedupe map: HIGH — every referenced test read in full. +- Pitfalls: HIGH — derived directly from source semantics (nullable strip, section formatting, culture). + +**Research date:** 2026-07-14 +**Valid until:** 2026-08-13 (stable internal codebase; re-verify only if `ValuesPopulator`/`TypeConverter`/exception types change before planning) From d3c454dd6d7dfd0db6383d6c70f504fc85c088f0 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:33:56 +0300 Subject: [PATCH 04/14] docs(02): add validation strategy --- .../02-VALIDATION.md | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-VALIDATION.md diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-VALIDATION.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-VALIDATION.md new file mode 100644 index 0000000..e34c29f --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-VALIDATION.md @@ -0,0 +1,76 @@ +--- +phase: 2 +slug: binding-correctness-engine-test-hardening +status: draft +nyquist_compliant: false +wave_0_complete: false +created: 2026-07-14 +--- + +# Phase 2 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. This is a test-only phase +> (no production code beyond fixtures), so every requirement is validated by the tests it adds. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | TUnit 1.58.0 on Microsoft.Testing.Platform (opted in via `src/global.json`) | +| **Config file** | `src/global.json`; `src/Tests/ExistForAll.SimpleSettings.UnitTests/ExistForAll.SimpleSettings.UnitTests.csproj` | +| **Quick run command** | `dotnet test src/Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0` (net10 installed locally) | +| **Full suite command** | `dotnet test src/ExistForAll.SimpleSettings.slnx` (CI runs net8.0 + net10.0) | +| **Estimated runtime** | ~10–20 s (unit suite) | + +--- + +## Sampling Rate + +- **After every task commit:** Run the quick run command (the new test file, net10) +- **After every plan wave:** Run the full suite command +- **Before `/gsd-verify-work`:** Full suite green on net10 (CI confirms net8 parity) +- **Max feedback latency:** ~20 s + +--- + +## Per-Task Verification Map + +> Populated by the planner/executor once plan + task IDs exist. Each new test task maps to its +> requirement and asserts a concrete, automated behavior (see RESEARCH.md `## Validation Architecture` +> for the specific gaps: binder precedence, null→value-type default, `Nullable` strip+convert, +> `ConverterType`-over-collection, scalar `Uri`/`DateTime` positive parse). + +| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| +| (pending planner) | — | — | TEST-01/02/03 | — | N/A (test-only) | unit | `dotnet test … --framework net10.0` | ❌ W0 | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +--- + +## Wave 0 Requirements + +- [ ] New test files under `src/Tests/ExistForAll.SimpleSettings.UnitTests/` (created by the plan tasks themselves — no separate stub wave) + +*Existing infrastructure (TUnit, `[InternalsVisibleTo]` from `Info.cs:3`, existing fixtures) covers all phase requirements; no framework install needed.* + +--- + +## Manual-Only Verifications + +*All open phase behaviors have automated verification (unit tests). ENG-01/T7 is already verified by committed concurrency stress tests (#29 — verify-only); COLL-01/C1 is deferred (not in scope).* + +--- + +## Validation Sign-Off + +- [ ] All tasks have `` verify or Wave 0 dependencies +- [ ] Sampling continuity: no 3 consecutive tasks without automated verify +- [ ] Wave 0 covers all MISSING references +- [ ] No watch-mode flags +- [ ] Feedback latency < 20s +- [ ] `nyquist_compliant: true` set in frontmatter + +**Approval:** pending From cbca08c7320fc01b2a3e1f332c99e68c26557650 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:45:25 +0300 Subject: [PATCH 05/14] docs(02): create Phase 2 binding-correctness test-hardening plans --- .planning/ROADMAP.md | 4 +- .../02-01-PLAN.md | 247 ++++++++++++++++++ .../02-02-PLAN.md | 155 +++++++++++ 3 files changed, 405 insertions(+), 1 deletion(-) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-02-PLAN.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index e3be8ec..341b156 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -52,7 +52,9 @@ value: config → typed settings maps accurately, and never leaks a secret doing 3. `TypeConverter` null / nullable / empty-enumerable / `AllowEmpty` / attribute-`ConverterType` paths are verified by tests. 4. Concurrent first-touch generation of the same interface returns one `ReferenceEquals` implementation with no duplicate-`DefineType` race. ✓ Met by #29 (T7). 5. `Uri`/`DateTime` and collection converters have parity tests passing on net8 and net10. -**Plans**: TBD +**Plans**: 2 plans +- [ ] 02-01-PLAN.md — Engine-core correctness tests (TEST-01 ValuesPopulator precedence/default; TEST-02 TypeConverter null/nullable/ConverterType-over-collection) + ENG-01 concurrency verify +- [ ] 02-02-PLAN.md — Converter scalar residual (TEST-03 scalar Uri/DateTime positive + one format-mismatch negative); COLL-01 documented as owner-deferred ### Phase 3: Public Surface, Packaging & Binder Cleanup **Goal**: The public API and packages carry only meaningful, correctly-scoped surface, and the command-line binder parses real-world arguments correctly — the remaining breaking changes batched before beta. diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md new file mode 100644 index 0000000..f751e6c --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md @@ -0,0 +1,247 @@ +--- +phase: 02-binding-correctness-engine-test-hardening +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs +autonomous: true +requirements: [TEST-01, TEST-02, ENG-01] + +must_haves: + truths: + - "Given two ordered binders that both set the same property, the resolved value is the LATER binder's (last-writer-wins)." + - "Given binders present but none of them setting a given scalar property, that property's [SettingsProperty] DefaultValue survives." + - "A null bound value for a non-nullable value-type property (int) resolves to that type's default (0)." + - "A Nullable property resolves null for a null bound value and the parsed int for a numeric string." + - "A property whose [SettingsProperty(ConverterType=...)] names a converter uses THAT converter, bypassing CollectionTypeConverter, even for an IEnumerable property." + - "Concurrent first-touch generation of the same interface returns one ReferenceEquals implementation (verified present + green; met by #29)." + artifacts: + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs + key_links: + - "Section-name formatting strips a leading 'I' (INameFormatter default): InMemoryCollection.Add(section,key,value) MUST use the formatted section name (ISample -> \"Sample\") or values never bind." + - "TypeConverter.CreateConversion(propertyInfo, attribute, options) is the internal seam reached via InternalsVisibleTo; SettingsOptions() auto-seeds the default converter chain (DateTime,Uri,Array,Enumerable,Enum,Default)." + - "Post-S1/C2 exception contract (SettingsPropertyValueException value-free, InnerException==null; SettingsBindingException primitives) is already locked by ExceptionHierarchyTests/ExceptionRedactionTests — new tests must NOT re-assert or weaken it." +--- + + +Lock the two genuinely-uncovered engine-core correctness behaviors with unit tests, and confirm the +already-shipped generator-concurrency fix satisfies its success criterion: + +- TEST-01 (`ValuesPopulator`): binder precedence (last-writer-wins) and "attribute default survives + when no binder sets a value" — neither is covered anywhere today. +- TEST-02 (`TypeConverter`): `null -> value-type default`, `Nullable` strip-and-convert, and + attribute `ConverterType` bypassing the collection converter — the three uncovered resolution paths. +- ENG-01 (verify-only): confirm `SettingsClassGenerator` + its committed stress tests satisfy Phase 2 + success criterion #4. NO implementation — the fix shipped pre-GSD (#29). + +Purpose: Phase 2 success criteria #2 (precedence) and #3 (converter null/nullable/ConverterType paths) +demand tests; #4 is met by #29 and only needs verification. These are the highest-value gaps. + +Output: Two new TUnit test files under `Core/` plus a passing verification of the existing concurrency +stress tests. No production code changes. + +Deferred this phase: COLL-01/C1 (`List`/`IList`/`ICollection` broaden-vs-throw decision) — see +02-02-PLAN.md "## Deferred This Phase" for the owner-deferred rationale. Do NOT implement it here. + + + +@/Users/guyludvig/frontegg/development/open-source/SimpleSettings/.claude/gsd-core/workflows/execute-plan.md +@/Users/guyludvig/frontegg/development/open-source/SimpleSettings/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/02-binding-correctness-engine-test-hardening/02-RESEARCH.md +@.planning/phases/02-binding-correctness-engine-test-hardening/02-PATTERNS.md + + + + + + Task 1: TEST-01 — ValuesPopulator binder precedence + default-survives tests + src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs + + - src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs (binder loop lines 41-67: last-writer-wins via `context.HasNewValue`; `tempValue` seeded from `propertyPlan.DefaultValue`) + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs (the `Build` helper at :131-146; the `InMemoryCollection`/`InMemoryBinder` wiring; nested `public interface` fixtures at :148-182) + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/DefaultTypeConverterTests.cs (section-name formatting note at :8-9 — leading "I" stripped; the `BuildWith` helper shape) + - src/Core/ExistForAll.SimpleSettings/Binder/InMemoryCollection.cs and InMemoryBinder.cs (Add(section,key,value) API; namespace `ExistForAll.SimpleSettings.Binder`, visible via InternalsVisibleTo) + - src/Core/ExistForAll.SimpleSettings/SettingsPropertyAttribute.cs (DefaultValue property) + + + Create `Core/ValuesPopulatorTests.cs` as `public class ValuesPopulatorTests` in a block-scoped + namespace `ExistForAll.SimpleSettings.UnitTests.Core` (mirror the analog convention). Add + `using ExistForAll.SimpleSettings.Binder;`. Use the integration build path + (`SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection)))` then + `builder.GetSettings()`) — RESEARCH Open Question #1 recommends integration over the internal + ctor for precedence/default cases; the internal `ValuesPopulator` ctor is only warranted if binder + ORDER isolation cannot be expressed via the builder (it can here). + + Cover exactly these three cases (the genuine gaps per 02-RESEARCH Don't-Duplicate map): + 1. Last-writer-wins: build TWO ordered binders over two InMemoryCollections that BOTH set the same + section+key to different values ("first" then "second"), add the "first" binder before the + "second"; assert the resolved property equals the SECOND binder's value. + 2. Earlier-value-survives-when-later-is-silent (complementary precedence proof): binder A sets the + key, binder B (added after A) does NOT contain the key; assert the resolved value is binder A's + (proves a later silent binder does not clobber an earlier set value — still precedence, not the + exception contract). + 3. Default-survives-with-binders-present: a fixture property carrying `[SettingsProperty(DefaultValue + = ...)]`; build with a binder whose collection sets a DIFFERENT key (so the binder loop runs but + never sets this property); assert the property equals its attribute DefaultValue. + + Use a scalar fixture interface (e.g. `ISample { string Name { get; set; } [SettingsProperty(DefaultValue + = "fallback")] string Label { get; set; } }`) declared as a nested `public interface` at the class + bottom. Remember the section name is the interface name with the leading "I" stripped ("Sample"). + + Do NOT re-assert the exception-wrapper contracts: binder-throws => SettingsBindingException and + conversion-throws => SettingsPropertyValueException are already locked by + ExceptionHierarchyTests.BinderThrows_ExposesBinderContext and + ExceptionHierarchyTests.ConversionFailure_ExposesSafeStructuredMetadata_AndNoChainedInner. Adding + them here is forbidden duplication (per 02-RESEARCH Anti-Patterns + Don't-Duplicate map). + + + From `src/`: `dotnet build SimpleSettings.slnx -c Debug` then `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build --filter "*ValuesPopulator*"` exits 0 with the three new test methods present and green. + + + - File `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs` exists with a `public class ValuesPopulatorTests` in namespace `ExistForAll.SimpleSettings.UnitTests.Core`. + - Exactly three `[Test] public async Task` methods: last-writer-wins (resolved == second binder's value), earlier-survives-when-later-silent (resolved == first binder's value), default-survives-with-binders-present (resolved == attribute DefaultValue). + - No assertion references `SettingsBindingException` or `SettingsPropertyValueException` (no exception-contract re-assertion). + - `dotnet test ... --framework net10.0 --filter "*ValuesPopulator*"` (run from `src/`) exits 0; all three named tests appear in the run and pass. + + Binder precedence (last-writer-wins + earlier-survives) and attribute-default-survives are proven by three green ValuesPopulator tests; no existing contract is duplicated. + + + + Task 2: TEST-02 — TypeConverter null/nullable/ConverterType-over-collection tests + src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs + + - src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeConverter.cs (CreateConversion :13-24; CreateNullResult :28-37 — enumerable->empty T[], value-type->Activator.CreateInstance, else null; GetConverter :39-58 — attribute.ConverterType wins at :43-44; StripIfNullable :60-65) + - src/Core/ExistForAll.SimpleSettings/Conversion/PropertyConversion.cs (Convert :31-42 — null-check + throwOnNull + dispatch) + - src/Core/ExistForAll.SimpleSettings/SettingsOptions.cs (public parameterless ctor auto-seeds `Converters`) + - src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsBuilderConversionsTests.cs (the ConverterType attribute + hand-written `ISettingsTypeConverter` pattern at :26-58) + - src/Core/ExistForAll.SimpleSettings/SettingsPropertyAttribute.cs (ConverterType, AllowEmpty defaults; AllowEmpty defaults true) + + + Create `Core/TypeConverterTests.cs` as `public class TypeConverterTests` in block-scoped namespace + `ExistForAll.SimpleSettings.UnitTests.Core`. Add `using ExistForAll.SimpleSettings.Conversion;` and + `using ExistForAll.SimpleSettings.Core.Reflection;` (both reachable via InternalsVisibleTo). + + Use the direct converter-orchestration seam (02-RESEARCH Pattern 2): build `var options = new + SettingsOptions();` and `var conv = new TypeConverter();`, resolve a property's conversion via + `conv.CreateConversion(prop, prop.GetCustomAttribute(inherit: true), + options)`, then exercise the returned `PropertyConversion.Convert(...)`. This tests resolution logic + directly without building a generated instance. + + Cover exactly these three genuine gaps (per 02-RESEARCH Don't-Duplicate map + scope control): + 1. null -> value-type default: an `int Count` property; `CreateConversion(...).Convert(null)` returns + `0` (CreateNullResult on a non-nullable value type). + 2. Nullable strip-and-convert: an `int? Maybe` property; `.Convert(null)` returns `null` + (nullResult built on the ORIGINAL nullable type — see RESEARCH Pitfall 3; NOT 0), and + `.Convert("42")` returns `42` (StripIfNullable unwraps to int, DefaultTypeConverter parses). + Assert `int` and `int?` distinctly. + 3. ConverterType bypasses the COLLECTION converter: a fixture property + `[SettingsProperty(ConverterType = typeof(SentinelConverter))] IEnumerable Values` where + `SentinelConverter : ISettingsTypeConverter` has `CanConvert` return true and `Convert(value,type)` + return a distinctive sentinel (e.g. `new[] { -1 }`). Resolve the conversion for that property and + call `.Convert("1,2,3")`; assert the result equals the sentinel `new[] { -1 }` (NOT `[1,2,3]`), + proving GetConverter returned the attribute's ConverterType before the collection-converter scan. + Scalar ConverterType is already covered by SettingsBuilderConversionsTests — the collection-typed + property is the residual. + + Declare fixtures as nested `public interface` (e.g. `ISample { int Count { get; set; } int? Maybe { + get; set; } }` and `IWithConverterOverride { [SettingsProperty(ConverterType=typeof(SentinelConverter))] + IEnumerable Values { get; set; } }`) and `SentinelConverter` as a nested `private class` at the + class bottom (mirror SettingsBuilderConversionsTests layout). + + Do NOT duplicate already-covered paths: empty-enumerable (CollectionConversionTests.Convert_Unbound + Enumerable_NoDefault_YieldsEmptyArray) and AllowEmpty=false=>SettingsPropertyNullException + (SettingsPropertyTests.Build_WhenAllowEmptyIsFalse_ShouldThrowException) are locked — do not re-add. + + + From `src/`: `dotnet build SimpleSettings.slnx -c Debug` then `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build --filter "*TypeConverter*"` exits 0 with the new test methods present and green. + + + - File `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs` exists (`public class TypeConverterTests`, namespace `...UnitTests.Core`). + - Tests assert: `int` conversion of `null` == `0`; `int?` conversion of `null` IsNull and of `"42"` == `42`; the ConverterType-decorated `IEnumerable` property converts `"1,2,3"` to the sentinel `new[] { -1 }` (SequenceEqual), not to `[1,2,3]`. + - Uses `new TypeConverter().CreateConversion(...)` seam and `new SettingsOptions()`; no empty-enumerable or AllowEmpty re-assertion. + - `dotnet test ... --framework net10.0 --filter "*TypeConverter*"` (from `src/`) exits 0; new tests present and green. + + The three uncovered TypeConverter resolution paths (null->default, nullable strip+convert, ConverterType-over-collection) are green; no already-covered path is duplicated. + + + + Task 3: ENG-01 — verify-only confirmation of the generator concurrency fix + (no new files — verification only; reads existing source + test) + + - src/Core/ExistForAll.SimpleSettings/Core/Reflection/SettingsClassGenerator.cs (the `_generationGate` field ~:27 and the double-checked `lock (_generationGate)` around DefineType with the lock-free warm `TryGetValue` at ~:45-56) + - src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsClassGeneratorTests.cs (the two stress tests at :103-166: GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType and GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree) + - .planning/ROADMAP.md (Phase 2 success criterion #4) + + + Verify-only — do NOT modify SettingsClassGenerator.cs, do NOT add or edit any test, do NOT + re-implement. This task confirms the pre-GSD fix (#29) satisfies Phase 2 success criterion #4 + (concurrent same-interface generation returns one ReferenceEquals impl with no duplicate-DefineType + race). + + Steps: + 1. Confirm the concurrency gate is present in SettingsClassGenerator.cs: a single `_generationGate` + object guarding all generation with a lock-free warm-path `TryGetValue` before the lock (grep for + `_generationGate` and `lock (`). + 2. Confirm both stress tests exist in SettingsClassGeneratorTests.cs (:103-166) and run green: + `GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType` (Parallel.For(0,128) => one distinct + type) and `GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree` (32 threads + Barrier + over 8 interfaces => zero failures, one shared impl each). + 3. Record the confirmation in the plan SUMMARY (source gate present + both tests green) as the ENG-01 + verify evidence. No code or test changes. + + + From `src/`: `dotnet build SimpleSettings.slnx -c Debug` then `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build --filter "*Concurrent*"` exits 0 with both stress tests present and green; AND `grep -nE '_generationGate|lock \(' Core/ExistForAll.SimpleSettings/Core/Reflection/SettingsClassGenerator.cs` returns at least one match for each token. + + + - `SettingsClassGenerator.cs` contains `_generationGate` and a `lock (_generationGate)` block (grep confirms; no modification made). + - Both `GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType` and `GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree` run and pass under `--filter "*Concurrent*"` (net10). + - No file under `src/` is modified by this task (verification only); git diff for this task is empty. + - SUMMARY records success criterion #4 as met by #29 (verify-only). + + Phase 2 success criterion #4 is confirmed met by the existing #29 fix + stress tests; no new work performed. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| config value → typed property | Untrusted string values cross into typed conversion; a failed conversion must never surface the raw (possibly secret) value. This phase adds tests that exercise this boundary but must not weaken the existing SEC-01 redaction invariant. | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-02-01 | Information Disclosure | new test files (ValuesPopulatorTests, TypeConverterTests) | medium | mitigate | Tasks 1–2 explicitly forbid re-asserting/printing the exception contract; any assertion that inspects an exception reuses the leak-safe pattern (assert Type/property/failure-type metadata only, never the raw bound value). SEC-01 stays locked by ExceptionHierarchyTests/ExceptionRedactionTests. | +| T-02-SC | Tampering | package installs (NuGet) | low | accept | N/A this phase — no new PackageReference, no npm/pip/cargo/NuGet install; all test doubles are hand-written (project convention, no mock framework). RESEARCH Package Legitimacy Audit: N/A — no external packages installed. | + + + +- From `src/`: `dotnet build SimpleSettings.slnx -c Debug` succeeds on net10 (net8 build-only locally; CI covers net8). +- `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build` passes with the new ValuesPopulator + TypeConverter tests green and the existing suite unbroken. +- Concurrency stress tests remain green (ENG-01 verify). +- No production source under `src/Core/` is modified by this plan. + + + +- Phase 2 success criterion #2 (binder precedence: last binder wins; attribute default applies when none set) is proven by ValuesPopulatorTests. +- Phase 2 success criterion #3 (TypeConverter null/nullable/ConverterType paths) is proven by TypeConverterTests (empty-enumerable + AllowEmpty already covered elsewhere). +- Phase 2 success criterion #4 (concurrent generation → one shared impl) is confirmed met by #29 (verify-only). +- No duplication of contracts owned by ExceptionHierarchyTests / ExceptionRedactionTests / CollectionConversionTests / SettingsPropertyTests. + + + +Create `.planning/phases/02-binding-correctness-engine-test-hardening/02-01-SUMMARY.md` when done. + diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-02-PLAN.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-02-PLAN.md new file mode 100644 index 0000000..b6fb9df --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-02-PLAN.md @@ -0,0 +1,155 @@ +--- +phase: 02-binding-correctness-engine-test-hardening +plan: 02 +type: execute +wave: 1 +depends_on: [] +files_modified: + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs +autonomous: true +requirements: [TEST-03] + +must_haves: + truths: + - "A scalar Uri property bound to a URL string resolves to the parsed Uri (new Uri(value))." + - "A scalar DateTime property bound to a string in the configured DateTimeFormat (default yyyy-MM-dd) resolves to the parsed DateTime." + - "A scalar DateTime string that does not match the configured format surfaces SettingsPropertyValueException (one negative case; redaction already proven elsewhere)." + - "Uri/DateTime scalar conversion passes on net8 and net10 (CI parity)." + artifacts: + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs + key_links: + - "DateTimeTypeConverter.Convert uses DateTime.ParseExact(value, options.DateTimeFormat, InvariantCulture); default format is \"yyyy-MM-dd\" — positive input must match it exactly." + - "UriTypeConvertor.Convert does new Uri((string)value) — value must be a string; assert against new Uri(expected)." + - "Section-name formatting strips a leading 'I' (IEndpoint -> \"Endpoint\"); InMemoryCollection keys must use the formatted section name." +--- + + +Close the scalar converter-coverage residual (TEST-03): P4 (#25) already covers Uri/DateTime as ARRAY +elements in CollectionConversionTests, but there is no test for scalar `Uri`/`DateTime` POSITIVE parsing. +This plan adds the missing scalar positive cases plus one DateTime format-mismatch negative. + +Purpose: Phase 2 success criterion #5 (Uri/DateTime + collection converters have parity tests on net8 +and net10) requires scalar coverage; only the array + failure-redaction paths exist today. + +Output: One new TUnit test file `Conversion/ScalarConversionTests.cs`. No production code changes. + +Scope discipline: Do NOT duplicate P4's array-of-Uri/array-of-DateTime cases +(CollectionConversionTests) and do NOT re-prove secret redaction (ExceptionRedactionTests). Exactly one +lightweight format-mismatch negative is allowed (assert the exception type only, no value/redaction +assertions). + + + +@/Users/guyludvig/frontegg/development/open-source/SimpleSettings/.claude/gsd-core/workflows/execute-plan.md +@/Users/guyludvig/frontegg/development/open-source/SimpleSettings/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/02-binding-correctness-engine-test-hardening/02-RESEARCH.md +@.planning/phases/02-binding-correctness-engine-test-hardening/02-PATTERNS.md + + + + + + Task 1: TEST-03 — scalar Uri/DateTime positive + one format-mismatch negative + src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs + + - src/Core/ExistForAll.SimpleSettings/Conversion/UriTypeConvertor.cs (Convert => new Uri((string)value)) + - src/Core/ExistForAll.SimpleSettings/Conversion/DateTimeTypeConverter.cs (Convert => DateTime.ParseExact(value, options.DateTimeFormat, InvariantCulture); default format yyyy-MM-dd) + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/CollectionConversionTests.cs (the `Build` helper at :131-146 and the Uri/DateTime ARRAY cases at :101-116 — mirror the shape, scope to scalar; nested `public interface` fixtures at :148-182) + - src/Core/ExistForAll.SimpleSettings/SettingsOptions.cs (DateTimeFormat default "yyyy-MM-dd") + - src/Core/ExistForAll.SimpleSettings/Binder/InMemoryCollection.cs and InMemoryBinder.cs (Add(section,key,value); namespace `ExistForAll.SimpleSettings.Binder`) + + + Create `Conversion/ScalarConversionTests.cs` as `public class ScalarConversionTests` in block-scoped + namespace `ExistForAll.SimpleSettings.UnitTests.Conversion`, `using ExistForAll.SimpleSettings.Binder;`. + (Claude's discretion per CONTEXT allows extending an existing converter class instead; a dedicated + file is preferred for a clean, greppable `*Scalar*` filter.) + + Use the integration `Build` pattern from CollectionConversionTests (build an InMemoryCollection, + add the formatted section + key + value, `SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new + InMemoryBinder(collection)))`, then `GetSettings()`). Fixture: a nested `public interface IEndpoint + { Uri Url { get; set; } DateTime When { get; set; } }`; the section is "Endpoint" (leading "I" stripped). + + Cover exactly these cases (genuine residual per 02-RESEARCH Don't-Duplicate map): + 1. Scalar Uri positive: bind "Endpoint"/Url = "https://a.example/"; assert result.Url equals + `new Uri("https://a.example/")`. + 2. Scalar DateTime positive (configured format): bind "Endpoint"/When = "2020-01-02" (matches default + "yyyy-MM-dd"); assert result.When equals `new DateTime(2020, 1, 2)`. + 3. One DateTime format-mismatch negative: bind When = "01/02/2020" (wrong format) and assert + `() => builder.GetSettings()` Throws(). Assert the + exception TYPE only — do NOT assert on the message, ToString(), value, or inner (redaction is + already locked by ExceptionRedactionTests; re-proving it is forbidden duplication). + + No culture mutation is needed (ParseExact uses InvariantCulture and the format is invariant), so + `[NotInParallel]` is not required. Do NOT add array-of-Uri or array-of-DateTime cases (P4 covers them + in CollectionConversionTests) — that is forbidden duplication. + + + From `src/`: `dotnet build SimpleSettings.slnx -c Debug` then `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build --filter "*Scalar*"` exits 0 with the three new test methods present and green. + + + - File `src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs` exists (`public class ScalarConversionTests`, namespace `...UnitTests.Conversion`). + - Three `[Test] public async Task` methods: scalar Uri positive (== new Uri(...)), scalar DateTime positive (== new DateTime(2020,1,2)), one format-mismatch negative (Throws). + - The negative test asserts only the exception type — no assertion on message/ToString/value/inner. + - No array-of-Uri/array-of-DateTime cases (no CollectionConversionTests duplication). + - `dotnet test ... --framework net10.0 --filter "*Scalar*"` (from `src/`) exits 0; new tests present and green. + + Scalar Uri and DateTime positive conversion (plus one format-mismatch negative) are green; array-of-* and redaction contracts are not duplicated. Success criterion #5 scalar coverage is satisfied on net10 (net8 via CI parity). + + + + + +## Deferred This Phase (COLL-01 / C1) — owner-deferred, NOT planned + +**COLL-01** (`List`/`IList`/`ICollection` support decision — broaden the converter or +document+throw, with a positive test) is **deferred by the owner** and is intentionally NOT implemented +in Phase 2. No collection-support implementation task and no `List` positive/doc test are planned. + +Why deferred: the broaden-vs-document+throw call is an open design decision the owner is holding. +Background for when it is decided: `TypeExtensions.IsEnumerable` +(`src/Core/ExistForAll.SimpleSettings/Core/Reflection/TypeExtensions.cs:23-28`) matches only exact +`IEnumerable<>`; other shapes fall through to `DefaultTypeConverter` and throw. `CollectionTypeConverter` +already materializes a `T[]`, which satisfies `IList`/`ICollection`/`IReadOnlyList`/ +`IReadOnlyCollection` in one shot — so broadening is low-cost when the owner decides. + +This note exists so COLL-01 is **visible in the plan set as an explicit deferral, not silently dropped**. +The requirements-coverage gate will (correctly) surface COLL-01 as intentionally deferred. Do NOT invent +a task for it here. + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| config value → typed property | Untrusted string values cross into scalar Uri/DateTime conversion; a failed conversion must never surface the raw (possibly secret) value. | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-02-02 | Information Disclosure | ScalarConversionTests format-mismatch negative | medium | mitigate | The negative case asserts only `Throws()` — no assertion on message/ToString/value/inner, so the SEC-01 redaction invariant (locked by ExceptionRedactionTests) is neither re-proved nor weakened. | +| T-02-SC | Tampering | package installs (NuGet) | low | accept | N/A this phase — no new PackageReference, no npm/pip/cargo/NuGet install; test doubles are the existing hand-written in-memory binder. RESEARCH Package Legitimacy Audit: N/A — no external packages installed. | + + + +- From `src/`: `dotnet build SimpleSettings.slnx -c Debug` succeeds on net10. +- `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build --filter "*Scalar*"` passes with the three new tests green. +- Full net10 suite remains green; no production source under `src/Core/` modified. + + + +- Phase 2 success criterion #5 scalar coverage: `Uri`/`DateTime` scalar conversion is proven by ScalarConversionTests on net10 (net8 parity via CI). +- No duplication of P4's array-of-Uri/DateTime coverage or of ExceptionRedactionTests. +- COLL-01 is explicitly documented as owner-deferred (see "## Deferred This Phase"), not silently dropped. + + + +Create `.planning/phases/02-binding-correctness-engine-test-hardening/02-02-SUMMARY.md` when done. + From 473a40c3d20c45bd79c49d264e503d834712d33f Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:46:14 +0300 Subject: [PATCH 06/14] docs(02): add phase artifacts inventory to plan 01 --- .../02-01-PLAN.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md index f751e6c..a7d2a9e 100644 --- a/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-PLAN.md @@ -48,6 +48,31 @@ Deferred this phase: COLL-01/C1 (`List`/`IList`/`ICollection` broaden-v 02-02-PLAN.md "## Deferred This Phase" for the owner-deferred rationale. Do NOT implement it here. + +## Artifacts This Phase Produces (both plans) + +New test files: +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs` (Plan 01, TEST-01) +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs` (Plan 01, TEST-02) +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs` (Plan 02, TEST-03) + +New fixture interfaces (nested `public interface` at each class bottom — names at executor discretion; suggested): +- `ValuesPopulatorTests.ISample` — scalar props incl. one `[SettingsProperty(DefaultValue=...)]` (precedence + default-survives) +- `TypeConverterTests.ISample` — `int Count` + `int? Maybe` (null->default, nullable strip+convert) +- `TypeConverterTests.IWithConverterOverride` — `[SettingsProperty(ConverterType=typeof(SentinelConverter))] IEnumerable Values` +- `ScalarConversionTests.IEndpoint` — `Uri Url` + `DateTime When` + +New test helper types: +- `TypeConverterTests.SentinelConverter : ISettingsTypeConverter` — returns a distinctive sentinel to prove ConverterType bypasses the collection converter. + +No new fake binder is introduced: TEST-01 uses the existing `InMemoryBinder`/`InMemoryCollection` +(RESEARCH Open Question #1 recommends the integration path). `ThrowingBinder` from ExceptionHierarchyTests +is NOT copied — the bind-throw contract is already locked and must not be re-asserted. + +No production source is created or modified by either plan (test-only phase). ENG-01 (Plan 01 Task 3) is +verify-only against the existing `SettingsClassGenerator.cs` + `SettingsClassGeneratorTests.cs`. + + @/Users/guyludvig/frontegg/development/open-source/SimpleSettings/.claude/gsd-core/workflows/execute-plan.md @/Users/guyludvig/frontegg/development/open-source/SimpleSettings/.claude/gsd-core/templates/summary.md From 6c5f380aed7f90ec1339b81092d3ba631719ade6 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 12:50:33 +0300 Subject: [PATCH 07/14] docs(02): add pattern map; mark phase planned (2 plans, ready to execute) --- .planning/STATE.md | 18 +- .../02-PATTERNS.md | 195 ++++++++++++++++++ 2 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-PATTERNS.md diff --git a/.planning/STATE.md b/.planning/STATE.md index 4fd7509..abd0a4b 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -1,6 +1,14 @@ --- -gsd_state_version: '1.0' # placeholder; syncStateFrontmatter overwrites on first state.* call +gsd_state_version: 1.0 +milestone: v2.0.0 +milestone_name: milestone +current_phase: 2 +current_phase_name: Binding Correctness & Engine Test Hardening status: planning +stopped_at: "Phase 1 (S1 #27, C2 #28) shipped; reconciled ROADMAP/STATE/REQUIREMENTS to mark Phase 1 complete. Next: plan Phase 2." +last_updated: "2026-07-14T09:49:18.804Z" +last_activity: 2026-07-14 +last_activity_desc: "ENG-01/T7 merged (#29, master @ 10f9275); GSD ownership cutover: .planning reconciled to reality, FIX-PLAN.md frozen as a historical reference (GSD is now source of truth)" progress: total_phases: 5 completed_phases: 1 @@ -21,15 +29,16 @@ See: .planning/PROJECT.md (updated 2026-07-13) ## Current Position Phase: 2 of 5 (Binding Correctness & Engine Test Hardening) -Plan: 0 of TBD in current phase -Status: In progress — ENG-01/T7 done (#29); COLL-01 (C1, deferred) + TEST-01/02/03 remain to plan -Last activity: 2026-07-14 — ENG-01/T7 merged (#29, master @ 10f9275); GSD ownership cutover: .planning reconciled to reality, FIX-PLAN.md frozen as a historical reference (GSD is now source of truth) +Plan: 0 of 2 executed in current phase (both planned + verified) +Status: Ready to execute — 2 plans (wave 1, parallel): TEST-01/02/03 + ENG-01 verify-only; COLL-01 deferred +Last activity: 2026-07-14 — Phase 2 planned via GSD (2 plans, plan-checker PASSED); prior: ENG-01/T7 merged (#29) + GSD ownership cutover (.planning is source of truth, FIX-PLAN.md frozen) Progress: [██░░░░░░░░] 20% ## Performance Metrics **Velocity:** + - Total plans completed: 0 - Average duration: — - Total execution time: 0 hours @@ -41,6 +50,7 @@ Progress: [██░░░░░░░░] 20% | - | - | - | - | **Recent Trend:** + - Last 5 plans: — - Trend: — diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-PATTERNS.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-PATTERNS.md new file mode 100644 index 0000000..c382806 --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-PATTERNS.md @@ -0,0 +1,195 @@ +# Phase 2: Binding Correctness & Engine Test Hardening - Pattern Map + +**Mapped:** 2026-07-14 +**Files analyzed:** 3 new test files (TEST-01, TEST-02, TEST-03) +**Analogs found:** 3 / 3 (all exact-role matches — existing TUnit test classes in the same project) + +This is a **test-only** phase. Every new file is a TUnit test class under +`src/Tests/ExistForAll.SimpleSettings.UnitTests/`. There is no production-code analog to copy; +the analogs are existing **test classes** whose TUnit shape, fixture layout, and engine-wiring +the new files mirror. Do NOT re-assert contracts already locked by `ExceptionHierarchyTests` / +`ExceptionRedactionTests` (see the Don't-Duplicate map in `02-RESEARCH.md`). + +## File Classification + +| New File | Role | Data Flow | Closest Analog | Match Quality | +|----------|------|-----------|----------------|---------------| +| `Core/ValuesPopulatorTests.cs` (TEST-01) | test | request-response (bind→convert→set) | `SimpleSettings/ExceptionHierarchyTests.cs` (fake binder + `CatchBase`) + `Conversion/CollectionConversionTests.cs` (two-binder integration) | exact | +| `Core/TypeConverterTests.cs` (TEST-02) | test | transform (null/nullable/converter-select) | `Conversion/CollectionConversionTests.cs` (integration build) + `SimpleSettings/SettingsBuilderConversionsTests.cs` (`ConverterType`) | exact | +| `Conversion/ScalarConversionTests.cs` (TEST-03) | test | transform (scalar parse) | `Conversion/CollectionConversionTests.cs` (`Build` helper, Uri/DateTime array cases) + `Conversion/DefaultTypeConverterTests.cs` (`[NotInParallel]` culture) | exact | + +Note: `Core/` is a new test subfolder mirroring the source `ExistForAll.SimpleSettings.Core` namespace. +TEST-03 may extend an existing class instead of a new `ScalarConversionTests.cs` (Claude's discretion). + +## Shared TUnit Conventions (apply to ALL three files) + +**Source of truth:** `Conversion/CollectionConversionTests.cs`, `Conversion/DefaultTypeConverterTests.cs`. + +- File is a plain `public class XxxTests` in namespace `ExistForAll.SimpleSettings.UnitTests.` + wrapped in a block-scoped namespace (project convention — see every analog). +- Test methods: `[Test] public async Task Name_State_Expected()`. +- Assertions: `await Assert.That(actual).IsEqualTo(expected)` / `.IsTrue()` / `.IsNull()` / + `.IsNotEqualTo(...)`; throwing: `await Assert.That(() => act()).Throws()`. +- Fixtures are **nested `public interface`** declarations at the bottom of the class + (`CollectionConversionTests.cs:148-182`), decorated with `[SettingsProperty(...)]` where a default + or `ConverterType` is needed. +- Build path: `SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection)))` + then `builder.GetSettings()`. +- Section-name gotcha: default `SectionNameFormatter` strips the leading `I` — `IThing` → `"Thing"`. + Use the formatted name in `collection.Add(section, key, value)` (see `DefaultTypeConverterTests.cs:9` + const `Section = "NumericSettings"` for `INumericSettings`). +- `InMemoryBinder`/`InMemoryCollection` live in namespace `ExistForAll.SimpleSettings.Binder` + (`using ExistForAll.SimpleSettings.Binder;` — visible via InternalsVisibleTo, `Info.cs:3`). +- Culture-mutating tests: `[NotInParallel]` + save/restore in `finally` + (`DefaultTypeConverterTests.cs:11-30`). + +--- + +## Pattern Assignments + +### `Core/ValuesPopulatorTests.cs` (TEST-01) + +**Analogs:** `SimpleSettings/ExceptionHierarchyTests.cs` (fake binder + catch helper), +`Conversion/CollectionConversionTests.cs` (integration `Build` helper). + +**Source under test:** `src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs` — binder loop +(`ValuesPopulator.cs:45-63`, last-writer-wins via `context.HasNewValue`), bind-catch wrapping +(`:59-61` → `SettingsBindingException`), convert wrapping (`:117-131`). + +**Genuine gaps to cover (per RESEARCH Don't-Duplicate map):** last-writer-wins precedence; +"binders present but none set value ⇒ `[SettingsProperty]` default survives" (scalar); optional +unit-level bind-throw ⇒ `SettingsBindingException`. +**Do NOT re-assert** the value-free `SettingsPropertyValueException` contract — owned by +`ExceptionHierarchyTests.ConversionFailure_ExposesSafeStructuredMetadata_AndNoChainedInner`. + +**Last-writer-wins pattern** (build two ordered binders; recommended integration route per +RESEARCH Open Question #1 — `CollectionConversionTests.cs:131-146` helper shape): +```csharp +var c1 = new InMemoryCollection(); c1.Add("Sample", nameof(ISample.Name), "first"); +var c2 = new InMemoryCollection(); c2.Add("Sample", nameof(ISample.Name), "second"); +var builder = SettingsBuilder.CreateBuilder(x => +{ + x.AddSectionBinder(new InMemoryBinder(c1)); + x.AddSectionBinder(new InMemoryBinder(c2)); // later binder wins +}); +await Assert.That(builder.GetSettings().Name).IsEqualTo("second"); +``` + +**Fake binder pattern** (mirror `ExceptionHierarchyTests.cs:116-120`; a *setting* fake calls +`context.SetNewValue(...)`): +```csharp +private class ThrowingBinder : ISectionBinder +{ + public void BindPropertySettings(BindingContext context) + => throw new InvalidOperationException("binder failed"); +} +``` + +**Bind-throw assertion** (mirror `ExceptionHierarchyTests.cs:79-90`, using the `CatchBase` helper +at `:92-104` — copy it verbatim if a unit-level bind-throw case is written): +```csharp +var ex = (SettingsBindingException)CatchBase(() => builder.GetSettings())!; +await Assert.That(ex.BinderType).IsEqualTo(typeof(ThrowingBinder)); +await Assert.That(ex.Section).IsEqualTo("IntSettings"); +await Assert.That(ex.Key).IsEqualTo(nameof(IIntSettings.Value)); +``` + +**Internal-ctor option** (only if binder-order isolation is needed the builder can't express — +`ValuesPopulator.cs:26-35`): `new ValuesPopulator(new TypePropertiesExtractor(), new TypeConverter())` +then `PopulateInstanceWithValues(instance, typeof(T), options, binders)`. RESEARCH recommends the +integration path first. + +--- + +### `Core/TypeConverterTests.cs` (TEST-02) + +**Analogs:** `Conversion/CollectionConversionTests.cs` (integration + fixture interfaces), +`SimpleSettings/SettingsBuilderConversionsTests.cs` (`ConverterType` attribute). + +**Source under test:** `Core/Reflection/TypeConverter.cs:13-24` (`CreateConversion`), +`:28-37` (`CreateNullResult` — enumerable→empty `T[]`, value-type→`Activator.CreateInstance`, +else null), `:39-58` (`GetConverter` — `attribute.ConverterType` wins at `:43-44`), +`:60-65` (`StripIfNullable`); runtime null-check in `Conversion/PropertyConversion.cs:31-42` +(`throwOnNull` ⇒ `SettingsPropertyNullException`). + +**Genuine gaps:** `null` → value-type default (`int` → `0`); `Nullable` strip+convert +(`int?` null → `null`, `"42"` → `42`); `ConverterType` bypasses the **collection** converter +(attribute on an `IEnumerable`/array property — the residual; scalar `ConverterType` already +covered by `SettingsBuilderConversionsTests`). Empty-enumerable + `AllowEmpty` are already covered — +do NOT duplicate. + +**Direct-seam pattern** (RESEARCH Pattern 2 — for pure resolution logic, no generated instance): +```csharp +var options = new SettingsOptions(); // Converters auto-seeded: DateTime,Uri,Array,Enumerable,Enum,Default +var conv = new TypeConverter(); +var intProp = typeof(ISample).GetProperty(nameof(ISample.Count))!; // int +var c = conv.CreateConversion(intProp, intProp.GetCustomAttribute(true), options); +await Assert.That(c.Convert(null)).IsEqualTo(0); // value-type default +// int? property: null -> null, "42" -> 42 (StripIfNullable, TypeConverter.cs:60-65) +``` +Nullable pitfall (RESEARCH Pitfall 3): `int?` null → `null` (nullResult built on the original +nullable type), NOT `0`. Test `int` and `int?` separately. + +**`ConverterType` fixture pattern** (mirror `SettingsBuilderConversionsTests.cs:32-58` — a +`SettingsPropertyAttribute` subclass with `ConverterType` set, plus an `ISettingsTypeConverter`; +here point it at a collection-typed property to prove it bypasses `CollectionTypeConverter`): +```csharp +public interface IThing +{ + [MyConv(ConverterType = typeof(FakeCollectionConverter))] + IEnumerable Values { get; set; } +} +``` + +--- + +### `Conversion/ScalarConversionTests.cs` (TEST-03) + +**Analog:** `Conversion/CollectionConversionTests.cs` (the `Build` helper at `:131-146` and the +Uri/DateTime **array** cases at `:101-116` — mirror shape, scope to **scalar**). + +**Source under test:** `Conversion/UriTypeConvertor.cs` (`new Uri((string)value)`), +`Conversion/DateTimeTypeConverter.cs` (`DateTime.ParseExact(value, options.DateTimeFormat, +InvariantCulture)`, default format `"yyyy-MM-dd"`). + +**Genuine gaps:** scalar `Uri` positive parse; scalar `DateTime` positive parse with configured +format. Array-of-Uri/DateTime are fully covered by P4 — do NOT duplicate. At most one DateTime +format-mismatch negative (redaction already locked — don't re-prove). + +**Scalar positive pattern** (integration `Build` shape from `CollectionConversionTests.cs:131-146`): +```csharp +var c = new InMemoryCollection(); +c.Add("Endpoint", nameof(IEndpoint.Url), "https://a.example/"); +c.Add("Endpoint", nameof(IEndpoint.When), "2020-01-02"); // default yyyy-MM-dd +var b = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(c))); +var r = b.GetSettings(); +await Assert.That(r.Url).IsEqualTo(new Uri("https://a.example/")); +await Assert.That(r.When).IsEqualTo(new DateTime(2020, 1, 2)); +``` +Fixture interface at class bottom (mirror `CollectionConversionTests.cs:148-182`): +`public interface IEndpoint { Uri Url { get; set; } DateTime When { get; set; } }`. + +--- + +## ENG-01 — Verify-Only (no new file) + +Not mapped as new work. The 2 stress tests already exist and satisfy success criterion #4: +- `SettingsClassGeneratorTests.cs:103-116` `GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType` + (`Parallel.For(0,128)` → `results.Distinct().Count() == 1`). +- `SettingsClassGeneratorTests.cs:118-166` `GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree` + (32 threads + `Barrier`, 8 interfaces, zero failures, one shared impl each). +Verify present + green; do NOT re-implement. + +## No Analog Found + +None. Every new file has a strong same-project test analog. COLL-01/C1 and D1/D2 are deferred/held +and intentionally unmapped. + +## Metadata + +**Analog search scope:** `src/Tests/ExistForAll.SimpleSettings.UnitTests/{Conversion,SimpleSettings}`, +`src/Core/ExistForAll.SimpleSettings/{,Core/Reflection,Conversion}`. +**Files scanned:** 7 (4 test analogs, 3 source-under-test). +**Pattern extraction date:** 2026-07-14 + + From 5ff86473884195472e26c8df30a119dc8eff03a1 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:00:40 +0300 Subject: [PATCH 08/14] test(02-01): add ValuesPopulator precedence + default-survives tests - last-writer-wins across two ordered binders - later silent binder does not clobber earlier set value - [SettingsProperty] DefaultValue survives when no binder sets the property --- .../Core/ValuesPopulatorTests.cs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs new file mode 100644 index 0000000..5898821 --- /dev/null +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs @@ -0,0 +1,81 @@ +using ExistForAll.SimpleSettings.Binder; + +namespace ExistForAll.SimpleSettings.UnitTests.Core +{ + // Locks the two genuinely-uncovered ValuesPopulator precedence behaviors (TEST-01): binder + // last-writer-wins, a later silent binder not clobbering an earlier set value, and the + // [SettingsProperty] DefaultValue surviving when binders run but none set the property. Uses the + // integration build path (RESEARCH Open Question #1) — binder ORDER is expressible via AddSectionBinder, + // so the internal ValuesPopulator ctor is not needed. Section name is the interface name with the + // leading "I" stripped ("Sample"). Does NOT re-assert the exception-wrapper contracts (owned by + // ExceptionHierarchyTests / ExceptionRedactionTests). + public class ValuesPopulatorTests + { + private const string Section = "Sample"; + + [Test] + public async Task Populate_WhenTwoOrderedBindersSetSameProperty_LaterBinderWins() + { + var first = new InMemoryCollection(); + first.Add(Section, nameof(ISample.Name), "first"); + + var second = new InMemoryCollection(); + second.Add(Section, nameof(ISample.Name), "second"); + + var builder = SettingsBuilder.CreateBuilder(x => + { + x.AddSectionBinder(new InMemoryBinder(first)); + x.AddSectionBinder(new InMemoryBinder(second)); // added later => wins + }); + + var result = builder.GetSettings(); + + await Assert.That(result.Name).IsEqualTo("second"); + } + + [Test] + public async Task Populate_WhenLaterBinderIsSilentOnProperty_EarlierValueSurvives() + { + var first = new InMemoryCollection(); + first.Add(Section, nameof(ISample.Name), "first"); + + // An active later binder that sets a DIFFERENT key and never touches Name: proves a silent + // later binder does not clobber the earlier set value (still precedence, not the exception contract). + var second = new InMemoryCollection(); + second.Add(Section, nameof(ISample.Label), "later-label"); + + var builder = SettingsBuilder.CreateBuilder(x => + { + x.AddSectionBinder(new InMemoryBinder(first)); + x.AddSectionBinder(new InMemoryBinder(second)); + }); + + var result = builder.GetSettings(); + + await Assert.That(result.Name).IsEqualTo("first"); + } + + [Test] + public async Task Populate_WhenBindersPresentButNoneSetProperty_AttributeDefaultSurvives() + { + // The binder runs (it sets a different key) but never sets Label, so Label keeps its + // [SettingsProperty(DefaultValue = "fallback")]. + var collection = new InMemoryCollection(); + collection.Add(Section, nameof(ISample.Name), "set-name"); + + var builder = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection))); + + var result = builder.GetSettings(); + + await Assert.That(result.Label).IsEqualTo("fallback"); + } + + public interface ISample + { + string Name { get; set; } + + [SettingsProperty(DefaultValue = "fallback")] + string Label { get; set; } + } + } +} From 42ff5219b93ac7bc5c599f145d7bd3126e175687 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:01:15 +0300 Subject: [PATCH 09/14] test(02-01): add TypeConverter null/nullable/ConverterType tests - null for non-nullable int resolves to 0 - Nullable null resolves to null; "42" strips and converts to 42 - ConverterType on IEnumerable bypasses the collection converter (sentinel wins) --- .../Core/TypeConverterTests.cs | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs new file mode 100644 index 0000000..1406921 --- /dev/null +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs @@ -0,0 +1,86 @@ +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using ExistForAll.SimpleSettings.Conversion; +using ExistForAll.SimpleSettings.Core.Reflection; + +namespace ExistForAll.SimpleSettings.UnitTests.Core +{ + // Locks the three genuinely-uncovered TypeConverter resolution paths (TEST-02) via the direct + // converter-orchestration seam (RESEARCH Pattern 2): null -> value-type default, Nullable + // strip-and-convert, and an attribute ConverterType bypassing the collection converter on an + // IEnumerable property. Reaches the internal seam via InternalsVisibleTo. Does NOT re-assert the + // empty-enumerable or AllowEmpty paths (owned by CollectionConversionTests / SettingsPropertyTests). + public class TypeConverterTests + { + [Test] + public async Task Convert_NullForNonNullableValueType_ReturnsTypeDefault() + { + var conversion = ResolveConversion(nameof(ISample.Count)); + + await Assert.That(conversion.Convert(null)).IsEqualTo(0); + } + + [Test] + public async Task Convert_NullForNullableValueType_ReturnsNull() + { + // RESEARCH Pitfall 3: the null result is built on the ORIGINAL nullable type, so int? null -> null + // (NOT 0). + var conversion = ResolveConversion(nameof(ISample.Maybe)); + + await Assert.That(conversion.Convert(null)).IsNull(); + } + + [Test] + public async Task Convert_NumericStringForNullableValueType_StripsAndConverts() + { + // StripIfNullable unwraps int? to int, DefaultTypeConverter parses "42" -> 42. + var conversion = ResolveConversion(nameof(ISample.Maybe)); + + await Assert.That(conversion.Convert("42")).IsEqualTo(42); + } + + [Test] + public async Task Convert_WhenConverterTypeSetOnCollectionProperty_BypassesCollectionConverter() + { + // GetConverter returns the attribute's ConverterType before the collection-converter scan, even + // for an IEnumerable property, so the sentinel wins over the parsed [1,2,3]. + var conversion = ResolveConversion(nameof(IWithConverterOverride.Values)); + + var result = conversion.Convert("1,2,3"); + + await Assert.That(result is int[]).IsTrue(); + await Assert.That(((int[])result!).SequenceEqual(new[] { -1 })).IsTrue(); + } + + private static PropertyConversion ResolveConversion(string propertyName) + { + var options = new SettingsOptions(); // Converters auto-seeded: DateTime,Uri,Array,Enumerable,Enum,Default + var conv = new TypeConverter(); + var prop = typeof(T).GetProperty(propertyName)!; + + return conv.CreateConversion(prop, prop.GetCustomAttribute(inherit: true), options); + } + + public interface ISample + { + int Count { get; set; } + int? Maybe { get; set; } + } + + public interface IWithConverterOverride + { + [SettingsProperty(ConverterType = typeof(SentinelConverter))] + IEnumerable Values { get; set; } + } + + // Returns a distinctive sentinel so a passing test proves the attribute ConverterType was chosen + // instead of CollectionTypeConverter. + private class SentinelConverter : ISettingsTypeConverter + { + public bool CanConvert(System.Type settingsType) => true; + + public object Convert(object value, System.Type settingsType) => new[] { -1 }; + } + } +} From 6ac91555c263ecd90371ed3444542c751704f918 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:03:56 +0300 Subject: [PATCH 10/14] docs(02-01): complete engine-core correctness test-hardening plan --- .planning/REQUIREMENTS.md | 9 +- .planning/ROADMAP.md | 23 ++- .planning/STATE.md | 34 ++-- .../02-01-SUMMARY.md | 174 ++++++++++++++++++ 4 files changed, 218 insertions(+), 22 deletions(-) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-01-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index f1aba2d..9805e08 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -45,8 +45,8 @@ Remaining open work (from `FIX-PLAN.md`), batched toward the first `v2.0.0-beta` ### Binding Correctness & Engine Tests - [ ] **COLL-01**: `List`/`IList`/`ICollection` support decision — broaden the converter or document + throw a clear error, with a positive test (C1) -- [ ] **TEST-01**: `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts (T4) -- [ ] **TEST-02**: `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths (T5) +- [x] **TEST-01**: `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts (T4) +- [x] **TEST-02**: `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths (T5) - [ ] **TEST-03**: Converter tests residual — `Uri`/`DateTime` + `List` doc test tied to C1 (T6) - [x] **ENG-01**: Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests (T7 — shipped pre-GSD via the FIX-PLAN track, merged #29: double-checked locking, one gate over all generation; same- + distinct-interface `Barrier` stress tests) @@ -96,8 +96,8 @@ Deferred / held. Tracked but not in the current roadmap. | SEC-02 | Phase 1 | ✓ Complete | | EXC-01 | Phase 1 | ✓ Complete (#28) | | COLL-01 | Phase 2 | Pending | -| TEST-01 | Phase 2 | Pending | -| TEST-02 | Phase 2 | Pending | +| TEST-01 | Phase 2 | Complete | +| TEST-02 | Phase 2 | Complete | | TEST-03 | Phase 2 | Pending | | ENG-01 | Phase 2 | ✓ Complete (#29) | | API-01 | Phase 3 | Pending | @@ -109,6 +109,7 @@ Deferred / held. Tracked but not in the current roadmap. | REL-01 | Phase 5 | Pending | **Coverage:** + - v1 requirements: 15 total - Mapped to phases: 15 - Unmapped: 0 ✓ diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 341b156..db55fed 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -17,6 +17,7 @@ value: config → typed settings maps accurately, and never leaks a secret doing ## Phases **Phase Numbering:** + - Integer phases (1, 2, 3): Planned milestone work - Decimal phases (2.1, 2.2): Urgent insertions (marked with INSERTED) @@ -29,63 +30,79 @@ value: config → typed settings maps accurately, and never leaks a secret doing ## Phase Details ### Phase 1: Exception Safety & Public Hierarchy + **Goal**: The library never leaks secret bound values through failures, and consumers can catch every library error as one public, structured category. **Depends on**: Nothing (first phase) **Requirements**: SEC-01, SEC-02, EXC-01 **Status**: ✓ COMPLETE — shipped outside GSD via the FIX-PLAN track (S1 #27 + C2 #28), merged to `master` @ `13b78dd` on 2026-07-14. All 5 success criteria met; secret-redaction is structural (`SettingsPropertyValueException` takes the failure `Type`, not the `Exception`); +11 tests (5 redaction + 6 hierarchy). **Success Criteria** (what must be TRUE): + 1. A secret sentinel bound to an `int`/`enum`/`DateTime`/`Uri`/custom-converter property is absent from the entire `ex.ToString()` chain of the thrown exception. 2. A "required value missing" failure still surfaces its full diagnostic message via `SettingsPropertyNullException` (not redacted). 3. A consumer can `catch (SimpleSettingsException)` and handle every boundary failure the library raises. 4. Boundary exceptions expose structured context (property name, target type, failure type name, binder/section/key) without the bound value. 5. No exception wrapper (binding, extraction, generation, value-conversion) embeds a bound value or chains a value-bearing inner. + **Plans**: n/a — delivered pre-GSD (S1 #27 + C2 #28) ### Phase 2: Binding Correctness & Engine Test Hardening + **Goal**: Binding maps config to typed settings accurately across every supported collection, nullable, and converter shape, with the engine's concurrency and precedence behavior locked by tests. **Depends on**: Phase 1 (engine tests assert the S1/C2 exception contract) **Requirements**: COLL-01, TEST-01, TEST-02, TEST-03, ENG-01 **Status**: In progress — ENG-01/T7 delivered pre-GSD (merged #29: generator concurrency race closed via double-checked locking + same/distinct-interface stress tests). COLL-01 (C1 — decision deferred) and TEST-01/02/03 (T4/T5/T6) remain. **Success Criteria** (what must be TRUE): + 1. A settings interface exposing `List`/`IList`/`ICollection` either binds correctly or fails with a clear, documented error (per the C1 decision), covered by a test. 2. Binder precedence (last binder wins; attribute default applies when none set) is verified by `ValuesPopulator` tests. 3. `TypeConverter` null / nullable / empty-enumerable / `AllowEmpty` / attribute-`ConverterType` paths are verified by tests. 4. Concurrent first-touch generation of the same interface returns one `ReferenceEquals` implementation with no duplicate-`DefineType` race. ✓ Met by #29 (T7). 5. `Uri`/`DateTime` and collection converters have parity tests passing on net8 and net10. -**Plans**: 2 plans -- [ ] 02-01-PLAN.md — Engine-core correctness tests (TEST-01 ValuesPopulator precedence/default; TEST-02 TypeConverter null/nullable/ConverterType-over-collection) + ENG-01 concurrency verify + +**Plans**: 1/2 plans executed + +- [x] 02-01-PLAN.md — Engine-core correctness tests (TEST-01 ValuesPopulator precedence/default; TEST-02 TypeConverter null/nullable/ConverterType-over-collection) + ENG-01 concurrency verify - [ ] 02-02-PLAN.md — Converter scalar residual (TEST-03 scalar Uri/DateTime positive + one format-mismatch negative); COLL-01 documented as owner-deferred ### Phase 3: Public Surface, Packaging & Binder Cleanup + **Goal**: The public API and packages carry only meaningful, correctly-scoped surface, and the command-line binder parses real-world arguments correctly — the remaining breaking changes batched before beta. **Depends on**: Phase 2 **Requirements**: API-01, PKG-01, PKG-02, SRC-02 **Success Criteria** (what must be TRUE): + 1. `SettingsHolder`/`ISettingsHolder` are internal and no longer appear on the public surface; build and suite stay green. 2. `Core.AspNet` either exposes a consumable public type or is removed from the solution. 3. A net8 consumer is no longer transitively forced onto `Microsoft.Extensions.* 10.x` (per-TFM floor), or the pin is documented with justification. 4. A quoted command-line value containing spaces binds correctly and the executable path (`arg[0]`) is skipped. + **Plans**: TBD ### Phase 4: AOT/Trim Honesty & Documentation + **Goal**: Consumers get honest signals about AOT/trim support and accurate, canonically-named documentation. **Depends on**: Phase 3 **Requirements**: AOT-01, DOC-01 **Success Criteria** (what must be TRUE): + 1. Public reflection-based entry points carry `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` annotations and/or the AOT/trim limitation is documented before stable. 2. Building an AOT/trimmed consumer surfaces a warning (or finds a clearly documented limitation) rather than failing silently. 3. README uses the canonical `ExistForAll.SimpleSettings` name and links to current repo/package paths (no legacy `existall`/`SimpleConfig` references). + **Plans**: TBD ### Phase 5: First v2.0.0-beta Release + **Goal**: All batched breaking changes and hardening ship as the first pre-release beta consumers can install. **Depends on**: Phases 1–4 (release gate — all breaking + hardening work complete) **Requirements**: REL-01 **Success Criteria** (what must be TRUE): + 1. A `v2.0.0-beta` tag exists and the packages are published to NuGet.org via the release workflow. 2. A net8 consumer and a net10 consumer can install and use the beta package. 3. All sub-packages (Core, Binders, Extensions.GenericHost, and Core.AspNet if retained) ship under the canonical `ExistForAll.SimpleSettings` identity. 4. The full test suite passes on net8 and net10 at the tagged commit. + **Plans**: TBD ## Progress @@ -96,7 +113,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| | 1. Exception Safety & Public Hierarchy | n/a (shipped) | ✓ Complete | 2026-07-14 (#27/#28) | -| 2. Binding Correctness & Engine Test Hardening | ENG-01 ✓ (#29); rest 0/TBD | ◆ In progress | - | +| 2. Binding Correctness & Engine Test Hardening | 1/2 | In Progress| | | 3. Public Surface, Packaging & Binder Cleanup | 0/TBD | Not started | - | | 4. AOT/Trim Honesty & Documentation | 0/TBD | Not started | - | | 5. First v2.0.0-beta Release | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index abd0a4b..7fec3b3 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,19 +2,19 @@ gsd_state_version: 1.0 milestone: v2.0.0 milestone_name: milestone -current_phase: 2 -current_phase_name: Binding Correctness & Engine Test Hardening -status: planning +current_phase: 02 +current_phase_name: binding-correctness-engine-test-hardening +status: executing stopped_at: "Phase 1 (S1 #27, C2 #28) shipped; reconciled ROADMAP/STATE/REQUIREMENTS to mark Phase 1 complete. Next: plan Phase 2." -last_updated: "2026-07-14T09:49:18.804Z" +last_updated: "2026-07-14T10:03:51.219Z" last_activity: 2026-07-14 -last_activity_desc: "ENG-01/T7 merged (#29, master @ 10f9275); GSD ownership cutover: .planning reconciled to reality, FIX-PLAN.md frozen as a historical reference (GSD is now source of truth)" +last_activity_desc: Phase 02 execution started progress: total_phases: 5 - completed_phases: 1 - total_plans: 0 - completed_plans: 0 - percent: 20 + completed_phases: 0 + total_plans: 2 + completed_plans: 1 + percent: 0 --- # Project State @@ -24,14 +24,14 @@ progress: See: .planning/PROJECT.md (updated 2026-07-13) **Core value:** Correctness of binding — config → strongly-typed settings maps accurately across every supported shape (sections, arrays/enumerables, defaults, nullable, custom converters). -**Current focus:** Phase 2 — Binding Correctness & Engine Test Hardening +**Current focus:** Phase 02 — binding-correctness-engine-test-hardening ## Current Position -Phase: 2 of 5 (Binding Correctness & Engine Test Hardening) -Plan: 0 of 2 executed in current phase (both planned + verified) -Status: Ready to execute — 2 plans (wave 1, parallel): TEST-01/02/03 + ENG-01 verify-only; COLL-01 deferred -Last activity: 2026-07-14 — Phase 2 planned via GSD (2 plans, plan-checker PASSED); prior: ENG-01/T7 merged (#29) + GSD ownership cutover (.planning is source of truth, FIX-PLAN.md frozen) +Phase: 02 (binding-correctness-engine-test-hardening) — EXECUTING +Plan: 2 of 2 +Status: Ready to execute +Last activity: 2026-07-14 — Phase 02 execution started Progress: [██░░░░░░░░] 20% @@ -55,6 +55,7 @@ Progress: [██░░░░░░░░] 20% - Trend: — *Updated after each plan completion* +| Phase 02 P01 | 2min | 3 tasks | 2 files | ## Accumulated Context @@ -68,6 +69,9 @@ Recent decisions affecting current work: - Public exception hierarchy (C2, merged #28): all library exceptions derive from `public abstract SimpleSettingsException` in the root namespace, enforced by a reflection invariant test; `SettingsTypeNotInterfaceException` replaces the `TypeIsNotInterface` throws (one runtime break → release notes). - Breaking changes are free until the first `v2.0.0-beta` and are batched before cutting it. - `Validations/*` (D1) and `EqualityCompererCreator` (D2) are HELD — do NOT delete (reserved for feature work). +- [Phase 02]: TEST-01/TEST-02 (Plan 01): engine-core correctness gaps locked — ValuesPopulator precedence (last-writer-wins, later-silent-preserves, attribute-default-survives) via integration binders; TypeConverter null->default, Nullable strip+convert, ConverterType-over-collection via the direct CreateConversion seam. +- [Phase 02]: ENG-01 verified (not re-implemented): SettingsClassGenerator _generationGate + both concurrency stress tests green — Phase 2 success criterion #4 met by pre-GSD #29. +- [Phase 02]: TUnit invocation uses --treenode-filter (Microsoft.Testing.Platform); legacy --filter returns zero tests / exit 5. ### Pending Todos @@ -89,6 +93,6 @@ Items acknowledged and carried forward: ## Session Continuity -Last session: 2026-07-14 +Last session: 2026-07-14T10:03:12.795Z Stopped at: Phase 1 (S1 #27, C2 #28) shipped; reconciled ROADMAP/STATE/REQUIREMENTS to mark Phase 1 complete. Next: plan Phase 2. Resume file: None diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-SUMMARY.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-SUMMARY.md new file mode 100644 index 0000000..cda6826 --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-01-SUMMARY.md @@ -0,0 +1,174 @@ +--- +phase: 02-binding-correctness-engine-test-hardening +plan: 01 +subsystem: testing +tags: [tunit, binding, type-conversion, concurrency, nullable, converter] + +# Dependency graph +requires: + - phase: 01 + provides: "Secret-safe exception contract (S1 #27, C2 #28) — the value-free SettingsPropertyValueException / SettingsBindingException hierarchy these tests must not re-assert" +provides: + - "ValuesPopulatorTests: binder precedence (last-writer-wins, later-silent-does-not-clobber) + [SettingsProperty] DefaultValue-survives coverage" + - "TypeConverterTests: null->value-type-default, Nullable strip+convert, and ConverterType-over-collection resolution coverage" + - "ENG-01 verification: confirmation that SettingsClassGenerator concurrency gate (#29) + its two stress tests satisfy Phase 2 success criterion #4" +affects: [02-02, binding-correctness, engine-test-hardening] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Direct converter-orchestration seam: new TypeConverter().CreateConversion(prop, attr, new SettingsOptions()) then PropertyConversion.Convert(...) to unit-test resolution logic without a generated instance" + - "Integration precedence proof via ordered AddSectionBinder(new InMemoryBinder(collection)) calls" + +key-files: + created: + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs + modified: [] + +key-decisions: + - "Used the integration build path (SettingsBuilder + ordered InMemoryBinders) for TEST-01 precedence rather than the internal ValuesPopulator ctor — binder ORDER is expressible via AddSectionBinder (RESEARCH Open Question #1)." + - "Split the Nullable gap into two explicit tests (null->null and \"42\"->42) for a distinct null vs strip-and-convert assertion." + - "SentinelConverter returns new[] { -1 } so a passing ConverterType test proves the attribute converter was chosen over CollectionTypeConverter." + +patterns-established: + - "New Core/ test subfolder mirrors the source ExistForAll.SimpleSettings.Core namespace; nested public interface fixtures + private helper converter at class bottom." + - "Run TUnit filters via --treenode-filter \"/*/*/ClassName/*\" (Microsoft.Testing.Platform), not the legacy --filter flag." + +requirements-completed: [TEST-01, TEST-02, ENG-01] + +coverage: + - id: D1 + description: "ValuesPopulator binder precedence: last-writer-wins across two ordered binders" + requirement: "TEST-01" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs#Populate_WhenTwoOrderedBindersSetSameProperty_LaterBinderWins" + status: pass + human_judgment: false + - id: D2 + description: "ValuesPopulator: a later silent binder does not clobber an earlier set value" + requirement: "TEST-01" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs#Populate_WhenLaterBinderIsSilentOnProperty_EarlierValueSurvives" + status: pass + human_judgment: false + - id: D3 + description: "ValuesPopulator: [SettingsProperty] DefaultValue survives when binders present but none set the property" + requirement: "TEST-01" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs#Populate_WhenBindersPresentButNoneSetProperty_AttributeDefaultSurvives" + status: pass + human_judgment: false + - id: D4 + description: "TypeConverter: null for non-nullable int resolves to type default (0)" + requirement: "TEST-02" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs#Convert_NullForNonNullableValueType_ReturnsTypeDefault" + status: pass + human_judgment: false + - id: D5 + description: "TypeConverter: Nullable resolves null->null and \"42\"->42 (strip + convert)" + requirement: "TEST-02" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs#Convert_NullForNullableValueType_ReturnsNull" + status: pass + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs#Convert_NumericStringForNullableValueType_StripsAndConverts" + status: pass + human_judgment: false + - id: D6 + description: "TypeConverter: ConverterType on an IEnumerable property bypasses the collection converter (sentinel wins)" + requirement: "TEST-02" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs#Convert_WhenConverterTypeSetOnCollectionProperty_BypassesCollectionConverter" + status: pass + human_judgment: false + - id: D7 + description: "ENG-01 verify-only: SettingsClassGenerator concurrency gate (#29) + two stress tests satisfy Phase 2 success criterion #4" + requirement: "ENG-01" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsClassGeneratorTests.cs#GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType" + status: pass + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/SimpleSettings/SettingsClassGeneratorTests.cs#GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree" + status: pass + human_judgment: false + +# Metrics +duration: 2min +completed: 2026-07-14 +status: complete +--- + +# Phase 2 Plan 01: Engine-Core Correctness Test Hardening Summary + +**Two new TUnit test files lock ValuesPopulator binder precedence + attribute-default-survives and the three uncovered TypeConverter resolution paths (null->default, Nullable strip+convert, ConverterType-over-collection); ENG-01 concurrency fix verified green — no production code touched.** + +## Performance + +- **Duration:** ~2 min +- **Started:** 2026-07-14T10:00:00Z +- **Completed:** 2026-07-14T10:01:51Z +- **Tasks:** 3 (2 code, 1 verify-only) +- **Files modified:** 2 created + +## Accomplishments +- TEST-01: `ValuesPopulatorTests` proves last-writer-wins across two ordered binders, that a later silent binder does not clobber an earlier set value, and that a `[SettingsProperty(DefaultValue=...)]` survives when binders run but none set the property. +- TEST-02: `TypeConverterTests` proves `null`->value-type default (`int`->`0`), `Nullable` null->`null` and `"42"`->`42`, and that an attribute `ConverterType` on an `IEnumerable` property bypasses `CollectionTypeConverter` (sentinel `new[]{-1}` wins over the parsed `[1,2,3]`). +- ENG-01 (verify-only): confirmed the `_generationGate` double-checked lock is present in `SettingsClassGenerator.cs` (lines 27 + 53) and both concurrency stress tests pass — Phase 2 success criterion #4 is met by the pre-GSD fix (#29), no work performed. +- Full net10 unit suite green: 91/91 passing (7 new tests added by this plan). + +## Task Commits + +1. **Task 1: TEST-01 ValuesPopulator precedence + default-survives** - `5ff8647` (test) +2. **Task 2: TEST-02 TypeConverter null/nullable/ConverterType** - `42ff521` (test) +3. **Task 3: ENG-01 verify-only** - no commit (no code diff; evidence recorded here) + +## Files Created/Modified +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs` - Three precedence/default tests via the integration build path (ordered `InMemoryBinder`s; section name `"Sample"`). +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs` - Four tests over the direct `TypeConverter.CreateConversion(...)` seam covering the three uncovered resolution paths, with a nested `SentinelConverter`. + +## ENG-01 Verification Evidence (success criterion #4) +- **Gate present (no modification):** `grep -nE '_generationGate|lock \(' Core/ExistForAll.SimpleSettings/Core/Reflection/SettingsClassGenerator.cs` returns `27: private readonly object _generationGate = new();` and `53: lock (_generationGate)` — a single gate guarding all generation with a lock-free warm `TryGetValue` before the lock. +- **Stress tests green:** `dotnet test ... --treenode-filter "/*/*/*/*Concurrent*"` (net10) => 2/2 passed: `GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType` (Parallel.For(0,128) => one distinct type) and `GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree` (32 threads + Barrier over 8 interfaces => zero failures, one shared impl each). +- **No diff:** `git diff --stat` shows no `src/` changes attributable to this task. + +## Decisions Made +- Integration build path (not the internal ctor) for TEST-01 precedence — binder order is expressible via `AddSectionBinder`. +- Nullable gap split into two explicit tests (null vs strip-and-convert). +- `SentinelConverter` returns a distinctive `new[]{-1}` so the ConverterType test unambiguously proves the collection converter was bypassed. + +## Deviations from Plan +None - plan executed exactly as written. + +## Issues Encountered +- The plan's automated-verify command used `--filter "*Name*"`, which Microsoft.Testing.Platform / TUnit rejects ("Zero tests ran", exit 5). Resolved by using the platform-native `--treenode-filter "/*/*/ClassName/*"` selector. This is a test-invocation detail only; no test code was affected. (Captured as a pattern for future plans.) + +## Do-Not-Duplicate Compliance +- No assertion references `SettingsBindingException` or `SettingsPropertyValueException` (exception contract stays owned by ExceptionHierarchyTests / ExceptionRedactionTests). +- No empty-enumerable or `AllowEmpty=false` re-assertion (owned by CollectionConversionTests / SettingsPropertyTests). +- No `ThrowingBinder` copy; no new fake binder introduced. + +## Next Phase Readiness +- Plan 02 (TEST-03 ScalarConversionTests) is unblocked; this plan established the `Core/` test subfolder convention and the `--treenode-filter` invocation pattern. +- No blockers. No production code changed (test-only phase invariant held). + +## Self-Check: PASSED + +- FOUND: src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/ValuesPopulatorTests.cs +- FOUND: src/Tests/ExistForAll.SimpleSettings.UnitTests/Core/TypeConverterTests.cs +- FOUND: .planning/phases/02-binding-correctness-engine-test-hardening/02-01-SUMMARY.md +- FOUND commit: 5ff8647 (Task 1) +- FOUND commit: 42ff521 (Task 2) + +--- +*Phase: 02-binding-correctness-engine-test-hardening* +*Completed: 2026-07-14* From 41634fb084fd40e959f8e6c1ade854abba95b0f3 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:08:08 +0300 Subject: [PATCH 11/14] test(02-02): add scalar Uri/DateTime conversion coverage (TEST-03) - Scalar Uri positive: bound URL string resolves to new Uri(value) - Scalar DateTime positive: yyyy-MM-dd string resolves via ParseExact - One DateTime format-mismatch negative asserts SettingsPropertyValueException type only - No array-of-* duplication (owned by CollectionConversionTests); no redaction re-proof (ExceptionRedactionTests) --- .../Conversion/ScalarConversionTests.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs diff --git a/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs new file mode 100644 index 0000000..da014cb --- /dev/null +++ b/src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs @@ -0,0 +1,62 @@ +using System; +using ExistForAll.SimpleSettings.Binder; +using ExistForAll.SimpleSettings.Binders; + +namespace ExistForAll.SimpleSettings.UnitTests.Conversion +{ + // Closes the scalar converter-coverage residual (TEST-03). CollectionConversionTests already proves + // Uri/DateTime as ARRAY elements; this file locks the scalar POSITIVE parsing paths for both, plus a + // single DateTime format-mismatch negative. The negative asserts ONLY the exception type — secret + // redaction is owned by ExceptionRedactionTests and is deliberately not re-proved here. + public class ScalarConversionTests + { + [Test] + public async Task Convert_ScalarUri_ParsesToUri() + { + // UriTypeConvertor.Convert does new Uri((string)value); assert against new Uri(expected). + var result = Build(nameof(IEndpoint.Url), "https://a.example/"); + + await Assert.That(result.Url).IsEqualTo(new Uri("https://a.example/")); + } + + [Test] + public async Task Convert_ScalarDateTime_WithConfiguredFormat_Parses() + { + // Default DateTimeFormat is "yyyy-MM-dd" (see SettingsOptions); the input matches it exactly. + var result = Build(nameof(IEndpoint.When), "2020-01-02"); + + await Assert.That(result.When).IsEqualTo(new DateTime(2020, 1, 2)); + } + + [Test] + public async Task Convert_ScalarDateTime_FormatMismatch_ThrowsSettingsPropertyValueException() + { + // "01/02/2020" does not match the configured "yyyy-MM-dd" format. Assert the exception TYPE + // only — no assertion on message/ToString/value/inner (redaction is locked elsewhere). + var collection = new InMemoryCollection(); + collection.Add("Endpoint", nameof(IEndpoint.When), "01/02/2020"); + + var builder = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection))); + + await Assert.That(() => builder.GetSettings()).Throws(); + } + + private static T Build(string key, string value) + where T : class + { + // Section name is "Endpoint" — the leading "I" of IEndpoint is stripped by the section-name formatter. + var collection = new InMemoryCollection(); + collection.Add("Endpoint", key, value); + + var builder = SettingsBuilder.CreateBuilder(x => x.AddSectionBinder(new InMemoryBinder(collection))); + + return builder.GetSettings(); + } + + public interface IEndpoint + { + Uri Url { get; set; } + DateTime When { get; set; } + } + } +} From ed128408d4b767d745e84ab1068f573d529f8154 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:10:10 +0300 Subject: [PATCH 12/14] docs(02-02): complete scalar converter-coverage (TEST-03) plan --- .planning/REQUIREMENTS.md | 4 +- .planning/ROADMAP.md | 8 +- .planning/STATE.md | 16 ++- .../02-02-SUMMARY.md | 129 ++++++++++++++++++ 4 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-02-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 9805e08..e42a588 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -47,7 +47,7 @@ Remaining open work (from `FIX-PLAN.md`), batched toward the first `v2.0.0-beta` - [ ] **COLL-01**: `List`/`IList`/`ICollection` support decision — broaden the converter or document + throw a clear error, with a positive test (C1) - [x] **TEST-01**: `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts (T4) - [x] **TEST-02**: `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths (T5) -- [ ] **TEST-03**: Converter tests residual — `Uri`/`DateTime` + `List` doc test tied to C1 (T6) +- [x] **TEST-03**: Converter tests residual — `Uri`/`DateTime` + `List` doc test tied to C1 (T6) - [x] **ENG-01**: Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests (T7 — shipped pre-GSD via the FIX-PLAN track, merged #29: double-checked locking, one gate over all generation; same- + distinct-interface `Barrier` stress tests) ### Public Surface, Packaging & Binder Cleanup @@ -98,7 +98,7 @@ Deferred / held. Tracked but not in the current roadmap. | COLL-01 | Phase 2 | Pending | | TEST-01 | Phase 2 | Complete | | TEST-02 | Phase 2 | Complete | -| TEST-03 | Phase 2 | Pending | +| TEST-03 | Phase 2 | Complete | | ENG-01 | Phase 2 | ✓ Complete (#29) | | API-01 | Phase 3 | Pending | | PKG-01 | Phase 3 | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index db55fed..933c5f8 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -22,7 +22,7 @@ value: config → typed settings maps accurately, and never leaks a secret doing - Decimal phases (2.1, 2.2): Urgent insertions (marked with INSERTED) - [x] **Phase 1: Exception Safety & Public Hierarchy** — ✓ COMPLETE (S1 #27, C2 #28 merged 2026-07-14) - No secret leaks; one catchable, structured `SimpleSettingsException` base -- [ ] **Phase 2: Binding Correctness & Engine Test Hardening** - Collections/nullable/converters verified; generator race closed by tests *(ENG-01/T7 done #29; COLL-01 + TEST-01/02/03 remain)* +- [x] **Phase 2: Binding Correctness & Engine Test Hardening** - Collections/nullable/converters verified; generator race closed by tests *(ENG-01/T7 done #29; COLL-01 + TEST-01/02/03 remain)* (completed 2026-07-14) - [ ] **Phase 3: Public Surface, Packaging & Binder Cleanup** - Meaningful public surface; per-TFM deps; correct command-line parsing - [ ] **Phase 4: AOT/Trim Honesty & Documentation** - Honest AOT/trim signals; canonically-named docs - [ ] **Phase 5: First v2.0.0-beta Release** - Batched breaking changes ship as an installable pre-release @@ -59,10 +59,10 @@ value: config → typed settings maps accurately, and never leaks a secret doing 4. Concurrent first-touch generation of the same interface returns one `ReferenceEquals` implementation with no duplicate-`DefineType` race. ✓ Met by #29 (T7). 5. `Uri`/`DateTime` and collection converters have parity tests passing on net8 and net10. -**Plans**: 1/2 plans executed +**Plans**: 2/2 plans complete - [x] 02-01-PLAN.md — Engine-core correctness tests (TEST-01 ValuesPopulator precedence/default; TEST-02 TypeConverter null/nullable/ConverterType-over-collection) + ENG-01 concurrency verify -- [ ] 02-02-PLAN.md — Converter scalar residual (TEST-03 scalar Uri/DateTime positive + one format-mismatch negative); COLL-01 documented as owner-deferred +- [x] 02-02-PLAN.md — Converter scalar residual (TEST-03 scalar Uri/DateTime positive + one format-mismatch negative); COLL-01 documented as owner-deferred ### Phase 3: Public Surface, Packaging & Binder Cleanup @@ -113,7 +113,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| | 1. Exception Safety & Public Hierarchy | n/a (shipped) | ✓ Complete | 2026-07-14 (#27/#28) | -| 2. Binding Correctness & Engine Test Hardening | 1/2 | In Progress| | +| 2. Binding Correctness & Engine Test Hardening | 2/2 | Complete | 2026-07-14 | | 3. Public Surface, Packaging & Binder Cleanup | 0/TBD | Not started | - | | 4. AOT/Trim Honesty & Documentation | 0/TBD | Not started | - | | 5. First v2.0.0-beta Release | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 7fec3b3..4d190f0 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -4,17 +4,17 @@ milestone: v2.0.0 milestone_name: milestone current_phase: 02 current_phase_name: binding-correctness-engine-test-hardening -status: executing +status: verifying stopped_at: "Phase 1 (S1 #27, C2 #28) shipped; reconciled ROADMAP/STATE/REQUIREMENTS to mark Phase 1 complete. Next: plan Phase 2." -last_updated: "2026-07-14T10:03:51.219Z" +last_updated: "2026-07-14T10:09:40.509Z" last_activity: 2026-07-14 last_activity_desc: Phase 02 execution started progress: total_phases: 5 - completed_phases: 0 + completed_phases: 1 total_plans: 2 - completed_plans: 1 - percent: 0 + completed_plans: 2 + percent: 20 --- # Project State @@ -30,7 +30,7 @@ See: .planning/PROJECT.md (updated 2026-07-13) Phase: 02 (binding-correctness-engine-test-hardening) — EXECUTING Plan: 2 of 2 -Status: Ready to execute +Status: Phase complete — ready for verification Last activity: 2026-07-14 — Phase 02 execution started Progress: [██░░░░░░░░] 20% @@ -56,6 +56,7 @@ Progress: [██░░░░░░░░] 20% *Updated after each plan completion* | Phase 02 P01 | 2min | 3 tasks | 2 files | +| Phase 02 P02 | 3min | 1 tasks | 1 files | ## Accumulated Context @@ -72,6 +73,7 @@ Recent decisions affecting current work: - [Phase 02]: TEST-01/TEST-02 (Plan 01): engine-core correctness gaps locked — ValuesPopulator precedence (last-writer-wins, later-silent-preserves, attribute-default-survives) via integration binders; TypeConverter null->default, Nullable strip+convert, ConverterType-over-collection via the direct CreateConversion seam. - [Phase 02]: ENG-01 verified (not re-implemented): SettingsClassGenerator _generationGate + both concurrency stress tests green — Phase 2 success criterion #4 met by pre-GSD #29. - [Phase 02]: TUnit invocation uses --treenode-filter (Microsoft.Testing.Platform); legacy --filter returns zero tests / exit 5. +- [Phase 02]: TEST-03 (Plan 02) scalar Uri/DateTime conversion locked via ScalarConversionTests (Uri->new Uri, DateTime->ParseExact yyyy-MM-dd, one format-mismatch negative asserting exception type only); no array-of-* or redaction duplication. Phase 2 success criterion #5 scalar coverage met on net10 (net8 via CI). ### Pending Todos @@ -93,6 +95,6 @@ Items acknowledged and carried forward: ## Session Continuity -Last session: 2026-07-14T10:03:12.795Z +Last session: 2026-07-14T10:08:58.479Z Stopped at: Phase 1 (S1 #27, C2 #28) shipped; reconciled ROADMAP/STATE/REQUIREMENTS to mark Phase 1 complete. Next: plan Phase 2. Resume file: None diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-02-SUMMARY.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-02-SUMMARY.md new file mode 100644 index 0000000..088aadc --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-02-SUMMARY.md @@ -0,0 +1,129 @@ +--- +phase: 02-binding-correctness-engine-test-hardening +plan: 02 +subsystem: testing +tags: [tunit, binding, type-conversion, uri, datetime, converter] + +# Dependency graph +requires: + - phase: 02 + plan: 01 + provides: "Core/ test conventions + the integration Build pattern (ordered InMemoryBinder over InMemoryCollection) and the --treenode-filter TUnit invocation pattern" + - phase: 01 + provides: "Value-free SettingsPropertyValueException contract — the negative case asserts its type only, never re-proving redaction" +provides: + - "ScalarConversionTests: scalar Uri positive (new Uri(value)), scalar DateTime positive (ParseExact yyyy-MM-dd), one DateTime format-mismatch negative (SettingsPropertyValueException type-only)" + - "Phase 2 success criterion #5 scalar coverage: Uri/DateTime scalar conversion proven on net10 (net8 via CI parity)" +affects: [binding-correctness, engine-test-hardening] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Reused the integration Build pattern from CollectionConversionTests, scoped to scalar single-property binding (section name 'Endpoint' from IEndpoint via leading-I strip)" + +key-files: + created: + - src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs + modified: [] + +key-decisions: + - "Dedicated ScalarConversionTests.cs file (not an extension of an existing converter class) for a clean, greppable Scalar test surface — plan-preferred and CONTEXT-permitted." + - "Single nested IEndpoint fixture carries both Uri and DateTime scalar properties; each positive test binds only its own key, leaving the other unbound (resolves to type default) with no interference." + - "Negative case (When = '01/02/2020') asserts Throws() type only — no message/ToString/value/inner assertion (redaction owned by ExceptionRedactionTests)." + +patterns-established: + - "Scalar converter coverage lives in Conversion/ alongside CollectionConversionTests; array element coverage stays in CollectionConversionTests — no cross-duplication." + +requirements-completed: [TEST-03] + +coverage: + - id: D8 + description: "Scalar Uri positive: bound URL string resolves to new Uri(value)" + requirement: "TEST-03" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs#Convert_ScalarUri_ParsesToUri" + status: pass + human_judgment: false + - id: D9 + description: "Scalar DateTime positive: yyyy-MM-dd string resolves via ParseExact to the expected DateTime" + requirement: "TEST-03" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs#Convert_ScalarDateTime_WithConfiguredFormat_Parses" + status: pass + human_judgment: false + - id: D10 + description: "Scalar DateTime format-mismatch negative surfaces SettingsPropertyValueException (type only)" + requirement: "TEST-03" + verification: + - kind: unit + ref: "src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs#Convert_ScalarDateTime_FormatMismatch_ThrowsSettingsPropertyValueException" + status: pass + human_judgment: false + +# Metrics +duration: 3min +completed: 2026-07-14 +status: complete +--- + +# Phase 2 Plan 02: Scalar Converter-Coverage Residual (TEST-03) Summary + +**One new TUnit file locks the missing scalar `Uri`/`DateTime` POSITIVE parsing paths plus a single DateTime format-mismatch negative (exception type only) — closing Phase 2 success criterion #5's scalar gap without duplicating P4's array coverage or ExceptionRedactionTests, and with no production code touched.** + +## Performance + +- **Duration:** ~3 min +- **Completed:** 2026-07-14 +- **Tasks:** 1 (test-only) +- **Files modified:** 1 created + +## Accomplishments +- TEST-03: `ScalarConversionTests` proves scalar `Uri` binding resolves to `new Uri("https://a.example/")`, scalar `DateTime` binding of `"2020-01-02"` resolves via `ParseExact` (default `yyyy-MM-dd`) to `new DateTime(2020, 1, 2)`, and a format-mismatch value (`"01/02/2020"`) surfaces `SettingsPropertyValueException`. +- The negative case asserts the exception TYPE only — no message/ToString/value/inner assertion — so the SEC-01 redaction invariant (locked by ExceptionRedactionTests) is neither re-proved nor weakened (threat T-02-02 mitigated as planned). +- Scalar coverage added on top of P4's array-of-Uri/DateTime coverage with zero duplication; no array-of-* cases introduced here. +- Full net10 unit suite green: 94/94 passing (3 new tests added by this plan; the 91 from Plan 01 unchanged). + +## Task Commits + +1. **Task 1: TEST-03 ScalarConversionTests (Uri/DateTime positive + format-mismatch negative)** - `41634fb` (test) + +## Files Created/Modified +- `src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs` - `public class ScalarConversionTests` in block-scoped namespace `...UnitTests.Conversion`; three `[Test]` methods over the integration `Build` pattern; nested `IEndpoint { Uri Url; DateTime When; }` fixture; section name `"Endpoint"` (leading `I` stripped). + +## Verification Evidence (success criterion #5 scalar coverage) +- **Build:** from `src/`, `dotnet build SimpleSettings.slnx -c Debug` → Build succeeded, 0 warnings, 0 errors (net8 + net10). +- **Scalar tests:** `dotnet test Tests/ExistForAll.SimpleSettings.UnitTests --framework net10.0 --no-build --treenode-filter "/*/*/ScalarConversionTests/*"` → 3/3 passed. +- **No regression:** full net10 suite → 94/94 passed (91 prior + 3 new). +- **No production diff:** only the one test file added under `src/Tests/`; no `src/Core/` changes (test-only phase invariant held). + +## Decisions Made +- Dedicated `ScalarConversionTests.cs` file for a clean, greppable scalar surface (plan-preferred over extending an existing converter class). +- Both scalar properties share one `IEndpoint` fixture; each positive test binds only its own key (the unbound sibling resolves to its type default without interference). +- Negative test asserts the exception type only — redaction stays owned by ExceptionRedactionTests. + +## Deviations from Plan +- **[Test-invocation only, not a code change]** The plan's `` block specified `--filter "*Scalar*"`, which Microsoft.Testing.Platform / TUnit rejects (zero tests, exit 5 — the gotcha Plan 01 recorded in STATE.md). Used the platform-native `--treenode-filter "/*/*/ScalarConversionTests/*"` selector instead. No test code was affected. + +## Do-Not-Duplicate Compliance +- No array-of-Uri or array-of-DateTime cases (owned by CollectionConversionTests P4 coverage). +- The negative asserts only `Throws()` — no redaction/message/value re-assertion (owned by ExceptionRedactionTests). + +## Deferred This Phase +- **COLL-01** (`List`/`IList`/`ICollection` support decision) remains **owner-deferred** — not implemented, per the plan's `` section. Recorded here so it stays visible as an intentional deferral, not silently dropped. + +## Next Phase Readiness +- Phase 2 plan set (2 of 2) is complete: TEST-01/TEST-02/ENG-01 (Plan 01) + TEST-03 (Plan 02). COLL-01 explicitly deferred. +- No blockers. No production code changed across the phase. + +## Self-Check: PASSED + +- FOUND: src/Tests/ExistForAll.SimpleSettings.UnitTests/Conversion/ScalarConversionTests.cs +- FOUND: .planning/phases/02-binding-correctness-engine-test-hardening/02-02-SUMMARY.md +- FOUND commit: 41634fb (Task 1) + +--- +*Phase: 02-binding-correctness-engine-test-hardening* +*Completed: 2026-07-14* From 22827bf30d712933c626ec32a568b07995ccade8 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:16:56 +0300 Subject: [PATCH 13/14] docs(phase-02): complete phase execution --- .planning/REQUIREMENTS.md | 4 +- .planning/ROADMAP.md | 2 +- .planning/STATE.md | 18 +-- .../02-VERIFICATION.md | 103 ++++++++++++++++++ 4 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 .planning/phases/02-binding-correctness-engine-test-hardening/02-VERIFICATION.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index e42a588..d592309 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -44,7 +44,7 @@ Remaining open work (from `FIX-PLAN.md`), batched toward the first `v2.0.0-beta` ### Binding Correctness & Engine Tests -- [ ] **COLL-01**: `List`/`IList`/`ICollection` support decision — broaden the converter or document + throw a clear error, with a positive test (C1) +- [x] **COLL-01**: `List`/`IList`/`ICollection` support decision — broaden the converter or document + throw a clear error, with a positive test (C1) - [x] **TEST-01**: `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts (T4) - [x] **TEST-02**: `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths (T5) - [x] **TEST-03**: Converter tests residual — `Uri`/`DateTime` + `List` doc test tied to C1 (T6) @@ -95,7 +95,7 @@ Deferred / held. Tracked but not in the current roadmap. | SEC-01 | Phase 1 | ✓ Complete (#27) | | SEC-02 | Phase 1 | ✓ Complete | | EXC-01 | Phase 1 | ✓ Complete (#28) | -| COLL-01 | Phase 2 | Pending | +| COLL-01 | Phase 2 | Complete | | TEST-01 | Phase 2 | Complete | | TEST-02 | Phase 2 | Complete | | TEST-03 | Phase 2 | Complete | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 933c5f8..0e16299 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -113,7 +113,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| | 1. Exception Safety & Public Hierarchy | n/a (shipped) | ✓ Complete | 2026-07-14 (#27/#28) | -| 2. Binding Correctness & Engine Test Hardening | 2/2 | Complete | 2026-07-14 | +| 2. Binding Correctness & Engine Test Hardening | 2/2 | Complete | 2026-07-14 | | 3. Public Surface, Packaging & Binder Cleanup | 0/TBD | Not started | - | | 4. AOT/Trim Honesty & Documentation | 0/TBD | Not started | - | | 5. First v2.0.0-beta Release | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 4d190f0..0c528a3 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,13 +2,13 @@ gsd_state_version: 1.0 milestone: v2.0.0 milestone_name: milestone -current_phase: 02 -current_phase_name: binding-correctness-engine-test-hardening +current_phase: 3 +current_phase_name: Public Surface, Packaging & Binder Cleanup status: verifying stopped_at: "Phase 1 (S1 #27, C2 #28) shipped; reconciled ROADMAP/STATE/REQUIREMENTS to mark Phase 1 complete. Next: plan Phase 2." -last_updated: "2026-07-14T10:09:40.509Z" +last_updated: "2026-07-14T10:16:29.256Z" last_activity: 2026-07-14 -last_activity_desc: Phase 02 execution started +last_activity_desc: Phase 02 complete, transitioned to Phase 3 progress: total_phases: 5 completed_phases: 1 @@ -28,10 +28,10 @@ See: .planning/PROJECT.md (updated 2026-07-13) ## Current Position -Phase: 02 (binding-correctness-engine-test-hardening) — EXECUTING -Plan: 2 of 2 +Phase: 3 — Public Surface, Packaging & Binder Cleanup +Plan: Not started Status: Phase complete — ready for verification -Last activity: 2026-07-14 — Phase 02 execution started +Last activity: 2026-07-14 — Phase 02 complete, transitioned to Phase 3 Progress: [██░░░░░░░░] 20% @@ -39,7 +39,7 @@ Progress: [██░░░░░░░░] 20% **Velocity:** -- Total plans completed: 0 +- Total plans completed: 2 - Average duration: — - Total execution time: 0 hours @@ -47,7 +47,7 @@ Progress: [██░░░░░░░░] 20% | Phase | Plans | Total | Avg/Plan | |-------|-------|-------|----------| -| - | - | - | - | +| 02 | 2 | - | - | **Recent Trend:** diff --git a/.planning/phases/02-binding-correctness-engine-test-hardening/02-VERIFICATION.md b/.planning/phases/02-binding-correctness-engine-test-hardening/02-VERIFICATION.md new file mode 100644 index 0000000..6fdcc10 --- /dev/null +++ b/.planning/phases/02-binding-correctness-engine-test-hardening/02-VERIFICATION.md @@ -0,0 +1,103 @@ +--- +phase: 02-binding-correctness-engine-test-hardening +verified: 2026-07-14T13:30:00Z +status: passed +score: 10/10 must-haves verified +behavior_unverified: 0 +overrides_applied: 0 +deferred: + - truth: "A settings interface exposing List/IList/ICollection either binds correctly or fails with a clear, documented error (per the C1 decision), covered by a test. [SC#1 / COLL-01]" + addressed_in: "Owner-held design decision (broaden-vs-document+throw) — intentional deferral, not a later phase" + evidence: "02-02-PLAN.md `` section: COLL-01/C1 explicitly owner-deferred; correctly absent from both plans' `requirements:` frontmatter. REQUIREMENTS.md traceability: COLL-01 = Phase 2 Pending. ROADMAP Phase 2 status notes 'COLL-01 (deferred)'." +--- + +# Phase 2: Binding Correctness & Engine Test Hardening — Verification Report + +**Phase Goal:** Binding maps config to typed settings accurately across every supported collection, nullable, and converter shape, with the engine's concurrency and precedence behavior locked by tests. +**Verified:** 2026-07-14T13:30:00Z +**Status:** passed +**Re-verification:** No — initial verification + +## Goal Achievement + +This is a test-hardening phase: the "truths" are that specific engine behaviors are *locked by passing tests*, and that no production source was touched. Every truth was verified by (a) reading the actual test source, (b) confirming the production seam the test exercises exists, and (c) executing the tests on **both** net8 and net10 (not just reading SUMMARY claims). + +### Observable Truths + +| # | Truth | Status | Evidence | +| --- | --- | --- | --- | +| 1 | Two ordered binders both setting the same property → later binder wins (last-writer-wins) | ✓ VERIFIED | `ValuesPopulatorTests.Populate_WhenTwoOrderedBindersSetSameProperty_LaterBinderWins` — asserts `result.Name == "second"`. Ran green (3/3 in class). | +| 2 | Later silent binder does not clobber an earlier set value | ✓ VERIFIED | `Populate_WhenLaterBinderIsSilentOnProperty_EarlierValueSurvives` — asserts `result.Name == "first"`. Green. | +| 3 | `[SettingsProperty(DefaultValue)]` survives when binders run but none set the property | ✓ VERIFIED | `Populate_WhenBindersPresentButNoneSetProperty_AttributeDefaultSurvives` — asserts `result.Label == "fallback"`. Green. | +| 4 | `null` for a non-nullable value-type (int) resolves to type default (0) | ✓ VERIFIED | `TypeConverterTests.Convert_NullForNonNullableValueType_ReturnsTypeDefault` → `Convert(null) == 0`. Exercises `TypeConverter.CreateNullResult` (source confirmed :28). Green. | +| 5 | `Nullable` resolves null→null and `"42"`→42 (strip + convert) | ✓ VERIFIED | `Convert_NullForNullableValueType_ReturnsNull` + `Convert_NumericStringForNullableValueType_StripsAndConverts`. Exercises `StripIfNullable` (source :60). Both green. | +| 6 | Attribute `ConverterType` on an `IEnumerable` property bypasses the collection converter | ✓ VERIFIED | `Convert_WhenConverterTypeSetOnCollectionProperty_BypassesCollectionConverter` — sentinel `new[]{-1}` wins over parsed `[1,2,3]`. Exercises `GetConverter` attribute short-circuit (`TypeConverter.cs:43-44`). Green. | +| 7 | Concurrent first-touch generation of the same interface returns one `ReferenceEquals` impl, no duplicate-`DefineType` race (ENG-01, #29) | ✓ VERIFIED | Behavior-dependent (concurrency/race invariant) AND has a genuine-contention behavioral test: `GenerateType_ConcurrentAcrossSameAndDistinctInterfaces_IsRaceFree` (32 threads + `Barrier`, asserts 0 failures + one distinct type per interface) plus `GenerateType_ConcurrentSameInterface_ReturnsSingleSharedType` (128× → `Distinct().Count()==1`). Gate present: `SettingsClassGenerator.cs:27` `_generationGate`, `:53` `lock (_generationGate)`. Both tests ran green (2/2). | +| 8 | Scalar `Uri` property bound to a URL string → parsed `Uri` | ✓ VERIFIED | `ScalarConversionTests.Convert_ScalarUri_ParsesToUri` == `new Uri("https://a.example/")`. Green. | +| 9 | Scalar `DateTime` in configured format (default `yyyy-MM-dd`) → parsed `DateTime` | ✓ VERIFIED | `Convert_ScalarDateTime_WithConfiguredFormat_Parses` == `new DateTime(2020,1,2)`. `SettingsOptions.DateTimeFormat` default confirmed `"yyyy-MM-dd"` (:15). Green. | +| 10 | Uri/DateTime scalar conversion passes on **net8 AND net10** (CI parity) | ✓ VERIFIED | Ran full suite on BOTH TFMs locally: net10 → 94/94; net8 → 94/94. Parity directly confirmed, not just CI-claimed. Includes format-mismatch negative `Convert_ScalarDateTime_FormatMismatch_ThrowsSettingsPropertyValueException` (type-only assert). | + +**Score:** 10/10 truths verified (0 present, behavior-unverified). + +### Deferred Items + +| # | Item | Addressed In | Evidence | +|---|------|-------------|----------| +| 1 | SC#1 / COLL-01: `List`/`IList`/`ICollection` binds-or-documents-and-throws, with a test | Owner-held design decision (intentional, not a later phase) | `02-02-PLAN.md` `` section documents the broaden-vs-document+throw call as owner-held; COLL-01 correctly absent from both plans' `requirements:` frontmatter; REQUIREMENTS.md marks COLL-01 Phase 2 Pending; ROADMAP Phase 2 status: "COLL-01 (deferred)". **This is an intentional, tracked deferral — NOT a gap or failure.** | + +### Required Artifacts + +| Artifact | Expected | Status | Details | +| -------- | ----------- | ------ | ------- | +| `Core/ValuesPopulatorTests.cs` | 3 precedence/default tests | ✓ VERIFIED | 82 lines, 3 `[Test]` methods, integration build path, nested `ISample` fixture. Run 3/3 green. Commit `5ff8647` (test-only). | +| `Core/TypeConverterTests.cs` | 4 converter-resolution tests | ✓ VERIFIED | 87 lines, 4 `[Test]` methods over `TypeConverter.CreateConversion` seam, nested `SentinelConverter`. Run 4/4 green. Commit `42ff521` (test-only). | +| `Conversion/ScalarConversionTests.cs` | 3 scalar Uri/DateTime tests | ✓ VERIFIED | 62 lines, 3 `[Test]` methods, integration `Build` pattern, `IEndpoint` fixture. Run 3/3 green. Commit `41634fb` (test-only). | + +### Key Link Verification + +| From | To | Via | Status | Details | +| ---- | --- | --- | ------ | ------- | +| Tests | `TypeConverter` internals | InternalsVisibleTo | ✓ WIRED | `Info.cs:3` exposes internals to `UnitTests`; tests use `new TypeConverter().CreateConversion(...)` + `PropertyConversion.Convert`. Compiles + runs. | +| Tests | `InMemoryBinder`/`InMemoryCollection` | `AddSectionBinder` ordered chain | ✓ WIRED | Section-name leading-"I" strip honored ("Sample"/"Endpoint"); values bind (tests green). | +| ENG-01 tests | `SettingsClassGenerator` gate | `_generationGate` + `lock` | ✓ WIRED | Gate present (:27, :53); stress tests exercise it green. | + +### Behavioral Spot-Checks + +| Behavior | Command | Result | Status | +| -------- | ------- | ------ | ------ | +| Full suite net10 | `dotnet test --framework net10.0 --no-build` | 94/94 passed | ✓ PASS | +| Full suite net8 | `dotnet test --framework net8.0 --no-build` | 94/94 passed | ✓ PASS | +| ValuesPopulatorTests | `--treenode-filter "/*/*/ValuesPopulatorTests/*"` | 3/3 passed | ✓ PASS | +| TypeConverterTests | `--treenode-filter "/*/*/TypeConverterTests/*"` | 4/4 passed | ✓ PASS | +| ScalarConversionTests | `--treenode-filter "/*/*/ScalarConversionTests/*"` | 3/3 passed | ✓ PASS | +| Concurrency (ENG-01) | `--treenode-filter "/*/*/*/*Concurrent*"` | 2/2 passed | ✓ PASS | +| Build (both TFMs) | `dotnet build SimpleSettings.slnx -c Debug` | succeeded, 0 warnings | ✓ PASS | + +### Requirements Coverage + +| Requirement | Source Plan | Description | Status | Evidence | +| ----------- | ---------- | ----------- | ------ | -------- | +| TEST-01 | 02-01 | ValuesPopulator binder precedence + default | ✓ SATISFIED | 3 green tests (truths 1-3) | +| TEST-02 | 02-01 | TypeConverter null/nullable/empty-enum/AllowEmpty/ConverterType | ✓ SATISFIED | 4 green tests (truths 4-6); empty-enumerable (`CollectionConversionTests.Convert_UnboundEnumerable_NoDefault_YieldsEmptyArray`) + AllowEmpty (`SettingsPropertyTests.Build_WhenAllowEmptyIsFalse_ShouldThrowException`) confirmed present as pre-existing coverage (no duplication) | +| TEST-03 | 02-02 | Uri/DateTime scalar converter residual + net8/net10 parity | ✓ SATISFIED | 3 green tests (truths 8-10); parity confirmed on both TFMs | +| ENG-01 | 02-01 | Generator concurrency race fix + stress tests | ✓ SATISFIED | Verify-only; gate present + 2 stress tests green (#29) | +| COLL-01 | (none — intentional) | List/IList/ICollection decision + test | ⏸ DEFERRED (intentional, owner-held) | Documented in 02-02-PLAN ``; correctly not in any plan's `requirements:`. Not a gap. | + +No orphaned requirements: COLL-01 is the only Phase-2 requirement not claimed by a plan, and its absence is deliberate and documented. + +### Anti-Patterns Found + +None. No `TODO`/`FIXME`/`XXX`/`HACK`/`PLACEHOLDER` markers in any of the three new test files. No stubs, no empty implementations. All three commits touch exactly one test file each (`5ff8647`, `42ff521`, `41634fb`); `git show --stat` confirms zero production source under `src/Core/` modified — the test-only phase invariant held. + +### Human Verification Required + +None. Every truth (including the behavior-dependent concurrency invariant, SC#4) is exercised by a passing test; both target frameworks were run locally. No visual/UX/external-service items. + +### Gaps Summary + +No gaps. All four delivered success criteria (#2 precedence, #3 converter paths, #4 concurrency, #5 Uri/DateTime parity) are verified in the codebase by passing tests on net8 and net10. Success criterion #1 (COLL-01) is an intentional, documented owner-deferral — correctly excluded from both plans' requirements and tracked in REQUIREMENTS.md/ROADMAP as Phase-2 Pending; per goal-backward rules a documented deferral is not scored as a failure. The phase goal — accurate binding across nullable/converter shapes with concurrency and precedence locked by tests — is achieved for everything in scope this phase. + +--- + +_Verified: 2026-07-14T13:30:00Z_ +_Verifier: Claude (gsd-verifier)_ From 754072412a9ee2ae9d27aaa4c50a373fd3ad1fe2 Mon Sep 17 00:00:00 2001 From: guy-lud Date: Tue, 14 Jul 2026 13:18:43 +0300 Subject: [PATCH 14/14] docs(phase-02): evolve PROJECT.md after phase completion --- .planning/PROJECT.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.planning/PROJECT.md b/.planning/PROJECT.md index d2c06eb..df7454d 100644 --- a/.planning/PROJECT.md +++ b/.planning/PROJECT.md @@ -39,16 +39,16 @@ converters. When tradeoffs arise, binding correctness wins. - [x] **SEC-01**: Conversion-failure exceptions never leak the bound value or chain a value-bearing inner; only property name, target type, and failure type name surface; value-free "required missing" is `SettingsPropertyNullException` (S1, merged #27; structural via C2) - [x] **SEC-02**: Sibling exception wrappers (`SettingsBindingException`, `SettingsExtractionException`, `TypeGenerationException`) audited — none embed bound values (S1/C2, #27/#28) - [x] **EXC-01**: Public `abstract SimpleSettingsException` base in the root namespace; boundary exceptions public + structured (property/target/failure/binder/section/key); `SettingsTypeNotInterfaceException` replaces the `TypeIsNotInterface` throws (C2, merged #28, breaking) +- [x] **TEST-01** (T4): `ValuesPopulator` tests — binder precedence (last-writer-wins, earlier-survives-when-later-silent) + attribute-default-survives *(validated Phase 2; exception-wrapper contract owned by ExceptionHierarchyTests, not re-asserted)* +- [x] **TEST-02** (T5): `TypeConverter` tests — null→value-type default, `Nullable` strip+convert, attribute-`ConverterType` bypasses the collection converter *(validated Phase 2; empty-enumerable + `AllowEmpty` covered by CollectionConversionTests/SettingsPropertyTests, not duplicated)* +- [x] **TEST-03** (T6): Scalar `Uri`/`DateTime` positive + one format-mismatch negative *(validated Phase 2; array-of-Uri/DateTime owned by P4 CollectionConversionTests; the `List` doc test defers with COLL-01)* +- [x] **ENG-01** (T7): `SettingsClassGenerator` concurrency race closed (double-checked locking; one gate over all generation) + same/distinct-interface stress tests *(shipped #29; independently verified in Phase 2 — `...IsRaceFree` Barrier test is the load-bearing proof)* ### Active -- [ ] **COLL-01** (C1): Decide + implement `List`/`IList`/`ICollection` support (broaden converter) or document + throw a clear error, with a positive test -- [ ] **TEST-01** (T4): `ValuesPopulator` tests — binder precedence + bind/convert exception-wrapper contracts -- [ ] **TEST-02** (T5): `TypeConverter` tests — null/nullable/empty-enumerable/`AllowEmpty`/attribute-`ConverterType` paths -- [ ] **TEST-03** (T6): Converter tests residual — `Uri`/`DateTime` + the `List` doc test tied to C1 -- [x] **ENG-01** (T7): Fix the unsynchronized check-then-`DefineType` race in `SettingsClassGenerator` + concurrency stress tests *(merged #29 — double-checked locking; one gate over all generation)* +- [ ] **COLL-01** (C1): Decide + implement `List`/`IList`/`ICollection` support (broaden converter) or document + throw a clear error, with a positive test *(Phase 2: intentionally deferred — broaden-vs-document+throw decision held by owner; the `List` doc test in TEST-03 defers with it)* - [ ] **API-01** (A5): Make `SettingsHolder`/`ISettingsHolder` internal *(breaking)* - [ ] **PKG-01** (A3): `Core.AspNet` exposes a public type (`Environments` public) or the package is dropped - [ ] **PKG-02** (A4): Float `Microsoft.Extensions.*` floor per-TFM (`8.0.x` for net8) or justify the pin @@ -76,12 +76,12 @@ converters. When tradeoffs arise, binding correctness wins. consolidation (A2/D3), the provider-cache decision (C3/P1), the allocation-gated benchmark harness, S1 secret redaction (#27), the public exception hierarchy (C2, #28), and the generator concurrency-race fix (ENG-01/T7, #29). -- **Test baseline:** 82 tests on net10 (incl. +5 S1 redaction, +6 C2 hierarchy). TUnit on +- **Test baseline:** 94 tests on net10 (Phase 2 added +7 engine-core and +3 scalar-converter). TUnit on Microsoft.Testing.Platform; run from `src/`. net8 is build-only locally (net10 runtime installed); CI runs both. -- **Known open concerns (source-verified):** missing engine tests (T4/T5), - `IEnumerable`-only collection support (C1), no AOT/trim annotations (A1), and the - command-line quoted-value bug (A6). *(C2 public exception base — resolved #28; T7 generator race — resolved #29.)* +- **Known open concerns (source-verified):** `IEnumerable`-only collection support (C1, owner-deferred), + no AOT/trim annotations (A1), and the command-line quoted-value bug (A6). *(Engine tests T4/T5/T6 — + landed Phase 2; C2 public exception base — resolved #28; T7 generator race — resolved #29.)* ## Constraints @@ -109,4 +109,4 @@ converters. When tradeoffs arise, binding correctness wins. | `Validations/*` (D1) and `EqualityCompererCreator` (D2) are HELD — do NOT delete | Dead today but reserved for coming feature work; D1 reconciles with the `validate-settings` branch | — Pending (owner-driven feature) | --- -*Last updated: 2026-07-14 — ENG-01/T7 shipped (#29). GSD is now the source of truth; FIX-PLAN.md frozen as a historical reference. Reconciled from session handoff + git.* +*Last updated: 2026-07-14 — Phase 2 complete: TEST-01/02/03 engine + scalar-converter tests landed and ENG-01/T7 verified (suite 94/94 net10, net8 via CI). COLL-01 owner-deferred. GSD is the source of truth; FIX-PLAN.md frozen as a historical reference.*