From 183c3e2c856c41306b9754bd51ce05589cf1d773 Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 13:06:40 +0000 Subject: [PATCH 1/7] Add HTTP characterization test suite pytest + tornado AsyncHTTPTestCase harness running against the real web.Application with a faked audio backend (no JACK, mod-host or hardware needed). Covers /files/list behavior (type mapping, extension filtering, subfolder recursion, ordering, per-request freshness), the index page render, and the CORS header conventions. Co-Authored-By: Claude Fable 5 --- README.rst | 23 +++++++ pytest.ini | 3 + test-requirements.txt | 4 ++ test/base.py | 78 ++++++++++++++++++++++++ test/conftest.py | 129 ++++++++++++++++++++++++++++++++++++++++ test/test_cors.py | 40 +++++++++++++ test/test_files_list.py | 109 +++++++++++++++++++++++++++++++++ test/test_index.py | 42 +++++++++++++ 8 files changed, 428 insertions(+) create mode 100644 pytest.ini create mode 100644 test-requirements.txt create mode 100644 test/base.py create mode 100644 test/conftest.py create mode 100644 test/test_cors.py create mode 100644 test/test_files_list.py create mode 100644 test/test_index.py diff --git a/README.rst b/README.rst index 8c9ac1da5..029b5057a 100644 --- a/README.rst +++ b/README.rst @@ -58,3 +58,26 @@ And now you are ready to start the webserver:: Setting the environment variables is needed when developing on a PC. Open your browser and point to http://localhost:8888/. + +Test +---- + +The test suite in ``test/`` contains HTTP-level characterization tests (pytest + tornado's testing tools). +They run against a faked audio backend, so no JACK, mod-host or MOD hardware is needed. + +First complete the Install section above (virtualenv, python requirements and ``make -C utils`` — the tests +import the webserver, which requires ``utils/libmod_utils.so``). + +On Python 3.10 or newer, the pinned tornado 4.3 needs a one-time patch (see the note in ``requirements.txt``):: + + $ sed -i 's/collections.MutableMapping/collections.abc.MutableMapping/' modui-env/lib/python3.*/site-packages/tornado/httputil.py + +Install the test requirements and run the suite from the repository root:: + + $ source modui-env/bin/activate + $ pip3 install -r test-requirements.txt + $ pytest + +NOTE: ``test/hmi-protocol-integrationtest.py`` is not part of this suite — it is a standalone integration +test for the HMI serial protocol that requires JACK, mod-host and a serial device, and pytest does not +collect it. diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 000000000..3b84f2109 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +testpaths = test +python_files = test_*.py diff --git a/test-requirements.txt b/test-requirements.txt new file mode 100644 index 000000000..6957b218d --- /dev/null +++ b/test-requirements.txt @@ -0,0 +1,4 @@ +pytest + +# Python 3.12 removed ssl.match_hostname, which tornado 4.3 falls back to importing from this package +backports.ssl_match_hostname; python_version >= "3.12" diff --git a/test/base.py b/test/base.py new file mode 100644 index 000000000..958dd9a0d --- /dev/null +++ b/test/base.py @@ -0,0 +1,78 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Shared base class for layer-1 characterization tests. + +Uses tornado 4.3's tornado.testing.AsyncHTTPTestCase (unittest-based, plain +tornado-4 idioms -- no async/await) against the real module-level +mod.webserver.application. conftest.py has already set every MOD_* env var +before this module (or any test module) imports mod.webserver. +""" + +import json + +from tornado.testing import AsyncHTTPTestCase + +from conftest import _UserFilesHelper, _USER_FILES_DIR + + +class ModUITestCase(AsyncHTTPTestCase): + # Every test_*.py imports this class directly (`from base import + # ModUITestCase`) so it can subclass it -- which also puts it in each + # test module's globals. pytest's unittest integration collects *any* + # TestCase subclass it finds at module scope, regardless of name (the + # usual python_classes="Test*" filter does not apply to TestCase + # subclasses), so without this it would try to collect ModUITestCase + # itself as a test in every module that imports it and fail with + # "no attribute 'runTest'" (it defines no test_* methods of its own). + # Concrete subclasses below must set __test__ = True to opt back in, + # since __test__ is looked up via normal attribute inheritance. + __test__ = False + + def runTest(self): + # Never actually invoked as a real test (real test_* methods are + # collected and run individually by pytest). Exists only because + # modern pytest (9.x) instantiates every unittest.TestCase subclass + # once with methodName="runTest" during collection, to register + # fixture factories (_pytest.unittest.UnitTestCase.newinstance()). + # tornado 4.3's own AsyncTestCase.__init__ (tornado/testing.py) + # unconditionally does getattr(self, methodName) -- unlike stdlib + # unittest.TestCase, it does not special-case a missing "runTest" -- + # so without this method that probe instantiation raises + # AttributeError and collection fails for every test class. + pass + + def get_app(self): + # Imported lazily (not at module top-level) so that conftest.py's + # env-var setup is guaranteed to have already run before mod.webserver + # -- and mod.settings, which reads MOD_* env vars at import time -- + # is ever imported. + import mod.webserver + return mod.webserver.application + + def seed(self, relpath, content=b"data"): + """Write USER_FILES_DIR/ (creating parent dirs) for a test. + + Thin wrapper around conftest's user_files fixture helper, exposed + here because pytest fixture return values can't be injected as + parameters into unittest.TestCase test methods (which is what these + AsyncHTTPTestCase-derived tests are). The autouse `user_files` + fixture in conftest.py still handles wiping USER_FILES_DIR between + tests. + """ + return _UserFilesHelper(_USER_FILES_DIR).seed(relpath, content=content) + + def fetch_json(self, path, **kwargs): + """GET/POST/etc. `path` and decode the response body as JSON. + + Asserts the response declares a JSON content type, matching the + JsonRequestHandler.write() convention used across mod/webserver.py. + """ + response = self.fetch(path, **kwargs) + content_type = response.headers.get("Content-Type", "") + assert "application/json" in content_type, ( + "expected JSON content type for %s, got %r (body: %r)" + % (path, content_type, response.body) + ) + return response, json.loads(response.body.decode("utf-8")) diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 000000000..47848e23d --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +""" +Layer-1 characterization test harness bootstrap. + +This module runs at collection time, BEFORE any test module gets a chance to +``import mod.webserver`` (and transitively ``mod.settings``, which reads every +``MOD_*`` env var at import time). So all environment setup below is plain +module-level code, not a fixture -- fixtures would run too late. + +These tests exercise the real ``mod.webserver.application`` (the module-level +tornado ``web.Application`` built at import time) against a throwaway on-disk +tree. Nothing here calls ``mod.webserver.run()`` or ``prepare()`` -- those pull +in real hardware / mod-host / JACK setup that isn't available in this dev +environment. +""" + +import atexit +import os +import shutil +import sys +import tempfile + +import pytest + +# ---------------------------------------------------------------------------- +# 0. Fail fast if the native extension mod.webserver depends on isn't built. +# ---------------------------------------------------------------------------- + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +_LIBMOD_UTILS_SO = os.path.join(REPO_ROOT, "utils", "libmod_utils.so") + +if not os.path.isfile(_LIBMOD_UTILS_SO): + pytest.exit( + "libmod_utils.so not found at {0}.\n" + "Build it first: `make -C utils` (from the repo root). See " + "mod-ui/CLAUDE.md for one-time environment setup.".format(_LIBMOD_UTILS_SO), + returncode=1, + ) + +# ---------------------------------------------------------------------------- +# 1. Build a throwaway data/user-files tree and point every MOD_* env var +# that mod.settings reads at it, BEFORE mod.webserver (and mod.settings) +# is ever imported by a test module. +# ---------------------------------------------------------------------------- + +_TEST_ROOT = tempfile.mkdtemp(prefix="modui-test-") + +_DATA_DIR = os.path.join(_TEST_ROOT, "data") +_USER_FILES_DIR = os.path.join(_TEST_ROOT, "user-files") + +os.makedirs(_DATA_DIR, exist_ok=True) +os.makedirs(_USER_FILES_DIR, exist_ok=True) + +# Replicate the bits of mod.check_environment() that handlers rely on but +# that we never call (check_environment() only runs inside prepare()). +with open(os.path.join(_DATA_DIR, "banks.json"), "w") as fh: + fh.write("[]") +with open(os.path.join(_DATA_DIR, "favorites.json"), "w") as fh: + fh.write("[]") + +os.environ["MOD_DEV_ENVIRONMENT"] = "1" +os.environ["MOD_LOG"] = "0" +os.environ["MOD_DATA_DIR"] = _DATA_DIR +os.environ["MOD_USER_FILES_DIR"] = _USER_FILES_DIR +os.environ["MOD_HTML_DIR"] = os.path.join(REPO_ROOT, "html") +os.environ["MOD_DEFAULT_PEDALBOARD"] = os.path.join(REPO_ROOT, "default.pedalboard") + +# Make the repo importable the same way server.py does (test/ is not on +# sys.path by default under some pytest invocations). +if REPO_ROOT not in sys.path: + sys.path.insert(0, REPO_ROOT) + + +def _cleanup_test_root(): + shutil.rmtree(_TEST_ROOT, ignore_errors=True) + + +atexit.register(_cleanup_test_root) + +# ---------------------------------------------------------------------------- +# 2. Fixtures +# ---------------------------------------------------------------------------- + + +class _UserFilesHelper(object): + """Exposes USER_FILES_DIR plus a seed() helper to test modules.""" + + def __init__(self, root): + self.root = root + + def seed(self, relpath, content=b"data"): + """Create USER_FILES_DIR/ (and any parent dirs) with content.""" + fullpath = os.path.join(self.root, relpath) + os.makedirs(os.path.dirname(fullpath), exist_ok=True) + mode = "wb" if isinstance(content, bytes) else "w" + with open(fullpath, mode) as fh: + fh.write(content) + return fullpath + + +def _reset_user_files_dir(): + if os.path.isdir(_USER_FILES_DIR): + shutil.rmtree(_USER_FILES_DIR) + os.makedirs(_USER_FILES_DIR, exist_ok=True) + + +@pytest.fixture(autouse=True) +def user_files(): + """Function-scoped, autouse: wipe and recreate USER_FILES_DIR around + every test, so tests never see leftovers from one another. + + USER_FILES_DIR itself is fixed for the whole process (mod.settings reads + MOD_USER_FILES_DIR once, at import time), so we can't swap directories + per-test -- instead we clear its contents. + + autouse=True (rather than requiring tests to depend on it explicitly) + because our test classes are unittest.TestCase subclasses + (tornado.testing.AsyncHTTPTestCase) -- pytest applies autouse fixtures' + setup/teardown around unittest-style tests too, but does NOT support + injecting a fixture's return value as a test-method parameter for them. + Tests that need to seed files use ModUITestCase.seed() (test/base.py) + instead, which points at this same directory. + """ + _reset_user_files_dir() + yield _UserFilesHelper(_USER_FILES_DIR) + _reset_user_files_dir() diff --git a/test/test_cors.py b/test/test_cors.py new file mode 100644 index 000000000..983164924 --- /dev/null +++ b/test/test_cors.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for CORS headers. + +Pins the constraint that shapes the Tone3000 download design: ordinary +JsonRequestHandler routes (e.g. /files/list) send no +Access-Control-Allow-Origin at all, while RemoteRequestHandler subclasses +(e.g. Hello, at /hello) echo it back only for the mod.audio/moddevices.com +allow-list (mod/webserver.py:279-292). +""" + +from base import ModUITestCase + + +class TestFilesListCors(ModUITestCase): + __test__ = True + + def test_files_list_has_no_cors_header(self): + response = self.fetch("/files/list?types=bogus") + self.assertEqual(response.code, 200) + self.assertNotIn("Access-Control-Allow-Origin", response.headers) + + +class TestHelloCors(ModUITestCase): + __test__ = True + + def test_hello_echoes_allowed_origin(self): + response = self.fetch("/hello", headers={"Origin": "https://mod.audio"}) + self.assertEqual(response.code, 200) + self.assertEqual( + response.headers.get("Access-Control-Allow-Origin"), + "https://mod.audio", + ) + + def test_hello_omits_header_for_foreign_origin(self): + response = self.fetch("/hello", headers={"Origin": "https://evil.example"}) + self.assertEqual(response.code, 200) + self.assertNotIn("Access-Control-Allow-Origin", response.headers) diff --git a/test/test_files_list.py b/test/test_files_list.py new file mode 100644 index 000000000..866be6ad5 --- /dev/null +++ b/test/test_files_list.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for GET /files/list (mod/webserver.py:FilesList). + +These pin the *current* behavior of the pipeline the Tone3000 download +feature will rely on: a fresh os.walk per request, recursion into +subfolders (needed for zip-pack downloads), sort-by-fullname ordering, and +the exact JSON entry shape. +""" + +from base import ModUITestCase + + +class TestFilesList(ModUITestCase): + __test__ = True + + def test_missing_types_param_is_501(self): + response = self.fetch("/files/list") + self.assertEqual(response.code, 501) + + def test_unknown_type_is_empty_ok(self): + response, body = self.fetch_json("/files/list?types=bogus") + self.assertEqual(response.code, 200) + self.assertEqual(body, {"ok": True, "files": []}) + + def test_nammodel_missing_folder_is_empty_ok(self): + # No NAM Models folder exists at all under USER_FILES_DIR. + response, body = self.fetch_json("/files/list?types=nammodel") + self.assertEqual(response.code, 200) + self.assertEqual(body, {"ok": True, "files": []}) + + def test_nammodel_lists_nam_files_and_excludes_decoys(self): + self.seed("NAM Models/amp.nam") + self.seed("NAM Models/readme.txt") + self.seed("NAM Models/ir.wav") + + response, body = self.fetch_json("/files/list?types=nammodel") + self.assertEqual(response.code, 200) + self.assertTrue(body["ok"]) + + basenames = [f["basename"] for f in body["files"]] + self.assertEqual(basenames, ["amp.nam"]) + + entry = body["files"][0] + self.assertEqual(entry["filetype"], "nammodel") + self.assertTrue(entry["fullname"].endswith("NAM Models/amp.nam")) + self.assertTrue(entry["fullname"].startswith("/"), "fullname should be absolute") + + def test_nammodel_extension_match_is_case_insensitive(self): + self.seed("NAM Models/AMP.NAM") + + response, body = self.fetch_json("/files/list?types=nammodel") + self.assertEqual(response.code, 200) + basenames = [f["basename"] for f in body["files"]] + self.assertEqual(basenames, ["AMP.NAM"]) + + def test_nammodel_recurses_into_subfolders(self): + # Pins DP1: a zip pack unzipped into NAM Models// will list fine. + self.seed("NAM Models/top.nam") + self.seed("NAM Models/MyPack/clean.nam") + self.seed("NAM Models/MyPack/Nested/deep.nam") + + response, body = self.fetch_json("/files/list?types=nammodel") + self.assertEqual(response.code, 200) + basenames = sorted(f["basename"] for f in body["files"]) + self.assertEqual(basenames, ["clean.nam", "deep.nam", "top.nam"]) + + def test_nammodel_results_sorted_by_full_path(self): + # Pins DP2's findability baseline: ordering is decided by full path. + self.seed("NAM Models/zzz.nam") + self.seed("NAM Models/aaa.nam") + self.seed("NAM Models/MyPack/mmm.nam") + + response, body = self.fetch_json("/files/list?types=nammodel") + self.assertEqual(response.code, 200) + fullnames = [f["fullname"] for f in body["files"]] + self.assertEqual(fullnames, sorted(fullnames)) + + def test_nammodel_walk_is_fresh_every_request(self): + # Pins that /files/list is NOT cached server-side: a file dropped in + # between two requests is visible on the second request with no + # extra signal needed (the staleness the Tone3000 feature must + # solve is 100% client-side). + self.seed("NAM Models/first.nam") + + _, body1 = self.fetch_json("/files/list?types=nammodel") + self.assertEqual([f["basename"] for f in body1["files"]], ["first.nam"]) + + self.seed("NAM Models/second.nam") + + _, body2 = self.fetch_json("/files/list?types=nammodel") + self.assertEqual( + sorted(f["basename"] for f in body2["files"]), + ["first.nam", "second.nam"], + ) + + def test_multiple_types_are_unioned_with_own_filetype_tag(self): + self.seed("NAM Models/amp.nam") + self.seed("Reverb IRs/hall.wav") + + response, body = self.fetch_json("/files/list?types=nammodel,ir") + self.assertEqual(response.code, 200) + + by_basename = {f["basename"]: f for f in body["files"]} + self.assertEqual(set(by_basename), {"amp.nam", "hall.wav"}) + self.assertEqual(by_basename["amp.nam"]["filetype"], "nammodel") + self.assertEqual(by_basename["hall.wav"]["filetype"], "ir") diff --git a/test/test_index.py b/test/test_index.py new file mode 100644 index 000000000..17227b9a2 --- /dev/null +++ b/test/test_index.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for GET / (mod/webserver.py:TemplateHandler). + +This is the page a future Tone3000-tab edit will touch (the #main-menu +trigger-icon cluster). We pin the redirect-without-?v= behavior and, if the +dev-fake environment can render the full page without JACK/hardware, that +the rendered HTML still contains the anchor points the future edit needs. +""" + +from base import ModUITestCase + + +class TestIndexRedirect(ModUITestCase): + __test__ = True + + def test_bare_get_redirects_with_version_query_arg(self): + response = self.fetch("/", follow_redirects=False) + self.assertEqual(response.code, 302) + location = response.headers.get("Location", "") + self.assertIn("v=", location) + + +class TestIndexRender(ModUITestCase): + __test__ = True + + # TemplateHandler.get (mod/webserver.py) is a gen.coroutine that awaits + # SESSION.wait_for_hardware_if_needed -- if that never calls back under + # the dev-fake environment, this test would hang. self.fetch(...) + # applies AsyncTestCase's own timeout (ASYNC_TEST_TIMEOUT env var, + # default 5s) as a guard; conftest/CI can raise it if 5s is too tight. + + def test_get_with_version_renders_index_html(self): + response = self.fetch("/?v=1") + self.assertEqual(response.code, 200) + self.assertIn("text/html", response.headers.get("Content-Type", "")) + + body = response.body.decode("utf-8") + self.assertIn('id="main-menu"', body) + self.assertIn('id="mod-file-manager"', body) From ea0c290f17dbe0cb4a0f295378c78a9760312d18 Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 13:57:52 +0000 Subject: [PATCH 2/7] Refactor CORS tests and harden test sandbox CORS: split the hello tests into one allowed-origin base class with the origin as a class property (subclassed per allowed origin variant) and a separate foreign-origin class. Conftest: point MOD_USER_PEDALBOARDS_DIR/MOD_USER_PLUGINS_DIR at the temp tree (defaults are the real ~/.pedalboards and ~/.lv2), abort the run if any writable mod.settings path escapes the test root, and document the routes tests must never call. Co-Authored-By: Claude Fable 5 --- test/conftest.py | 49 +++++++++++++++++++++++++++++++++++++++++++++++ test/test_cors.py | 29 +++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 47848e23d..4d0dfe8f3 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -15,6 +15,25 @@ tree. Nothing here calls ``mod.webserver.run()`` or ``prepare()`` -- those pull in real hardware / mod-host / JACK setup that isn't available in this dev environment. + +NEVER-CALL LIST -- routes no test in this suite may fetch. They run +subprocesses, write outside the sandboxed temp tree, or block forever: + +- ``/system/cleanup`` deletes ``~/.pedalboards`` and ``~/.lv2`` +- ``/system/exechange`` reboot/systemctl/mod-backup subprocesses +- ``/update/download/``, ``/update/begin`` writes under /data, /tmp +- ``/controlchain/download/`` firmware download, subprocess mv to /tmp +- ``/switch_cpu_freq/`` writes /sys/devices/system/cpu +- ``/recording/*`` needs JACK; ``play/wait`` long-polls forever +- ``/pedalboard/pack_bundle``, ``/pedalboard/load_web``, + ``/pedalboard/factorycopy``, ``/pedalboard/image/generate`` subprocesses +- ``/effect/install``, ``/sdk/install``, ``/package/uninstall`` touch the + plugin dir via subprocess tar / rmtree +- ``/effect/list``, ``/effect/get*``, ``/effect/bulk``, ``/effect/add``, + ``/effect/image``, ``/effect/file``, ``/resources/(.*)?uri=`` dereference + the global lilv world, which is NULL until ``modtools.utils.init()`` runs + -- calling them without init SEGFAULTS the test process (see the phase-6 + spec in docs/ before touching these) """ import atexit @@ -50,9 +69,13 @@ _DATA_DIR = os.path.join(_TEST_ROOT, "data") _USER_FILES_DIR = os.path.join(_TEST_ROOT, "user-files") +_PEDALBOARDS_DIR = os.path.join(_TEST_ROOT, "pedalboards") +_PLUGINS_DIR = os.path.join(_TEST_ROOT, "lv2") os.makedirs(_DATA_DIR, exist_ok=True) os.makedirs(_USER_FILES_DIR, exist_ok=True) +os.makedirs(_PEDALBOARDS_DIR, exist_ok=True) +os.makedirs(_PLUGINS_DIR, exist_ok=True) # Replicate the bits of mod.check_environment() that handlers rely on but # that we never call (check_environment() only runs inside prepare()). @@ -67,6 +90,32 @@ os.environ["MOD_USER_FILES_DIR"] = _USER_FILES_DIR os.environ["MOD_HTML_DIR"] = os.path.join(REPO_ROOT, "html") os.environ["MOD_DEFAULT_PEDALBOARD"] = os.path.join(REPO_ROOT, "default.pedalboard") +# Without these two, mod.settings defaults LV2_PEDALBOARDS_DIR/LV2_PLUGIN_DIR +# to ~/.pedalboards and ~/.lv2 -- and pedalboard save/remove handlers would +# write into the REAL user home instead of the sandbox. +os.environ["MOD_USER_PEDALBOARDS_DIR"] = _PEDALBOARDS_DIR +os.environ["MOD_USER_PLUGINS_DIR"] = _PLUGINS_DIR + +# ---------------------------------------------------------------------------- +# 1b. Sandbox guard: every writable path mod.settings resolves must live +# under the throwaway test root. mod.settings only reads env vars and +# stdlib (importing it here is cheap and does NOT load libmod_utils.so), +# so this catches a broken/renamed MOD_* env var before any test can +# write outside the sandbox. +# ---------------------------------------------------------------------------- + +from mod import settings as _mod_settings # noqa: E402 (needs env set above) + +for _name in ("DATA_DIR", "USER_FILES_DIR", "LV2_PEDALBOARDS_DIR", "LV2_PLUGIN_DIR"): + _path = os.path.realpath(getattr(_mod_settings, _name)) + if not _path.startswith(os.path.realpath(_TEST_ROOT) + os.sep): + pytest.exit( + "SANDBOX GUARD: mod.settings.{0} resolves to {1}, which is outside " + "the test root {2}. Refusing to run -- tests could write into real " + "user/system directories. Check the MOD_* env vars set in " + "test/conftest.py against mod/settings.py.".format(_name, _path, _TEST_ROOT), + returncode=1, + ) # Make the repo importable the same way server.py does (test/ is not on # sys.path by default under some pytest invocations). diff --git a/test/test_cors.py b/test/test_cors.py index 983164924..9bfc5972d 100644 --- a/test/test_cors.py +++ b/test/test_cors.py @@ -23,17 +23,40 @@ def test_files_list_has_no_cors_header(self): self.assertNotIn("Access-Control-Allow-Origin", response.headers) -class TestHelloCors(ModUITestCase): +class TestHelloAllowedOriginCors(ModUITestCase): + """Base case for the allow-list: subclasses only change ``origin``.""" + __test__ = True + origin = "https://mod.audio" def test_hello_echoes_allowed_origin(self): - response = self.fetch("/hello", headers={"Origin": "https://mod.audio"}) + response = self.fetch("/hello", headers={"Origin": self.origin}) self.assertEqual(response.code, 200) self.assertEqual( response.headers.get("Access-Control-Allow-Origin"), - "https://mod.audio", + self.origin, ) + +class TestHelloAllowedOriginModdevicesCors(TestHelloAllowedOriginCors): + origin = "https://moddevices.com" + + +class TestHelloAllowedOriginPlainHttpCors(TestHelloAllowedOriginCors): + origin = "http://mod.audio" + + +class TestHelloAllowedOriginModAudioSubdomainCors(TestHelloAllowedOriginCors): + origin = "https://pedalboards.mod.audio" + + +class TestHelloAllowedOriginModdevicesSubdomainCors(TestHelloAllowedOriginCors): + origin = "https://cloud.moddevices.com" + + +class TestHelloForeignOriginCors(ModUITestCase): + __test__ = True + def test_hello_omits_header_for_foreign_origin(self): response = self.fetch("/hello", headers={"Origin": "https://evil.example"}) self.assertEqual(response.code, 200) From 26124bd804f805358cd1ef8ce2e10ba13859c2d8 Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 14:04:38 +0000 Subject: [PATCH 3/7] Add config and user-data characterization tests (phase 2) Covers favorites, config/set, save_user_id, tokens, banks, system info/prefs, hello, ping, template loaders, static-file and JSON handler header conventions. Pins current behavior, including 500s on /auth/nonce without device credentials, /tokens/save without expires_in_days, and missing templates, plus the banks.json rewrite side effect of GET /banks/. Co-Authored-By: Claude Fable 5 --- test/test_banks.py | 101 ++++++++++++++++++++++ test/test_config.py | 188 +++++++++++++++++++++++++++++++++++++++++ test/test_system.py | 90 ++++++++++++++++++++ test/test_templates.py | 110 ++++++++++++++++++++++++ test/test_tokens.py | 141 +++++++++++++++++++++++++++++++ 5 files changed, 630 insertions(+) create mode 100644 test/test_banks.py create mode 100644 test/test_config.py create mode 100644 test/test_system.py create mode 100644 test/test_templates.py create mode 100644 test/test_tokens.py diff --git a/test/test_banks.py b/test/test_banks.py new file mode 100644 index 000000000..2da07013a --- /dev/null +++ b/test/test_banks.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for /banks/save (BankSave) and /banks/ (BankLoad), +mod/webserver.py. + +conftest.py pre-seeds DATA_DIR/banks.json with "[]" (mimicking +check_environment(), which real prepare() calls but this sandbox never +does). Both handlers use mod.settings.USER_BANKS_JSON_FILE, so this module +snapshots/restores that file around every test. +""" + +import json +import os + +import pytest + +from base import ModUITestCase + + +@pytest.fixture(autouse=True) +def _snapshot_banks_json(): + from mod import settings + + with open(settings.USER_BANKS_JSON_FILE, "r") as fh: + before = fh.read() + + yield + + with open(settings.USER_BANKS_JSON_FILE, "w") as fh: + fh.write(before) + + +class TestBankLoadEmpty(ModUITestCase): + __test__ = True + + def test_load_with_empty_banks_json_returns_empty_list(self): + response, body = self.fetch_json("/banks/") + self.assertEqual(response.code, 200) + self.assertEqual(body, []) + + +class TestBankSave(ModUITestCase): + __test__ = True + + def test_save_writes_banks_json_and_returns_true(self): + banks = [{"title": "Bank 1", "pedalboards": []}] + response = self.fetch("/banks/save", method="POST", body=json.dumps(banks)) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + from mod import settings + + with open(settings.USER_BANKS_JSON_FILE, "r") as fh: + on_disk = json.load(fh) + self.assertEqual(on_disk, banks) + + +class TestBankRoundTripNonexistentPedalboard(ModUITestCase): + __test__ = True + + def test_save_bank_referencing_missing_pedalboard_is_filtered_on_load(self): + # DP1/round-trip case from the spec: does BankLoad filter out a + # pedalboard whose bundle path doesn't exist in the sandboxed temp + # pedalboards dir (which is always empty here)? + banks = [ + { + "title": "Bank 1", + "pedalboards": [ + {"bundle": "/nonexistent/thing.pedalboard", "title": "Ghost"} + ], + } + ] + save_response = self.fetch( + "/banks/save", method="POST", body=json.dumps(banks) + ) + self.assertEqual(save_response.body, b"true") + + load_response, body = self.fetch_json("/banks/") + self.assertEqual(load_response.code, 200) + + # The bank itself survives, but its pedalboards list is filtered + # empty: list_banks() (mod/bank.py) drops any pedalboard whose + # 'bundle' path doesn't os.path.exists(), independent of whether it + # is "broken" -- and the temp pedalboards dir is always empty in + # this sandbox, so get_all_pedalboards() never repopulates it + # either. + self.assertEqual(len(body), 1) + self.assertEqual(body[0]["title"], "Bank 1") + self.assertEqual(body[0]["pedalboards"], []) + + # Observed side effect: list_banks() auto-rewrites banks.json to + # drop the now-filtered pedalboard entry as a side effect of a GET + # request (mod/bank.py:58-59, changed=True + shouldSave=True by + # default). So a plain GET /banks/ is not read-only on disk. + from mod import settings + + with open(settings.USER_BANKS_JSON_FILE, "r") as fh: + on_disk = json.load(fh) + self.assertEqual(on_disk[0]["pedalboards"], []) diff --git a/test/test_config.py b/test/test_config.py new file mode 100644 index 000000000..a898094fe --- /dev/null +++ b/test/test_config.py @@ -0,0 +1,188 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the small config/user-data POST endpoints: +favorites (mod/webserver.py:FavoritesAdd/FavoritesRemove), /config/set +(SaveSingleConfigValue), /save_user_id/ (SaveUserId) and /auth/nonce +(AuthNonce). + +These all mutate either an in-process singleton (gState.favorites) or a +DATA_DIR JSON file (favorites.json, prefs.json, user-id.json). Since +conftest.py's autouse `user_files` fixture only resets USER_FILES_DIR, this +module snapshots/restores gState.favorites and the DATA_DIR files itself so +tests stay order-independent (per characterization-phase-2.md ground rule 5). +""" + +import json +import os + +import pytest + +from base import ModUITestCase + + +def _read_or_none(path): + if not os.path.exists(path): + return None + with open(path, "r") as fh: + return fh.read() + + +def _restore(path, original): + if original is None: + if os.path.exists(path): + os.remove(path) + else: + with open(path, "w") as fh: + fh.write(original) + + +@pytest.fixture(autouse=True) +def _snapshot_data_dir_state(): + import mod.webserver as webserver + from mod import settings + + favorites_snapshot = list(webserver.gState.favorites) + prefs_before = _read_or_none(settings.PREFERENCES_JSON_FILE) + user_id_before = _read_or_none(settings.USER_ID_JSON_FILE) + favorites_file_before = _read_or_none(settings.FAVORITES_JSON_FILE) + + yield + + webserver.gState.favorites[:] = favorites_snapshot + _restore(settings.PREFERENCES_JSON_FILE, prefs_before) + _restore(settings.USER_ID_JSON_FILE, user_id_before) + _restore(settings.FAVORITES_JSON_FILE, favorites_file_before) + + +class TestFavoritesAdd(ModUITestCase): + __test__ = True + + def test_add_writes_favorites_json_and_returns_true(self): + response = self.fetch( + "/favorites/add", method="POST", body="uri=http%3A%2F%2Fexample.org%2Ffx" + ) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + from mod import settings + + with open(settings.FAVORITES_JSON_FILE, "r") as fh: + data = json.load(fh) + self.assertEqual(data, ["http://example.org/fx"]) + + def test_add_duplicate_returns_false_and_does_not_duplicate(self): + body = "uri=http%3A%2F%2Fexample.org%2Ffx" + first = self.fetch("/favorites/add", method="POST", body=body) + self.assertEqual(first.body, b"true") + + second = self.fetch("/favorites/add", method="POST", body=body) + self.assertEqual(second.code, 200) + self.assertEqual(second.body, b"false") + + from mod import settings + + with open(settings.FAVORITES_JSON_FILE, "r") as fh: + data = json.load(fh) + self.assertEqual(data, ["http://example.org/fx"]) + + def test_add_missing_uri_argument_is_400(self): + response = self.fetch("/favorites/add", method="POST", body="") + self.assertEqual(response.code, 400) + + +class TestFavoritesRemove(ModUITestCase): + __test__ = True + + def test_add_then_remove_empties_favorites_json(self): + body = "uri=http%3A%2F%2Fexample.org%2Ffx" + self.fetch("/favorites/add", method="POST", body=body) + + response = self.fetch("/favorites/remove", method="POST", body=body) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + from mod import settings + + with open(settings.FAVORITES_JSON_FILE, "r") as fh: + data = json.load(fh) + self.assertEqual(data, []) + + def test_remove_unknown_uri_returns_false(self): + response = self.fetch( + "/favorites/remove", + method="POST", + body="uri=http%3A%2F%2Fnever-added.example%2Ffx", + ) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"false") + + +class TestSaveSingleConfigValue(ModUITestCase): + __test__ = True + + def test_set_writes_prefs_json_and_returns_true(self): + response = self.fetch( + "/config/set", method="POST", body="key=some-key&value=some-value" + ) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + from mod import settings + + with open(settings.PREFERENCES_JSON_FILE, "r") as fh: + data = json.load(fh) + # Values are stored verbatim as whatever get_argument() returned: + # tornado decodes form params to str, so both key and value land as + # plain strings in prefs.json (no int/bool coercion happens here). + self.assertEqual(data["some-key"], "some-value") + self.assertIsInstance(data["some-key"], str) + + def test_set_missing_key_argument_is_400(self): + response = self.fetch("/config/set", method="POST", body="value=x") + self.assertEqual(response.code, 400) + + +class TestSaveUserId(ModUITestCase): + __test__ = True + + def test_save_writes_user_id_json_and_returns_true(self): + response = self.fetch( + "/save_user_id/", + method="POST", + body="name=Ada&email=ada%40example.org", + ) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + from mod import settings + + with open(settings.USER_ID_JSON_FILE, "r") as fh: + data = json.load(fh) + self.assertEqual(data, {"name": "Ada", "email": "ada@example.org"}) + + def test_save_missing_name_argument_is_400(self): + response = self.fetch( + "/save_user_id/", method="POST", body="email=ada%40example.org" + ) + self.assertEqual(response.code, 400) + + +class TestAuthNonce(ModUITestCase): + __test__ = True + + def test_auth_nonce_observed_behavior(self): + # Spec: "if the crypto `token` module is None -> {}; otherwise may + # error -- OBSERVE and pin actual." In this dev sandbox + # mod.communication.token imports fine (no exception at import + # time), so `token` is NOT None; but MOD_DEVICE_KEY / MOD_DEVICE_TAG + # are unset (conftest.py never sets them), so + # mod.communication.device.get_tag() raises "Missing device tag" + # once AuthNonce actually calls token.create_token_message(). That + # exception is uncaught by the handler, so tornado turns it into a + # 500. + response = self.fetch( + "/auth/nonce", method="POST", body=json.dumps({"nonce": "abc123"}) + ) + self.assertEqual(response.code, 500) diff --git a/test/test_system.py b/test/test_system.py new file mode 100644 index 000000000..7ab0058f2 --- /dev/null +++ b/test/test_system.py @@ -0,0 +1,90 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for /system/info (SystemInfo), /system/prefs +(SystemPreferences), /hello/ (Hello) and /ping/ (Ping), mod/webserver.py. + +None of these write anything -- they only read read-only system paths +(/etc/mod-release/system, /data/*) that don't exist on a plain dev machine, +so no snapshot/restore fixture is needed here. +""" + +from base import ModUITestCase + + +class TestSystemInfo(ModUITestCase): + __test__ = True + + def test_info_defaults_when_no_hardware_descriptor(self): + response, body = self.fetch_json("/system/info") + self.assertEqual(response.code, 200) + + # /etc/mod-hardware-descriptor.json and /etc/mod-release/system + # don't exist on this dev machine, so every hwdesc-derived field + # falls back to "Unknown" and sysdate falls back to "Unknown". + self.assertEqual(body["hwname"], "Unknown") + self.assertEqual(body["architecture"], "Unknown") + self.assertEqual(body["cpu"], "Unknown") + self.assertEqual(body["platform"], "Unknown") + self.assertEqual(body["bin_compat"], "Unknown") + self.assertEqual(body["model"], "Unknown") + self.assertEqual(body["sysdate"], "Unknown") + + self.assertIn("version", body["python"]) + self.assertIn("machine", body["uname"]) + self.assertIn("release", body["uname"]) + self.assertIn("sysname", body["uname"]) + self.assertIn("version", body["uname"]) + + +class TestSystemPreferences(ModUITestCase): + __test__ = True + + def test_prefs_defaults_when_no_data_files(self): + response, body = self.fetch_json("/system/prefs") + self.assertEqual(response.code, 200) + + # SystemPreferences reads hardcoded absolute "/data/..." paths (NOT + # mod.settings.DATA_DIR -- these are never sandboxed by conftest.py, + # but /data doesn't exist on this dev machine so every pref falls + # back to its default). + self.assertEqual(body["jack_buffer_size"], 128) + self.assertEqual(body["jack_mono_copy"], False) + self.assertEqual(body["jack_sync_mode"], False) + self.assertEqual(body["separate_spdif_outs"], False) + # "service_mod_peakmeter" is an OPTION_FILE_NOT_EXISTS pref, so it's + # True exactly when the disable-flag file is absent. + self.assertEqual(body["service_mod_peakmeter"], True) + self.assertEqual(body["service_mod_sdk"], False) + self.assertEqual(body["service_netmanager"], False) + self.assertEqual(body["autorestart_hmi"], False) + # bluetooth_name has no valdef override (defaults to None), so the + # key is present with a JSON null, not omitted. + self.assertIn("bluetooth_name", body) + self.assertIsNone(body["bluetooth_name"]) + + +class TestHello(ModUITestCase): + __test__ = True + + def test_hello_shape(self): + response, body = self.fetch_json("/hello/") + self.assertEqual(response.code, 200) + # No websocket ever connects in this test harness. + self.assertEqual(body["online"], False) + # IMAGE_VERSION is None in this dev sandbox (no /etc/mod-release/system). + self.assertIsNone(body["version"]) + + +class TestPing(ModUITestCase): + __test__ = True + + def test_ping_reports_hmi_offline(self): + response, body = self.fetch_json("/ping/") + self.assertEqual(response.code, 200) + # The fake HMI (mod.development.FakeHMI under MOD_DEV_ENVIRONMENT=1) + # is never .initialized, so web_ping() calls back False synchronously + # -- no 5s gen.with_timeout wait is actually exercised here. + self.assertEqual(body["ihm_online"], False) + self.assertEqual(body["ihm_time"], 0) diff --git a/test/test_templates.py b/test/test_templates.py new file mode 100644 index 000000000..94d9d97df --- /dev/null +++ b/test/test_templates.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the template/static-file loaders and the +header conventions shared by every handler base class in mod/webserver.py: + +- TemplateLoader (/load_template/.html) +- BulkTemplateLoader (/js/templates.js) +- TimelessStaticFileHandler (any static file, e.g. /js/desktop.js) +- header pinning: JsonRequestHandler (no Date, no cache headers) vs + CachedJsonRequestHandler (Cache-Control + fixed Expires) + +None of these write anything, so no snapshot/restore fixture is needed. +""" + +from base import ModUITestCase + + +class TestTemplateLoader(ModUITestCase): + __test__ = True + + def test_loads_known_template_from_html_include(self): + response = self.fetch("/load_template/pedalboard.html") + self.assertEqual(response.code, 200) + self.assertIn("text/plain", response.headers.get("Content-Type", "")) + self.assertTrue(len(response.body) > 0) + + def test_unknown_template_is_500(self): + # TemplateLoader.get() open()s the file directly with no + # os.path.exists guard, so a missing template surfaces as an + # uncaught FileNotFoundError -> tornado 500, not a 404. + response = self.fetch("/load_template/does_not_exist.html") + self.assertEqual(response.code, 500) + + +class TestBulkTemplateLoader(ModUITestCase): + __test__ = True + + def test_bundles_html_include_into_templates_object(self): + response = self.fetch("/js/templates.js") + self.assertEqual(response.code, 200) + self.assertIn("text/javascript", response.headers.get("Content-Type", "")) + + body = response.body.decode("utf-8") + self.assertIn("TEMPLATES['pedalboard']", body) + + def test_has_cache_control_and_expires_headers(self): + # BulkTemplateLoader can't subclass CachedJsonRequestHandler (it's + # not JSON), so it sets the same two headers by hand -- pin that + # both routes converge on the same cache contract. + response = self.fetch("/js/templates.js") + self.assertEqual( + response.headers.get("Cache-Control"), "public, max-age=31536000" + ) + self.assertEqual( + response.headers.get("Expires"), "Mon, 31 Dec 2035 12:00:00 gmt" + ) + self.assertNotIn("Date", response.headers) + + +class TestTimelessStaticFileHandler(ModUITestCase): + __test__ = True + + def test_static_file_has_no_date_header(self): + response = self.fetch("/js/desktop.js") + self.assertEqual(response.code, 200) + self.assertNotIn("Date", response.headers) + self.assertEqual( + response.headers.get("Cache-Control"), "public, max-age=31536000" + ) + self.assertEqual( + response.headers.get("Expires"), "Mon, 31 Dec 2035 12:00:00 gmt" + ) + + +class TestJsonRequestHandlerHeaders(ModUITestCase): + __test__ = True + + def test_plain_json_route_has_no_date_and_no_cache_headers(self): + response = self.fetch("/files/list?types=bogus") + self.assertEqual(response.code, 200) + self.assertNotIn("Date", response.headers) + self.assertNotIn("Cache-Control", response.headers) + self.assertNotIn("Expires", response.headers) + + +class TestCachedJsonRequestHandlerHeaders(ModUITestCase): + __test__ = True + + def test_pedalboard_image_check_has_cache_headers(self): + # PedalboardImageCheck (CachedJsonRequestHandler) is filesystem-only + # (SESSION.screenshot_generator.check_screenshot just stats a + # thumbnail.png next to the given bundlepath) -- safe to call + # without lilv/JACK, unlike the /effect/* routes on the NEVER-CALL + # list. The other CachedJsonRequestHandler route, EffectGet, does + # need the global lilv world and is skipped (see NEVER-CALL list). + response, body = self.fetch_json( + "/pedalboard/image/check?bundlepath=/nonexistent/thing.pedalboard" + ) + self.assertEqual(response.code, 200) + self.assertEqual(body["status"], -1) + + self.assertEqual( + response.headers.get("Cache-Control"), "public, max-age=31536000" + ) + self.assertEqual( + response.headers.get("Expires"), "Mon, 31 Dec 2035 12:00:00 gmt" + ) + self.assertNotIn("Date", response.headers) diff --git a/test/test_tokens.py b/test/test_tokens.py new file mode 100644 index 000000000..10ea292c0 --- /dev/null +++ b/test/test_tokens.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for /tokens/save, /tokens/get, /tokens/delete +(mod/webserver.py:TokensSave/TokensGet/TokensDelete). + +These read/write DATA_DIR/tokens.conf directly (not via mod.settings, the +handlers build the path from the DATA_DIR global inline), so this module +snapshots/restores that file around every test to stay order-independent. +""" + +import json +import os + +import pytest + +from base import ModUITestCase + + +@pytest.fixture(autouse=True) +def _snapshot_tokens_conf(): + from mod import settings + + tokens_conf = os.path.join(settings.DATA_DIR, "tokens.conf") + existed = os.path.exists(tokens_conf) + before = None + if existed: + with open(tokens_conf, "r") as fh: + before = fh.read() + + yield + + if existed: + with open(tokens_conf, "w") as fh: + fh.write(before) + elif os.path.exists(tokens_conf): + os.remove(tokens_conf) + + +class TestTokensGetMissing(ModUITestCase): + __test__ = True + + def test_get_with_no_file_returns_ok_false(self): + response, body = self.fetch_json("/tokens/get") + self.assertEqual(response.code, 200) + self.assertEqual(body, {"ok": False}) + + +class TestTokensDeleteMissing(ModUITestCase): + __test__ = True + + def test_delete_with_no_file_is_a_noop_returning_true(self): + response = self.fetch("/tokens/delete") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + +class TestTokensRoundTrip(ModUITestCase): + __test__ = True + + def _payload(self): + return { + "user_id": "u1", + "access_token": "at1", + "refresh_token": "rt1", + "expires_in_days": 30, + } + + def test_save_then_get_returns_saved_payload_minus_expires(self): + save_response = self.fetch( + "/tokens/save", method="POST", body=json.dumps(self._payload()) + ) + self.assertEqual(save_response.code, 200) + self.assertEqual(save_response.body, b"true") + + from mod import settings + + tokens_conf = os.path.join(settings.DATA_DIR, "tokens.conf") + with open(tokens_conf, "r") as fh: + on_disk = json.load(fh) + # TokensSave pops "expires_in_days" before writing to disk. + self.assertEqual( + on_disk, {"user_id": "u1", "access_token": "at1", "refresh_token": "rt1"} + ) + + get_response, body = self.fetch_json("/tokens/get") + self.assertEqual(get_response.code, 200) + self.assertEqual( + body, + { + "user_id": "u1", + "access_token": "at1", + "refresh_token": "rt1", + "ok": True, + }, + ) + + def test_save_missing_expires_in_days_is_500_and_does_not_write(self): + # TokensSave unconditionally data.pop("expires_in_days") before + # writing -- a payload that omits it raises an uncaught KeyError + # (500), and nothing is written to tokens.conf. + partial = {"user_id": "u1", "access_token": "at1", "refresh_token": "rt1"} + response = self.fetch( + "/tokens/save", method="POST", body=json.dumps(partial) + ) + self.assertEqual(response.code, 500) + + get_response, body = self.fetch_json("/tokens/get") + self.assertEqual(get_response.code, 200) + self.assertEqual(body, {"ok": False}) + + def test_get_missing_a_required_key_is_ok_false(self): + partial = {"user_id": "u1", "access_token": "at1", "expires_in_days": 30} + save_response = self.fetch( + "/tokens/save", method="POST", body=json.dumps(partial) + ) + self.assertEqual(save_response.body, b"true") + + response, body = self.fetch_json("/tokens/get") + self.assertEqual(response.code, 200) + self.assertEqual(body["ok"], False) + self.assertEqual(body["user_id"], "u1") + self.assertEqual(body["access_token"], "at1") + self.assertNotIn("refresh_token", body) + + def test_save_then_delete_then_get_is_ok_false_again(self): + self.fetch("/tokens/save", method="POST", body=json.dumps(self._payload())) + + delete_response = self.fetch("/tokens/delete") + self.assertEqual(delete_response.code, 200) + self.assertEqual(delete_response.body, b"true") + + from mod import settings + + tokens_conf = os.path.join(settings.DATA_DIR, "tokens.conf") + self.assertFalse(os.path.exists(tokens_conf)) + + get_response, body = self.fetch_json("/tokens/get") + self.assertEqual(get_response.code, 200) + self.assertEqual(body, {"ok": False}) From 504a4151526eb4f9f95d94685bf01b7407f23372 Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 14:19:40 +0000 Subject: [PATCH 4/7] Add pedalboard and snapshot characterization tests (phase 3) Save/info/remove/load_bundle round trip against the sandboxed pedalboards dir under the fake host, plus snapshot list/name/save/saveas behavior (including the Default snapshot seeded by SESSION.reset). Key finding recorded in the conftest never-call list: /pedalboard/list and /banks/ segfault (lilv_new_uri on the uninitialized global lilv world) as soon as a real pedalboard bundle exists on disk; both are only safe against an empty pedalboards dir, so the round trip verifies bundle presence via the filesystem and only lists once the dir is empty again. Conftest also gains an autouse fixture resetting the pedalboards dir between tests. Co-Authored-By: Claude Fable 5 --- test/conftest.py | 43 ++++++ test/test_pedalboards.py | 289 +++++++++++++++++++++++++++++++++++++++ test/test_snapshots.py | 158 +++++++++++++++++++++ 3 files changed, 490 insertions(+) create mode 100644 test/test_pedalboards.py create mode 100644 test/test_snapshots.py diff --git a/test/conftest.py b/test/conftest.py index 4d0dfe8f3..7033b54a8 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -34,6 +34,11 @@ the global lilv world, which is NULL until ``modtools.utils.init()`` runs -- calling them without init SEGFAULTS the test process (see the phase-6 spec in docs/ before touching these) +- ``/pedalboard/list``, ``/banks/`` while a real pedalboard bundle exists in + the sandbox pedalboards dir -- get_all_pedalboards SEGFAULTS parsing it + (NamespaceDefinitions::init / lilv_new_uri, needs the uninitialized global + lilv world). Both are safe against an EMPTY pedalboards dir; save/remove a + bundle within one test and only list after the dir is empty again """ import atexit @@ -176,3 +181,41 @@ def user_files(): _reset_user_files_dir() yield _UserFilesHelper(_USER_FILES_DIR) _reset_user_files_dir() + + +def _reset_pedalboards_dir(): + if os.path.isdir(_PEDALBOARDS_DIR): + shutil.rmtree(_PEDALBOARDS_DIR) + os.makedirs(_PEDALBOARDS_DIR, exist_ok=True) + + +@pytest.fixture(autouse=True) +def pedalboards_dir(): + """Function-scoped, autouse: wipe/recreate LV2_PEDALBOARDS_DIR and reset + the process-global SESSION around every test. + + Mirrors ``user_files`` above, for the same reason: LV2_PEDALBOARDS_DIR is + fixed for the whole process (mod.settings reads MOD_USER_PEDALBOARDS_DIR + once, at import time), so we clear its contents instead of swapping + directories. Phase 3 (docs/characterization-phase-3.md) is the first + phase whose tests write real pedalboard bundles to disk via + ``SESSION.host.save()`` (through ``POST /pedalboard/save``) and mutate + process-global state on ``SESSION.host`` (``pedalboard_path``, + ``pedalboard_snapshots``, ``current_pedalboard_snapshot_id``, ...) via + ``/pedalboard/load_bundle/`` and ``/snapshot/*``. Neither the on-disk + bundles nor that in-memory state may leak between tests or modules. + + ``SESSION.reset(callback)`` (mod/session.py) runs its callback + synchronously here: under ``MOD_DEV_ENVIRONMENT=1`` the FakeHMI is never + "initialized" (see module docstring), so ``Session.reset`` takes its + synchronous branch straight to ``Host.reset``, and ``FakeHost`` (see + ``mod/development.py``) invokes every ``send_notmodified``/ + ``send_modified`` callback immediately with ``True`` -- no real + mod-host, no IOLoop pump required. + """ + _reset_pedalboards_dir() + from mod.webserver import SESSION + SESSION.reset(lambda ok: None) + yield + SESSION.reset(lambda ok: None) + _reset_pedalboards_dir() diff --git a/test/test_pedalboards.py b/test/test_pedalboards.py new file mode 100644 index 000000000..6252d77e2 --- /dev/null +++ b/test/test_pedalboards.py @@ -0,0 +1,289 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the pedalboard lifecycle: list, save, info, +remove, load_bundle (mod/webserver.py: PedalboardList, PedalboardSave, +PedalboardInfo, PedalboardRemove, PedalboardLoadBundle). + +CRITICAL, spec-overriding finding (see docs/characterization-phase-3.md, +which claimed /pedalboard/list and /banks/ are "self-contained lilv calls" +and safe): **GET /pedalboard/list and GET /banks/ SEGFAULT the whole test +process the moment the sandboxed pedalboards dir contains a real, on-disk +pedalboard bundle** (one written by POST /pedalboard/save). Root cause, +confirmed with faulthandler/gdb: modtools.utils.get_all_pedalboards() +(mod/webserver.py:1316 for PedalboardList, :1685 for BankLoad) invalidates +its Python-side cache and calls into utils.get_all_pedalboards() (the +libmod_utils.so C extension), which crashes inside +``NamespaceDefinitions::init(LilvWorldImpl*)`` -> ``lilv_new_uri()`` while +building its lilv world over a *real* bundle -- reproduced identically via +both PedalboardList and BankLoad, deterministically, every time, and +independent of how many times /pedalboard/list was called before (repeated +calls against an *empty* dir never crash; a *single* call against a dir +containing one real saved bundle crashes every time). By contrast, +GET /pedalboard/info/ (single-bundle parse via get_pedalboard_info(), a +different C entry point) and GET /pedalboard/remove/ are safe with a real +bundle present -- confirmed by direct probing with faulthandler enabled. + +Consequence for this suite: no test here calls GET /pedalboard/list or +GET /banks/ while a real bundle exists in the sandboxed pedalboards dir. +The "list now includes TestBoard" step from the phase-3 spec's core +round-trip is NOT executed as a live HTTP call; the bundle's presence is +instead verified directly on disk (os.path), which is what a real GET +/pedalboard/list would enumerate. The bundle is always removed via +GET /pedalboard/remove/ before any subsequent GET /pedalboard/list call in +the same test. + +The autouse `pedalboards_dir` fixture (test/conftest.py) wipes +LV2_PEDALBOARDS_DIR and resets SESSION around every test, so tests are +order-independent regarding on-disk bundles and SESSION.host state. +""" + +import os +import urllib.parse + +from base import ModUITestCase + + +def _quote(bundlepath): + return urllib.parse.quote(bundlepath, safe="") + + +class TestPedalboardListBaseline(ModUITestCase): + __test__ = True + + def test_list_on_empty_sandbox_is_empty_list(self): + # Safe: the sandboxed pedalboards dir is always empty here (no save + # has happened yet in this test). + response, body = self.fetch_json("/pedalboard/list") + self.assertEqual(response.code, 200) + self.assertEqual(body, []) + + +class TestPedalboardSave(ModUITestCase): + __test__ = True + + def test_save_missing_title_is_400(self): + response = self.fetch("/pedalboard/save", method="POST", body="asNew=1") + self.assertEqual(response.code, 400) + + def test_save_missing_asnew_is_400(self): + response = self.fetch("/pedalboard/save", method="POST", body="title=X") + self.assertEqual(response.code, 400) + + def test_save_creates_bundle_on_disk_with_expected_json_shape(self): + import mod.settings as settings + + body = urllib.parse.urlencode({"title": "TestBoard", "asNew": "1"}) + response, payload = self.fetch_json( + "/pedalboard/save", method="POST", body=body + ) + self.assertEqual(response.code, 200) + self.assertEqual( + payload, {"ok": True, "bundlepath": payload["bundlepath"], "title": "TestBoard"} + ) + + bundlepath = payload["bundlepath"] + self.assertTrue(bundlepath.startswith(settings.LV2_PEDALBOARDS_DIR)) + self.assertTrue(bundlepath.endswith(".pedalboard")) + self.assertTrue(os.path.isdir(bundlepath)) + + # A .ttl bundle got written (host.save_state_to_ttl), NOT observed + # through /pedalboard/list (see module docstring -- that would + # segfault) but directly on disk, which is exactly what a real + # /pedalboard/list GET would enumerate. + entries = os.listdir(bundlepath) + self.assertTrue(any(name.endswith(".ttl") for name in entries)) + self.assertIn("manifest.ttl", entries) + + # cleanup: remove before this test method returns, so no real + # bundle is left for the next test even though the autouse fixture + # would also wipe it. + remove_response = self.fetch( + "/pedalboard/remove/?bundlepath=" + _quote(bundlepath) + ) + self.assertEqual(remove_response.body, b"true") + + def test_save_asnew_0_over_fresh_session_behaves_like_asnew_1(self): + # No pedalboard was ever loaded/saved in this session yet, so + # host.pedalboard_path is "" -- host.save()'s "save over existing" + # branch requires a truthy, on-disk, sandbox-rooted pedalboard_path, + # so asNew=0 here takes the same "save new" branch as asNew=1. + body = urllib.parse.urlencode({"title": "FreshBoard", "asNew": "0"}) + response, payload = self.fetch_json( + "/pedalboard/save", method="POST", body=body + ) + self.assertEqual(response.code, 200) + self.assertTrue(payload["ok"]) + self.assertTrue(os.path.isdir(payload["bundlepath"])) + + self.fetch("/pedalboard/remove/?bundlepath=" + _quote(payload["bundlepath"])) + + def test_save_asnew_0_after_asnew_1_overwrites_same_bundle(self): + body1 = urllib.parse.urlencode({"title": "TestBoard", "asNew": "1"}) + _, payload1 = self.fetch_json("/pedalboard/save", method="POST", body=body1) + bundlepath1 = payload1["bundlepath"] + + body2 = urllib.parse.urlencode({"title": "TestBoard", "asNew": "0"}) + _, payload2 = self.fetch_json("/pedalboard/save", method="POST", body=body2) + bundlepath2 = payload2["bundlepath"] + + self.assertEqual(bundlepath1, bundlepath2) + + import mod.settings as settings + + self.assertEqual(os.listdir(settings.LV2_PEDALBOARDS_DIR), [os.path.basename(bundlepath1)]) + + self.fetch("/pedalboard/remove/?bundlepath=" + _quote(bundlepath1)) + + +class TestPedalboardInfo(ModUITestCase): + __test__ = True + + def test_info_on_bogus_bundlepath_is_500(self): + # get_pedalboard_info() raises a bare Exception on failure + # (modtools/utils.py); PedalboardInfo.get() does not catch it, so it + # surfaces as tornado's generic uncaught-exception 500 HTML page -- + # NOT a JsonRequestHandler JSON error body. + response = self.fetch( + "/pedalboard/info/?bundlepath=" + _quote("/nonexistent/thing.pedalboard") + ) + self.assertEqual(response.code, 500) + self.assertIn("text/html", response.headers.get("Content-Type", "")) + + def test_info_on_real_bundle_matches_saved_title(self): + save_body = urllib.parse.urlencode({"title": "TestBoard", "asNew": "1"}) + _, save_payload = self.fetch_json( + "/pedalboard/save", method="POST", body=save_body + ) + bundlepath = save_payload["bundlepath"] + + response, info = self.fetch_json( + "/pedalboard/info/?bundlepath=" + _quote(bundlepath) + ) + self.assertEqual(response.code, 200) + self.assertEqual(info["title"], "TestBoard") + self.assertIn("plugins", info) + self.assertIn("width", info) + self.assertIn("height", info) + + self.fetch("/pedalboard/remove/?bundlepath=" + _quote(bundlepath)) + + +class TestPedalboardRemove(ModUITestCase): + __test__ = True + + def test_remove_bogus_bundlepath_returns_false(self): + response = self.fetch( + "/pedalboard/remove/?bundlepath=" + _quote("/nonexistent/thing.pedalboard") + ) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"false") + + def test_remove_nonexistent_path_outside_sandbox_returns_false_and_touches_nothing(self): + # PedalboardRemove gates on os.path.exists(bundlepath) before ever + # calling shutil.rmtree -- a nonexistent path outside the sandbox is + # therefore a safe no-op observation, not a real escape attempt. + outside_path = "/tmp/modui-characterization-does-not-exist.pedalboard" + self.assertFalse(os.path.exists(outside_path)) + + response = self.fetch("/pedalboard/remove/?bundlepath=" + _quote(outside_path)) + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"false") + + +class TestPedalboardCoreRoundTrip(ModUITestCase): + __test__ = True + + def test_save_info_remove_round_trip(self): + """The highest-value test (spec's "core round-trip"), adapted for + the /pedalboard/list segfault documented in the module docstring: + the bundle's presence/absence is asserted via the filesystem and + via /pedalboard/info/, never via a live /pedalboard/list call while + the bundle exists. + """ + import mod.settings as settings + + # 1. Baseline: safe, dir is empty (autouse fixture guarantees this). + response, body = self.fetch_json("/pedalboard/list") + self.assertEqual(body, []) + + # 2. Save. + save_body = urllib.parse.urlencode({"title": "TestBoard", "asNew": "1"}) + save_response, save_payload = self.fetch_json( + "/pedalboard/save", method="POST", body=save_body + ) + self.assertEqual(save_response.code, 200) + self.assertTrue(save_payload["ok"]) + bundlepath = save_payload["bundlepath"] + self.assertEqual(save_payload["title"], "TestBoard") + + # 3. "List now includes TestBoard" -- verified on disk (this is + # exactly what get_all_pedalboards() would scan), NOT via a live + # GET /pedalboard/list call, which segfaults with a real bundle + # present (see module docstring). + self.assertTrue(os.path.isdir(bundlepath)) + self.assertEqual( + os.listdir(settings.LV2_PEDALBOARDS_DIR), + [os.path.basename(bundlepath)], + ) + + # 4. Info. + info_response, info = self.fetch_json( + "/pedalboard/info/?bundlepath=" + _quote(bundlepath) + ) + self.assertEqual(info_response.code, 200) + self.assertEqual(info["title"], "TestBoard") + + # 5. Remove. + remove_response = self.fetch( + "/pedalboard/remove/?bundlepath=" + _quote(bundlepath) + ) + self.assertEqual(remove_response.code, 200) + self.assertEqual(remove_response.body, b"true") + self.assertFalse(os.path.exists(bundlepath)) + + # Now the dir is empty again, so a live /pedalboard/list call is + # safe once more (see module docstring: only a NON-empty dir + # segfaults) and should be back to baseline. + final_response, final_body = self.fetch_json("/pedalboard/list") + self.assertEqual(final_response.code, 200) + self.assertEqual(final_body, []) + + +class TestPedalboardLoadBundle(ModUITestCase): + __test__ = True + + def test_load_bundle_on_bogus_bundlepath_returns_ok_false(self): + body = urllib.parse.urlencode( + {"bundlepath": "/nonexistent/thing.pedalboard", "isDefault": "0"} + ) + response, payload = self.fetch_json( + "/pedalboard/load_bundle/", method="POST", body=body + ) + self.assertEqual(response.code, 200) + self.assertEqual(payload, {"ok": False, "name": ""}) + + def test_load_bundle_on_real_saved_bundle_returns_ok_true_with_name(self): + save_body = urllib.parse.urlencode({"title": "TestBoard", "asNew": "1"}) + _, save_payload = self.fetch_json( + "/pedalboard/save", method="POST", body=save_body + ) + bundlepath = save_payload["bundlepath"] + + load_body = urllib.parse.urlencode( + {"bundlepath": bundlepath, "isDefault": "0"} + ) + response, payload = self.fetch_json( + "/pedalboard/load_bundle/", method="POST", body=load_body + ) + self.assertEqual(response.code, 200) + self.assertEqual(payload, {"ok": True, "name": "TestBoard"}) + + # Reset SESSION before cleanup, per phase-3 spec's order-dependence + # warning: SESSION.host is a process-global singleton, so a test + # that loads a pedalboard must reset afterwards. + reset_response = self.fetch("/reset/") + self.assertEqual(reset_response.code, 200) + + self.fetch("/pedalboard/remove/?bundlepath=" + _quote(bundlepath)) diff --git a/test/test_snapshots.py b/test/test_snapshots.py new file mode 100644 index 000000000..566a9e3a3 --- /dev/null +++ b/test/test_snapshots.py @@ -0,0 +1,158 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the snapshot endpoints (mod/webserver.py: +SnapshotList, SnapshotName, SnapshotSave, SnapshotSaveAs). + +Snapshots live entirely on SESSION.host (Python lists/dicts) -- unlike the +pedalboard endpoints in test_pedalboards.py, nothing here calls into lilv, +so there is no segfault risk (confirmed by direct probing). + +SESSION.host is a process-global singleton (mod/session.py), and these +handlers mutate it (pedalboard_snapshots, current_pedalboard_snapshot_id). +The autouse `pedalboards_dir` fixture (test/conftest.py) calls +SESSION.reset() before and after every test, so each test method here +starts from a known-fresh state: pedalboard_snapshots == [], and +current_pedalboard_snapshot_id == -1 (mod/host.py Host.__init__ defaults; +SESSION.reset() restores current_pedalboard_snapshot_id to 0 with a single +"Default" snapshot via host.snapshot_clear() -- see the "after /reset" +tests below, which pin that distinction). +""" + +import urllib.parse + +from base import ModUITestCase + + +class TestSnapshotFreshSession(ModUITestCase): + __test__ = True + + def test_list_after_fixture_reset_has_one_default_entry(self): + # NOT {} -- host.snapshot_clear() (invoked by SESSION.reset(), which + # the autouse pedalboards_dir fixture runs before every test) seeds + # exactly one "Default" snapshot at index 0 (host.py:3052-3053). + # See TestSnapshotSaveWithoutAnyReset below for the true "nothing at + # all" shape ({}), reached only by clearing the list by hand. + response, body = self.fetch_json("/snapshot/list") + self.assertEqual(response.code, 200) + self.assertEqual(body, {"0": "Default"}) + + def test_name_with_default_id_after_reset(self): + response, body = self.fetch_json("/snapshot/name?id=0") + self.assertEqual(response.code, 200) + self.assertEqual(body, {"ok": True, "name": "Default"}) + + def test_name_with_out_of_range_id_falls_back_to_default_name(self): + # snapshot_name(idx) returns None for an out-of-range idx, and the + # handler falls back to DEFAULT_SNAPSHOT_NAME ("Default") -- so this + # is NOT a 404/error shape, it's indistinguishable in its "ok": True + # shape from a real snapshot named "Default". + response, body = self.fetch_json("/snapshot/name?id=99") + self.assertEqual(response.code, 200) + self.assertEqual(body, {"ok": True, "name": "Default"}) + + def test_save_after_reset_succeeds_since_reset_seeds_snapshot_0(self): + # SESSION.reset() leaves current_pedalboard_snapshot_id == 0 with a + # real snapshot at index 0 (see host.py snapshot_clear()), so + # SnapshotSave succeeds here -- contrast with + # TestSnapshotSaveWithoutAnyReset below, which pins the *true* + # "nothing to save" shape (current_pedalboard_snapshot_id == -1). + response = self.fetch("/snapshot/save", method="POST", body="") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"true") + + def test_saveas_creates_new_snapshot_and_appears_in_list(self): + response, payload = self.fetch_json("/snapshot/saveas?title=Foo") + self.assertEqual(response.code, 200) + self.assertEqual(payload["ok"], True) + self.assertEqual(payload["title"], "Foo") + new_id = payload["id"] + + list_response, list_body = self.fetch_json("/snapshot/list") + self.assertEqual(list_response.code, 200) + self.assertEqual(list_body[str(new_id)], "Foo") + # the pre-existing "Default" snapshot (seeded by SESSION.reset()) + # is still present alongside the new one. + self.assertIn("0", list_body) + + +class TestSnapshotSaveWithoutAnyReset(ModUITestCase): + __test__ = True + + def test_save_returns_false_when_current_snapshot_id_is_negative_one(self): + # Pins the "truly nothing to save" shape from a session that has + # never had snapshot_clear() run at all: Host.__init__ (mod/host.py) + # defaults current_pedalboard_snapshot_id to -1, and snapshot_save() + # returns False whenever that index is out of range. We reach this + # state by resetting the underlying attribute directly (the HTTP + # surface has no route that produces it, since even GET /reset + # seeds a "Default" snapshot at id 0 -- see module docstring). + from mod.webserver import SESSION + + SESSION.host.pedalboard_snapshots = [] + SESSION.host.current_pedalboard_snapshot_id = -1 + + response = self.fetch("/snapshot/save", method="POST", body="") + self.assertEqual(response.code, 200) + self.assertEqual(response.body, b"false") + + def test_list_is_empty_dict_when_snapshots_list_is_empty(self): + from mod.webserver import SESSION + + SESSION.host.pedalboard_snapshots = [] + + response, body = self.fetch_json("/snapshot/list") + self.assertEqual(response.code, 200) + self.assertEqual(body, {}) + + +class TestSnapshotRoundTripWithRealPedalboard(ModUITestCase): + __test__ = True + + def test_saveas_and_list_after_loading_a_real_pedalboard(self): + # Save a real bundle, load it back (this reseeds + # pedalboard_snapshots with a single "Default" snapshot via + # host.load() -> save_state_snapshots()/snapshots.json handling), + # then exercise snapshot/saveas + snapshot/list against it. + save_body = urllib.parse.urlencode({"title": "TestBoard", "asNew": "1"}) + _, save_payload = self.fetch_json( + "/pedalboard/save", method="POST", body=save_body + ) + bundlepath = save_payload["bundlepath"] + + load_body = urllib.parse.urlencode( + {"bundlepath": bundlepath, "isDefault": "0"} + ) + load_response, load_payload = self.fetch_json( + "/pedalboard/load_bundle/", method="POST", body=load_body + ) + self.assertEqual(load_response.code, 200) + self.assertTrue(load_payload["ok"]) + + list_response, list_body = self.fetch_json("/snapshot/list") + self.assertEqual(list_response.code, 200) + self.assertEqual(list_body, {"0": "Default"}) + + saveas_response, saveas_payload = self.fetch_json( + "/snapshot/saveas?title=Verse" + ) + self.assertEqual(saveas_response.code, 200) + self.assertEqual(saveas_payload["ok"], True) + self.assertEqual(saveas_payload["title"], "Verse") + + list_response2, list_body2 = self.fetch_json("/snapshot/list") + self.assertEqual(list_body2, {"0": "Default", "1": "Verse"}) + + # Order-dependence warning (phase-3 spec): reset SESSION.host before + # removing the bundle, so later tests see a clean session -- not + # via a live /pedalboard/list call (segfaults with a real bundle + # present, see test_pedalboards.py's module docstring), so we go + # straight to /reset/ then remove. + reset_response = self.fetch("/reset/") + self.assertEqual(reset_response.code, 200) + + self.fetch( + "/pedalboard/remove/?bundlepath=" + + urllib.parse.quote(bundlepath, safe="") + ) From bb2b10385c584a23529b9d473d8fc91d9b0b3cc9 Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 14:27:35 +0000 Subject: [PATCH 5/7] Add fake-host command characterization tests (phase 4) Pins webserver handler behavior under FakeHost/FakeHMI (not the production mod-host protocol): connect/disconnect always true, parameter set short-circuits on uninitialized HMI, reset, buffersize {ok:false,size:0} in dev, xruns, midi device shapes (500 on missing body keys), truebypass false, transport sync modes, cv port add 500 without plugin instances. /effect/remove/ is added to the never-call list: for any unregistered instance the KeyError from mapper.get_id_without_creating is swallowed by the gen.coroutine future in Host.remove_plugin, the callback never fires, and the request hangs forever. Co-Authored-By: Claude Fable 5 --- test/conftest.py | 14 ++ test/test_host_commands.py | 296 +++++++++++++++++++++++++++++++++++++ 2 files changed, 310 insertions(+) create mode 100644 test/test_host_commands.py diff --git a/test/conftest.py b/test/conftest.py index 7033b54a8..d7f489118 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -39,6 +39,20 @@ (NamespaceDefinitions::init / lilv_new_uri, needs the uninitialized global lilv world). Both are safe against an EMPTY pedalboards dir; save/remove a bundle within one test and only list after the dir is empty again +- ``/effect/remove/`` HANGS (no crash, no timeout server-side -- + the response is simply never sent) for any instance name not already + registered in ``SESSION.host.mapper``, which is every instance name in + this sandbox (``POST /effect/add`` is itself banned above, so no instance + can ever be registered). Root cause: ``Host.remove_plugin`` + (``mod/host.py:2604``, ``@gen.coroutine``) calls + ``self.mapper.get_id_without_creating(instance)`` *before* its own + try/except KeyError guard around ``self.plugins.pop(...)`` a few lines + down -- the KeyError from the lookup itself is swallowed into the + coroutine's Future instead of propagating, so the handler's + ``callback(False)`` is never reached and ``gen.Task`` in ``EffectRemove`` + never resolves. Confirmed by probing with a 4s client-side + ``request_timeout``: HTTP 599 (client timeout), not a fast response. See + ``test/test_host_commands.py`` module docstring (phase 4). """ import atexit diff --git a/test/test_host_commands.py b/test/test_host_commands.py new file mode 100644 index 000000000..27ca1cbab --- /dev/null +++ b/test/test_host_commands.py @@ -0,0 +1,296 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the handlers that route into +SESSION.host/SESSION.hmi (mod/webserver.py: EffectConnect, EffectDisconnect, +EffectParameterSet, EffectParameterAddress, DashboardClean, SetBufferSize, +ResetXruns, JackGetMidiDevices, JackSetMidiDevices, TrueBypass, +PedalboardTransportSetSyncMode, PedalboardCvAddressingPluginPortAdd). + +Honest caveat: these tests pin the behavior of the mod/webserver.py handler +code under FakeHost/FakeHMI (mod/development.py), NOT the production +mod-host protocol. They are a regression net for webserver.py edits, +nothing more. + +CRITICAL, spec-overriding finding: GET /effect/remove/ HANGS (does +not error, does not time out server-side -- the HTTP response is simply +never sent) when was never registered with SESSION.host.mapper, +which is true of *every* instance name in this sandbox since we cannot call +POST /effect/add (banned -- dereferences the uninitialized global lilv +world, see test/conftest.py NEVER-CALL LIST). Root cause, confirmed by +direct probing with a 4s client-side request_timeout (got HTTP 599 after +the timeout, not a fast response): Host.remove_plugin (mod/host.py:2604, +decorated @gen.coroutine) calls self.mapper.get_id_without_creating(instance) +*before* its own try/except KeyError guard around self.plugins.pop(...). A +nonexistent instance means that lookup itself raises KeyError, which the +@gen.coroutine machinery captures into the returned (never awaited-on) +Future instead of propagating synchronously -- so the handler's +`callback(False)` line is never reached, gen.Task in EffectRemove never +resolves, and the request hangs until the HTTP client's own timeout. This +class does not call GET /effect/remove/* at all; the route has been added to +the test/conftest.py NEVER-CALL LIST with this reasoning. + +Every test below that mutates SESSION.host state relies on the autouse +`pedalboards_dir` fixture (test/conftest.py) to call SESSION.reset() before +and after each test method, EXCEPT sync-mode: SESSION.reset() does not touch +SESSION.host.profile, so the sync-mode test explicitly restores the default +("/none") at the end. +""" + +import json +import urllib.parse + +from base import ModUITestCase + + +class TestEffectConnectDisconnect(ModUITestCase): + __test__ = True + + def test_connect_between_system_ports_returns_true_and_is_idempotent(self): + # "/graph/capture_1" and "/graph/playback_1" are hardware-port + # shortcuts handled entirely inside Host._fix_host_connection_port + # (mod/host.py) without touching the plugin mapper, so they are safe + # "syntactically valid but nonexistent instance" stand-ins per the + # spec's guidance. FakeHost.send_modified (mod/development.py) + # invokes its callback immediately with True regardless of whether + # the ports are real JACK ports -- there is no validation under the + # fakes. + response, body = self.fetch_json( + "/effect/connect/graph/capture_1,/graph/playback_1" + ) + self.assertEqual(response.code, 200) + self.assertIs(body, True) + + # Second call: (port_from, port_to) is now already in + # self.connections, so Host.connect short-circuits to callback(True) + # without going through send_modified again -- still True. + response2, body2 = self.fetch_json( + "/effect/connect/graph/capture_1,/graph/playback_1" + ) + self.assertEqual(response2.code, 200) + self.assertIs(body2, True) + + # Clean up: disconnect so SESSION.host.connections is restored to + # empty (belt-and-braces -- the autouse fixture would clear it too). + response3, body3 = self.fetch_json( + "/effect/disconnect/graph/capture_1,/graph/playback_1" + ) + self.assertEqual(response3.code, 200) + self.assertIs(body3, True) + + def test_disconnect_with_no_active_connections_returns_true(self): + # Spec guessed this might be `false`. Observed: Host.disconnect's + # inner host_callback(ok) *always* calls callback(True) regardless + # of ok (mod/host.py:3422-3436, "always return true" per its own + # comment) -- even the len(self.connections) == 0 short-circuit + # path (host_callback(False)) ends up reporting True to the client. + response, body = self.fetch_json( + "/effect/disconnect/graph/capture_1,/graph/playback_1" + ) + self.assertEqual(response.code, 200) + self.assertIs(body, True) + + +class TestEffectParameterSet(ModUITestCase): + __test__ = True + + def test_parameter_set_short_circuits_true_when_hmi_uninitialized(self): + # FakeHMI.initialized is always False (mod/development.py), so + # EffectParameterSet.post's `if not SESSION.hmi.initialized: + # self.write(True); return` guard fires before the request body is + # even parsed -- an empty body is fine. + response, body = self.fetch_json( + "/effect/parameter/set/", method="POST", body=b"" + ) + self.assertEqual(response.code, 200) + self.assertIs(body, True) + + +class TestEffectParameterAddress(ModUITestCase): + __test__ = True + + def test_address_nonexistent_instance_returns_false(self): + # Unlike EffectRemove, Host.address (mod/host.py:4795) uses + # self.mapper.get_id(instance) -- the *creating* variant, which + # never raises -- so a never-seen instance safely resolves to + # pluginData is None and callback(False) fires synchronously. + body_json = json.dumps( + {"uri": "https://example.org/actuator", "minimum": 0, "maximum": 1, "value": 0.5} + ).encode("utf-8") + response, body = self.fetch_json( + "/effect/parameter/address/graph/nonexistent/gain", + method="POST", + body=body_json, + ) + self.assertEqual(response.code, 200) + self.assertIs(body, False) + + def test_address_missing_uri_is_404(self): + # EffectParameterAddress.post checks `data.get('uri', None) is + # None` before anything else and raises web.HTTPError(404). + response = self.fetch( + "/effect/parameter/address/graph/nonexistent/gain", + method="POST", + body=b"{}", + ) + self.assertEqual(response.code, 404) + + +class TestReset(ModUITestCase): + __test__ = True + + def test_reset_returns_true(self): + # SESSION.reset under FakeHMI (never initialized) takes the + # synchronous reset_host(True) branch straight to Host.reset, and + # FakeHost invokes every send_notmodified callback immediately with + # True. + response, body = self.fetch_json("/reset/") + self.assertEqual(response.code, 200) + self.assertIs(body, True) + + +class TestSetBufferSize(ModUITestCase): + __test__ = True + + def test_128_and_256_return_ok_false_size_zero(self): + # Matches the spec's own prediction: under MOD_DEV_ENVIRONMENT, + # IMAGE_VERSION is None so the /data/jack-buffer-size write is + # skipped, and set_jack_buffer_size (utils/utils_jack.cpp) is a + # null-guarded no-op that returns 0 without a JACK client -- so + # newsize (0) never equals the requested size, and 'ok' is always + # False. + for size in ("128", "256"): + response, body = self.fetch_json( + "/set_buffersize/%s" % size, method="POST", body=b"" + ) + self.assertEqual(response.code, 200) + self.assertEqual(body, {"ok": False, "size": 0}) + + +class TestResetXruns(ModUITestCase): + __test__ = True + + def test_reset_xruns_returns_true(self): + response, body = self.fetch_json("/reset_xruns/", method="POST", body=b"") + self.assertEqual(response.code, 200) + self.assertIs(body, True) + + +class TestJackMidiDevices(ModUITestCase): + __test__ = True + + def test_get_midi_devices_shape_under_fake_host(self): + # No JACK client -> Host.get_midi_ports() (backing + # web_get_midi_device_list) reports no devices; midiAggregatedMode + # defaults to True (mod/host.py:368, Host.__init__). + response, body = self.fetch_json("/jack/get_midi_devices") + self.assertEqual(response.code, 200) + self.assertEqual( + body, + { + "devsInUse": [], + "devList": [], + "names": {}, + "midiAggregatedMode": True, + }, + ) + + def test_set_midi_devices_matching_current_state_is_a_true_noop(self): + # devs=[], midiAggregatedMode=True, midiLoopback=False exactly match + # Host.__init__'s defaults (mod/host.py:368-369), so both the + # "mode changed" and "loopback changed" branches inside + # Host.set_midi_devices are skipped -- a side-effect-free call that + # still returns True (the handler always writes True on success). + body_json = json.dumps( + {"devs": [], "midiAggregatedMode": True, "midiLoopback": False} + ).encode("utf-8") + response, body = self.fetch_json( + "/jack/set_midi_devices", method="POST", body=body_json + ) + self.assertEqual(response.code, 200) + self.assertIs(body, True) + + def test_set_midi_devices_missing_key_is_500(self): + # Spec guessed "echo/true -- observe". Observed: JackSetMidiDevices + # .post does three unguarded dict subscripts (data['devs'], + # data['midiAggregatedMode'], data['midiLoopback']) with no + # try/except -- a body missing any of them raises an uncaught + # KeyError before any yield, which tornado turns into a plain 500, + # not a JSON error shape. + body_json = json.dumps({"devs": []}).encode("utf-8") + response = self.fetch( + "/jack/set_midi_devices", method="POST", body=body_json + ) + self.assertEqual(response.code, 500) + + +class TestTrueBypass(ModUITestCase): + __test__ = True + + def test_truebypass_returns_false_under_fake_jack(self): + # Matches the spec's own guess. set_truebypass_value (utils_jack) + # is null-guarded without a JACK client and its C-side setter + # reports failure, so the handler always writes False under the + # fakes, both channels, both requested states. + response, body = self.fetch_json("/truebypass/Left/true") + self.assertEqual(response.code, 200) + self.assertIs(body, False) + + response2, body2 = self.fetch_json("/truebypass/Right/false") + self.assertEqual(response2.code, 200) + self.assertIs(body2, False) + + +class TestTransportSetSyncMode(ModUITestCase): + __test__ = True + + def test_known_modes_return_true_then_restore_default(self): + # SESSION.reset() does NOT touch SESSION.host.profile, so this test + # explicitly restores the "/none" (internal/default) mode at the + # end to avoid leaking Profile state into later tests. + try: + for mode in ("/none", "/midi_clock_slave", "/link"): + response, body = self.fetch_json( + "/pedalboard/transport/set_sync_mode%s" % mode, + method="POST", + body=b"", + ) + self.assertEqual(response.code, 200) + self.assertIs(body, True) + finally: + self.fetch_json( + "/pedalboard/transport/set_sync_mode/none", method="POST", body=b"" + ) + + def test_invalid_mode_returns_false_not_error(self): + response, body = self.fetch_json( + "/pedalboard/transport/set_sync_mode/bogus", method="POST", body=b"" + ) + self.assertEqual(response.code, 200) + self.assertIs(body, False) + + +class TestCVAddressingPluginPortAdd(ModUITestCase): + __test__ = True + + def test_add_500s_without_an_existing_addressed_plugin_instance(self): + # Spec said "observe". Observed: PedalboardCvAddressingPluginPortAdd + # calls SESSION.web_cv_addressing_plugin_port_add synchronously + # (not yielded), which ends up in + # Host.addr_task_get_plugin_cv_port_op_mode (mod/host.py:970), + # which does self.mapper.get_id_without_creating(instance) on the + # instance embedded in the uri. Since no plugin instance can exist + # in this sandbox (POST /effect/add is banned -- see + # test/conftest.py NEVER-CALL LIST), this *always* raises KeyError + # and always 500s -- there is no well-formed uri that succeeds here + # without lilv. Unlike EffectRemove this is NOT a hang: the + # exception is raised synchronously (no @gen.coroutine boundary + # swallows it), so tornado's normal error path returns a fast 500. + body = urllib.parse.urlencode( + {"uri": "/cv/graph/nonexistent/cvout", "name": "test"} + ) + response = self.fetch( + "/pedalboard/cv_addressing_plugin_port/add", method="POST", body=body + ) + self.assertEqual(response.code, 500) From 9bce915266820deb09ba80ad3db5b89f3eca09f5 Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 14:34:49 +0000 Subject: [PATCH 6/7] Add WebSocket handshake characterization tests (phase 5) Pins the connect-time report_current_state burst pushed to every socket (sys_stats, stats, transport, truebypass, loading_start, size, then the loading_end ready marker), payload shapes for transport/truebypass/size, the identical burst on a second concurrent connection, and clean client close tolerance. Prefixes and arg counts only; volatile payloads unpinned. Co-Authored-By: Claude Fable 5 --- test/test_websocket.py | 284 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 284 insertions(+) create mode 100644 test/test_websocket.py diff --git a/test/test_websocket.py b/test/test_websocket.py new file mode 100644 index 000000000..c7214d7f3 --- /dev/null +++ b/test/test_websocket.py @@ -0,0 +1,284 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the /websocket connect-time handshake +(mod/webserver.py:ServerWebSocket, mod/session.py:Session.websocket_opened, +mod/host.py:Host.report_current_state -- all wired through +mod/development.py:FakeHost.open_connection_if_needed in this dev harness). + +Pins the message-PUSH sequence a client sees right after connecting: this is +the ``msg_callback`` broadcast path a future Tone3000 "file added" push +notification would ride. + +Exploratory findings (see docs/characterization-phase-5.md, verified against +this sandbox -- FakeHMI uninitialized, empty pedalboard, no CC devices, no +hardware ports, /proc/meminfo present so Host.memtimer is armed): + +- The connect-time push on the FIRST socket of the process is exactly 7 + ordered messages, by PREFIX (first whitespace-delimited word): + sys_stats, stats, transport, truebypass, loading_start, size, loading_end + The spec's sketch guessed a "stop"/ready-ish closing token; the real + ready-marker is ``loading_end `` + (mod/host.py:Host.report_current_state, last line before the per-plugin + addressing/HW sections, which are all empty in this sandbox). +- Immediately AFTER that 7-message burst, the first socket receives one + EXTRA ``sys_stats ...`` message. This is not part of + report_current_state's own push -- it is + FakeHost.open_connection_if_needed (mod/development.py) calling + ``self.memtimer_callback()`` synchronously, once, right after + ``report_current_state()`` returns, on the branch where + ``self.readsock``/``self.writesock`` were previously None (i.e. only for + the connection that actually "establishes" the fake host link). This is a + detail the spec's sketch did not anticipate; tests below treat it as an + unpinned trailing message (present in practice in this sandbox, but not + asserted on) rather than pin an exact total message count, per the phase's + "pin prefixes/arg-counts, not volatile detail" and "consume/ignore extra + stats messages" guidance. +- A SECOND, concurrent connection (opened while the first is still open) + gets the exact same 7-message report_current_state burst (every new + websocket gets its own full state dump -- Session.websocket_opened's + first-vs-not-first branch, session.py:236, only decides whether + Host.start_session runs first; it does not change what gets pushed to the + new socket). The observable difference is narrower than "first socket + session branch": it is FakeHost.open_connection_if_needed's *own* branch + on ``self.readsock is not None and self.writesock is not None`` -- true + for every connection after the first -- which skips + statstimer.start()/memtimer_callback() entirely. So only the very first + socket of the process gets the extra trailing sys_stats; the second socket + does not, even though both get an identical initial 7-message burst. +- PeriodicCallback (mod/host.py Host.statstimer/memtimer) binds + ``IOLoop.current()`` at construction time (tornado 4.3 + ioloop.py:PeriodicCallback.__init__), and SESSION.host is a process-level + singleton constructed at ``mod.webserver`` import time -- before any + per-test IOLoop exists. So ``statstimer.start()``/``memtimer.start()`` + schedule ticks on a *different*, never-pumped IOLoop; in practice no + additional "stats ..."/"sys_stats ..." messages arrive during a test's own + ~2s collection window. Tests still avoid asserting exact trailing counts, + since this is an artifact of import ordering, not a documented guarantee. +- Closing the client side of the connection is NOT synchronously reflected + server-side: right after ``conn.close()`` returns, GET /hello still + reports ``online: true`` (SESSION.websockets still has the entry) because + ``ServerWebSocket.on_close`` -> ``SESSION.websocket_closed`` only runs + once the server's IOLoop actually polls and processes the close frame. + This turned out FLAKY to observe deterministically: a bounded number of + ``gen.moment`` spins (pure callback-queue turns, no real I/O wait) is + sometimes enough and sometimes not. Per the phase spec's own guidance + ("count may update asynchronously; drop the assertion rather than + sleep-loop if flaky"), the close test below does not assert on the + post-close ``online`` value -- it only pins that the server tolerates the + close without erroring and keeps answering HTTP requests. +""" + +from tornado import gen, websocket +from tornado.testing import gen_test + +from base import ModUITestCase + +# Generous but bounded -- nothing here should ever legitimately take this +# long; a real hang (behavior regression) fails fast instead of hanging the +# whole suite. +_READ_TIMEOUT = 5.0 + +# Exact pinned prefix sequence of the connect-time report_current_state +# burst in this sandbox (empty pedalboard, no CC devices, no HW ports). +_EXPECTED_PREFIXES = [ + "sys_stats", + "stats", + "transport", + "truebypass", + "loading_start", + "size", + "loading_end", +] + +_READY_MARKER_PREFIX = "loading_end" + + +class WebSocketTestCase(ModUITestCase): + """Shared helpers. Not collected directly (__test__ stays False).""" + + __test__ = False + + def ws_url(self): + return self.get_url("/websocket").replace("http://", "ws://") + + @gen.coroutine + def read_one(self, conn, timeout=_READ_TIMEOUT): + """Read a single message with a hard timeout so a behavior change + (e.g. the ready marker disappearing) fails fast instead of hanging. + """ + msg = yield gen.with_timeout(self.io_loop.time() + timeout, conn.read_message()) + raise gen.Return(msg) + + @gen.coroutine + def drain_until_ready(self, conn, timeout=_READ_TIMEOUT): + """Collect messages up to and including the ready marker + (``loading_end ...``). Returns the list of messages, ready marker + included. Raises (via with_timeout / assertion) if the marker never + arrives or the socket closes first. + """ + messages = [] + while True: + msg = yield self.read_one(conn, timeout=timeout) + if msg is None: + self.fail( + "server closed the websocket before the ready marker " + "(%r) arrived; messages so far: %r" + % (_READY_MARKER_PREFIX, messages) + ) + messages.append(msg) + if msg.split(" ", 1)[0] == _READY_MARKER_PREFIX: + break + raise gen.Return(messages) + + +class TestWebSocketConnectSequence(WebSocketTestCase): + __test__ = True + + @gen_test(timeout=10) + def test_connect_pushes_pinned_prefix_sequence(self): + conn = yield websocket.websocket_connect(self.ws_url()) + try: + messages = yield self.drain_until_ready(conn) + prefixes = [m.split(" ", 1)[0] for m in messages] + self.assertEqual(prefixes, _EXPECTED_PREFIXES) + finally: + conn.close() + + @gen_test(timeout=10) + def test_transport_message_shape(self): + conn = yield websocket.websocket_connect(self.ws_url()) + try: + messages = yield self.drain_until_ready(conn) + transport_msgs = [m for m in messages if m.split(" ", 1)[0] == "transport"] + self.assertEqual(len(transport_msgs), 1) + + parts = transport_msgs[0].split(" ") + # "transport %i %f %f %s" -- prefix + 4 fields. + self.assertEqual(len(parts), 5) + rolling, bpb, bpm, sync = parts[1:] + + # arg-count + parseability is the load-bearing assertion; the + # concrete defaults below are deterministic Profile() config + # (not volatile like cpu%/timestamps), so pinning them is safe + # in this fresh-sandbox harness. + self.assertEqual(int(rolling), 0) + self.assertEqual(float(bpb), 4.0) + self.assertEqual(float(bpm), 120.0) + self.assertEqual(sync, "none") + finally: + conn.close() + + @gen_test(timeout=10) + def test_truebypass_message_reflects_defaults(self): + conn = yield websocket.websocket_connect(self.ws_url()) + try: + messages = yield self.drain_until_ready(conn) + tb_msgs = [m for m in messages if m.split(" ", 1)[0] == "truebypass"] + self.assertEqual(len(tb_msgs), 1) + + parts = tb_msgs[0].split(" ") + # "truebypass %i %i" -- prefix + 2 fields. + self.assertEqual(len(parts), 3) + left, right = int(parts[1]), int(parts[2]) + # Observed default (Host.last_true_bypass_left/right initial + # state in this dev sandbox): both channels report "true bypass + # on". + self.assertEqual((left, right), (1, 1)) + finally: + conn.close() + + @gen_test(timeout=10) + def test_size_message_present_with_two_numeric_args(self): + conn = yield websocket.websocket_connect(self.ws_url()) + try: + messages = yield self.drain_until_ready(conn) + size_msgs = [m for m in messages if m.split(" ", 1)[0] == "size"] + self.assertEqual(len(size_msgs), 1) + + parts = size_msgs[0].split(" ") + # "size %d %d" -- prefix + 2 fields. + self.assertEqual(len(parts), 3) + width, height = int(parts[1]), int(parts[2]) + # Empty/default pedalboard in a fresh sandbox -> zero size. + self.assertEqual((width, height), (0, 0)) + finally: + conn.close() + + @gen_test(timeout=10) + def test_second_concurrent_connection_gets_full_burst_but_no_trailing_extra(self): + conn1 = yield websocket.websocket_connect(self.ws_url()) + try: + messages1 = yield self.drain_until_ready(conn1) + prefixes1 = [m.split(" ", 1)[0] for m in messages1] + self.assertEqual(prefixes1, _EXPECTED_PREFIXES) + + # The first socket also gets one extra trailing sys_stats + # (FakeHost.open_connection_if_needed's memtimer_callback() + # kick, see module docstring). Consume it so it can't bleed + # into the second connection's read below; don't fail if it + # doesn't show up (it is an unpinned, environment-dependent + # detail). + try: + extra = yield self.read_one(conn1, timeout=1.0) + if extra is not None: + self.assertEqual(extra.split(" ", 1)[0], "sys_stats") + except gen.TimeoutError: + pass + + # Second connection, opened while the first is still open -- + # exercises Session.websocket_opened's "not the first socket" + # branch (session.py:236). + conn2 = yield websocket.websocket_connect(self.ws_url()) + try: + messages2 = yield self.drain_until_ready(conn2) + prefixes2 = [m.split(" ", 1)[0] for m in messages2] + # Same full report_current_state burst as the first socket. + self.assertEqual(prefixes2, _EXPECTED_PREFIXES) + + # But NOT the extra trailing sys_stats: FakeHost's + # readsock/writesock are already set by the time the second + # socket connects, so open_connection_if_needed takes the + # early-return branch (report_current_state only, no + # statstimer/memtimer kick). + with self.assertRaises(gen.TimeoutError): + yield self.read_one(conn2, timeout=1.0) + finally: + conn2.close() + finally: + conn1.close() + + +class TestWebSocketClose(WebSocketTestCase): + __test__ = True + + @gen_test(timeout=10) + def test_clean_close_no_server_error_and_hello_reflects_it(self): + conn = yield websocket.websocket_connect(self.ws_url()) + yield self.drain_until_ready(conn) + + response = yield self.http_client.fetch(self.get_url("/hello/")) + self.assertEqual(response.code, 200) + self.assertIn(b'"online": true', response.body) + + conn.close() + + # DROPPED (per phase-5 spec §6: "count may update asynchronously; + # drop the assertion rather than sleep-loop if flaky"): a strict + # assertion that /hello's `online` flips to false shortly after + # close. websocket_closed (session.py) only runs once the server's + # IOLoop actually polls and processes the close frame -- draining a + # bounded number of `gen.moment`s (pure callback-queue turns, no + # real I/O wait) was NOT enough to observe it in this harness + # (confirmed flaky: failed after 10 moments in a run where the + # exploratory script's 5-moments-after-two-fetches version happened + # to see it flip). A real wait would require an actual sleep/poll, + # which the spec explicitly says not to add. So this test only pins + # what's reliable: the server tolerates a client-initiated close + # without erroring, and a subsequent HTTP request still succeeds + # (well-formed JSON, 200) -- it does not assert on the `online` + # value after close. + response = yield self.http_client.fetch(self.get_url("/hello/")) + self.assertEqual(response.code, 200) + self.assertIn(b'"online"', response.body) From 37b07d5c43faf08b7503daabe048b1eb8787c5db Mon Sep 17 00:00:00 2001 From: Luis Fagundes Date: Thu, 9 Jul 2026 14:47:48 +0000 Subject: [PATCH 7/7] Add opt-in lilv plugin-endpoint characterization tests (phase 6) /effect/* endpoints need the global lilv world initialized or they segfault, so these tests are excluded from the default run via markers (lilv: empty-world behavior; lilv_fixture: a binaryless .lv2 fixture bundle). LV2_PATH points at an empty sandbox dir for determinism. Pins: /effect/list -> [], /effect/get on unknown uri -> 404 HTML, /effect/bulk skips unknown uris and 501s without a JSON content type, /effect/add on unknown uri mutates host state before failing late with 404. The two marker sets must never run in one process: a second modtools.utils.init() corrupts lilv's namespace singleton and segfaults at interpreter exit (documented in pytest.ini and both test modules). Co-Authored-By: Claude Fable 5 --- pytest.ini | 33 ++++ test/conftest.py | 21 +++ test/fixtures/phase6-fixture.lv2/manifest.ttl | 7 + .../phase6-fixture.lv2/phase6-fixture.ttl | 25 +++ test/test_effects_lilv.py | 174 ++++++++++++++++++ test/test_effects_lilv_fixture.py | 147 +++++++++++++++ 6 files changed, 407 insertions(+) create mode 100644 test/fixtures/phase6-fixture.lv2/manifest.ttl create mode 100644 test/fixtures/phase6-fixture.lv2/phase6-fixture.ttl create mode 100644 test/test_effects_lilv.py create mode 100644 test/test_effects_lilv_fixture.py diff --git a/pytest.ini b/pytest.ini index 3b84f2109..3655b1c20 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,3 +1,36 @@ [pytest] testpaths = test python_files = test_*.py +markers = + lilv: tests that initialize the global lilv world (opt-in; run with -m lilv). + Deliberately excluded from the default run by addopts below -- see + docs/characterization-phase-6.md. These tests call modtools.utils.init(), + which is process-global and has no de-init in this suite (cleanup() + exists but is intentionally not called -- see test/test_effects_lilv.py). + A combined run (`pytest -m "(lilv or not lilv) and not lilv_fixture"` + -- see the lilv_fixture marker below for why the "and not lilv_fixture" + is required) was verified green with no observed leakage into other + tests as of phase 6, but -m lilv should still be treated as its own + invocation in CI to keep the blast radius of a lilv/JACK-adjacent + regression contained to one job. + lilv_fixture: the phase-6 stretch-goal tests (test/test_effects_lilv_fixture.py) + that scan a hand-written, binaryless .lv2 bundle. Opt-in; run with + -m lilv_fixture, and ONLY that -- as its OWN, separate pytest + invocation. DO NOT run together with -m lilv: modtools.utils.init() + is only safe to call ONCE per process. Calling it a second time (this + module's fixture vs. test_effects_lilv.py's) leaves lilv's + process-global NamespaceDefinitions singleton bound to a freed world + -- confirmed by direct reproduction to silently return empty + ports/category data and then SIGSEGV the interpreter on exit. Keeping + this marker's tests in a file of their own, with their own single + init() call and their own `pytest -m lilv_fixture` invocation, is not + a style preference, it is the only way to keep this suite + segfault-free. + DANGER, easy to get wrong: a bare `-m "lilv or not lilv"` (the + phase-6 spec's combined-run check) ALSO collects lilv_fixture tests, + because a lilv_fixture-only test is NOT marked lilv and therefore + satisfies "not lilv" -- that recreates the exact double-init crash + above. When running that combined check, always add + `and not lilv_fixture`: + `pytest -m "(lilv or not lilv) and not lilv_fixture"`. +addopts = -m "not lilv and not lilv_fixture" diff --git a/test/conftest.py b/test/conftest.py index d7f489118..a480d3325 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -90,11 +90,17 @@ _USER_FILES_DIR = os.path.join(_TEST_ROOT, "user-files") _PEDALBOARDS_DIR = os.path.join(_TEST_ROOT, "pedalboards") _PLUGINS_DIR = os.path.join(_TEST_ROOT, "lv2") +# Empty, dedicated -- deliberately NOT the same dir as _PLUGINS_DIR +# (MOD_USER_PLUGINS_DIR) above, to avoid conflating "where mod-ui thinks user +# plugins live" with "what lilv scans for the phase-6 tests" even though +# both happen to be empty today. +_LV2_SCAN_DIR = os.path.join(_TEST_ROOT, "lv2-path-empty") os.makedirs(_DATA_DIR, exist_ok=True) os.makedirs(_USER_FILES_DIR, exist_ok=True) os.makedirs(_PEDALBOARDS_DIR, exist_ok=True) os.makedirs(_PLUGINS_DIR, exist_ok=True) +os.makedirs(_LV2_SCAN_DIR, exist_ok=True) # Replicate the bits of mod.check_environment() that handlers rely on but # that we never call (check_environment() only runs inside prepare()). @@ -115,6 +121,21 @@ os.environ["MOD_USER_PEDALBOARDS_DIR"] = _PEDALBOARDS_DIR os.environ["MOD_USER_PLUGINS_DIR"] = _PLUGINS_DIR +# Phase 6 (docs/characterization-phase-6.md): lilv (the C++ library backing +# modtools.utils.init()) reads the LV2_PATH env var itself, via plain getenv, +# at lilv_world_load_all() time inside init() -- not at Python import time +# and not cached anywhere in Python. Confirmed by reading utils/utils_lilv.cpp +# init() (~:3893): it only calls lilv_world_new()/lilv_world_load_all(W), with +# no explicit LV2_PATH manipulation of its own (unlike e.g. get_all_pedalboards +# a few hundred lines below, which saves/restores LV2_PATH around its own +# private world). So setting os.environ["LV2_PATH"] here, well before any +# phase-6 test module even calls init() at fixture time, is sufficient -- +# CPython's os.environ.__setitem__ calls os.putenv() under the hood, which is +# what a later getenv() in the C library will see. Point it at an empty, +# sandboxed dir so the lilv world phase 6 builds is always empty and +# deterministic, never the machine's real ~/.lv2 or /usr/lib/lv2. +os.environ["LV2_PATH"] = _LV2_SCAN_DIR + # ---------------------------------------------------------------------------- # 1b. Sandbox guard: every writable path mod.settings resolves must live # under the throwaway test root. mod.settings only reads env vars and diff --git a/test/fixtures/phase6-fixture.lv2/manifest.ttl b/test/fixtures/phase6-fixture.lv2/manifest.ttl new file mode 100644 index 000000000..636a5adb3 --- /dev/null +++ b/test/fixtures/phase6-fixture.lv2/manifest.ttl @@ -0,0 +1,7 @@ +@prefix lv2: . +@prefix rdfs: . + + + a lv2:Plugin ; + lv2:binary ; + rdfs:seeAlso . diff --git a/test/fixtures/phase6-fixture.lv2/phase6-fixture.ttl b/test/fixtures/phase6-fixture.lv2/phase6-fixture.ttl new file mode 100644 index 000000000..8c8561d25 --- /dev/null +++ b/test/fixtures/phase6-fixture.lv2/phase6-fixture.ttl @@ -0,0 +1,25 @@ +@prefix lv2: . +@prefix doap: . +@prefix rdfs: . +@prefix foaf: . + + + a lv2:Plugin, lv2:UtilityPlugin ; + doap:name "Phase 6 Fixture" ; + doap:license ; + doap:maintainer [ + foaf:name "mod-ui characterization tests" + ] ; + lv2:microVersion 0 ; + lv2:minorVersion 1 ; + lv2:port [ + a lv2:InputPort, lv2:AudioPort ; + lv2:index 0 ; + lv2:symbol "in" ; + lv2:name "In" + ] , [ + a lv2:OutputPort, lv2:AudioPort ; + lv2:index 1 ; + lv2:symbol "out" ; + lv2:name "Out" + ] . diff --git a/test/test_effects_lilv.py b/test/test_effects_lilv.py new file mode 100644 index 000000000..6262d40c0 --- /dev/null +++ b/test/test_effects_lilv.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Characterization tests for the /effect/* plugin-info handlers +(mod/webserver.py: EffectList, EffectGet, EffectGetNonCached, EffectBulk, +EffectAdd) that dereference the global lilv world (utils/utils_lilv.cpp +W/PLUGINS, initialized by modtools.utils.init()). + +Phase 6 (docs/characterization-phase-6.md): this whole module is the +controlled, opt-in exception to the test/conftest.py NEVER-CALL LIST entry +for these exact routes. Every test here runs against an EMPTY lilv world +(LV2_PATH pointed at a dedicated empty sandbox dir in test/conftest.py) -- +no real plugin is ever scanned, so every URI used below is, from lilv's +point of view, unknown/nonexistent. + +Opt-in via the `lilv` pytest marker (see pytest.ini): `pytestmark` below +marks every test in this module, and pytest.ini's `addopts = -m "not lilv"` +excludes the whole module from the default `pytest` run. Run explicitly with +`pytest -m lilv`. + +modtools.utils.init() is the exact function mod/webserver.py's lv2_init() +(aliased import at webserver.py:52, called inside prepare() at :2451) calls +-- confirmed by reading modtools/utils.py's init() (:708), which is a bare +`utils.init()` ctypes call into utils/utils_lilv.cpp init() (:3893: +lilv_world_free + lilv_world_new + lilv_world_load_all + namespace setup, +no JACK calls). This fixture calls the same modtools.utils.init(), skipping +the rest of webserver.prepare() (HMI/host/JACK setup), matching the spec's +guidance to call the same underlying function without the surrounding +production bootstrap. + +No de-init: modtools.utils.cleanup() exists but is deliberately not called +here -- it frees the global lilv world (W = nullptr), and any other test +process code that runs after it (including non-lilv tests, if ever run in +the same process) would then be back to square one, NULL-W, for any /effect/* +call. Leaving the world initialized-but-empty for the rest of the process +is the safer of two bad options and is exactly why this suite keeps these +tests opt-in/marker-gated rather than trying to isolate them automatically. + +The stretch-goal hand-written-.lv2-bundle tests (docs/characterization- +phase-6.md) live in a SEPARATE file, test/test_effects_lilv_fixture.py, +under a SEPARATE marker (`lilv_fixture`), run as its own invocation +(`pytest -m lilv_fixture`) -- NEVER combined in the same process as this +module's `lilv_world` fixture. Reason (load-bearing finding, see that +file's docstring): modtools.utils.init() is safe to call once per process, +but calling it a SECOND time (e.g. to reload with a bundle present) does +not correctly reset lilv's NamespaceDefinitions::getStaticInstance() +singleton (utils/utils_lilv.cpp ~:540), which is a process-global object +independent of the LilvWorld it was last init()ed with. A second init() +call left get_plugin_info()'s ports/category silently empty (instead of +the real data) and reliably crashed the interpreter on process exit +(SIGSEGV during native static/global teardown) -- confirmed by direct +reproduction, not a guess. One init() per process is a hard constraint of +this C extension as used here. +""" + +import json + +from base import ModUITestCase + +import pytest + +pytestmark = pytest.mark.lilv + + +@pytest.fixture(scope="module", autouse=True) +def lilv_world(): + from modtools import utils + utils.init() + yield + # Deliberately no cleanup()/de-init -- see module docstring. + + +class TestEffectList(ModUITestCase): + __test__ = True + + def test_empty_world_returns_empty_list(self): + response = self.fetch("/effect/list") + self.assertEqual(response.code, 200) + content_type = response.headers.get("Content-Type", "") + self.assertIn("application/json", content_type) + self.assertEqual(json.loads(response.body.decode("utf-8")), []) + + +class TestEffectGet(ModUITestCase): + __test__ = True + + def test_unknown_uri_is_404(self): + # get_plugin_info() (modtools/utils.py) raises when the C lookup + # returns NULL for an unknown URI; EffectGet's bare `except:` around + # it turns that into web.HTTPError(404). CachedJsonRequestHandler + # does not override write_error, so this is tornado's default HTML + # error page, NOT a JSON body -- unlike every other handler in this + # suite, do not decode this response as JSON. + response = self.fetch("/effect/get?uri=urn:nonexistent") + self.assertEqual(response.code, 404) + + +class TestEffectGetNonCached(ModUITestCase): + __test__ = True + + def test_unknown_uri_is_404(self): + # Same shape as EffectGet: get_non_cached_plugin_info() raises for + # an unknown URI, caught by the handler's bare `except:`, re-raised + # as web.HTTPError(404) -> tornado's default HTML error page. + response = self.fetch("/effect/get_non_cached?uri=urn:nonexistent") + self.assertEqual(response.code, 404) + + +class TestEffectBulk(ModUITestCase): + __test__ = True + + def _post_bulk(self, uris): + return self.fetch( + "/effect/bulk/", + method="POST", + headers={"Content-Type": "application/json"}, + body=json.dumps(uris), + ) + + def test_empty_list_returns_empty_object(self): + response = self._post_bulk([]) + self.assertEqual(response.code, 200) + content_type = response.headers.get("Content-Type", "") + self.assertIn("application/json", content_type) + self.assertEqual(json.loads(response.body.decode("utf-8")), {}) + + def test_unknown_uris_are_silently_skipped(self): + # EffectBulk.post() calls get_plugin_info(uri) per URI inside a bare + # try/except that `continue`s on failure -- an unknown URI simply + # never makes it into the result dict, no error surfaced at all. + response = self._post_bulk(["urn:nonexistent-1", "urn:nonexistent-2"]) + self.assertEqual(response.code, 200) + self.assertEqual(json.loads(response.body.decode("utf-8")), {}) + + def test_missing_content_type_is_501(self): + # EffectBulk.prepare() requires "application/json" in Content-Type, + # else raises web.HTTPError(501) before post() ever runs. + response = self.fetch("/effect/bulk/", method="POST", body=json.dumps([])) + self.assertEqual(response.code, 501) + + +class TestEffectAdd(ModUITestCase): + __test__ = True + + def test_unknown_uri_ends_in_404_not_false(self): + # SPEC DEVIATION (docs/characterization-phase-6.md guessed "false?"): + # observed behavior is HTTP 404, not a JSON `false` body. Traced: + # 1. EffectAdd.get() does `ok = yield gen.Task(SESSION.web_add, ...)`. + # 2. web_add -> Host.add_plugin -> FakeHost.send_modified(msg, host_callback) + # (mod/development.py) calls host_callback(True) unconditionally + # and synchronously -- no real mod-host round trip. + # 3. Inside add_plugin's host_callback, `resp` is `True`; the guard + # is `if resp < 0`, and `True < 0` is False in Python, so it does + # NOT bail out early despite the URI being unknown. + # 4. It then calls get_plugin_info_essentials(uri) (modtools/utils.py), + # which is NULL-safe: an unknown URI returns a defaults dict + # (`{'error': True, 'controlInputs': [], ...}`) instead of + # raising. So add_plugin finishes normally, registers a fake + # plugin instance in SESSION.host.plugins, and calls + # callback(True) -- i.e. `ok` is True back in EffectAdd.get(). + # 5. Because ok is truthy, EffectAdd.get() proceeds to + # `data = get_plugin_info(uri)` (NOT get_plugin_info_essentials) + # -- this one DOES raise for an unknown URI (see TestEffectGet + # above), caught by EffectAdd's own bare `except:`, which raises + # web.HTTPError(404). + # Net effect: an unknown-URI add is NOT rejected up front; it silently + # registers bookkeeping state on SESSION.host before failing late, + # with a 404 (tornado's default HTML error page) as the only visible + # signal to the caller -- no JSON, no `false`. + response = self.fetch( + "/effect/add/lilv_phase6_unknown?uri=urn:nonexistent&x=0&y=0" + ) + self.assertEqual(response.code, 404) diff --git a/test/test_effects_lilv_fixture.py b/test/test_effects_lilv_fixture.py new file mode 100644 index 000000000..5bb357d75 --- /dev/null +++ b/test/test_effects_lilv_fixture.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2012-2023 MOD Audio UG +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Phase 6 (docs/characterization-phase-6.md) STRETCH GOAL: a minimal +hand-written .lv2 bundle (test/fixtures/phase6-fixture.lv2/ -- manifest.ttl ++ phase6-fixture.ttl, deliberately NO binary/.so file), scanned into its own +lilv world, to pin one real /effect/list entry shape and one real +/effect/get field set -- as opposed to test_effects_lilv.py, which only +exercises an empty world / unknown URIs. + +Confirmed: lilv does NOT need the plugin binary to exist on disk for +metadata listing or get_plugin_info(). lilv_world_load_all() only RDF-parses +manifest.ttl and the rdfs:seeAlso'd turtle file; it never dlopen()s +lv2:binary at scan time (only real audio instantiation would). manifest.ttl +here points lv2:binary at "phase6-fixture.so", a file that is never created +anywhere -- the scan and both endpoints below work anyway. So the spec's +"if lilv refuses a binaryless bundle, drop the stretch" escape hatch was not +needed for the *lilv* half of this stretch goal. + +THIS FILE MUST BE RUN AS ITS OWN INVOCATION: `pytest -m lilv_fixture`, never +combined with `-m lilv` (test_effects_lilv.py) in the same process, and +never via a marker expression that would pull both in (e.g. do NOT run +`pytest -m "lilv or lilv_fixture"`). This is a hard constraint, not +caution for its own sake -- it is the actual reason the stretch goal ended +up in its own file instead of a second test class inside +test_effects_lilv.py (which is where it lived during development). + +What went wrong when it *was* a second class in the same file/process, +reproduced directly: that class's setUpClass copied the fixture bundle into +a fresh directory, repointed LV2_PATH, and called modtools.utils.init() a +SECOND time in the process (the module-scoped `lilv_world` fixture in +test_effects_lilv.py having already called it once for the empty world). +modtools/utils.py init() is a bare ctypes call into utils/utils_lilv.cpp +init() (:3893), which does free the previous LilvWorld and build a new one +-- but it also re-runs `NamespaceDefinitions::getStaticInstance(W).init(W)` +(:3898). getStaticInstance() (~:540) is a process-wide C++ singleton +(function-local `static`), NOT reset or freed between the two init() calls +(cleanup() -- the only code path that calls +`NamespaceDefinitions::...cleanup()` -- is, per the phase-6 spec, never +called in this suite). The result, reproduced directly: the second world's +plugin was found and get_all_plugins()/get_plugin_info() returned it, but +its `ports` and `category` fields were silently empty (`[]`) instead of the +real port/category data -- no exception, no crash at that point, just wrong +data, because the namespace lookups the C++ code uses to classify ports/ +categories were resolved against the FIRST (already-freed) world's nodes. +Worse, the process then reliably SIGSEGV'd during native static/global +destructor teardown at interpreter exit (after pytest had already printed +its full PASSED/FAILED summary -- so a naive glance at "tests passed" would +have missed it; `timeout ... ; echo $?` showed "dumped core"). This is +exactly the DANGER-level failure mode the phase-6 spec's NEVER-CALL-LIST +reasoning warns about, just triggered by a second init() instead of a +pre-init call. + +The fix applied: this module now calls modtools.utils.init() exactly ONCE, +as the ONLY init() call in its own process, matching the one-call invariant +that makes test_effects_lilv.py itself safe. +""" + +import json +import os +import shutil + +from base import ModUITestCase + +import pytest + +pytestmark = pytest.mark.lilv_fixture + +_FIXTURE_BUNDLE_SRC = os.path.join( + os.path.dirname(os.path.abspath(__file__)), "fixtures", "phase6-fixture.lv2" +) +_FIXTURE_PLUGIN_URI = "http://example.org/plugins/phase6-fixture" + + +@pytest.fixture(scope="module", autouse=True) +def lilv_world_with_fixture_bundle(): + from conftest import _TEST_ROOT + from modtools import utils + + scan_dir = os.path.join(_TEST_ROOT, "lv2-path-fixture-bundle") + if os.path.isdir(scan_dir): + shutil.rmtree(scan_dir) + os.makedirs(scan_dir) + shutil.copytree(_FIXTURE_BUNDLE_SRC, os.path.join(scan_dir, "phase6-fixture.lv2")) + + # See module docstring: os.environ["LV2_PATH"] is read by lilv itself, + # inside init(), at call time (test/conftest.py sets an initial empty-dir + # value for test_effects_lilv.py; here we override it before this + # module's ONE AND ONLY init() call in this process). + os.environ["LV2_PATH"] = scan_dir + utils.init() + yield + # No cleanup()/de-init -- same reasoning as test_effects_lilv.py, and + # moot here since this is the last thing this process ever does. + + +class TestEffectListWithFixtureBundle(ModUITestCase): + __test__ = True + + def test_effect_list_contains_fixture_plugin_mini_shape(self): + response = self.fetch("/effect/list") + self.assertEqual(response.code, 200) + plugins = json.loads(response.body.decode("utf-8")) + + by_uri = {p["uri"]: p for p in plugins} + self.assertIn(_FIXTURE_PLUGIN_URI, by_uri) + + entry = by_uri[_FIXTURE_PLUGIN_URI] + # Pin the PluginInfo_Mini field set (get_all_plugins), not the full + # PluginInfo shape (that's EffectGet, checked separately below). + self.assertEqual(entry["name"], "Phase 6 Fixture") + self.assertEqual(entry["label"], "Phase 6 Fixture") + self.assertEqual(entry["category"], ["Utility"]) + self.assertEqual( + set(entry.keys()), + { + "uri", "name", "brand", "label", "comment", "buildEnvironment", + "category", "microVersion", "minorVersion", "release", + "builder", "licensed", "iotype", "gui", + }, + ) + + +class TestEffectGetWithFixtureBundle(ModUITestCase): + __test__ = True + + def test_effect_get_returns_full_info_for_fixture_plugin(self): + response = self.fetch("/effect/get?uri=" + _FIXTURE_PLUGIN_URI) + self.assertEqual(response.code, 200) + content_type = response.headers.get("Content-Type", "") + self.assertIn("application/json", content_type) + + data = json.loads(response.body.decode("utf-8")) + self.assertTrue(data["valid"]) + self.assertEqual(data["uri"], _FIXTURE_PLUGIN_URI) + self.assertEqual(data["name"], "Phase 6 Fixture") + # lv2:binary in manifest.ttl points at a .so that was never created + # on disk -- confirms the binary is not required for full plugin + # info (only real audio instantiation would need it). + self.assertEqual(data["binary"], "") + self.assertEqual( + [p["symbol"] for p in data["ports"]["audio"]["input"]], ["in"] + ) + self.assertEqual( + [p["symbol"] for p in data["ports"]["audio"]["output"]], ["out"] + )