From 0b027f49fdf7d649cf782bf585d5ff30610cd756 Mon Sep 17 00:00:00 2001 From: k9ert <117085+k9ert@users.noreply.github.com> Date: Sat, 8 Aug 2026 14:12:46 +0200 Subject: [PATCH] fix: dev-server crash in the specter-desktop repo after the project-rename (#2686) Co-authored-by: Claude Opus 5 --- .github/workflows/test.yml | 25 ++++++++++ src/cryptoadvance/specter/util/reflection.py | 52 ++++++++++++-------- tests/test_util_reflection.py | 51 +++++++++++++++++-- tests/test_util_shell.py | 15 +++++- 4 files changed, 116 insertions(+), 27 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 58f6a44c5..6611f4827 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -196,6 +196,31 @@ jobs: pip3 install -r requirements.txt --require-hashes pip3 install -e ".[test]" + - name: Dev-server smoketest (from the repo-root) + # No job used to start the dev-server from the repo-root the way + # docs/development.md describes it. That's why #2526 (renaming the + # project to cryptoadvance_specter) could break it unnoticed. + run: | + source ./.env/bin/activate + export SPECTER_DATA_FOLDER=$(mktemp -d) + # --debug enables the werkzeug-reloader which forks a child-process, so + # start a new process-group we can kill as a whole further down + setsid python3 -m cryptoadvance.specter server --config DevelopmentConfig --debug > specterd.log 2>&1 & + specterd_pid=$! + started="" + for i in $(seq 1 30); do + if curl -sf http://127.0.0.1:25441/ > /dev/null; then started="yes"; break; fi + sleep 2 + done + kill -- -$specterd_pid || true + # the next step needs port 25441, so make sure it's free again + for i in $(seq 1 10); do + curl -sf http://127.0.0.1:25441/ > /dev/null || break + sleep 1 + done + cat specterd.log + if [ -z "$started" ]; then echo "The dev-server did not come up!"; exit 1; fi + - name: Extension smoketest run: | git config --global user.name "CI CD" diff --git a/src/cryptoadvance/specter/util/reflection.py b/src/cryptoadvance/specter/util/reflection.py index 708730c28..70cfca46f 100644 --- a/src/cryptoadvance/specter/util/reflection.py +++ b/src/cryptoadvance/specter/util/reflection.py @@ -5,11 +5,11 @@ import os from pathlib import Path import pkgutil from pkgutil import iter_modules +import re import sys from typing import List from .common import camelcase2snake_case from ..specter_error import SpecterError, SpecterInternalException -from .shell import grep from .reflection_fs import detect_extension_style_in_cwd, search_dirs_in_path @@ -130,6 +130,29 @@ def get_classlist_of_type_clazz_from_modulelist(clazz, modulelist, skip_missing= return class_list +def is_specter_desktop_project(cwd=".") -> bool: + """Whether cwd is the specter-desktop project itself rather than an + extension-project. Detected via the project-name in the pyproject.toml. + Hmm, a bit hackish but we don't want to depend on toml-parsing libs. + """ + try: + with open(Path(cwd, "pyproject.toml")) as pyproject_file: + for line in pyproject_file: + line = line.strip().replace(" ", "").replace("'", "").replace('"', "") + if not line.startswith("name="): + continue + # PEP 503: ".", "-" and "_" are equivalent in project-names, so + # "cryptoadvance.specter" and "cryptoadvance_specter" are the same + name = re.sub(r"[-_.]+", "-", line[len("name=") :]).lower() + if name == "cryptoadvance-specter": + return True + except FileNotFoundError: + # Expected for adhoc-style extension-projects: those have no + # pyproject.toml at all and are therefore not specter-desktop + pass + return False + + def get_subclasses_for_clazz_in_cwd(clazz, cwd=".") -> List[type]: """Returns all subclasses of class clazz located in the CWD if the cwd is not a specter-desktop dev-env-kind-of-dir or contains any .py-file @@ -140,37 +163,26 @@ def get_subclasses_for_clazz_in_cwd(clazz, cwd=".") -> List[type]: return [] # if not testing but in a folder which looks like specter-desktop/src --> No dynamic extensions - if "PYTEST_CURRENT_TEST" not in os.environ: - # Hmm, a bit hackish but if the pyproject.toml specifies cryptoadvance.specter as a name and - # we don't need to depend on toml-parsing libs, that should be ok. - try: - found, line = grep("./pyproject.toml", 'name = "cryptoadvance.specter"') - if found: - return [] - if line: - line = line.replace(" ", "").replace("'", "").replace('"', "") - if line == "name=cryptoadvance.specter": - return [] - except FileNotFoundError: - pass + if "PYTEST_CURRENT_TEST" not in os.environ and is_specter_desktop_project(cwd): + return [] - # Depending on the style we either add "." or "./src" to the searchpath + # Depending on the style we either add cwd or cwd/src to the searchpath - extension_style = detect_extension_style_in_cwd() + extension_style = detect_extension_style_in_cwd(cwd) # raise Exception(extension_style) if extension_style == "adhoc": - package_dirs.append(Path(".")) + package_dirs.append(Path(cwd)) elif extension_style == "publish-ready": - package_dirs.extend(search_dirs_in_path(Path("./src"))) + package_dirs.extend(search_dirs_in_path(Path(cwd, "src"))) elif extension_style == "specter-desktop": if "PYTEST_CURRENT_TEST" in os.environ: # I admit, ugly hack logger.info("We're in testing mode. Adding CWD to searchpath") - package_dirs.append(Path("./src")) + package_dirs.append(Path(cwd, "src")) else: raise Exception( f""" - We checked before that we're not in the specter-desktop home + We checked before that we're not in the specter-desktop home directory but now the extension-style is 'specter-desktop' ?! This should not happen! """ diff --git a/tests/test_util_reflection.py b/tests/test_util_reflection.py index 6890a6039..fb737515e 100644 --- a/tests/test_util_reflection.py +++ b/tests/test_util_reflection.py @@ -11,6 +11,7 @@ from cryptoadvance.specter.util.reflection import ( get_subclasses_for_clazz, get_subclasses_for_clazz_in_cwd, get_classlist_of_type_clazz_from_modulelist, + is_specter_desktop_project, _get_module_from_class, get_package_dir_for_subclasses_of, search_dirs_in_path, @@ -119,13 +120,53 @@ def test_get_classlist_raises_on_missing_module_by_default(): get_classlist_of_type_clazz_from_modulelist(Service, modulelist) +repo_root = Path(__file__).parent.parent +xtestdata = repo_root / "tests" / "xtestdata_testextensions" + + +def test_is_specter_desktop_project(): + """The specter-desktop project detects itself via the name in its own + pyproject.toml. If that name changes (PEP 503 allows "." "-" and "_" to be + used interchangeably), the dev-server dies on startup, see #2526.""" + assert is_specter_desktop_project(repo_root) + assert not is_specter_desktop_project(xtestdata / "ext_root_fully_qualified_1") + assert not is_specter_desktop_project(xtestdata) + + +def test_is_specter_desktop_project_pep503_names(tmp_path): + for name in [ + "cryptoadvance.specter", + "cryptoadvance_specter", + "Cryptoadvance-Specter", + ]: + (tmp_path / "pyproject.toml").write_text( + f'[project]\nname = "{name}"\nversion = "1.2.3"\n' + ) + assert is_specter_desktop_project(tmp_path), f"{name} should be detected" + + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "boatacccorp.tretboot"\n' + ) + assert not is_specter_desktop_project(tmp_path) + + +def test_get_subclasses_for_clazz_in_cwd_in_specter_desktop_project(monkeypatch): + """No dynamic extension-discovery in the specter-desktop project itself. + Regression test: this used to raise "This should not happen!" when the + project got renamed to cryptoadvance_specter, breaking + `python3 -m cryptoadvance.specter server --config DevelopmentConfig`""" + # the production code takes a shortcut for tests, so pretend we're not testing + monkeypatch.delenv("PYTEST_CURRENT_TEST", raising=False) + assert get_subclasses_for_clazz_in_cwd(Service, cwd=repo_root) == [] + + def test_get_subclasses_for_clazz_in_cwd(caplog): caplog.set_level(logging.DEBUG) - classlist: List[type] = get_subclasses_for_clazz_in_cwd( - Service, cwd="./tests/xtestdata_testextensions" - ) - # damn, this is difficult to test - # assert len(classlist) == 3 + classlist: List[type] = get_subclasses_for_clazz_in_cwd(Service, cwd=xtestdata) + # That folder is a container of extension-projects, not an extension-project + # itself, so there is nothing importable in there + assert classlist == [] + assert "Detected Extension-style: adhoc" in caplog.text def test_get_subclasses_for_class(caplog): diff --git a/tests/test_util_shell.py b/tests/test_util_shell.py index 9b08b078e..73a7d9c30 100644 --- a/tests/test_util_shell.py +++ b/tests/test_util_shell.py @@ -23,7 +23,18 @@ def test_last_lines(caplog): assert lines[-2].startswith("OUT OF OR IN CONNECTION WITH THE SOFTWARE ") -def test_grep(): +def test_grep(tmp_path): + """grep returns a (found, line)-tuple. Asserting on the tuple itself is + always truthy, so always assert on the first element!""" from cryptoadvance.specter.util.shell import grep - assert grep("./pyproject.toml", 'name = "cryptoadvance.specter"') + some_file = tmp_path / "some_file.txt" + some_file.write_text('name = "cryptoadvance_specter"\nversion = "1.2.3"\n') + + found, line = grep(str(some_file), 'name = "cryptoadvance_specter"') + assert found + assert line.strip() == 'name = "cryptoadvance_specter"' + + found, line = grep(str(some_file), "does not exist") + assert not found + assert line is None