Skip to content

test: pin the default output-device entry as the clear-the-override path - #1930

Merged
laurentiu021 merged 1 commit into
mainfrom
test/audio-route-default-entry-contract
Aug 19, 2026
Merged

test: pin the default output-device entry as the clear-the-override path#1930
laurentiu021 merged 1 commit into
mainfrom
test/audio-route-default-entry-contract

Conversation

@laurentiu021

Copy link
Copy Markdown
Owner

What does this PR do?

Volume Control's output picker holds real endpoints only — there is no "System default" item — so
the entry flagged IsDefault carries that meaning instead, and an empty endpoint id is the pivot in
both directions:

  • Read: AudioSessionRowViewModel.SetOutputDeviceFromService resolves an empty id to the
    IsDefault entry (:238-241).
  • Write: OnSelectedOutputDeviceChanged sends string.Empty when the picked entry IsDefault
    (:209), and SetPersistedDefaultAudioEndpoint treats empty as clear the override
    (AudioPolicyConfig.cs:97-100).

So the default entry is the only way a user can stop routing an app, and nothing pinned it.
value.IsDefault ? string.Empty : value.Id reads like a needless special case; simplifying it to
value.Id compiles, keeps every pre-existing test green, and silently removes that capability — the
override would stay in Windows forever, re-pinned to whichever device was default at the time of the
pick. Same invisible-capability-loss class as a command bound by no XAML.

Three tests, four cases:

Test Pins
PuttingAnAppBackOnTheDefaultDevice_ClearsTheOverride the sequence is ("{hdst}") then ("")
RoutingAnAppToANonDefaultDevice_SendsThatDevicesEndpointId an ordinary pick keeps its own id
ARouteTheServiceCannotResolve_SelectsTheDefaultEntry_AndWritesNothing ×2 empty and gone-device routes select the default entry and write nothing

The clear test goes headset-then-default rather than selecting the default directly: the row is
constructed with the default already selected (the route-read is a stub returning empty), so
assigning it again is not a property change, the write path would never run, and the test would pass
while asserting nothing.

Also corrects the note on GetPersistedDefaultEndpoint. It claimed "the UI shows System default",
which is not what happens — the null becomes an empty id and the picker preselects the current
default device by name, so an app genuinely routed elsewhere is indistinguishable from one
following the default. That is the live half of #1585 and the comment was pointing away from it.

Related issues

Partially addresses #1658's neighbour #1585 — see the note below. Not closed.

Type of change

  • Tests (test:)

Checklist

  • Branch created from main
  • Code compiles with 0 errors — all four projects, 0 warnings
  • dotnet format --verify-no-changes passes on both touched projects
  • Tests added/updated and passing locally — audio class 55 green, 0 red; with AudioPolicyConfigTests
    + ArchitectureTests, 105 green
  • Author headers on all modified files
  • Self-review completed
  • README updated (if features changed) — n/a, no behaviour change
  • No AI/IDE tool references

How this was verified

Five mutations, each expected test red, file restored byte-for-byte:

Mutation Red Shows
value.IsDefault ? string.Empty : value.Idvalue.Id clear test only the plausible simplification is caught, and the ordinary-pick test is not what catches it
always send string.Empty pick test (+ clear) the ordinary-pick test discriminates too, rather than passing on any write
empty id → null instead of the default entry read row "" only
unknown-id fallback dropped read row {unplugged} only the two Theory rows cover different branches, not one twice
echo suppression removed from the read path all four the read path would write its own snapshot back

Mutations 1, 3 and 4 each fire exactly one case, so every branch is individually pinned rather than
covered by one blanket assertion.

What this deliberately does NOT do

AudioPolicyConfigFactory.GetPersistedDefaultEndpoint is still a stub returning null
(AudioPolicyConfig.cs:90-95), so the picker still cannot show an app's actual persisted route —
the live half of #1585. Implementing it means adding an undocumented COM vtable slot whose
out-string marshaling is the most build-variant part of the interface, which is exactly why it was
skipped, and it cannot be exercised on a machine that never runs the app. The alternative — showing a
neutral placeholder instead of a device name — changes what the picker means and where the
clear-the-override action lives, which is a product decision rather than a mechanical fix. This PR
pins the contract that any such change must preserve, and #1585 keeps the decision.

The Volume Control picker holds real endpoints only — there is no "System
default" item — so the entry flagged IsDefault carries that meaning instead, and
an empty endpoint id is the pivot in both directions. Reading, an empty id
resolves to that entry; writing, selecting it sends an empty id, which
SetPersistedDefaultAudioEndpoint treats as CLEAR THE OVERRIDE. That makes the
default entry the only way a user can stop routing an app.

Neither direction was pinned by a test. `value.IsDefault ? string.Empty :
value.Id` reads like a needless special case, and simplifying it to `value.Id`
compiles, keeps every pre-existing test green, and silently removes that
capability: the override would stay in Windows forever, re-pinned to whichever
device happened to be default at the time of the pick. Same
invisible-capability-loss class as a command bound by no XAML.

Three tests, four cases. Selecting a device and then putting the app back on the
default must produce ("{hdst}") then (""); an ordinary pick must keep its own
endpoint id; and a route the service cannot resolve — an empty one, or one naming
a device that is gone — must select the default entry while writing nothing back,
because a refresh that echoed its own snapshot would re-assert a route every pass
and, on the failure branch, report an error the user never caused.

The clear test goes headset-then-default rather than selecting the default
directly: the row is constructed with the default already selected, so assigning
it again is not a property change, the write path would never run, and the test
would pass while asserting nothing.

Also corrects the note on GetPersistedDefaultEndpoint. It claimed the UI shows
"System default", which is not what happens — the null becomes an empty id and
the picker preselects the current default device BY NAME, so an app genuinely
routed elsewhere is indistinguishable from one following the default (#1585).
Calls are captured inside the stub rather than asserted with Received(n), for the
reason the neighbouring test already gives: an NSubstitute substitute is not
thread-safe.
@laurentiu021
laurentiu021 merged commit 28fc0d3 into main Aug 19, 2026
5 checks passed
@laurentiu021
laurentiu021 deleted the test/audio-route-default-entry-contract branch August 19, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant