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.
This commit is contained in:
Marc Billow
2026-08-03 19:32:44 +00:00
parent e89aa4bab5
commit da25d567cb
3 changed files with 18 additions and 23 deletions
@@ -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
-11
View File
@@ -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(
-12
View File
@@ -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