From 29aa2c8f18516e82429a9e751a74d48284f67e9c Mon Sep 17 00:00:00 2001 From: historia Date: Wed, 26 Aug 2026 00:22:34 -0400 Subject: fix: stop remote form entries overwriting each other --- app/tests/test_hub.py | 179 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 128 insertions(+), 51 deletions(-) (limited to 'app/tests/test_hub.py') diff --git a/app/tests/test_hub.py b/app/tests/test_hub.py index f350072..9f72ad9 100644 --- a/app/tests/test_hub.py +++ b/app/tests/test_hub.py @@ -649,12 +649,23 @@ class ConvertFlowTests(unittest.TestCase): patcher.start() self.addCleanup(patcher.stop) + # Keys shared by every backend entry; a "-remote" backend's other + # option keys are namespaced under "." in the form dict + # (mirroring hub.py), so _form_values maps them automatically. + _COMMON_KEYS = frozenset(("backend", "output_format", "speed", + "single_file", "debug", "stop_and_exit")) + def _form_values(self, **overrides): """A fully-populated form result, with sensible defaults.""" values = {"output_format": "m4b", "speed": "1.0", "single_file": False, "debug": False, "stop_and_exit": True} values.update(overrides) + backend = values.get("backend") or "" + if backend.endswith("-remote"): + prefix = f"{backend}." + values = {(prefix + key if key not in self._COMMON_KEYS else key): + value for key, value in values.items()} return values def _answer_form(self, **overrides): @@ -662,7 +673,8 @@ class ConvertFlowTests(unittest.TestCase): def _field(self, key, form_index=-1): _, fields, _ = self.tui.forms_seen[form_index] - return next(f for f in fields if f["key"] == key) + return next(f for f in fields if f["key"] == key + or f["key"].endswith("." + key)) def _ready(self, key, label): """A backend status that is ready to convert with.""" @@ -748,9 +760,10 @@ class ConvertFlowTests(unittest.TestCase): title, fields, form_kwargs = self.tui.forms_seen[0] self.assertEqual(title, "Generate audiobooks") self.assertEqual([f["key"] for f in fields], - ["backend", "model_id", "audiocpp_voice", - "instructions", "output_format", "speed", - "single_file", "debug", "stop_and_exit"]) + ["backend", "audiocpp-remote.model_id", + "audiocpp-remote.audiocpp_voice", + "audiocpp-remote.instructions", "output_format", + "speed", "single_file", "debug", "stop_and_exit"]) self.assertEqual(form_kwargs["buttons"], ("Generate!", "Cancel")) self.assertTrue(form_kwargs["start_on_buttons"]) # The stop-and-exit toggle ships on by default. @@ -1002,6 +1015,59 @@ class ConvertFlowTests(unittest.TestCase): self.assertEqual(fields[0]["choices"], [("audio.cpp", "audiocpp"), ("audio.cpp [remote]", "audiocpp-remote")]) + # The two entries' fields are namespaced, so both carry their own + # values and picking one never leaks the other's into the run. + keys = [f["key"] for f in fields] + self.assertIn("model_id", keys) + self.assertIn("audiocpp-remote.model_id", keys) + + def test_managed_and_remote_entries_do_not_overwrite_each_other(self): + # Regression: the form returns one flat {key: value} dict. When + # managed and remote entries shared field keys, the remote entry's + # hidden defaults silently won over the user's edits on whichever + # entry was selected. + self._patch_remote( + [{"id": "higgs", "family": "higgs_audio_tts", "task": "tts"}], + voices=["narrator"]) + statuses = [self._ready("audiocpp", "audio.cpp"), + self._remote("audiocpp", "audio.cpp")] + common = {"output_format": "m4b", "speed": "1.0", + "single_file": False, "debug": False, + "stop_and_exit": True} + with tempfile.TemporaryDirectory() as td: + root = Path(td) + (root / "server.json").write_text(json.dumps({ + "models": [{"id": "qwen", "family": "qwen3_tts", + "task": "tts"}], + }), encoding="utf-8") + with patch.object(hub.audiocpp_backend, "find_local_checkout", + return_value=root), \ + patch.object(hub.config, "AUDIOCPP_INSTRUCTIONS", ""): + # Managed selected: its picks must survive next to the + # remote entry's same-shaped fields. + self.tui.form_script.append({ + "backend": "audiocpp", "model_id": "qwen", + "audiocpp_voice": "", "instructions": "", + "audiocpp-remote.model_id": "higgs", + "audiocpp-remote.audiocpp_voice": "narrator", + **common}) + managed_cmd = self._convert(None, statuses) + self.assertIsNotNone(managed_cmd) + self.assertEqual(managed_cmd[2]["model_id"], "qwen") + # The remote entry's hidden "narrator" voice must not leak + # into the managed run (the old duplicate-key behavior). + self.assertIsNone(managed_cmd[2]["voice"]) + # Remote selected: its picks win instead. + self.tui.form_script.append({ + "backend": "audiocpp-remote", + "audiocpp-remote.model_id": "higgs", + "audiocpp-remote.audiocpp_voice": "narrator", + "model_id": "qwen", + **common}) + remote_cmd = self._convert(None, statuses) + self.assertIsNotNone(remote_cmd) + self.assertEqual(remote_cmd[2]["model_id"], "higgs") + self.assertEqual(remote_cmd[2]["voice"], "narrator") def test_audiocpp_remote_mapper_adds_api_url(self): self._patch_remote( @@ -1038,15 +1104,20 @@ class ConvertFlowTests(unittest.TestCase): patch.object(hub.config, "SPEAKER", "Vivian"): self._answer_form(backend="qwen", mode="custom", speaker="Serena", clone="") - with patch.object(hub.common, "update_config_value") as mk_update: + # The fake mirrors the real update_config_value contract: + # persisting a value also lands it on the imported module. + def fake_update(key, value, config_path=None): + setattr(hub.config, key, value) + return True + + with patch.object(hub.common, "update_config_value", + fake_update): cmd = self._convert(None, [self._ready("qwen", "qwen-tts")]) speaker_in_memory = hub.config.SPEAKER self.assertEqual(cmd[0], "convert") self.assertEqual(cmd[1], hub.BACKEND_QWEN) self.assertIsNone(cmd[2]["clone"]) - # The speaker choice is persisted for future runs too. - mk_update.assert_called_once_with("SPEAKER", "Serena") self.assertEqual(speaker_in_memory, "Serena") fields = self.tui.forms_seen[0][1] self.assertEqual([f["key"] for f in fields], @@ -1512,8 +1583,25 @@ class AddAutostartTests(unittest.TestCase): class SettingsTests(unittest.TestCase): """Settings menu: field collection, validation, config.py writing.""" - def test_write_config_preserves_comments_and_other_lines(self): + # Keys _apply_settings persists; every test that triggers a real or + # fake config write restores these afterwards. + _SETTING_KEYS = ("AUDIO_FORMAT", "AUDIO_BITRATE", "LANGUAGE", + "CHUNK_SIZE", "STOP_SERVER_AND_EXIT", + "AUDIOCPP_UNLOAD_MODELS", + "QWEN_API_URL", "CLONE_API_URL", + "FASTER_API_URL", "AUDIOCPP_API_URL", + "QWEN_REMOTE_URL", "CLONE_REMOTE_URL", + "FASTER_REMOTE_URL", "AUDIOCPP_REMOTE_URL") + + def _snapshot_settings(self): + original = {name: getattr(hub.config, name) for name in + self._SETTING_KEYS} + self.addCleanup(lambda: [setattr(hub.config, name, value) + for name, value in original.items()]) + + def test_update_config_value_preserves_comments_and_other_lines(self): import tempfile + self._snapshot_settings() with tempfile.TemporaryDirectory() as td: path = Path(td) / "config.py" path.write_text( @@ -1524,11 +1612,12 @@ class SettingsTests(unittest.TestCase): "\n" "CHUNK_SIZE = 250 # words per request\n", encoding="utf-8") - with patch.object(hub.config, "__file__", str(path)): - hub._write_config({"AUDIO_FORMAT": "mp3", - "AUDIO_BITRATE": "192k", - "LANGUAGE": "Japanese", - "CHUNK_SIZE": 300}) + for key, value in (("AUDIO_FORMAT", "mp3"), + ("AUDIO_BITRATE", "192k"), + ("LANGUAGE", "Japanese"), + ("CHUNK_SIZE", 300)): + self.assertTrue(hub.common.update_config_value( + key, value, config_path=path)) text = path.read_text(encoding="utf-8") self.assertEqual( text, @@ -1538,32 +1627,28 @@ class SettingsTests(unittest.TestCase): 'LANGUAGE = "Japanese"\n' "\n" "CHUNK_SIZE = 300 # words per request\n") + # The imported module mirrors the file immediately. + self.assertEqual(hub.config.AUDIO_FORMAT, "mp3") + self.assertEqual(hub.config.CHUNK_SIZE, 300) - def test_write_config_missing_key_raises(self): + def test_update_config_value_missing_key_returns_false(self): import tempfile + self._snapshot_settings() with tempfile.TemporaryDirectory() as td: path = Path(td) / "config.py" path.write_text("X = 1\n", encoding="utf-8") - with patch.object(hub.config, "__file__", str(path)): - with self.assertRaises(ValueError): - hub._write_config({"AUDIO_FORMAT": "mp3"}) + self.assertFalse(hub.common.update_config_value( + "AUDIO_FORMAT", "mp3", config_path=path)) def test_apply_settings_writes_and_reloads_in_memory(self): written = {} - def fake_write(updates): - written.update(updates) + def fake_update(key, value, config_path=None): + written[key] = value + setattr(hub.config, key, value) + return True - original = {name: getattr(hub.config, name) for name in - ("AUDIO_FORMAT", "AUDIO_BITRATE", "LANGUAGE", - "CHUNK_SIZE", "STOP_SERVER_AND_EXIT", - "AUDIOCPP_UNLOAD_MODELS", - "QWEN_API_URL", "CLONE_API_URL", - "FASTER_API_URL", "AUDIOCPP_API_URL", - "QWEN_REMOTE_URL", "CLONE_REMOTE_URL", - "FASTER_REMOTE_URL", "AUDIOCPP_REMOTE_URL")} - self.addCleanup(lambda: [setattr(hub.config, name, value) - for name, value in original.items()]) + self._snapshot_settings() values = {"audio_format": "ogg", "audio_bitrate": " 192k ", "language": "en", "chunk_size": "300", "stop_and_exit": False, @@ -1574,7 +1659,8 @@ class SettingsTests(unittest.TestCase): "faster_remote_url": "http://10.0.0.6:8000", "qwen_custom_remote_url": "", "qwen_clone_remote_url": ""} - with patch.object(hub, "_write_config", fake_write), \ + with patch.object(hub.common, "update_config_value", + fake_update), \ patch.object(hub, "_sync_audiocpp_server_port"): hub._apply_settings(values) # Values are trimmed and language normalized to a display name; @@ -1609,23 +1695,14 @@ class SettingsTests(unittest.TestCase): "http://10.0.0.5:8080") def test_apply_settings_rejects_bad_values(self): - original = {name: getattr(hub.config, name) for name in - ("AUDIO_FORMAT", "AUDIO_BITRATE", "LANGUAGE", - "CHUNK_SIZE", "STOP_SERVER_AND_EXIT", - "AUDIOCPP_UNLOAD_MODELS", - "QWEN_API_URL", "CLONE_API_URL", - "FASTER_API_URL", "AUDIOCPP_API_URL", - "QWEN_REMOTE_URL", "CLONE_REMOTE_URL", - "FASTER_REMOTE_URL", "AUDIOCPP_REMOTE_URL")} - self.addCleanup(lambda: [setattr(hub.config, name, value) - for name, value in original.items()]) + self._snapshot_settings() base = {"audio_format": "m4b", "audio_bitrate": "128k", "language": "English", "chunk_size": "250", "stop_and_exit": True, "unload_models": True, "qwen_custom_port": "7860", "qwen_clone_port": "7861", "faster_port": "8000", "audiocpp_port": "8080"} - with patch.object(hub, "_write_config") as mk_write: + with patch.object(hub.common, "update_config_value") as mk_update: with self.assertRaises(ValueError): hub._apply_settings({**base, "language": "Klingon"}) with self.assertRaises(ValueError): @@ -1635,7 +1712,7 @@ class SettingsTests(unittest.TestCase): with self.assertRaises(ValueError): hub._apply_settings({**base, "audiocpp_remote_url": "not a url"}) - mk_write.assert_not_called() + mk_update.assert_not_called() def test_field_validators(self): self.assertIsNone(hub._validate_bitrate("128k")) @@ -1831,7 +1908,8 @@ class SettingsTests(unittest.TestCase): 'FASTER_REMOTE_URL = "http://127.0.0.1:8000"\n' 'AUDIOCPP_REMOTE_URL = "http://127.0.0.1:8080"\n', encoding="utf-8") - with patch.object(hub.config, "__file__", str(path)): + with patch.object(hub.common, "CONFIG_PATH", path), \ + patch.object(hub, "_sync_audiocpp_server_port"): # Down to Chunk size, Enter -> editor, Ctrl-U + '300', # Enter; Tab -> Save, Enter; a key dismisses the flash. screen = FakeScreen(keys=[ @@ -1860,14 +1938,13 @@ class SettingsTests(unittest.TestCase): 'FASTER_API_URL = "http://127.0.0.1:8000"\n' 'AUDIOCPP_API_URL = "http://127.0.0.1:8080"\n', encoding="utf-8") - with patch.object(hub.config, "__file__", str(path)), \ - patch.object(hub, "_sync_audiocpp_server_port"): - hub._write_config({ - "QWEN_API_URL": "http://127.0.0.1:7862", - "CLONE_API_URL": "http://127.0.0.1:7863", - "FASTER_API_URL": "http://127.0.0.1:8001", - "AUDIOCPP_API_URL": "http://127.0.0.1:8081", - }) + self._snapshot_settings() + for key, value in ( + ("QWEN_API_URL", "http://127.0.0.1:7862"), + ("CLONE_API_URL", "http://127.0.0.1:7863"), + ("FASTER_API_URL", "http://127.0.0.1:8001"), + ("AUDIOCPP_API_URL", "http://127.0.0.1:8081")): + hub.common.update_config_value(key, value, config_path=path) text = path.read_text(encoding="utf-8") self.assertIn('QWEN_API_URL = "http://127.0.0.1:7862"', text) self.assertIn('CLONE_API_URL = "http://127.0.0.1:7863"', text) -- cgit v1.2.3