From 33e9abdd2048a1f8de020261a88a4bcb5c61fa23 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 26 Jul 2026 11:31:11 +0200 Subject: [PATCH] fix(633): assert the cap set, not the presence of one true cap claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review finding, and a correction to what the previous commit claimed. That commit said the regex matched the cap "as a whole token" and called the result "exact". The whole-token part was true and did fix the 100-within-1000 substring hole. "Exact" was not: `capped at 100(?!\d)` asks only whether a correct claim is PRESENT, which is not the same as asking whether an incorrect one is ABSENT. A description reading "not capped at 1000 for this endpoint; capped at 100 …" satisfied it while publishing a wrong number to every consumer. Enumerate every `capped at ` in the description instead and require the set to be exactly one number, the right one. Mutation-verified on the constructed case: /logs naming both 1000 and 100 now reddens the test, where it passed under the previous form. This is the third round on this one assertion, and each round found the previous fix's blind spot rather than a fresh mistake — the failure mode was consistently "the new check tests presence of the right thing, not absence of the wrong thing." Note on verification: the reviewer could not run the suite (its sandbox could not create a temp dir, and a direct VSTest invocation could not bind its IPC socket), so it explicitly flagged the 1905/0 result as unverified rather than trusting it. That figure comes from my own run in this worktree, re-run after this change, and CI is the independent confirmation. Refs #633 Decisions-Edit: yes --- .../Controllers/OpenApiPagingContractTests.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs index 3f1ccedf9..bb7e9ebf2 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs @@ -129,11 +129,19 @@ public class OpenApiPagingContractTests foreach ((string key, int cap) in expectedCaps) { - // Match the number as a WHOLE token, not a substring: "capped at 1000" contains - // "capped at 100", so a plain ShouldContain would pass a cap-100 endpoint whose - // description claims 1000 — the exact wrong-cap defect this test exists to catch. - Regex.IsMatch(Description(key, "pageSize"), $@"capped at {cap}(?!\d)", RegexOptions.IgnoreCase) - .ShouldBeTrue($"{key} pageSize should document a cap of exactly {cap}"); + // Enumerate EVERY cap claim in the description and require the set to be exactly one + // number, the right one. Two weaker forms were rejected on the way here: + // - ShouldContain("capped at 100") is satisfied by the string "capped at 1000", so a + // cap-100 endpoint claiming 1000 passed — the very defect this test exists to catch. + // - Matching one occurrence as a whole token ("capped at 100(?!\d)") fixes that, but + // still passes a description that names a wrong cap somewhere ELSE in the sentence + // and the right one later. Presence of a true claim is not absence of a false one. + List claimedCaps = Regex + .Matches(Description(key, "pageSize"), @"capped at (\d+)", RegexOptions.IgnoreCase) + .Select(match => int.Parse(match.Groups[1].Value)) + .ToList(); + + claimedCaps.ShouldBe([cap], $"{key} pageSize should make exactly one cap claim, of {cap}"); } }