From da25d567cb571020e825c461f93ffad7b69b78d6 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Mon, 3 Aug 2026 19:32:44 +0000 Subject: [PATCH] Remove pointless catalog-literal tests; document the anti-pattern Two tests added while triaging #244/#226 just re-asserted a translation string against the catalog entry that had been written moments earlier (dryer_cycle_table_03's '51'/'53'/'4e', dishwasher_cycle's '83'/'86'). Neither exercises any code path -- they pass by construction and only break when someone later edits the label text for wording, not when the actual code/value mapping regresses. tests/test_translations.py already holds the invariants that matter for catalog data. Documents the anti-pattern in the adding-device-support skill so future translation-only fixes don't reach for this pattern again. --- .claude/skills/adding-device-support/SKILL.md | 18 ++++++++++++++++++ tests/test_dishwasher_capabilities.py | 11 ----------- tests/test_dryer_capabilities.py | 12 ------------ 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/.claude/skills/adding-device-support/SKILL.md b/.claude/skills/adding-device-support/SKILL.md index 57078d7..1feae3f 100644 --- a/.claude/skills/adding-device-support/SKILL.md +++ b/.claude/skills/adding-device-support/SKILL.md @@ -399,6 +399,24 @@ no `[%key:...%]` resolution (that's Core build tooling). Every other language must mirror `en.json` key for key — also enforced by `tests/test_translations.py`. +**Don't write a test that just re-asserts a translation string.** Adding +labels is a data change, not a logic change, and `tests/test_translations.py` +already holds the invariants that matter for data (every descriptor has a +catalog entry, every language mirrors English key-for-key, no unresolved +`[%key:...%]`). A test that loads the catalog and asserts +`catalog["select"]["foo"]["state"]["16"] == "Cotton"` right after you just +wrote that exact line into `en.json` doesn't exercise any code path — it +re-states the JSON file in Python, passes by construction, and only ever +fails when someone *correctly* edits the label later (a wording fix, a +translator's improvement). It's not a regression test, because there's no +`select.py`/`adapter.py` logic between "the JSON says X" and "the test reads +X" for it to catch drift in. If a code/label mapping is worth locking in, +test it through the code that actually consumes it instead — a write +contract (`desc.write_fn(...)` returns the right raw code), a read contract +(`flatten()` produces the right raw value from a fixture rep), or a routing +decision — never a bare literal-string comparison against the catalog you +just edited. + ## 8. Coverage discipline: bound or ignored Every href in the dump must resolve, or the repair fires. If a resource isn't diff --git a/tests/test_dishwasher_capabilities.py b/tests/test_dishwasher_capabilities.py index 7ad5599..1e9d232 100644 --- a/tests/test_dishwasher_capabilities.py +++ b/tests/test_dishwasher_capabilities.py @@ -33,17 +33,6 @@ class TestCycleOptions: assert body == {"x.com.samsung.da.options": ["Course_90"]} -def test_normal_and_express_60_labels_are_not_transposed(): - """Issue #226: '83'/'86' were swapped in the shipped catalog -- '86' is - Express 60, '83' is Normal (see dishwasher.py's CYCLE_OPTIONS comment for - how this was confirmed).""" - from custom_components.localthings.catalog import _ENTITY_CATALOG - - state = _ENTITY_CATALOG["select"]["dishwasher_cycle"]["state"] - assert state["83"] == "Normal" - assert state["86"] == "Express 60" - - class TestDishwasherOptions: def test_storm_wash_read_and_write(self): desc = next( diff --git a/tests/test_dryer_capabilities.py b/tests/test_dryer_capabilities.py index cb5aa42..1791efb 100644 --- a/tests/test_dryer_capabilities.py +++ b/tests/test_dryer_capabilities.py @@ -94,15 +94,3 @@ def test_st_dryercourse_is_ignored(): ignored_hrefs = {c.href for c in ignored.IGNORED} assert "/st/dryercourse/vs/0" in ignored_hrefs assert "/st/washercourse/vs/0" in ignored_hrefs - - -def test_table_03_labels_confirmed_on_dv90dg6845lhu5(): - """Issue #244: codes 51/53/4e were confirmed by selecting each program - directly on the physical DV90DG6845LHU5 and reading back the resulting - raw course code -- same verification method as issue #80's '52'/'54'/'60' - on the washer's own course table.""" - from custom_components.localthings.catalog import translated_states - - known = translated_states("select", "dryer_cycle_table_03") - for code in ("51", "53", "4e", "17", "21", "4c"): - assert code in known, code