From 8525666913d6d03b20eb5e8cf53a2eddb8ddc7bb Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 22 Sep 2026 00:22:30 +0200 Subject: [PATCH] =?UTF-8?q?test:=20round=204=20=E2=80=94=20an=20assertion?= =?UTF-8?q?=20that=20could=20not=20fail,=20and=20a=20comment=20that=20did?= =?UTF-8?q?=20not=20add=20up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings, both in the review's own work; nothing in the product code. That is the shift that ends the loop: 22 → 13 → 6 → 2, and the last two are a test and a comment rather than a defect. The "an override that is fine is still honoured" leg of the bounds-fallback test stored `spec.max` — which, for this key, IS the env default, because the spec's max is derived from it. So the assertion was satisfied by the fallback it existed to rule out: mutating `_coerce` to reject EVERY int override left all three legs green. It stores `spec.max - 1` now and asserts the value differs from the default, so the leg can actually fail. And the shrink test's comment claimed `3 - 5 = -2` for an expression that computes `chips.length - visible` = `2 - 3 = -1` — naming neither operand nor the result, in a comment that was rewritten one round earlier precisely for miscounting. --- backend/tests/test_body_limit.py | 9 +++++++-- frontend/src/components/TagFilterChips.dom.test.tsx | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/backend/tests/test_body_limit.py b/backend/tests/test_body_limit.py index c11d4187..05bebba7 100644 --- a/backend/tests/test_body_limit.py +++ b/backend/tests/test_body_limit.py @@ -284,9 +284,14 @@ def test_a_stored_override_outside_the_bounds_falls_back_on_READ(db): db.commit() assert sysconfig.get_int(db, "download_upload_max_bytes") == default - row.value = str(spec.max) # still in range — an override that is fine is still honoured + # An override that IS in range must still be honoured — otherwise a bounds predicate written + # backwards would make every override inert and the two assertions above could not tell. + # Deliberately not `spec.max`: that equals the env default for this key, so the assertion would + # be satisfied by the fallback it is meant to rule out. + row.value = str(spec.max - 1) db.commit() - assert sysconfig.get_int(db, "download_upload_max_bytes") == spec.max + assert spec.max - 1 != default + assert sysconfig.get_int(db, "download_upload_max_bytes") == spec.max - 1 def test_the_cap_sits_inside_the_response_decorators_and_outside_the_setup_gate(): diff --git a/frontend/src/components/TagFilterChips.dom.test.tsx b/frontend/src/components/TagFilterChips.dom.test.tsx index 8a12615f..cd2513fe 100644 --- a/frontend/src/components/TagFilterChips.dom.test.tsx +++ b/frontend/src/components/TagFilterChips.dom.test.tsx @@ -75,7 +75,7 @@ describe("TagFilterChips — the overflow menu", () => { // // So `fit.visible` is left ALONE across the shrink — that staleness is the precondition, not a // convenience of the mock. Five chips of which three fit, then three of them are deleted in the - // tag manager: 3 - 5 = -2. + // tag manager — so the shrink render computes `chips.length - visible` = 2 - 3 = -1. const five = ["alpha", "beta", "gamma", "delta", "epsilon"].map(chip); fit.visible = 3; const { rerender } = render();