Skip to content

Commit eaef3da

Browse files
chrisutheclaude
andcommitted
Assert the resolved URL, not the dial, in the Avahi discovery check
The first CI run showed the discovery half working and the check failing anyway. `-s mdns:<name>` filters on the TXT `name` record rather than the instance label, and avahi-publish was given only the REQUIRED `path`, so select_server() matched nothing and no dial was ever logged -- while the line that matters was there all along: I discovery: found server "..." (name: <unnamed>) at ws://10.1.0.147:8927/sendspin That line is the better assertion as well as the passing one. It is emitted the moment discovered_server_url() succeeds, which cannot happen with an empty address list, which only DNSServiceQueryRecord fills -- so it pins the compat-layer claim directly, where the dial only pinned it via RetryPacer's schedule. The published service now carries a `name` record too, so the -s filter is exercised rather than silently matching nothing. The tree also stops claiming three things this makes untrue: that the Linux dns_sd path has only been read (item 5 and the mdns_dnssd.cpp header), that `main` does not compile on Linux (item 6), and that the Linux dual-backend configuration is unbuilt (item 4). Item 4's is narrowed rather than dropped -- the matrix builds that configuration and asserts the backend list, but nothing has run `-l` or heard ALSA on Linux. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9566b3b commit eaef3da

3 files changed

Lines changed: 28 additions & 22 deletions

File tree

.github/workflows/ci.yml

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,15 @@ jobs:
217217
trap - EXIT
218218
219219
# Half two: a server published into the same daemon, discovered and resolved by our
220-
# own dns_sd code. Nothing is listening on 8927, so the dial that follows will fail;
221-
# what is being asserted is the URL, which only exists if QueryRecord answered.
222-
avahi-publish -s "$instance-server" _sendspin-server._tcp 8927 'path=/sendspin' &
220+
# own dns_sd code. Nothing is listening on 8927 and no dial is asserted -- what is
221+
# asserted is that a ws:// URL was built at all, which takes an address, which takes
222+
# a QueryRecord that answered.
223+
#
224+
# TXT `name` as well as the REQUIRED `path`, because -s mdns:<name> filters on the
225+
# former: without it the instance resolves but matches nothing, and the filter goes
226+
# untested.
227+
avahi-publish -s "$instance-server" _sendspin-server._tcp 8927 \
228+
'path=/sendspin' "name=$instance-server" &
223229
publisher=$!
224230
trap 'kill -TERM "$publisher" 2>/dev/null || true' EXIT
225231
@@ -228,16 +234,15 @@ jobs:
228234
client=$!
229235
trap 'kill -TERM "$client" "$publisher" 2>/dev/null || true' EXIT
230236
237+
resolved="found server \"$instance-server\" .* at ws://"
231238
for _ in $(seq 1 300); do
232-
if grep -q "Connecting to ws://.*(server \"$instance-server\")" discover.log; then
233-
break
234-
fi
239+
if grep -q "$resolved" discover.log; then break; fi
235240
sleep 0.1
236241
done
237242
echo 'What the discovering client logged:'
238243
cat discover.log
239-
grep -q "Connecting to ws://.*(server \"$instance-server\")" discover.log || {
240-
echo '::error::the server was published but never discovered and resolved to a URL'
244+
grep -q "$resolved" discover.log || {
245+
echo '::error::the server was published but never resolved to a ws:// URL'
241246
exit 1
242247
}
243248

docs/ROADMAP.md

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,10 @@ through Core Audio at 48 kHz/16-bit, 44.1 kHz/32-bit and 44.1 kHz/24-bit, with e
195195
volume and mute, the same-format restart, a mid-stream format change, recovery from a refused
196196
format, and the shutdown latch. It has **not** yet been driven by a real Sendspin server, so
197197
the sync loop converging over a live stream, controller volume, and pause/resume/next-track
198-
are unproven in the field. The Linux dual-backend path (`null, stdout, alsa, portaudio`, both
199-
`-l` sections) is likewise unexercised — this was built and tested on macOS. Item 12's build
200-
matrix is where that gets closed.
198+
are unproven in the field. Item 12's matrix now builds the Linux dual-backend configuration
199+
on every push and asserts from the configure output that it really is `null, stdout, alsa,
200+
portaudio` — but building it is all that proves. Neither `-l` section nor a note out of
201+
`AlsaAudioSink` has been exercised on Linux: this was written and heard on macOS.
201202

202203
One known rough edge, and only on Linux: `AlsaAudioSink` routes libasound's own stderr
203204
diagnostics through the CLI logger, but it installs that handler from its *own* `probe()`
@@ -382,11 +383,11 @@ refcount and the restart backoff are the riskiest code here and are exercised on
382383
on macOS. (`Impl` is factored so a fake dns_sd could drive it; that is item 12's job.) And
383384
the property that a per-tick redial would cancel the attempt in flight is reasoned from
384385
`ConnectionManager::connect_to()`'s source, not exercised — what the `RetryPacer` suite
385-
proves is the schedule. **The Linux path is unexercised**
386-
`libavahi-compat-libdnssd` has not been run against, only read; that is item 12's build
387-
matrix to close, and the Avahi compat findings above are the specific thing it should
388-
check. A mid-session server disappearance and the reconnect-after-drop path were
389-
covered by unit tests rather than in the field.
386+
proves is the schedule. The Linux path is no longer only read: item 12's matrix advertises,
387+
browses and resolves through a real `avahi-daemon` on every push, which is what confirms the
388+
Avahi compat findings above — a `ws://` URL cannot be built without an address, so it cannot
389+
be logged unless `DNSServiceQueryRecord` answered. A mid-session server disappearance and the
390+
reconnect-after-drop path were covered by unit tests rather than in the field.
390391

391392
### 6. Daemonization and logging — *shipped*
392393

@@ -527,10 +528,9 @@ precise about.** The code *in* this item builds warning-free and passes 148/149
527528
bookworm against real `libasound`, `portaudio` and `libavahi-compat-libdnssd`; the one
528529
failure is `LastServer.AnUnwritableDirectoryFails`, which fails only because the container
529530
runs as root and root ignores directory permissions — it passes as a normal user in the same
530-
container. But **`main` itself does not compile on Linux**, for a reason that predates this
531-
task: see item 12. Getting far enough to run these tests needed that break patched out in a
532-
throwaway copy, so "green on Linux" is a statement about this item's code, not about the
533-
tree.
531+
container. The `src/mdns_dnssd.cpp` break that made `main` itself uncompilable on Linux, and
532+
that had to be patched out in a throwaway copy to get this far, is fixed under item 12 — the
533+
whole tree now builds and tests on Linux in CI.
534534

535535
**Two things this item does not claim.** The library's own lines are **not timestamped**
536536
under `-f` and cannot be, for the same missing-sink-hook reason its category cannot be

src/mdns_dnssd.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@
2626
/// resolve chain on the one thread -- where the POSIX getaddrinfo() upstream's
2727
/// `examples/tui_client` falls back to needs a thread per lookup.
2828
///
29-
/// The Linux half of that is read from avahi's own `compat.c` and `unsupported.c`, not run:
30-
/// this has only been exercised against Bonjour. See docs/ROADMAP.md item 12.
29+
/// That the Linux half really behaves that way is checked rather than assumed: CI advertises,
30+
/// browses and resolves through a real `avahi-daemon` on every push, and a resolve cannot
31+
/// produce a URL without an address. See docs/ROADMAP.md item 12.
3132

3233
#include "mdns.h"
3334

0 commit comments

Comments
 (0)