From da17cd8939bbc256db087b36e607c3d657ae2452 Mon Sep 17 00:00:00 2001 From: Vadim Peretokin Date: Tue, 11 Aug 2026 08:08:03 +0200 Subject: [PATCH] infrastructure: test coverage for the IRC configuration and media playback APIs (#9772) #### Brief overview of PR changes/additions - **49 busted specs** for the last uncovered IRC and media functions: the IRC configuration round-trips through the profile with no client and no connection, and both the ordered-argument and table-argument form of every public media call is now exercised, which is what reaches the ~26 private `*AsOrderedArguments`/`*AsTableArgument` helpers. - The **video family gets its first coverage at all** - `playVideoFile`, `getPlayingVideos`, `pauseVideos`, `getPausedVideos`, `stopVideos` and the widget lookup behind them, both when the request's key names a label and when it names nothing. - Two things found on the way and fixed here: busted's `finally()` holds one function rather than a list, so `Media_spec.lua` specs with two things to undo kept only the last (a moved-aside media directory stayed moved, handlers outlived their spec, the speech rate/pitch/volume were never restored); and handing a player a video widget is the only thing in the suite that brings a GL context up, whose driver initialisation leaks unsuppressibly on the leak job's Mesa - so that one spec stands aside there, the way `Other_spec` already does for `show3dMapView`. #### Motivation for adding to Mudlet Part of the Lua API test-coverage programme; this is the residue wave for `net-media-tts`. Two behaviours these specs pin were previously unheld anywhere: the numeric-key protection all fourteen media table parsers carry, and a video request being silently refused when its key matches no widget. Bugs found while writing them, none of them specced (filed separately): a preload that has to download its file then plays it; the load family never sets a media type; `loadSoundFile`/`loadVideoFile` report a missing name as `loadMusicFile`; `playMusicFile`'s ordered fade errors name `playSoundFile`; `setIrcServer` blanks the stored IRC password whenever it is called without one, and rejects an explicit `nil` where it accepts an omission. #### Other info (issues closed, discussion etc) `Networking_spec.lua` is appended to only, and no `mmcp*` function is touched, to stay clear of the open #9744 (Fix: Several identified MMCP issues) which edits the middle of that file. `openIRC` is left pending with its reason: it creates an IRC dialog nothing in the Lua API closes again, after which the getters stop reading the profile from disk for the rest of the run. **Test case:** full busted suite green twice on a fresh profile and twice on a reused one (2477 successes, 0 failures, ~+2s), with leak detection on; 19 sabotage edits were verified to fail 23 of the 49 new specs. Assisted-by: Claude:claude-opus-5 --- src/mudlet-lua/tests/Media_spec.lua | 515 ++++++++++++++++++++++- src/mudlet-lua/tests/Networking_spec.lua | 228 ++++++++++ 2 files changed, 734 insertions(+), 9 deletions(-) diff --git a/src/mudlet-lua/tests/Media_spec.lua b/src/mudlet-lua/tests/Media_spec.lua index 5986cf5b8..7a7b7af79 100644 --- a/src/mudlet-lua/tests/Media_spec.lua +++ b/src/mudlet-lua/tests/Media_spec.lua @@ -171,6 +171,136 @@ describe("Media playback functions validate their parameters", function() end) end) +describe("Media load functions validate their parameters", function() + -- loadMusicFile/loadSoundFile/loadVideoFile are one preload request behind + -- three names: they share a pair of parsers which set no media type at all, + -- so what actually differs between them is the name in their error messages + -- and loadVideoFile taking the table form only. Nothing here names a file + -- that exists, so no preload gets as far as the media engine. + it("each raises a Lua error when called with no arguments", function() + assertArgError(function() loadMusicFile() end, "loadMusicFile: need at least one argument") + assertArgError(function() loadSoundFile() end, "loadSoundFile: need at least one argument") + assertArgError(function() loadVideoFile() end, "loadVideoFile: need at least one argument") + end) + + it("loadVideoFile raises a Lua error when its argument is not a table", function() + -- the video calls take the table form only + assertArgError(function() loadVideoFile("busted-media-absent.mkv") end, "loadVideoFile: needs to be a table") + end) + + it("the ordered form returns nil when it is given no file name", function() + local ok, err = loadSoundFile(nil) + assert.is_nil(ok) + assert.is_true(contains(err, "missing argument 1"), tostring(err)) + + ok, err = loadMusicFile("") + assert.is_nil(ok) + assert.is_true(contains(err, "missing argument 1"), tostring(err)) + end) + + it("the table form raises a Lua error when it is given no name", function() + -- Only the tail of the message: all three loads report this one as + -- loadMusicFile, whichever was called, and pinning that here would hold + -- that in place. + assertArgError(function() loadSoundFile({}) end, "missing name") + end) + + it("the ordered form raises a Lua error when the url is not a string", function() + assertArgError(function() loadSoundFile("busted-media-absent.wav", {}) end, "url as string expected, got table!") + end) + + it("the table form raises a Lua error for a wrongly typed name or url", function() + assertArgError(function() loadMusicFile({name = {}}) end, "value for name as string expected, got table!") + assertArgError(function() loadMusicFile({name = "busted-media-absent.mp3", url = {}}) end, "value for url as string expected, got table!") + end) +end) + +describe("Media query and stop functions validate their parameters", function() + -- The video calls, the pause calls and the paused-media queries take a table + -- and nothing else; the sound and music queries and stops take either form. + -- pauseSounds and pauseMusic have this same refusal checked above + local tableOnly = { + "getPlayingVideos", "getPausedSounds", "getPausedMusic", "getPausedVideos", + "pauseVideos", "stopVideos", + } + + for _, fnName in ipairs(tableOnly) do + it(fnName .. " raises a Lua error when its argument is not a table", function() + assertArgError(function() _G[fnName](5) end, fnName .. ": needs to be a table") + end) + end + + it("the ordered query forms raise a Lua error for a wrongly typed filter", function() + assertArgError(function() getPlayingSounds("busted-media-absent.wav", {}) end, "key as string expected, got table!") + assertArgError(function() getPlayingSounds("busted-media-absent.wav", "k", {}) end, "tag as string expected, got table!") + assertArgError(function() getPlayingSounds("busted-media-absent.wav", "k", "t", "loud") end, "priority as number expected, got string!") + assertArgError(function() getPlayingMusic("busted-media-absent.mp3", {}) end, "key as string expected, got table!") + end) + + it("the table query forms raise a Lua error for a wrongly typed filter", function() + assertArgError(function() getPlayingMusic({name = {}}) end, "value for name as string expected, got table!") + assertArgError(function() getPausedSounds({key = {}}) end, "value for key as string expected, got table!") + assertArgError(function() getPausedMusic({tag = {}}) end, "value for tag as string expected, got table!") + assertArgError(function() getPausedVideos({name = {}}) end, "value for name as string expected, got table!") + assertArgError(function() getPlayingVideos({key = {}}) end, "value for key as string expected, got table!") + end) + + it("the ordered stop forms raise a Lua error for a wrongly typed argument", function() + assertArgError(function() stopSounds("busted-media-absent.wav", "k", "t", "loud") end, "priority as number expected, got string!") + assertArgError(function() stopSounds("busted-media-absent.wav", "k", "t", 10, "yes") end, "fadeaway as boolean expected, got string!") + assertArgError(function() stopMusic("busted-media-absent.mp3", "k", "t", "yes") end, "fadeaway as boolean expected, got string!") + assertArgError(function() stopMusic("busted-media-absent.mp3", "k", "t", true, -1) end, "bad argument range for fadeout") + end) + + it("the table pause and stop forms raise a Lua error for a wrongly typed filter", function() + assertArgError(function() pauseSounds({name = {}}) end, "value for name as string expected, got table!") + assertArgError(function() pauseMusic({key = {}}) end, "value for key as string expected, got table!") + assertArgError(function() pauseVideos({tag = {}}) end, "value for tag as string expected, got table!") + assertArgError(function() stopVideos({name = {}}) end, "value for name as string expected, got table!") + end) + + it("the ordered play forms raise a Lua error for a wrongly typed argument", function() + assertArgError(function() playMusicFile("busted-media-absent.mp3", {}) end, "volume as number expected, got table!") + assertArgError(function() playMusicFile("busted-media-absent.mp3", 50, 0, 0, 0, 1, {}) end, "key as string expected, got table!") + assertArgError(function() playSoundFile("busted-media-absent.wav", 50, 0, 0, 0, 1, "k", {}) end, "tag as string expected, got table!") + end) + + it("a numeric key in a table argument does not stop the rest of it being read", function() + -- Reading a numeric key with lua_tostring() converts it in place, and the + -- step of the iteration that follows then refuses the key it is handed, so + -- every table parser reads its keys from a copy. Lua walks a table's array + -- part first, which puts the numeric key ahead of the named ones here. + assert.is_true(playSoundFile({[1] = "junk", name = "busted-media-absent.wav"})) + assert.is_true(stopSounds({[1] = "junk", key = "busted-media-no-such-key"})) + assert.is_table(getPlayingMusic({[1] = "junk", name = "busted-media-absent.mp3"})) + assert.is_table(getPausedVideos({[1] = "junk", key = "busted-media-no-such-key"})) + end) + + it("the ordered play forms refuse a negative fade", function() + -- Only the range refusal, not the whole message: the music parser's fade + -- messages name playSoundFile, and pinning that here would hold it in + -- place. + assertArgError(function() playMusicFile("busted-media-absent.mp3", 50, -1) end, "bad argument range for fadein") + assertArgError(function() playSoundFile("busted-media-absent.wav", 50, 0, -1) end, "bad argument range for fadeout") + end) + + it("every query returns an empty table while nothing is playing", function() + -- with everything stopped, each of the six queries answers with a table + -- rather than with nil or a false-plus-message pair + assert.is_true(stopSounds()) + assert.is_true(stopMusic()) + assert.is_true(stopVideos()) + + for _, query in ipairs({getPlayingSounds, getPlayingMusic, getPlayingVideos, getPausedSounds, getPausedMusic, getPausedVideos}) do + local result = query() + assert.is_table(result) + assert.equals(0, #result) + -- and the same with a filter that matches nothing + assert.same({}, query({key = "busted-media-no-such-key"})) + end + end) +end) + describe("Media playback effects with a generated sound file", function() -- The API media functions play files out of the profile's own media -- directory, so instead of shipping a binary fixture these specs write a @@ -229,6 +359,18 @@ describe("Media playback effects with a generated sound file", function() writeMediaFile(otherLongSoundFile, 10000) end + -- Cleanups to run at the end of the current spec. busted's finally() holds + -- one function rather than a list (busted/init.lua: `env.finally = + -- function(fn) finally = fn end`), so a spec that has two things to undo - + -- and several here do - would keep only the last of them. after_each drains + -- this instead, in reverse, and runs whatever a failed spec got as far as + -- registering. + local cleanups = {} + + local function onCleanup(undo) + cleanups[#cleanups + 1] = undo + end + -- purgeMediaCache() empties the whole media directory, not just the fixtures -- these specs wrote, and the self-test profile persists between runs on a -- developer's machine. Anything else already in there is moved aside for the @@ -248,7 +390,7 @@ describe("Media playback effects with a generated sound file", function() for _, entry in ipairs(preserved) do os.rename(mediaDirectory .. "/" .. entry, stash .. "/" .. entry) end - finally(function() + onCleanup(function() lfs.mkdir(mediaDirectory) for _, entry in ipairs(preserved) do os.rename(stash .. "/" .. entry, mediaDirectory .. "/" .. entry) @@ -265,7 +407,7 @@ describe("Media playback effects with a generated sound file", function() local handler = registerAnonymousEventHandler(eventName, function(_, file, path, mediaType, key, tag) into[#into + 1] = {file = file, path = path, mediaType = mediaType, key = key, tag = tag} end) - finally(function() killAnonymousEventHandler(handler) end) + onCleanup(function() killAnonymousEventHandler(handler) end) end -- Waits until collected holds count entries. A media event can be raised @@ -317,9 +459,93 @@ describe("Media playback effects with a generated sound file", function() return true end + -- The fixture server of CI/http-fixture-server.py, when the harness started + -- one and handed its ephemeral port over. A preload's only observable effect + -- is the fetch it starts for a file the profile does not have, so the two + -- load specs below are the media ones that need a server to talk to. + local httpPort = os.getenv("MUDLET_TEST_HTTP_PORT") + local requireFixture = os.getenv("MUDLET_TEST_REQUIRE_HTTP_FIXTURE") + -- the file CI/http-fixtures/ serves, and its contents + local fixtureFile = "fixture.txt" + local fixtureBody = "Mudlet self-test HTTP fixture.\n" + + local function noFixtureServer() + if httpPort then + return false + end + if requireFixture then + assert.is_true(false, "MUDLET_TEST_REQUIRE_HTTP_FIXTURE is set but MUDLET_TEST_HTTP_PORT is not - the fixture server was not started") + end + pending("no local HTTP fixture server (set MUDLET_TEST_HTTP_PORT)") + return true + end + + -- The url a media request is given is a directory: TMedia appends the file + -- name to it. + local function fixtureUrl() + return "http://127.0.0.1:" .. httpPort + end + + local function readFile(path) + local handle = io.open(path, "rb") + if not handle then + return nil + end + local contents = handle:read("*a") + handle:close() + return contents + end + + -- Video playback draws into a widget the request names with its key: + -- TMainConsole::setupVideoOutput() looks that key up among the profile's + -- labels and user windows, and refuses the request when it finds neither. The + -- label is not deleted afterwards, because the player that was handed its + -- video widget outlives the spec - it is only hidden again, so it does not + -- sit over the main console for every spec that runs later. + local videoLabel = "busted-media-video-label" + local videoLabelReady + + -- Handing a player that widget is the only thing this suite does that brings + -- a GL context up: Qt loads its XCB GL integration, and Mesa initialises and + -- then - at shutdown, with the context - unloads a driver. On the leak job's + -- Mesa that driver initialisation leaks around 240 bytes, and by the time + -- LeakSanitizer looks, the library holding the allocating frame is gone, so + -- no leak: line in asan-suppressions.txt can name it. That file asks for + -- exactly this: keep the context from being created test-side, which is also + -- why Other_spec leaves show3dMapView alone. The refusal spec below needs no + -- widget and no context, and every leg without leak checking - Windows CI and + -- a developer's own run - still plays the video. + local leakChecked = (os.getenv("ASAN_OPTIONS") or ""):find("detect_leaks=1", 1, true) ~= nil + + local function videoWidgetUnavailable() + if leakChecked then + pending("a video widget's GL context leaks in this job's GL driver, where nothing is left to suppress by name") + return true + end + return false + end + + local function withVideoLabel() + if not videoLabelReady then + createLabel(videoLabel, 0, 0, 40, 40, 1) + assert.equals("label", windowType(videoLabel)) + videoLabelReady = true + end + onCleanup(function() hideWindow(videoLabel) end) + end + after_each(function() + -- before the stops below, not after: a spec's own event handlers have to + -- be gone before anything raises sysMediaFinished at them, or a handler + -- that starts a sound of its own leaves one playing into the next spec + for index = #cleanups, 1, -1 do + cleanups[index]() + end + cleanups = {} + stopSounds() stopMusic() + stopVideos() end) it("playSoundFile plays the file and reports it from start to finish", function() @@ -564,7 +790,7 @@ describe("Media playback effects with a generated sound file", function() reentered = reentered + 1 playSoundFile({name = otherLongSoundFile, key = "busted-handler-sound"}) end) - finally(function() killAnonymousEventHandler(handler) end) + onCleanup(function() killAnonymousEventHandler(handler) end) assert.is_true(playSoundFile({name = longSoundFile, key = "busted-quiet", priority = 10})) -- stops the sound above while it is still loading, which raises @@ -682,7 +908,7 @@ describe("Media playback effects with a generated sound file", function() handle:write("pinned") handle:close() os.execute("chmod 500 '" .. lockedDirectory .. "'") - finally(function() + onCleanup(function() os.execute("chmod 700 '" .. lockedDirectory .. "'") os.remove(pinnedFile) lfs.rmdir(lockedDirectory) @@ -705,7 +931,7 @@ describe("Media playback effects with a generated sound file", function() local handler = registerAnonymousEventHandler("sysDownloadError", function(_, message, path) errors[#errors + 1] = {message = message, path = path} end) - finally(function() killAnonymousEventHandler(handler) end) + onCleanup(function() killAnonymousEventHandler(handler) end) -- a file the media directory does not have, so the url is the only way to get it assert.is_true(playSoundFile({name = "busted-media-absent-scheme.wav", url = "ftp://example.invalid/sounds"})) @@ -714,6 +940,264 @@ describe("Media playback effects with a generated sound file", function() assert.is_true(contains(errors[1].message, "http"), tostring(errors[1].message)) assert.is_true(contains(errors[1].path, "busted-media-absent-scheme.wav"), tostring(errors[1].path)) end) + + it("loadSoundFile fetches a file the media directory does not have and keeps it", function() + if noFixtureServer() then + return + end + local downloaded = mediaDirectory .. "/" .. fixtureFile + lfs.mkdir(mediaDirectory) + -- the download has to be the only file of that name, and a reused profile + -- may well have one of its own already + preserveMediaDirectory() + os.remove(downloaded) + onCleanup(function() os.remove(downloaded) end) + + local done = {} + collect("sysDownloadDone", done) + assert.is_true(loadSoundFile({name = fixtureFile, url = fixtureUrl()})) + waitForCount("sysDownloadDone", done, 1) + + assert.equals(1, #done) + assert.equals(fixtureBody, readFile(downloaded)) + end) + + it("loadMusicFile fetches from the url given in the ordered argument form", function() + if noFixtureServer() then + return + end + -- name[,url]: the ordered form has a parser of its own + local downloaded = mediaDirectory .. "/" .. fixtureFile + lfs.mkdir(mediaDirectory) + -- the download has to be the only file of that name, and a reused profile + -- may well have one of its own already + preserveMediaDirectory() + os.remove(downloaded) + onCleanup(function() os.remove(downloaded) end) + + local done = {} + collect("sysDownloadDone", done) + assert.is_true(loadMusicFile(fixtureFile, fixtureUrl())) + waitForCount("sysDownloadDone", done, 1) + + assert.equals(1, #done) + assert.equals(fixtureBody, readFile(downloaded)) + end) + + it("loadVideoFile reports a download error for a url it cannot fetch from", function() + -- The preload reaches the same fetch as a play would, so the refusal of a + -- url that is not http(s) is where a spec can see a load act on its url + -- without a server to answer it. + local errors = {} + local handler = registerAnonymousEventHandler("sysDownloadError", function(_, message, path) + errors[#errors + 1] = {message = message, path = path} + end) + onCleanup(function() killAnonymousEventHandler(handler) end) + + assert.is_true(loadVideoFile({name = "busted-media-absent-load.mkv", url = "ftp://example.invalid/videos"})) + waitForCount("sysDownloadError", errors, 1) + + -- picked out by name rather than by position: the collector sees every + -- download error, not only this one's + local reported + for _, failure in ipairs(errors) do + if contains(failure.path, "busted-media-absent-load.mkv") then + reported = failure + end + end + assert.is_not_nil(reported, "no download error named the file the load asked for") + assert.is_true(contains(reported.message, "http"), tostring(reported.message)) + end) + + it("playMusicFile starts a track given in the ordered argument form", function() + if mediaPlaybackUnavailable() then + return + end + writeSoundFiles() + -- name[,volume][,fadein][,fadeout][,start][,loops][,key][,tag] + assert.is_true(playMusicFile(longSoundFile, 70, 0, 0, 0, 1, "busted-music-ordered", "busted-music-ordered-tag")) + assert.equals("sysMediaStarted", (waitForEvent("sysMediaStarted", 5000))) + + local music = getPlayingMusic() + assert.equals(1, #music) + assert.equals(longSoundFile, music[1].name) + assert.equals(70, music[1].volume) + assert.equals("busted-music-ordered", music[1].key) + assert.equals("busted-music-ordered-tag", music[1].tag) + end) + + it("getPlayingMusic filters by name in both argument forms", function() + if mediaPlaybackUnavailable() then + return + end + writeSoundFiles() + assert.is_true(playMusicFile({name = longSoundFile, key = "busted-music-filter"})) + assert.equals("sysMediaStarted", (waitForEvent("sysMediaStarted", 5000))) + + -- name[,key][,tag] as ordered arguments + assert.equals(1, #getPlayingMusic(longSoundFile)) + assert.equals(1, #getPlayingMusic(longSoundFile, "busted-music-filter")) + assert.equals(0, #getPlayingMusic(longSoundFile, "busted-music-elsewhere")) + assert.equals(0, #getPlayingMusic(otherLongSoundFile)) + -- and the same filters as a table + assert.equals(1, #getPlayingMusic({name = longSoundFile})) + assert.equals(0, #getPlayingMusic({key = "busted-music-elsewhere"})) + end) + + it("getPlayingSounds filters by name, key and tag in the ordered argument form", function() + if mediaPlaybackUnavailable() then + return + end + writeSoundFiles() + assert.is_true(playSoundFile({name = longSoundFile, key = "busted-ordered-key", tag = "busted-ordered-tag"})) + assert.equals("sysMediaStarted", (waitForEvent("sysMediaStarted", 5000))) + + -- name[,key][,tag][,priority] + assert.equals(1, #getPlayingSounds(longSoundFile)) + assert.equals(1, #getPlayingSounds(longSoundFile, "busted-ordered-key", "busted-ordered-tag")) + assert.equals(0, #getPlayingSounds(longSoundFile, "busted-ordered-key", "busted-other-tag")) + assert.equals(0, #getPlayingSounds(otherLongSoundFile)) + end) + + it("stopSounds stops only the sound named in the ordered argument form", function() + if mediaPlaybackUnavailable() then + return + end + local started = {} + collect("sysMediaStarted", started) + + writeSoundFiles() + assert.is_true(playSoundFile({name = longSoundFile, key = "busted-stop-named"})) + waitForCount("sysMediaStarted", started, 1) + assert.is_true(playSoundFile({name = otherLongSoundFile, key = "busted-stop-spared"})) + waitForCount("sysMediaStarted", started, 2) + assert.equals(2, #getPlayingSounds()) + + -- name[,key][,tag][,priority][,fadeaway][,fadeout] + assert.is_true(stopSounds(longSoundFile)) + local playing = getPlayingSounds() + assert.equals(1, #playing) + assert.equals(otherLongSoundFile, playing[1].name) + end) + + it("stopMusic stops only the track named in the ordered argument form", function() + if mediaPlaybackUnavailable() then + return + end + local started = {} + collect("sysMediaStarted", started) + + writeSoundFiles() + assert.is_true(playMusicFile({name = longSoundFile, key = "busted-music-stop-named"})) + waitForCount("sysMediaStarted", started, 1) + assert.is_true(playMusicFile({name = otherLongSoundFile, key = "busted-music-stop-spared"})) + waitForCount("sysMediaStarted", started, 2) + assert.equals(2, #getPlayingMusic()) + + -- name[,key][,tag][,fadeaway][,fadeout] + assert.is_true(stopMusic(longSoundFile)) + local music = getPlayingMusic() + assert.equals(1, #music) + assert.equals(otherLongSoundFile, music[1].name) + end) + + it("pauseMusic and getPausedMusic take the same key filter", function() + if mediaPlaybackUnavailable() then + return + end + local started = {} + collect("sysMediaStarted", started) + + writeSoundFiles() + assert.is_true(playMusicFile({name = longSoundFile, key = "busted-music-parked-key"})) + waitForCount("sysMediaStarted", started, 1) + assert.is_true(playMusicFile({name = otherLongSoundFile, key = "busted-music-playing-key"})) + waitForCount("sysMediaStarted", started, 2) + + assert.is_true(pauseMusic({key = "busted-music-parked-key"})) + assert.equals(1, #getPlayingMusic()) + local paused = getPausedMusic() + assert.equals(1, #paused) + assert.equals(longSoundFile, paused[1].name) + assert.equals(1, #getPausedMusic({key = "busted-music-parked-key"})) + assert.equals(0, #getPausedMusic({key = "busted-music-playing-key"})) + end) + + it("getPausedSounds takes the same key filter as the sound that was paused", function() + if mediaPlaybackUnavailable() then + return + end + local started = {} + collect("sysMediaStarted", started) + + writeSoundFiles() + assert.is_true(playSoundFile({name = longSoundFile, key = "busted-sound-parked-key"})) + waitForCount("sysMediaStarted", started, 1) + assert.is_true(pauseSounds({key = "busted-sound-parked-key"})) + + assert.equals(1, #getPausedSounds({key = "busted-sound-parked-key"})) + assert.equals(0, #getPausedSounds({key = "busted-sound-never-played"})) + assert.equals(0, #getPausedSounds({name = otherLongSoundFile})) + end) + + it("playVideoFile plays into the label its key names and the video family reports it", function() + if videoWidgetUnavailable() or mediaPlaybackUnavailable() then + return + end + -- The file is the same silent WAV the sound specs use: what makes this a + -- video request is the type it is made as, which is what decides the widget + -- setup, the list it is tracked in and the media type its events carry. A + -- decodable picture would only change what the video widget draws. + withVideoLabel() + writeSoundFiles() + assert.equals(0, #getPlayingVideos()) + + assert.is_true(playVideoFile({name = longSoundFile, key = videoLabel, tag = "busted-video-tag"})) + local event, file, _, mediaType, key, tag = waitForEvent("sysMediaStarted", 5000) + assert.equals("sysMediaStarted", event) + assert.equals(longSoundFile, file) + assert.equals("video", mediaType) + assert.equals(videoLabel, key) + assert.equals("busted-video-tag", tag) + + local playing = getPlayingVideos() + assert.equals(1, #playing) + assert.equals(longSoundFile, playing[1].name) + assert.equals(videoLabel, playing[1].key) + -- videos are tracked apart from sounds and music + assert.equals(0, #getPlayingSounds()) + assert.equals(0, #getPlayingMusic()) + assert.equals(1, #getPlayingVideos({key = videoLabel})) + assert.equals(0, #getPlayingVideos({key = "busted-video-other-key"})) + + assert.is_true(pauseVideos()) + assert.equals(0, #getPlayingVideos()) + local paused = getPausedVideos() + assert.equals(1, #paused) + assert.equals(longSoundFile, paused[1].name) + assert.equals(1, #getPausedVideos({name = longSoundFile})) + + -- resumed by playing the same file again, like sounds and music are + assert.is_true(playVideoFile({name = longSoundFile, key = videoLabel})) + assert.equals(1, #getPlayingVideos()) + assert.equals(0, #getPausedVideos()) + + assert.is_true(stopVideos()) + assert.equals(0, #getPlayingVideos()) + end) + + it("playVideoFile starts nothing when its key names no widget to draw into", function() + if mediaPlaybackUnavailable() then + return + end + writeSoundFiles() + -- The request is understood, so it reports success; the widget lookup then + -- turns up nothing and the playback never starts. Nothing but the video + -- list says so, which is why this is worth holding to. + assert.is_true(playVideoFile({name = longSoundFile, key = "busted-media-no-such-widget"})) + assert.equals(0, #getPlayingVideos()) + assert.equals(0, #getPausedVideos()) + end) end) describe("receiveMSP reports MSP is not enabled while offline", function() @@ -783,6 +1267,15 @@ describe("Tests the text-to-speech Lua API", function() return true end + -- Undone at the end of the current spec, for the same reason the media + -- specs above keep a list: busted's finally() holds one function, not a + -- list, and these specs have several things to put back. + local cleanups = {} + + local function onCleanup(undo) + cleanups[#cleanups + 1] = undo + end + -- Collects every occurrence of an event for the duration of one spec. -- The mock engine changes state inside the ttsSpeak()/ttsSkip() call -- itself, so the matching event is raised before a waitForEvent() could @@ -791,7 +1284,7 @@ describe("Tests the text-to-speech Lua API", function() local handler = registerAnonymousEventHandler(eventName, function(_, first) into[#into + 1] = first == nil and true or first end) - finally(function() killAnonymousEventHandler(handler) end) + onCleanup(function() killAnonymousEventHandler(handler) end) end -- The mock engine speaks in real time at roughly a tenth of a second per @@ -803,6 +1296,10 @@ describe("Tests the text-to-speech Lua API", function() ttsClearQueue() ttsSkip() end + for index = #cleanups, 1, -1 do + cleanups[index]() + end + cleanups = {} end) it("ttsSpeak rejects whitespace-only text", function() @@ -1092,7 +1589,7 @@ describe("Tests the text-to-speech Lua API", function() collect("ttsPitchChanged", pitches) collect("ttsVolumeChanged", volumes) local rate, pitch, volume = ttsGetRate(), ttsGetPitch(), ttsGetVolume() - finally(function() + onCleanup(function() ttsSetRate(rate) ttsSetPitch(pitch) ttsSetVolume(volume) @@ -1132,7 +1629,7 @@ describe("Tests the text-to-speech Lua API", function() local changes = {} local originalVoice = ttsGetCurrentVoice() collect("ttsVoiceChanged", changes) - finally(function() ttsSetVoiceByName(originalVoice) end) + onCleanup(function() ttsSetVoiceByName(originalVoice) end) assert.is_true(ttsSetVoiceByName(voices[2])) assert.equals(voices[2], ttsGetCurrentVoice()) @@ -1239,7 +1736,7 @@ describe("Tests the text-to-speech Lua API", function() local changes = {} collect("ttsVoiceChanged", changes) local originalVoice = ttsGetCurrentVoice() - finally(function() ttsSetVoiceByName(originalVoice) end) + onCleanup(function() ttsSetVoiceByName(originalVoice) end) assert.is_true(ttsSetVoiceByName(voices[2])) assert.equals(voices[2], ttsGetCurrentVoice()) diff --git a/src/mudlet-lua/tests/Networking_spec.lua b/src/mudlet-lua/tests/Networking_spec.lua index df3975d2b..677fe190d 100644 --- a/src/mudlet-lua/tests/Networking_spec.lua +++ b/src/mudlet-lua/tests/Networking_spec.lua @@ -1736,3 +1736,231 @@ describe("Discord Lua API availability contract", function() end) end) end) + +describe("The IRC configuration functions round-trip through the profile", function() + -- While a profile has no IRC dialog - none of these specs opens one - the + -- getters read the profile's own configuration off disk, which is what the + -- setters write to. So the round trip is testable with no IRC server and no + -- connection anywhere in sight. + -- + -- The profile's own IRC configuration is put back afterwards, because the + -- self-test profile is reused between runs. Two things the restore cannot + -- reach, both of which matter to a developer running the suite against a + -- config root that is not a throwaway one: + -- + -- - the IRC password. setIrcServer() writes it on every call and blanks it + -- when none is passed, and no getter reads it back, so any password the + -- profile had is gone either way. + -- - the last-used nick, which setIrcNick() also writes to a file shared by + -- every profile (mudlet's data directory, not the profile's). Putting the + -- profile's nick back writes that file again rather than restoring it. + local function restoreIrcConfiguration() + local nick = getIrcNick() + local hostName, port, secure = getIrcServer() + local channels = getIrcChannels() + finally(function() + setIrcNick(nick) + setIrcServer(hostName, port, secure) + setIrcChannels(channels) + end) + end + + describe("getIrcNick, getIrcServer and getIrcChannels", function() + it("report a nick, a server and a channel list without an IRC client", function() + -- with nothing configured each getter falls back to a built-in default + -- rather than to nil, which is what makes them safe to read before + -- anything has been set + local nick = getIrcNick() + assert.is_string(nick) + assert.is_true(#nick > 0) + + local hostName, port, secure = getIrcServer() + assert.is_string(hostName) + assert.is_true(#hostName > 0) + assert.is_number(port) + assert.is_true(port >= 1 and port <= 65535, tostring(port)) + assert.is_boolean(secure) + + local channels = getIrcChannels() + assert.is_table(channels) + assert.is_true(#channels > 0) + for _, channel in ipairs(channels) do + assert.is_string(channel) + end + end) + end) + + describe("setIrcNick", function() + it("raises a Lua error when the nick is missing or not a string", function() + assertArgError(function() setIrcNick() end, "setIrcNick: bad argument #1 type (nick as string expected") + assertArgError(function() setIrcNick({}) end, "setIrcNick: bad argument #1 type (nick as string expected, got table!)") + end) + + it("returns nil and a message for an empty nick, leaving the stored one alone", function() + restoreIrcConfiguration() + assert.is_true(setIrcNick("BustedKeptNick")) + + local ok, err = setIrcNick("") + assert.is_nil(ok) + assert.is_true(contains(err, "nick must not be empty"), tostring(err)) + assert.equals("BustedKeptNick", getIrcNick()) + end) + + it("stores the nick where getIrcNick reads it back", function() + restoreIrcConfiguration() + assert.is_true(setIrcNick("BustedNickOne")) + assert.equals("BustedNickOne", getIrcNick()) + + assert.is_true(setIrcNick("BustedNickTwo")) + assert.equals("BustedNickTwo", getIrcNick()) + end) + end) + + describe("setIrcServer", function() + it("raises a Lua error when the hostname or an optional argument is wrongly typed", function() + assertArgError(function() setIrcServer() end, "setIrcServer: bad argument #1 type (hostname as string expected") + assertArgError(function() setIrcServer({}) end, "setIrcServer: bad argument #1 type (hostname as string expected, got table!)") + assertArgError(function() setIrcServer("irc.busted.invalid", {}) end, "port number") + assertArgError(function() setIrcServer("irc.busted.invalid", 6667, "yes") end, "secure") + assertArgError(function() setIrcServer("irc.busted.invalid", 6667, false, {}) end, "server password") + end) + + it("returns nil and a message for an empty hostname or an out-of-range port", function() + restoreIrcConfiguration() + assert.is_true(setIrcServer("irc.busted-kept.invalid", 6690)) + + local ok, err = setIrcServer("") + assert.is_nil(ok) + assert.is_true(contains(err, "hostname must not be empty"), tostring(err)) + + ok, err = setIrcServer("irc.busted.invalid", 70000) + assert.is_nil(ok) + assert.is_true(contains(err, "invalid port number 70000"), tostring(err)) + + ok, err = setIrcServer("irc.busted.invalid", 0) + assert.is_nil(ok) + assert.is_true(contains(err, "invalid port number 0"), tostring(err)) + + -- a refused call stored nothing + local hostName, port = getIrcServer() + assert.equals("irc.busted-kept.invalid", hostName) + assert.equals(6690, port) + end) + + it("stores the hostname, port and secure flag where getIrcServer reads them back", function() + restoreIrcConfiguration() + -- it reports success as true plus a nil second value + local ok, extra = setIrcServer("irc.busted-one.invalid", 6697, true) + assert.is_true(ok) + assert.is_nil(extra) + + local hostName, port, secure = getIrcServer() + assert.equals("irc.busted-one.invalid", hostName) + assert.equals(6697, port) + assert.is_true(secure) + + -- the secure flag is stored, not merely defaulted: turn it back off + assert.is_true(setIrcServer("irc.busted-two.invalid", 6668, false)) + hostName, port, secure = getIrcServer() + assert.equals("irc.busted-two.invalid", hostName) + assert.equals(6668, port) + assert.is_false(secure) + end) + + it("falls back to port 6667 and an insecure connection when only a hostname is given", function() + restoreIrcConfiguration() + assert.is_true(setIrcServer("irc.busted-secure.invalid", 6697, true)) + + assert.is_true(setIrcServer("irc.busted-default.invalid")) + local hostName, port, secure = getIrcServer() + assert.equals("irc.busted-default.invalid", hostName) + assert.equals(6667, port) + assert.is_false(secure) + end) + end) + + describe("setIrcChannels", function() + it("raises a Lua error when the channels are not a table", function() + assertArgError(function() setIrcChannels("#mudlet") end, "setIrcChannels: bad argument #1 type (channels as table expected, got string!)") + assertArgError(function() setIrcChannels() end, "setIrcChannels: bad argument #1 type (channels as table expected, got no value!)") + end) + + it("returns nil and a message when no entry is a usable channel name", function() + restoreIrcConfiguration() + assert.is_true(setIrcChannels({"#busted-kept"})) + + local ok, err = setIrcChannels({}) + assert.is_nil(ok) + assert.is_true(contains(err, "no (valid) channel names provided"), tostring(err)) + + -- a channel name has to start with #, & or +, and only strings are read + ok, err = setIrcChannels({"mudlet", 42, ""}) + assert.is_nil(ok) + assert.is_true(contains(err, "no (valid) channel names provided"), tostring(err)) + assert.same({"#busted-kept"}, getIrcChannels()) + end) + + it("stores the channel list where getIrcChannels reads it back", function() + restoreIrcConfiguration() + assert.is_true(setIrcChannels({"#busted-one", "&busted-two", "+busted-three"})) + assert.same({"#busted-one", "&busted-two", "+busted-three"}, getIrcChannels()) + end) + + it("keeps the usable channel names out of a mixed list and drops the rest", function() + restoreIrcConfiguration() + assert.is_true(setIrcChannels({"#busted-good", "busted-bad", "&busted-also-good"})) + assert.same({"#busted-good", "&busted-also-good"}, getIrcChannels()) + end) + end) + + describe("getIrcConnectedHost and restartIrc without a client", function() + -- Both of these read whether the profile has an IRC dialog, and nothing in + -- the suite creates one - see the openIRC spec below for why. Should + -- something start doing so, these are where it shows up first. + it("getIrcConnectedHost returns false and says there is no client", function() + local ok, err = getIrcConnectedHost() + assert.is_false(ok) + assert.equals("no client active", err) + end) + + it("restartIrc returns false", function() + -- there is no client to restart, and it says so by returning false + -- rather than by opening one + assert.is_false(restartIrc(), "something in this run opened an IRC client") + end) + end) + + describe("sendIrc", function() + -- Both arguments are checked before the IRC dialog would be created, so + -- these calls open no client. A well-formed sendIrc() does create one, + -- which is why there is no spec here for the delivery path. + it("raises a Lua error when the target or the message is missing or wrongly typed", function() + assertArgError(function() sendIrc() end, "sendIrc: bad argument #1 type (target as string expected") + assertArgError(function() sendIrc("#mudlet") end, "sendIrc: bad argument #2 type (message as string expected") + assertArgError(function() sendIrc({}, "hello") end, "sendIrc: bad argument #1 type (target as string expected, got table!)") + assertArgError(function() sendIrc("#mudlet", {}) end, "sendIrc: bad argument #2 type (message as string expected, got table!)") + end) + end) + + describe("openIRC", function() + it("opens the IRC client window", function() + pending("openIRC creates the profile's IRC dialog and nothing in the Lua API closes it again. " + .. "From then on the getters answer out of the copy the dialog read when it was constructed - " + .. "a setIrcNick() while it is open is not seen by getIrcNick() until restartIrc() - so the " + .. "round trips above would stop working for the rest of the run, and the dialog dials the " + .. "configured server and raises a window over the specs that follow") + end) + end) +end) + +describe("getNetworkLatency", function() + it("reports zero on a profile whose game socket has never been timed", function() + -- The latency is measured between a command going out and the prompt that + -- answers it, and nothing in the suite connects the game socket - so the + -- untouched value is what this reads, which is also what pins it to the + -- right member. A meaningful reading needs a game server. + local latency = getNetworkLatency() + assert.is_number(latency) + assert.equals(0, latency) + end) +end)