Skip to content

Smoke test spike: headless client/server connect in CI #9

Open

Description

@dtinth-claw

Part of #3 (wayfinder map)

Question

Explore the smoke-testing aspect of getting Jamulus under test (per upstream #2428's sketch: headless server + headless client, verify a connection, Linux first, keep it simple). Build a cheap spike to react to: can a headless client and server actually connect in CI without sound hardware, what does "connection verified" concretely mean (protocol handshake? channel list? audio packets flowing?), how flaky is it, and how long does it take? Distinct from the unit-test harness (#4) — different confidence level, separate eventual PR.

Activity

  1. dtinth-claw commented on Jul 22, 2026

    @dtinth-claw
    Author

    🔒 Claimed (orchestrator dispatch, 2026-07-22) — smoke-spike agent starting on branch smoke-test-spike.

  2. dtinth-claw commented on Jul 22, 2026

    @dtinth-claw
    Author

    Spike ready for reaction — outcome: a clean "yes"

    Branch smoke-test-spike @ 44eebdfc (off upstream main): tools/smoke-test.sh, tools/smoke-test-jrpc.py, and the full writeup in research/smoke-test-spike.md.

    The five spike questions, answered:

    1. Headless build: clean — qmake6 Jamulus.pro CONFIG+=headless && make -j8 builds a real client+server binary in ~21–30s (incl. vendored Opus) on the dev box. Gotcha: embed_translations needs lrelease (qt6-l10n-tools) even headless.
    2. Server: ./Jamulus -s -n + JSON-RPC flags works as expected (-n/--nogui is the runtime flag; CONFIG+=headless is build-time).
    3. Client without sound hardware: jackd -d dummy worked out of the box — no plan B needed. Two harmless RT warnings in unprivileged containers, non-fatal.
    4. "Connection verified" means: both sides agree via their own JSON-RPC — client jamulusclient/getClientInfo → connected:true and server jamulusserver/getClients → ≥1 connection. Chosen over log-scraping/UDP-sniffing because it's Jamulus's documented structured state, and two-sided checking prevents one-sided false positives.
    5. Timing/flakiness: verification 0.86–0.88s, ~1.2s total wall-clock; 5/5 runs passed. Failure path verified too: bad target → nonzero exit, log dump, and zero leaked processes either way.

    Notable for upstream: no containers needed — plain localhost processes suffice, which speaks directly to the "would a localhost test be sufficient?" question raised in #2428's thread (the original sketch had a two-container setup). CI additions needed: jackd2, libjack-jackd2-dev, qt6-l10n-tools, no X, no privileges. Also noteworthy: the client-side JSON-RPC interface is what makes clean verification possible — a concrete testing-value data point for it.

    Open questions (in the writeup, for a later grilling — no answers needed now): where this runs in CI (every PR vs. nightly), whether the full ~30s+ Jamulus build inline is acceptable or should reuse artifacts/ccache, jamulussoftware#2428's version-compatibility-matrix ambitions now or later, and whether long-term reliance on JSON-RPC is acceptable given its docs disclaim semantic versioning.

    @dtinth — prototype ticket: your keep/change/rethink resolves it.

  3. dtinth commented on Jul 22, 2026

    @dtinth
    Owner

    I read your write up already. My thoughts:

    1. Yes, a job in CI.
    2. Yes, let's do for all platforms as we already spiked with unit testing.
    3. The job can build the binary itself (so it can do ASan/UBSan if needed?)
    4. Keep it there for now.
    5. Yes, keep as it is for now, but is it practical to combine both shell and Python scripts into a single Python script?

    Can you try setting up GitHub Actions for smoke testing?

  4. dtinth-claw commented on Jul 22, 2026

    @dtinth-claw
    Author

    Recorded all five answers: CI job ✔, all platforms ✔, job builds the binary itself (which indeed enables an ASan/UBSan variant later) ✔, script stays in tools/ for now ✔, JSON-RPC verification stays ✔.

    On combining shell + Python into one Python script: yes — practical and actively better for the all-platforms goal, since a single cross-platform tools/smoke-test.py (spawn/verify/teardown all in Python's subprocess) removes the bash dependency exactly where it hurts (Windows). Doing that as part of the CI work.

    Dispatching now on branch smoke-test-spike: merge the scripts, add a smoke-test.yml workflow, shepherd Linux to green first, then attempt macOS and Windows. Honest expectation-setting on "all platforms": the spike only proved Linux (jackd -d dummy). macOS (CoreAudio on a soundless runner) and Windows (client builds need the ASIO SDK, which upstream's autobuild downloads at build time) are genuinely open questions — if either proves infeasible at reasonable effort, the finding lands here with specifics rather than a forced flaky job.

  5. dtinth-claw commented on Jul 22, 2026

    @dtinth-claw
    Author

    Smoke-test CI ready for reaction — all three platforms green

    Run 29933941324 on branch smoke-test-spike @ 5297f584:

    Platform Result Job total Build phase Verify phase
    Linux (Qt 6, distro pkgs, jackd dummy) ✅ first try 1m13s ~40s 0.79s
    macOS (Qt 6, CoreAudio — no JACK needed) ✅ 1m37s ~55s 1.34s
    Windows (Qt 6, MSVC, jackonwindows — ASIO SDK avoided entirely) ✅ 5m22s ~3m53s 5.55s

    Files: .github/workflows/smoke-test.yml (house style, same aqtinstall/Xcode/MSVC conventions as unit-tests.yml), tools/smoke-test.py (your Q5 — the shell+Python pair is now one pure-stdlib cross-platform script, with process-group teardown verified leak-free on all platforms), updated writeup.

    The headline discovery — two genuine upstream bugs. CONFIG+=headless has apparently never compiled on macOS or Windows (only Linux, where the Debian package ships it):

    1. macOS: src/sound/coreaudio-mac/sound.h unconditionally #include <QMessageBox> — unused and ungated, so headless macOS builds fail.
    2. Windows: src/main.cpp's #ifdef _WIN32 block calls QApplication::applicationDirPath(), but the #include <QApplication> is gated #ifndef HEADLESS.

    Both are one-line fixes (delete the unused include; use QCoreApplication::applicationDirPath()). The CI works around them with build flags only (QT+=widgets, an MSVC force-include) — deliberately not patched in src/, because shipping source changes silently inside a spike branch felt like a product decision, not an agent's. They're arguably nice tiny standalone upstream contributions in their own right.

    Open questions (for a later grilling, no answers needed now): fix the two upstream bugs now vs. file/leave them; are CI-only workarounds acceptable long-term vs. insisting on the source fix first; plus the carried-over ones (trigger scope, script location, JSON-RPC experimental-API reliance).

    @dtinth — your keep/change/rethink resolves this ticket.

  6. mcfnord commented on Jul 22, 2026

    @mcfnord

    Note

    📡 STAND BY FOR AN LLM-AUTHORED MESSAGE.

    Hi @dtinth — cross-pollination from the other AI-assisted effort in this space (mcfnord, working upstream on measurement/analysis). Your smoke-test spike here is exactly the right foundation, and it lines up with a request a Jamulus maintainer made on the TCP-fallback PR.

    On jamulussoftware/jamulus#3636 (softins's "Support TCP for protocol messages"), @pljones asked back in June: "It really would be good to write up a set of test cases, with the logging enabled to show it's working right. Things like explaining how to test over a LAN... (I'd not have a clue.)" That request is still open.

    Your tools/smoke-test.py + smoke-test.yml answers the happy-path half of it — "does a headless client and server connect", verified two-sidedly over JSON-RPC, no privileges, three platforms. That's the part we would not rebuild.

    The part it can't reach, by design, is the feature under test in jamulussoftware#3636: TCP fallback only fires when a UDP directory/client-list is lost to fragmentation. A clean localhost connect never produces that, so exercising it needs deliberate fault injection — force IP-fragment loss so the UDP list drops, then confirm the client re-requests it over TCP and reassembles it byte-exact. We've built and run that as a Linux netns harness (real built binaries), covering: forced-fragmentation → TCP fallback (v4 and v6), connect-refused / firewall-blocked degradation, mid-session TCP drop, multi-segment reassembly (throttled with tc netem so the message genuinely splits across reads), and an anti-hijack check (spoofed channel-ID from a wrong source IP is rejected). A couple of gotchas we already paid for, in case they save you time: for IPv4, iptables -A INPUT -f silently no-ops because conntrack reassembles first — you need iptables -t raw -A PREROUTING -f; and message size alone won't split a TCP stream on a fast link, you have to throttle it.

    So the two tiers compose cleanly: your happy-path smoke test as the base, a fault-injection integration tier on top for protocol features like this one — ideally reusing your JSON-RPC connected / getClients oracle as the pass criterion rather than log-scraping. The one real cost is that fault injection needs CAP_NET_ADMIN (netns/iptables/tc), so it's a local/manual or privileged-optional job, not a per-PR default like yours.

    Happy to share our harness scripts if any of it is useful to you, and glad to keep the seam de-duplicated either way. Flagging softins's/pljones's request mainly so it's on your radar — it maps almost one-to-one onto what you're already building.

  7. dtinth commented on Jul 22, 2026

    @dtinth
    Owner

    re. upstream bugs. if they are one-line fixes, then we should include it in the same PR as the smoke testing harness.

    FYI, please also check out this comment: #9 (comment)

    @mcfnord Smoke test harness is still being explored. Thanks for the heads up, I will have my agent check it out when we get back to this. I am currently focusing more on the unit test harness and this is more like a side-quest.

  8. dtinth-claw commented on Jul 22, 2026

    @dtinth-claw
    Author

    Recorded both:

    1. Decision: the two one-line headless build fixes ride in the same future PR as the smoke-testing harness (noted on the map; this ticket stays open as the smoke side-quest's home, parked while the unit-test harness is the focus).
    2. The linked comment — read and assessed (treating its technical claims as unverified third-party report): the proposed two-tier composition is architecturally sound — our unprivileged happy-path smoke job as the per-PR base; a fault-injection tier (fragmentation-loss → TCP fallback, per pljones's still-open test-case request on Support TCP for protocol messages jamulussoftware/jamulus#3636) as a separate privileged/manual job, since netns/iptables/tc needs CAP_NET_ADMIN that a default CI job shouldn't have. Both the tier idea and the script-sharing offer are captured in the map's Not yet specified section for when this side-quest resumes — nothing dispatched now, per your prioritization.
  9. ann0see commented on Jul 22, 2026

    @ann0see

    If not done already, yes please open issues for the bugs.

  10. dtinth commented on Jul 23, 2026

    @dtinth
    Owner

    @ann0see Thanks for the pointer, I will open an issue for the bugs.

  11. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    Since you're opening the upstream issue(s) yourself — here are verified, ready-to-paste drafts (line numbers checked against current upstream main @ 49f4f8d6; edit freely). They can be one combined issue ("headless doesn't compile on macOS/Windows") or two separate ones.


    Draft 1 — title: CONFIG+=headless does not compile on macOS: unconditional QMessageBox include in coreaudio sound.h

    Building with qmake Jamulus.pro CONFIG+=headless fails on macOS because src/sound/coreaudio-mac/sound.h:53 unconditionally does #include <QMessageBox>, while headless builds drop the widgets module (QT -= gui). QMessageBox is not referenced anywhere in src/sound/coreaudio-mac/ (verified by grep), so the include appears to be a leftover.

    Fix: delete the include (one line). Workaround until then: build with QT+=widgets linked in, which defeats part of headless's purpose.

    Found while setting up a headless client/server smoke test in CI; Linux headless builds fine (it's what the jamulus-headless Debian package ships), which suggests headless has simply never been compiled on macOS.


    Draft 2 — title: CONFIG+=headless does not compile on Windows: QApplication used in _WIN32 block but its include is gated on !HEADLESS

    In src/main.cpp, the #include <QApplication> is inside #ifndef HEADLESS (lines 51–52), but the #ifdef _WIN32 block at line 912 calls QApplication::applicationDirPath() unconditionally — so a headless Windows build fails to compile.

    Fix: applicationDirPath() is a static on QCoreApplication (always available), so QDir ApplDir ( QCoreApplication::applicationDirPath() ); is a one-line fix with identical behavior in GUI builds.

    Same discovery context as the macOS issue: headless appears never to have been compiled on Windows before.


    (Per the earlier decision these one-liner fixes would ride in the future smoke-harness PR — but with issues open, upstream may well fix them sooner, which is even better: the smoke PR then needs no production changes at all.)

  12. dtinth commented on Jul 23, 2026

    @dtinth
    Owner

    Seems like you haven't checked the issue template haha. I'm thinking of creating just one issue "headless doesn't compile on macOS/Windows"

    Can you help with:

    • To Reproduce (exact commands on Windows and Mac)
    • Actual behavior (exact log messages you found)
    • Screenshots (link me to GHA run, I will capture screenshot myself)
    • Operating system
    • Version of Jamulus
  13. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    You're right, I skipped the template 😅 — here's everything for the single-issue shape, field by field:

    Describe the bug

    CONFIG+=headless builds fail to compile on macOS and Windows (they work on Linux — it's what the jamulus-headless Debian package ships). Two independent one-line causes:

    1. macOS: src/sound/coreaudio-mac/sound.h:53 unconditionally #include <QMessageBox>, but headless builds exclude the widgets module. QMessageBox is not used anywhere in src/sound/coreaudio-mac/.
    2. Windows: src/main.cpp:912 (inside #ifdef _WIN32) calls QApplication::applicationDirPath(), but #include <QApplication> is gated behind #ifndef HEADLESS (lines 51–52).

    To Reproduce

    macOS (arm64, Qt 6.10.2 via aqtinstall, Xcode 16.3.0):

    qmake Jamulus.pro CONFIG+=headless QMAKE_APPLE_DEVICE_ARCHS=arm64
    make -j3
    

    Windows (Qt 6.10.2 MSVC 2022, x64 Native Tools prompt; JACK variant shown, but the error is in an unconditional _WIN32 block so any headless Windows build hits it):

    qmake Jamulus.pro CONFIG+=headless CONFIG+=jackonwindows "CONFIG-=debug_and_release" "CONFIG+=release"
    nmake
    

    Expected behavior

    Compiles, as the identical configuration does on Linux.

    Actual behavior (exact compiler output)

    macOS:

    ../src/sound/coreaudio-mac/sound.h:53:10: fatal error: 'QMessageBox' file not found
       53 | #include <QMessageBox>
    1 error generated.
    

    Windows:

    ..\src\main.cpp(912): error C2027: use of undefined type 'QApplication'
    C:\Qt\6.10.2\msvc2022_64\include\QtCore/qobject.h(380): note: see declaration of 'QApplication'
    ..\src\main.cpp(912): error C3861: 'applicationDirPath': identifier not found
    

    Screenshots — for your capture: failed run https://github.com/dtinth/jamulus/actions/runs/29933460318 — macOS job and Windows job; the errors are in each job's "Build Jamulus (headless)" step. (Same run shows the Linux job green for contrast.)

    Operating system

    GitHub Actions runners, building from source: macos-15 (Apple Silicon, Xcode 16.3.0, Qt 6.10.2) and windows-latest (MSVC 2022, Qt 6.10.2). Not observed on Linux (ubuntu-24.04, distro Qt 6 — builds and runs fine).

    Version of Jamulus

    Built from source at current main (49f4f8d6, reports 3.12.2dev).

    Additional context

    Suggested one-line fixes: delete the unused include in sound.h; use QCoreApplication::applicationDirPath() in main.cpp (static lives on the base class, identical behavior in GUI builds). Found while setting up a headless client/server smoke test in CI (ref jamulussoftware#2428); given Linux-only distribution of headless builds, these paths have likely never been compiled on macOS/Windows.

    One caveat for the macOS repro: the plain command above also needs the QMAKE_APPLE_DEVICE_ARCHS=arm64 override on Apple Silicon runners (qmake otherwise tries Opus's x86 SSE sources) — that's a separate pre-existing quirk, included in the command so the repro actually reaches the QMessageBox error.

  14. dtinth commented on Jul 23, 2026

    @dtinth
    Owner

    For fixes, can you format a patch file. If it's a one line change we should have a small Git diff output.

  15. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    Here are both fixes as real git diffs against upstream main (49f4f8d6) — generated from actual edits, so they apply cleanly with git apply:

    macOS — remove the unused include:

    diff --git a/src/sound/coreaudio-mac/sound.h b/src/sound/coreaudio-mac/sound.h
    index 9ee27965..970c6371 100644
    --- a/src/sound/coreaudio-mac/sound.h
    +++ b/src/sound/coreaudio-mac/sound.h
    @@ -50,7 +50,6 @@
     #include <AudioToolbox/AudioToolbox.h>
     #include <CoreMIDI/CoreMIDI.h>
     #include <QMutex>
    -#include <QMessageBox>
     #include "../soundbase.h"
     #include "../../global.h"
     

    Windows — use the base-class static (identical behavior in GUI builds):

    diff --git a/src/main.cpp b/src/main.cpp
    index a518c2e5..7bca23fb 100644
    --- a/src/main.cpp
    +++ b/src/main.cpp
    @@ -909,7 +909,7 @@ int main ( int argc, char** argv )
         // For accessible support we need to add a plugin to qt. The plugin has to
         // be located in the install directory of the software by the installer.
         // Here, we set the path to our application path.
    -    QDir ApplDir ( QApplication::applicationDirPath() );
    +    QDir ApplDir ( QCoreApplication::applicationDirPath() );
         pApp->addLibraryPath ( QString ( ApplDir.absolutePath() ) );
     #endif
     

    (Note for the issue: in GUI builds QApplication::applicationDirPath() was already just calling the inherited QCoreApplication static, so the second change is behavior-neutral everywhere.)

  16. dtinth commented on Jul 23, 2026

    @dtinth
    Owner

    By the way, if you have already isolated the fixes, you can just give me the commit number because I can fetch the patch file directly from GitHub.

  17. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    Done — branch fix/headless-macos-windows (off upstream main), one commit per fix:

    Both have proper commit messages explaining the why, so they're cherry-pickable as-is.

  18. dtinth commented on Jul 23, 2026

    @dtinth
    Owner
  19. dtinth commented on Jul 23, 2026

    @dtinth
    Owner

    Got a go-ahead to open a PR. Can you draft a fill-in the PR template?

  20. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    Drafting now — one integrity step first: the isolated fix commits were never themselves CI-tested (the smoke CI passed with the workarounds, which mask exactly these bugs). A verification run is in flight: verify/headless-fixes = the two fix commits + the smoke workflow with workarounds removed, so macOS/Windows build the pristine fixes. When it's green, the filled-in PR template lands here with that run as the "I tested my code" evidence.

  21. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    The verification run paid for itself — it found a third bug the workaround had been masking. Run 29988917176 (fixes only, no workarounds):

    • Windows: fully verified ✅ — with just the QCoreApplication one-liner, headless compiles, links, and passes the smoke test (4m31s). That fix is proven sufficient on its own.
    • macOS: fix correct but not sufficient — everything compiles cleanly now (the QMessageBox fix works), but the link fails on something new:
      Undefined symbols for architecture arm64:
        "qt_set_sequence_auto_mnemonic(bool)", referenced from: _main in main.o
      
      Cause: src/main.cpp lines 67–89 — the qt_set_sequence_auto_mnemonic ( true ) extern+call is gated only on Q_OS_MACOS, not HEADLESS, and that symbol only exists when QtWidgets is linked. The old QT+=widgets workaround masked both macOS bugs at once; the second one was invisible because the build never reached the link stage until the first was fixed.

    Proceeding on the standing "one-line fixes ride together" rule: preparing a third commit on fix/headless-macos-windows gating the extern+call with && !defined ( HEADLESS ) (menu mnemonics are meaningless without a GUI; the adjacent mac/activity.h app-nap machinery stays unconditional since headless wants it too), then re-running the no-workaround verification. PR template draft lands here once macOS is green — and jamulussoftware#3825's "Additional context" deserves a one-line update about the third bug when you next touch it. Shout if you'd rather I stop before the third commit.

  22. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    All three platforms now build headless with zero workarounds — verification run 29989445107: Linux ✅, macOS ✅ (compiles, links, smoke passes), Windows ✅. The onion had exactly three layers; no further masked bugs.

    Open the PR from dtinth:fix/headless-macos-windows → jamulussoftware:main (3 commits: 9c575a85, 2e74c057, 8a3f969c). Filled template below — paste and edit to taste:


    Short description of changes

    Three one-line fixes so that CONFIG+=headless compiles (and links) on macOS and Windows, as it already does on Linux:

    • macOS: remove an unused #include <QMessageBox> from src/sound/coreaudio-mac/sound.h (widgets aren't available in headless builds).
    • macOS: gate the qt_set_sequence_auto_mnemonic() extern/call in src/main.cpp behind !defined ( HEADLESS ) — the symbol only exists when QtWidgets is linked, and menu accelerator mnemonics have no meaning without a GUI. (This one only became visible once the first fix let the build reach the link stage.)
    • Windows: use QCoreApplication::applicationDirPath() instead of QApplication::applicationDirPath() in the #ifdef _WIN32 block of src/main.cpp — the <QApplication> include is (correctly) gated on !HEADLESS, and the static is inherited from QCoreApplication, so this is behavior-neutral in GUI builds.

    CHANGELOG: Build: Fixed headless builds (CONFIG+=headless) not compiling on macOS and Windows.

    Context: Fixes an issue?

    Fixes: jamulussoftware#3825

    Does this change need documentation? What needs to be documented and how?

    No — no user-facing behavior changes; CONFIG+=headless is already documented in COMPILING.md and now works on two more platforms.

    Status of this Pull Request

    Working implementation.

    What is missing until this pull request can be merged?

    Nothing known. Verified in a fork with a headless client/server build-and-connect job on all three platforms: failing before (macOS: fatal error: 'QMessageBox' file not found, then Undefined symbols … qt_set_sequence_auto_mnemonic(bool); Windows: error C2027: use of undefined type 'QApplication') and green after with only these three commits.

    Checklist

    • I've verified that this Pull Request follows the general code principles
    • I tested my code and it does what I want
    • My code follows the style guide
    • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
    • I've filled all the content above

    AUTOBUILD: Please build all targets


    Two notes for you, @dtinth:

    1. The PR body above mentions the third bug, which headless doesn't compile on macOS/Windows jamulussoftware/jamulus#3825 doesn't yet — worth a one-liner comment there when convenient (e.g. "turns out there's a third gap: qt_set_sequence_auto_mnemonic is widgets-only and gated just on Q_OS_MACOS — it surfaced once the include fix let the build reach the link stage; the PR covers it").
    2. I left AUTOBUILD: Please build all targets uncommented since this touches platform-specific code paths — full platform builds on the PR seem prudent for exactly this class of bug.
  23. dtinth commented on Jul 23, 2026

    @dtinth
    Owner
  24. dtinth commented on Jul 23, 2026

    @dtinth
    Owner

    Got a review on the opened PR.

  25. dtinth-claw commented on Jul 23, 2026

    @dtinth-claw
    Author

    Upstream review triage for PR #3826:

    • Copilot bot review: summary only, zero comments — nothing to act on.
    • ann0see's inline question on the sound.h change ("Why is this sound?"): the archaeology gives a complete answer. Suggested reply for you to paste/adapt:

    The include has been orphaned since December 2020: 270310d0 (2020-11-24, "show warning message if previous driver is no longer available") added a QMessageBox::warning(...) to the mac sound.cpp along with this include, and one week later 4f9d1460 (2020-12-01, "improved audio device management") removed that warning again — but left the include behind. Today nothing in src/sound/coreaudio-mac/ references QMessageBox (verified by grep), and the headless CI build in my fork compiles and links this file cleanly without it (green run). In GUI builds it changes nothing — the header was never needed there either, since no code in the sound layer shows dialogs anymore.

    (If their question instead meant "why did sound code ever include a message-box header" — same answer, it's the 2020 leftover.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions