Problem
lint.yml builds, formats and tests the C# project only. Nothing in src/MultiRoomAudio/wwwroot/js/ is covered by CI, and there is no JS test infrastructure in the repo at all.
That gap got noticeably wider with #282 / #283, which added real branching logic to wwwroot/js/utils.js:
buildRemapSinkDefaults(masterLabel, channels, existingNames) — slugifies the device name, appends an abbreviated channel suffix, enforces the 100-char cap from RemapSinkCreateRequest by truncating the slug (not the suffix), and walks a _2, _3, … ordinal until it finds a free name.makeCardDisambiguator(items, getLabel, getCardNumber) — appends (card N) only to labels shared by more than one entry.
Plus the autofill wiring that consumes them, implemented once in app.js (autofillRemapSinkNaming, markRemapFieldEdited, getSelectedRemapChannels) and again in wizard.js (Wizard.autofillRemapNaming, Wizard.markRemapFieldEdited). The two are meant to behave identically — that equivalence is exactly the kind of thing that quietly rots, and today nothing catches it.
The concrete risk: a generated name that violates ^[a-zA-Z0-9_\-\.]+$ or exceeds 100 chars gets rejected by the API at create time, and a broken dirty-flag stops the form regenerating (or worse, clobbers what someone typed). Both are silent until a user hits them.
What has already been proven to work
While building #283 I verified the behaviour with a throwaway node + jsdom harness. It was not committed — the repo had nowhere to put it — but it establishes the approach is cheap and that no refactor is needed:
- Load
utils.js and wizard.js as real <script> elements so top-level const lands in global script scope, the way the browser loads them.
- Pull the remap modal markup straight out of
index.html rather than hand-rolling a fixture, so the test exercises the real element ids and inline oninput/onchange handlers.
runScripts: 'dangerously' is required — jsdom will not compile inline handler attributes otherwise, and the sticky-autofill tests silently pass for the wrong reason without it.- For
app.js (5900 lines, top-level side effects), slice out the handful of needed top-level functions by name instead of evaluating the whole file.
Cases it covered, as a starting checklist — 15 on the main modal, 10 on the wizard:
- no autofill before a master device is chosen
- prefills both name and description on master selection
- regenerates on channel change while both fields are untouched
- a hand-edited field stops regenerating; the other field keeps going
- reopening the modal / re-rendering the wizard step resets the dirty flags
- collision with an existing sink yields the
_2 suffix
- mono mode yields a single channel in both name and description
- editing an existing sink rewrites neither field, and the name input stays disabled
- an absurdly long device name still produces a valid name ≤ 100 chars ending in the channel suffix, and a description ≤ 200 chars
- duplicate master devices get
(card N) in the dropdown, and the generated name carries it through
- the wizard's combine checkboxes and identify list disambiguate too
Suggested shape
A tests/js/ directory with package.json, jsdom as the only dependency, and either node --test or vitest. Then a step in lint.yml. Deliberately small — the goal is guarding these helpers, not adopting a front-end toolchain.
Worth deciding as part of this: whether to keep testing through the real DOM (higher fidelity, catches wiring bugs like a missing oninput) or to export the pure helpers from utils.js and unit-test those alone (simpler, misses the wiring). The harness did the former and the wiring bugs it caught argue for keeping it.
Related
tests/MultiRoomAudio.E2ETests/ contains two .cs files but no .csproj, and is referenced by neither the solution nor CI — so CustomSinksE2ETests and TriggersE2ETests cannot be run at all. Separate problem, but it is the other half of why UI changes go unverified in CI.
Problem
lint.ymlbuilds, formats and tests the C# project only. Nothing insrc/MultiRoomAudio/wwwroot/js/is covered by CI, and there is no JS test infrastructure in the repo at all.That gap got noticeably wider with #282 / #283, which added real branching logic to
wwwroot/js/utils.js:buildRemapSinkDefaults(masterLabel, channels, existingNames)— slugifies the device name, appends an abbreviated channel suffix, enforces the 100-char cap fromRemapSinkCreateRequestby truncating the slug (not the suffix), and walks a_2,_3, … ordinal until it finds a free name.makeCardDisambiguator(items, getLabel, getCardNumber)— appends(card N)only to labels shared by more than one entry.Plus the autofill wiring that consumes them, implemented once in
app.js(autofillRemapSinkNaming,markRemapFieldEdited,getSelectedRemapChannels) and again inwizard.js(Wizard.autofillRemapNaming,Wizard.markRemapFieldEdited). The two are meant to behave identically — that equivalence is exactly the kind of thing that quietly rots, and today nothing catches it.The concrete risk: a generated name that violates
^[a-zA-Z0-9_\-\.]+$or exceeds 100 chars gets rejected by the API at create time, and a broken dirty-flag stops the form regenerating (or worse, clobbers what someone typed). Both are silent until a user hits them.What has already been proven to work
While building #283 I verified the behaviour with a throwaway
node+jsdomharness. It was not committed — the repo had nowhere to put it — but it establishes the approach is cheap and that no refactor is needed:utils.jsandwizard.jsas real<script>elements so top-levelconstlands in global script scope, the way the browser loads them.index.htmlrather than hand-rolling a fixture, so the test exercises the real element ids and inlineoninput/onchangehandlers.runScripts: 'dangerously'is required — jsdom will not compile inline handler attributes otherwise, and the sticky-autofill tests silently pass for the wrong reason without it.app.js(5900 lines, top-level side effects), slice out the handful of needed top-level functions by name instead of evaluating the whole file.Cases it covered, as a starting checklist — 15 on the main modal, 10 on the wizard:
_2suffix(card N)in the dropdown, and the generated name carries it throughSuggested shape
A
tests/js/directory withpackage.json,jsdomas the only dependency, and eithernode --testor vitest. Then a step inlint.yml. Deliberately small — the goal is guarding these helpers, not adopting a front-end toolchain.Worth deciding as part of this: whether to keep testing through the real DOM (higher fidelity, catches wiring bugs like a missing
oninput) or to export the pure helpers fromutils.jsand unit-test those alone (simpler, misses the wiring). The harness did the former and the wiring bugs it caught argue for keeping it.Related
tests/MultiRoomAudio.E2ETests/contains two.csfiles but no.csproj, and is referenced by neither the solution nor CI — soCustomSinksE2ETestsandTriggersE2ETestscannot be run at all. Separate problem, but it is the other half of why UI changes go unverified in CI.