From 592c52224426c6015ea4284f29ef7f3ac1980e3d Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:15:39 +0000 Subject: [PATCH] tidy the integration harness and its comments Trim the explanatory comments to short quirk notes, drop the module headers, and fail cleanly: navidrome/gonic now kill their process if they never become ready and the entrypoint always tears down. The prune test waits for the removal to appear in the album list rather than for the scan flag. Correct the AGENTS token-auth note (lms answers 42, Nextcloud 41) and give CI more headroom. --- AGENTS.md | 2 +- .../data/sync/PruneSyncIntegrationTest.kt | 5 +-- .../data/source/subsonic/SubsonicClient.kt | 3 +- flake.nix | 6 +--- tools/integration-test.nu | 26 ++++++-------- tools/integration/servers.nu | 35 ++++++++++--------- tools/integration/subsonic.nu | 15 +++----- tools/integration/util.nu | 2 -- 8 files changed, 39 insertions(+), 55 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 37916475..33de5fc4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,6 @@ Everything comes from the Nix flake devshell; do not install toolchains by hand. - `applicationId` is `com.subtracks.next` for the beta; change it to `com.subtracks` before any store release. - The CI image provides `nix-ld`, `jq`, `sqlite`, `node`, `zstd` and a `runner` user that `cache-nix-action` expects. None of that is needed locally beyond the devshell. - Integration servers: navidrome on 4533 (`admin`/`password`), gonic on 4747 (`admin`/`admin`), lms on 5082 (`admin`/`subtracks-lms`), Nextcloud Music on 8090 (`admin`/`subtracks-nextcloud`, base URL `http://localhost:8090/index.php/apps/music/subsonic/`). The test music is cached in `.integration/music` (gitignored). -- lms and Nextcloud Music authenticate the Subsonic API with per-user **API keys**, not passwords or the `t`/`s` token scheme (both reject token auth with error 41). The harness seeds an API key and the integration tests use `useTokenAuth = false`, so the app's token-auth fallback is currently not exercised by integration tests. +- lms and Nextcloud Music authenticate the Subsonic API with per-user **API keys** instead of passwords. The harness seeds a key and most tests use `useTokenAuth = false`; `SubsonicAuthIntegrationTest` drives `useTokenAuth = true` and asserts the fallback to plaintext (lms answers code 42, Nextcloud 41). - The integration shell takes its server packages from a pinned stable nixpkgs (`nixpkgs-stable`), because lms 3.80 in nixpkgs-unstable segfaults at startup with wt 4.14.3. - `nix develop` does not source your shell rc files (so tools like atuin are not active); use direnv if you want your normal interactive shell. diff --git a/app/src/integrationTest/kotlin/com/subtracks/data/sync/PruneSyncIntegrationTest.kt b/app/src/integrationTest/kotlin/com/subtracks/data/sync/PruneSyncIntegrationTest.kt index 4ccef057..cd012390 100644 --- a/app/src/integrationTest/kotlin/com/subtracks/data/sync/PruneSyncIntegrationTest.kt +++ b/app/src/integrationTest/kotlin/com/subtracks/data/sync/PruneSyncIntegrationTest.kt @@ -82,10 +82,10 @@ class PruneSyncIntegrationTest { call("startScan") repeat(120) { - if (!call("getScanStatus").contains("\"scanning\":true")) return + if (!call("getAlbumList2", "&type=newest&size=500").contains(REMOVED_ALBUM_NAME)) return Thread.sleep(500) } - error("the prune server did not finish rescanning") + error("the prune server still lists $REMOVED_ALBUM_NAME after rescanning") } private fun requireProperty(name: String): String = @@ -94,5 +94,6 @@ class PruneSyncIntegrationTest { private companion object { const val REMOVED_ALBUM_DIR = "197" + const val REMOVED_ALBUM_NAME = "Retroconnaissance EP" } } diff --git a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt index 31672819..8cfb4152 100644 --- a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt +++ b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt @@ -96,8 +96,7 @@ class SubsonicClient( const val API_VERSION = "1.15.0" const val CLIENT = "subtracks" - // 41 is the classic Subsonic code for "token authentication not supported"; OpenSubsonic - // servers (lms) answer 42 instead, so both must disable token auth and retry. + // Classic Subsonic uses 41; OpenSubsonic servers (lms) answer 42, so both disable token auth. val TOKEN_AUTH_UNSUPPORTED_CODES = setOf(41, 42) private fun randomSalt(): String = (1..4).map { ('a'..'z').random() }.joinToString("") diff --git a/flake.nix b/flake.nix index 96d59532..f33f32d4 100644 --- a/flake.nix +++ b/flake.nix @@ -3,9 +3,7 @@ inputs = { nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable"; - # The integration shell runs real servers; pin them to a stable release. lms 3.80 (the - # version in unstable) segfaults at startup with wt 4.14.3, so the server packages come from - # here instead. The default shell never touches this input. + # lms 3.80 (unstable) segfaults at startup with wt 4.14.3; stable ships a working 3.78. nixpkgs-stable.url = "github:NixOS/nixpkgs/nixos-26.05"; }; @@ -70,8 +68,6 @@ inherit shellHook; }; - # The integration harness starts real Subsonic servers, none of which the app build or - # unit tests need. Kept out of the default shell so those stay free of the server deps. integration = pkgs.mkShell { packages = basePackages ++ [ servers.navidrome diff --git a/tools/integration-test.nu b/tools/integration-test.nu index 6c355a18..0dc244b7 100755 --- a/tools/integration-test.nu +++ b/tools/integration-test.nu @@ -1,8 +1,6 @@ #!/usr/bin/env nu -# Starts the test servers, prepares the fixture data the Kotlin integration tests expect, runs -# them, then tears everything down. Enter the integration shell first: -# +# Run inside the integration shell: # nix develop .#integration --command nu tools/integration-test.nu use ./integration/util.nu [port-in-use] @@ -21,15 +19,16 @@ def main [] { let work = (^mktemp -d | str trim) - print "starting navidrome" - let nav = (start-navidrome $work) - print "starting gonic" - let gonic = (start-gonic $work) - + mut nav = -1 + mut gonic = -1 mut lms = -1 mut nc = -1 mut prune = -1 let outcome = (try { + print "starting navidrome" + $nav = (start-navidrome $work) + print "starting gonic" + $gonic = (start-gonic $work) print "starting lms" $lms = (start-lms $work) print "starting nextcloud" @@ -50,14 +49,9 @@ def main [] { ($env.LAST_EXIT_CODE? | default 1) }) - let lms_id = $lms - let nc_id = $nc - let prune_id = $prune - do -i { job kill $nav } - do -i { job kill $gonic } - if $lms_id >= 0 { do -i { job kill $lms_id } } - if $nc_id >= 0 { do -i { job kill $nc_id } } - if $prune_id >= 0 { do -i { job kill $prune_id } } + for id in [$nav $gonic $lms $nc $prune] { + if $id >= 0 { do -i { job kill $id } } + } rm -rf $work exit $outcome diff --git a/tools/integration/servers.nu b/tools/integration/servers.nu index bfeb29e7..482b8a8d 100644 --- a/tools/integration/servers.nu +++ b/tools/integration/servers.nu @@ -1,5 +1,3 @@ -# Test servers: fixtures, lifecycle and the credentials the tests rely on. - use ./util.nu * use ./subsonic.nu * @@ -19,8 +17,7 @@ const MUSIC = [ { id: 321, name: "Brad Sucks - I Don't Know What I'm Doing", songs: 12 } ] -# The servers the Kotlin integration tests and the fixtures below share. Keep this in step with -# TestServers.kt. +# Keep in step with TestServers.kt. export def test-servers [] { [ { name: "navidrome", base: $"http://localhost:($NAV_PORT)/", user: "admin", pass: "password" } @@ -55,10 +52,14 @@ export def start-navidrome [work: string] { let data = ($work | path join "navidrome") mkdir $data let daemon = (job spawn { ^navidrome --address 127.0.0.1 --port $NAV_PORT --musicfolder $LIB --datafolder $data --loglevel warn out+err> ($work | path join "navidrome.log") }) - poll "navidrome" {|| - try { http get -m 5sec $"http://127.0.0.1:($NAV_PORT)/rest/ping.view?u=x&p=x&v=1.13.0&c=test" | ignore; true } catch { null } - } - http post -m 10sec -t application/json $"http://127.0.0.1:($NAV_PORT)/auth/createAdmin" { username: "admin", password: "password" } | ignore + let ready = (try { + poll "navidrome" {|| + try { http get -m 5sec $"http://127.0.0.1:($NAV_PORT)/rest/ping.view?u=x&p=x&v=1.13.0&c=test" | ignore; true } catch { null } + } + http post -m 10sec -t application/json $"http://127.0.0.1:($NAV_PORT)/auth/createAdmin" { username: "admin", password: "password" } | ignore + true + } catch { false }) + if not $ready { do -i { job kill $daemon }; error make { msg: "navidrome did not become ready" } } $daemon } @@ -67,9 +68,13 @@ export def start-gonic [work: string] { let daemon = (job spawn { ^gonic -music-path $LIB -listen-addr $"127.0.0.1:($GONIC_PORT)" -db-path ($work | path join "gonic.db") -cache-path ($work | path join "gonic-cache") -playlists-path ($work | path join "gonic-playlists") -podcast-path ($work | path join "gonic-podcasts") -scan-at-start-enabled -http-log=false out+err> ($work | path join "gonic.log") }) - poll "gonic" {|| - try { http get -m 5sec $"http://127.0.0.1:($GONIC_PORT)/rest/ping.view?u=x&p=x&v=1.13.0&c=test" | ignore; true } catch { null } - } + let ready = (try { + poll "gonic" {|| + try { http get -m 5sec $"http://127.0.0.1:($GONIC_PORT)/rest/ping.view?u=x&p=x&v=1.13.0&c=test" | ignore; true } catch { null } + } + true + } catch { false }) + if not $ready { do -i { job kill $daemon }; error make { msg: "gonic did not become ready" } } $daemon } @@ -84,7 +89,7 @@ export def start-lms [work: string] { | str replace --all 'listen-addr = "0.0.0.0"' 'listen-addr = "127.0.0.1"' | save -f $conf - # The first start creates and migrates the database; stop it before seeding, then run for real. + # Seed after the first start has migrated the schema, then run for real. let bootstrap = (job spawn { ^$lms_bin $conf out+err> ($dir | path join "bootstrap.log") }) let ready = (try { poll -n 60 "lms database" {|| @@ -152,8 +157,7 @@ export def start-nextcloud [work: string] { $daemon } -# A second, disposable navidrome over a private copy of two albums. The prune test deletes one -# of them and rescans, so this must not share a library with the servers above. +# The prune test deletes an album from this private copy and rescans, so it must not be shared. export def start-prune [work: string] { let dir = ($work | path join "prune") let music = ($dir | path join "music") @@ -181,8 +185,7 @@ export def start-prune [work: string] { $daemon } -# Wait for a server to index the fixture music, then star an album and artist and add a playlist -# and scrobbles. The Kotlin integration tests assert against this state. +# Wait for the library, then leave the star/playlist/scrobble state the Kotlin tests assert on. export def prepare-fixtures [server: record] { let base = $server.base let user = $server.user diff --git a/tools/integration/subsonic.nu b/tools/integration/subsonic.nu index 7b6aec9f..749ce5a3 100644 --- a/tools/integration/subsonic.nu +++ b/tools/integration/subsonic.nu @@ -1,21 +1,14 @@ -# Thin Subsonic API client used to inspect and prepare the test servers. - -# Nextcloud's Music app reads repeated (array) parameters straight from the raw query string -# without URL-decoding them, so ids must be sent with their unreserved characters intact. -export def url-escape [value: string] { +# Nextcloud reads repeated (array) params from the raw query string without URL-decoding, so ids +# must keep their unreserved characters. +def url-escape [value: string] { $value | url encode | str replace --all '%2D' '-' - | str replace --all '%2d' '-' - | str replace --all '%2E' '.' - | str replace --all '%2e' '.' | str replace --all '%5F' '_' - | str replace --all '%5f' '_' | str replace --all '%7E' '~' - | str replace --all '%7e' '~' } -export def sub-url [base: string, user: string, pass: string, method: string, params: list] { +def sub-url [base: string, user: string, pass: string, method: string, params: list] { let all = ([["u" $user] ["p" $pass] ["v" "1.16.1"] ["c" "subtracks-test"] ["f" "json"]] | append $params) let query = ($all | each {|p| $"($p.0)=(url-escape ($p.1 | into string))" } | str join "&") $"($base)rest/($method).view?($query)" diff --git a/tools/integration/util.nu b/tools/integration/util.nu index 371cda03..1bdf6f6e 100644 --- a/tools/integration/util.nu +++ b/tools/integration/util.nu @@ -1,5 +1,3 @@ -# Shared helpers for the integration harness: waiting and port checks. - export def poll [label: string, check: closure, --attempts (-n): int = 120] { for attempt in 0..<$attempts { let value = (try { do $check } catch { null })