diff --git a/.github/workflows/build-plugin.yml b/.github/workflows/build-plugin.yml index bb95e4a..1382388 100644 --- a/.github/workflows/build-plugin.yml +++ b/.github/workflows/build-plugin.yml @@ -52,10 +52,35 @@ jobs: cd electrum_swapgui python -m unittest discover -s tests -v + - name: Determine plugin version + id: version + run: | + cd electrum_swapgui + if [[ "$GITHUB_REF" == refs/tags/v* ]]; then + # The tag is the single source of truth for a release version. + version="${GITHUB_REF_NAME#v}" + if [ -z "$version" ]; then + echo "::error::tag $GITHUB_REF_NAME yields an empty version" + exit 1 + fi + else + # Not a release: stamp a dev marker traceable to the commit, so any + # zip handed out from a branch or PR build can be identified. + version="dev-g$(git rev-parse --short=7 HEAD)" + fi + echo "version=$version" >> "$GITHUB_OUTPUT" + echo "plugin version: $version" + - name: Build plugin zip run: | cd electrum_swapgui - bash contrib/make_zip.sh "$GITHUB_WORKSPACE/dist" + bash contrib/make_zip.sh "$GITHUB_WORKSPACE/dist" "${{ steps.version.outputs.version }}" + + - name: Verify the stamped version + run: | + cd "$GITHUB_WORKSPACE" + unzip -p dist/swapserver_gui.zip swapserver_gui/_version.py \ + | grep -Fx '__version__ = "${{ steps.version.outputs.version }}"' - name: Upload plugin zip artifact uses: actions/upload-artifact@v4 diff --git a/README.md b/README.md index d1c79ef..c26686a 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ listener can be shut down cleanly (`swapserver_gui.py:ManagedHttpSwapServer`). plugins/swapserver_gui/ manifest.json # available_for: ["qt"], min_electrum_version __init__.py # registers plugins.swapserver_gui.autostart + _version.py # plugin version, stamped from the git tag at build time swapserver_gui.py # GUI-agnostic server lifecycle + history helpers qt.py # the "Swap Server" tab + Plugin(load_wallet/close_wallet) tests/ # unit + HTTP end-to-end tests (no GUI needed) @@ -48,6 +49,25 @@ contrib/make_zip.sh # build the external-plugin zip contrib/regtest_demo/ # one-command local regtest + nostr demo ``` +## Versioning + +The tab header shows the installed plugin version, so users can tell which build +they are on. **The git tag is the single source of truth.** `_version.py` is +committed with the placeholder `dev`; `contrib/make_zip.sh` stamps the real value +into a staging copy while building the archive, so the working tree is never +modified: + +```bash +bash contrib/make_zip.sh dist 0.2.1 # -> tab shows "v0.2.1" +bash contrib/make_zip.sh dist # -> tab shows "dev" +``` + +CI passes `${GITHUB_REF_NAME#v}` for `refs/tags/v*` builds and `dev-g` +otherwise, then verifies the stamp landed in the zip. A release asset therefore +can never disagree with the tag it was built from, and any zip from a branch or +PR build is traceable to its commit. To cut a release, push a `vX.Y.Z` tag — +nothing in the tree needs bumping. + ## Installing **As an external zip (for users):** @@ -83,6 +103,24 @@ ELECTRUM_SRC=/path/to/electrum python3 -m unittest discover -s tests -v - `tests/test_http_endpoint.py` — starts the real `ManagedHttpSwapServer`, does a live `GET /getpairs`, validates the JSON, and asserts the port is released on stop. +- `tests/test_settings_save.py` — persisting settings against a **real** + `SimpleConfig`, including disabling the HTTP port (see the note below). +- `tests/test_version.py` / `tests/test_zip_plugin_load.py` — version formatting + and the end-to-end build-time stamping chain. + +### A note on the HTTP port + +A disabled HTTP port is stored as `0`, never `None`. `SWAPSERVER_PORT` is the +dotted key `plugins.swapserver.port`, and assigning `None` to it routes into +`SimpleConfig`'s key-*deletion* branch, whose recursion dereferences the +intermediate `plugins.swapserver` dict without checking that it exists — which it +usually does not, since this plugin only writes `plugins.swapserver_gui.*`. The +result was `AttributeError: 'NoneType' object has no attribute 'pop'` on every +"Save settings" with the port disabled. Every reader tests the key for +truthiness, so `0` is equivalent and takes the safe write path. This is an +upstream Electrum bug; the workaround lives here because users run released +AppImages. All writes go through `swapserver_gui.save_settings`, and a static +guard in the tests keeps direct assignments from creeping back into `qt.py`. For a **full GUI e2e** — a real Electrum Qt GUI (offscreen) with the plugin loaded, auto-starting the server and serving a real `/getpairs` — see diff --git a/contrib/make_zip.sh b/contrib/make_zip.sh index 84d6d8a..ace4e8e 100755 --- a/contrib/make_zip.sh +++ b/contrib/make_zip.sh @@ -15,25 +15,72 @@ # The zip is a plain archive: no signing key or secret is required to build it. # The end user authorises it locally (with their own plugin password) the first # time they install it via Electrum's Plugins dialog. +# +# Usage: make_zip.sh [OUT_DIR] [VERSION] +# +# VERSION (or the PLUGIN_VERSION environment variable) is stamped into +# swapserver_gui/_version.py inside the archive, and is what the plugin shows in +# its tab header. The stamping happens in a temporary staging copy, so building +# never modifies the working tree. With no version given the committed +# placeholder ('dev') is kept. set -euo pipefail HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT="$(dirname "$HERE")" # electrum_swapgui/ (our repo) PLUGIN_DIR="$ROOT/plugins/swapserver_gui" OUT_DIR="${1:-$ROOT/dist}" +VERSION="${2:-${PLUGIN_VERSION:-}}" OUT="$OUT_DIR/swapserver_gui.zip" if [ ! -f "$PLUGIN_DIR/manifest.json" ]; then echo "error: $PLUGIN_DIR/manifest.json not found" >&2 exit 1 fi +if [ ! -f "$PLUGIN_DIR/_version.py" ]; then + echo "error: $PLUGIN_DIR/_version.py not found" >&2 + exit 1 +fi mkdir -p "$OUT_DIR" +# Resolve to an absolute path before we cd into the staging dir below. +OUT_DIR="$(cd "$OUT_DIR" && pwd)" +OUT="$OUT_DIR/swapserver_gui.zip" rm -f "$OUT" -# Zip from plugins/ so paths are prefixed with 'swapserver_gui/'. Exclude -# caches and any local pyc files. -( cd "$ROOT/plugins" && \ +# Stage the package so the version can be stamped without touching the tree. +STAGE="$(mktemp -d)" +trap 'rm -rf "$STAGE"' EXIT +cp -r "$PLUGIN_DIR" "$STAGE/swapserver_gui" +rm -rf "$STAGE/swapserver_gui/__pycache__" + +if [ -n "$VERSION" ]; then + # Reject anything that would need quoting inside the Python string literal, + # so a bad tag fails the build instead of emitting a broken _version.py. + case "$VERSION" in + *[!A-Za-z0-9.+_-]*) + echo "error: refusing to stamp unsafe version string: $VERSION" >&2 + exit 1 ;; + esac + python3 - "$STAGE/swapserver_gui/_version.py" "$VERSION" <<'PY' +import re, sys +path, version = sys.argv[1], sys.argv[2] +with open(path, encoding="utf-8") as f: + src = f.read() +new, n = re.subn(r'^__version__ = ".*"$', f'__version__ = "{version}"', + src, count=1, flags=re.MULTILINE) +if n != 1: + raise SystemExit(f"error: could not find __version__ assignment in {path}") +with open(path, "w", encoding="utf-8") as f: + f.write(new) +PY + echo "stamped version: $VERSION" +else + echo "no version given; keeping the committed placeholder" +fi + +# Zip from the staging dir so paths are prefixed with 'swapserver_gui/'. +# Exclude caches and any local pyc files. +( cd "$STAGE" && \ zip -r -X "$OUT" swapserver_gui \ -x '*/__pycache__/*' -x '*.pyc' >/dev/null ) diff --git a/plugins/swapserver_gui/_version.py b/plugins/swapserver_gui/_version.py new file mode 100644 index 0000000..fbb5c74 --- /dev/null +++ b/plugins/swapserver_gui/_version.py @@ -0,0 +1,34 @@ +#!/usr/bin/env python +# +# swapserver_gui - a Qt GUI plugin for Electrum's submarine swap server. +# This file is released into the public domain (The Unlicense); see LICENSE. +# +# The plugin version, stamped into the zip at build time. +# +# The value committed here is deliberately the placeholder ``dev``: the release +# version is the *git tag*, and ``contrib/make_zip.sh`` rewrites the literal +# below in a staging copy of the package while building the archive (the working +# tree is never modified). ``.github/workflows/build-plugin.yml`` passes +# ``${GITHUB_REF_NAME#v}`` for ``refs/tags/v*`` builds and ``dev-g`` for +# every other build, so a release zip can never disagree with the tag it was +# built from and a dev zip is always traceable to the commit that produced it. +# +# Keep this module free of imports: ``contrib/make_zip.sh`` rewrites it with a +# line-oriented substitution, and the test suite parses it with ``ast`` without +# executing it. + +__version__ = "dev" + + +def format_version(version: str) -> str: + """Render a version string for display in the GUI. + + Real releases are numeric and conventionally shown with a leading ``v`` to + match the git tag / GitHub release name (``0.2.1`` -> ``v0.2.1``). The + placeholder and CI dev stamps (``dev``, ``dev-g1a2b3c``) are not versions in + that sense, so they are shown verbatim -- ``vdev`` would read as a release. + """ + version = (version or "").strip() + if not version: + return "unknown" + return "v" + version if version[0].isdigit() else version diff --git a/plugins/swapserver_gui/qt.py b/plugins/swapserver_gui/qt.py index 9d1a6f8..f928af0 100644 --- a/plugins/swapserver_gui/qt.py +++ b/plugins/swapserver_gui/qt.py @@ -22,10 +22,12 @@ from .swapserver_gui import ( SwapServerGuiPlugin, SwapServerError, get_swap_history, get_swap_summary, + save_settings, ) # NB: not ``from . import pow as swap_pow`` -- that form breaks when Electrum # loads this plugin from a zip. See the long comment in swapserver_gui.py. swap_pow = importlib.import_module('.pow', __package__) +_version = importlib.import_module('._version', __package__) # Above this many bits a proof-of-work grind stops being a "wait a few minutes" # affair (see the estimate shown next to the spinbox), so we ask for confirmation. @@ -62,6 +64,15 @@ def __init__(self, plugin: 'Plugin', window: 'ElectrumWindow') -> None: self.status_label = QLabel() self.status_label.setTextFormat(Qt.TextFormat.RichText) header.addWidget(self.status_label, 1) + # Which build of the plugin this is. Stamped into the zip at build time + # from the git tag, so it matches the GitHub release (see _version.py). + self.version_label = QLabel(_version.format_version(_version.__version__)) + self.version_label.setToolTip( + _("Installed version of the Swap Server (GUI) plugin.")) + self.version_label.setTextInteractionFlags( + Qt.TextInteractionFlag.TextSelectableByMouse) + self.version_label.setEnabled(False) # renders dimmed, like secondary text + header.addWidget(self.version_label) self.toggle_btn = QPushButton() self.toggle_btn.clicked.connect(self.on_toggle) header.addWidget(self.toggle_btn) @@ -200,10 +211,14 @@ def _relays_from_widget(self) -> str: return ",".join(parts) def on_save(self) -> None: - new_port = self.port_spin.value() or None + # 0 == "disabled"; never None. Assigning None to SWAPSERVER_PORT crashes + # Electrum -- see the docstring of swapserver_gui.save_settings. + new_port = self.port_spin.value() new_relays = self._relays_from_widget() new_pow = self.pow_spin.value() - old_port = self.config.SWAPSERVER_PORT + # Normalise the old value too: configs written by earlier versions of + # this plugin may hold None, which must compare equal to a disabled 0. + old_port = int(self.config.SWAPSERVER_PORT or 0) old_relays = self.config.NOSTR_RELAYS or "" old_pow = int(self.config.SWAPSERVER_POW_TARGET or 0) @@ -219,11 +234,25 @@ def on_save(self) -> None: "Continue?")): return - # persist - self.config.SWAPSERVER_PORT = new_port - self.config.SWAPSERVER_FEE_MILLIONTHS = self.fee_spin.value() - self.config.SWAPSERVER_POW_TARGET = new_pow - self.config.NOSTR_RELAYS = new_relays + # persist. A failure here must not escape into the Qt event loop: it + # would surface as an unhandled-exception dialog (or just a traceback on + # the console) with the user left unsure what, if anything, was saved. + try: + save_settings( + self.config, + port=new_port, + fee_millionths=self.fee_spin.value(), + pow_target=new_pow, + relays=new_relays, + ) + except Exception as e: + self.plugin.logger.exception("failed to save swap server settings") + self.window.show_error( + _("Could not save the swap server settings: {}").format(e)) + # Put the widgets back in sync with what is actually stored, so the + # form never claims a value that was not persisted. + self.load_settings_into_widgets() + return self.load_settings_into_widgets() # A port/relay change needs a server restart. So does *raising* the PoW # target, since the running server is announcing with a nonce that no diff --git a/plugins/swapserver_gui/swapserver_gui.py b/plugins/swapserver_gui/swapserver_gui.py index a6486f4..d7b080f 100644 --- a/plugins/swapserver_gui/swapserver_gui.py +++ b/plugins/swapserver_gui/swapserver_gui.py @@ -474,6 +474,57 @@ def status(self) -> Dict[str, Any]: return data +def save_settings( + config: 'SimpleConfig', + *, + port: int, + fee_millionths: int, + pow_target: int, + relays: str, +) -> None: + """Persist the swap-server settings edited in the GUI. + + Lives here rather than in ``qt.py`` so it can be tested against a real + ``SimpleConfig``: PyQt6 is not installed in CI, so anything importable only + through ``qt.py`` is effectively untestable. + + ``port`` MUST be an ``int``; ``0`` means "HTTP endpoint disabled". Passing + ``None`` for a disabled port -- the obvious way to express it, since + ``SWAPSERVER_PORT`` defaults to ``None`` -- crashes Electrum: + + AttributeError: 'NoneType' object has no attribute 'pop' + + ``SWAPSERVER_PORT`` is registered by Electrum's bundled swapserver plugin as + the *dotted* key ``plugins.swapserver.port`` + (electrum/plugins/swapserver/__init__.py). Assigning ``None`` to a ConfigVar + routes into ``SimpleConfig._set_key_in_user_config``'s key-*deletion* branch, + whose recursion walks the dotted path without checking that each intermediate + dict exists (electrum/simple_config.py, ``delete_key``):: + + prefix, suffix = key.split('.', 1) + d2 = d.get(prefix) # None when 'plugins.swapserver' was never written + empty = delete_key(d2, suffix) + + This plugin only ever writes ``plugins.swapserver_gui.*``, so unless the user + has separately enabled and configured the *bundled* swapserver plugin, the + ``plugins.swapserver`` sub-dict does not exist and the delete always raises. + + Storing ``0`` takes the ordinary write branch and is behaviourally identical: + every reader of the key tests it for truthiness rather than comparing it to + ``None`` -- ``electrum/submarine_swaps.py`` (``if self.config.SWAPSERVER_PORT:``), + ``electrum/plugins/swapserver/server.py``, and this plugin's own ``can_run``, + ``start_server`` and ``status``. + + This is an upstream Electrum bug, but the fix has to live here: users run + released AppImages we cannot patch. + """ + assert isinstance(port, int), f"port must be an int, got {port!r}" + config.SWAPSERVER_PORT = port + config.SWAPSERVER_FEE_MILLIONTHS = int(fee_millionths) + config.SWAPSERVER_POW_TARGET = int(pow_target) + config.NOSTR_RELAYS = relays + + def get_swap_history(wallet: 'Abstract_Wallet') -> List[Dict[str, Any]]: """Confirmed swaps served by this node (mirrors the bundled swapserver plugin's ``get_history`` command, but as a plain sync helper).""" diff --git a/tests/test_settings_save.py b/tests/test_settings_save.py new file mode 100644 index 0000000..23effe6 --- /dev/null +++ b/tests/test_settings_save.py @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +"""Regression tests for persisting the swap-server settings. + +The bug these cover: clicking "Save settings" with the HTTP port set to 0 +("disabled") crashed Electrum with + + File ".../swapserver_gui/qt.py", line 223, in on_save + self.config.SWAPSERVER_PORT = new_port + ... + File ".../electrum/simple_config.py", line 316, in delete_key + d.pop(key, None) + AttributeError: 'NoneType' object has no attribute 'pop' + +``qt.py`` turned a spinbox value of 0 into ``None``, and ``SWAPSERVER_PORT`` is +the *dotted* key ``plugins.swapserver.port``. Assigning ``None`` routes into +``SimpleConfig``'s key-deletion branch, whose recursion dereferences the +intermediate ``plugins.swapserver`` dict without checking it exists. This +plugin only ever writes ``plugins.swapserver_gui.*``, so that dict normally does +not exist and the delete always raises. See ``save_settings``' docstring. + +These tests drive a **real** ``SimpleConfig`` rather than the attribute-bag fake +the rest of the suite uses -- the fake is precisely why this was never caught. + +Run with: python3 -m pytest tests/test_settings_save.py +""" +import ast +import json +import os +import sys +import tempfile +import unittest +from typing import Any, Dict, List + +# --- make electrum + the plugin importable --------------------------------- +_HERE = os.path.dirname(os.path.abspath(__file__)) +_REPO_ROOT = os.path.dirname(_HERE) # electrum_swapgui/ (our repo) +_PROJECT_ROOT = os.path.dirname(_REPO_ROOT) +_ELECTRUM_SRC = os.environ.get("ELECTRUM_SRC", os.path.join(_PROJECT_ROOT, "electrum")) +_PLUGINS_DIR = os.path.join(_REPO_ROOT, "plugins") +for p in (_ELECTRUM_SRC, _PLUGINS_DIR): + if p not in sys.path: + sys.path.insert(0, p) + +from electrum.simple_config import SimpleConfig # noqa: E402 +# Registers the shared ConfigVars (plugins.swapserver.port, .fee_millionths). +import electrum.plugins.swapserver # noqa: E402,F401 + +from swapserver_gui.swapserver_gui import save_settings # noqa: E402 + +_QT_PY = os.path.join(_PLUGINS_DIR, "swapserver_gui", "qt.py") + + +def _make_config(seed: Dict[str, Any]) -> SimpleConfig: + """A real SimpleConfig on a throwaway datadir, pre-seeded with `seed`.""" + tmpdir = tempfile.mkdtemp() + config = SimpleConfig({"electrum_path": tmpdir}) + config.user_config.update(seed) + return config + + +class SaveSettingsTests(unittest.TestCase): + """save_settings must survive every shape of user_config we can be handed.""" + + # The four states, from the reproduction. 'plugins present without + # swapserver' is the one users actually hit: enabling *this* plugin writes + # plugins.swapserver_gui.enabled, which creates 'plugins' but not + # 'plugins.swapserver'. + SEEDS = { + "fresh config": {}, + "plugins present, no swapserver": { + "plugins": {"swapserver_gui": {"enabled": True}}}, + "full path already present": { + "plugins": {"swapserver": {"port": 5455}}}, + "swapserver present but empty": { + "plugins": {"swapserver": {}}}, + } + + def test_disabling_port_does_not_crash(self) -> None: + """The regression: port 0 must save cleanly from any starting state.""" + for label, seed in self.SEEDS.items(): + with self.subTest(seed=label): + config = _make_config(seed) + save_settings(config, port=0, fee_millionths=5000, + pow_target=30, relays="wss://relay.example.com") + self.assertFalse(config.SWAPSERVER_PORT, + "a disabled port must read back as falsy") + + def test_enabling_port_does_not_crash(self) -> None: + for label, seed in self.SEEDS.items(): + with self.subTest(seed=label): + config = _make_config(seed) + save_settings(config, port=5455, fee_millionths=5000, + pow_target=30, relays="wss://relay.example.com") + self.assertEqual(config.SWAPSERVER_PORT, 5455) + + def test_all_values_round_trip(self) -> None: + config = _make_config({}) + save_settings(config, port=5455, fee_millionths=1234, pow_target=22, + relays="wss://a.example.com,wss://b.example.com") + self.assertEqual(config.SWAPSERVER_PORT, 5455) + self.assertEqual(config.SWAPSERVER_FEE_MILLIONTHS, 1234) + self.assertEqual(config.SWAPSERVER_POW_TARGET, 22) + self.assertEqual(config.NOSTR_RELAYS, + "wss://a.example.com,wss://b.example.com") + + def test_later_writes_are_not_lost_when_port_is_disabled(self) -> None: + """The port write comes first; before the fix its crash discarded the rest.""" + config = _make_config({"plugins": {"swapserver_gui": {"enabled": True}}}) + save_settings(config, port=0, fee_millionths=7777, pow_target=18, + relays="wss://c.example.com") + self.assertEqual(config.SWAPSERVER_FEE_MILLIONTHS, 7777) + self.assertEqual(config.SWAPSERVER_POW_TARGET, 18) + self.assertEqual(config.NOSTR_RELAYS, "wss://c.example.com") + + def test_settings_persist_to_disk(self) -> None: + """Values must survive a restart, not just live in memory.""" + tmpdir = tempfile.mkdtemp() + config = SimpleConfig({"electrum_path": tmpdir}) + save_settings(config, port=0, fee_millionths=4321, pow_target=12, + relays="wss://d.example.com") + with open(os.path.join(tmpdir, "config"), encoding="utf-8") as f: + on_disk = json.load(f) + self.assertEqual(on_disk["plugins"]["swapserver"]["fee_millionths"], 4321) + + reloaded = SimpleConfig({"electrum_path": tmpdir}) + self.assertFalse(reloaded.SWAPSERVER_PORT) + self.assertEqual(reloaded.SWAPSERVER_FEE_MILLIONTHS, 4321) + self.assertEqual(reloaded.SWAPSERVER_POW_TARGET, 12) + self.assertEqual(reloaded.NOSTR_RELAYS, "wss://d.example.com") + + def test_does_not_disable_this_plugin(self) -> None: + """Upstream's delete path prunes empty parent dicts; ours must not run at all. + + ``plugins.swapserver_gui.enabled`` is what keeps this plugin installed -- + collateral damage there would uninstall the plugin on save. + """ + config = _make_config({ + "plugins": {"swapserver_gui": {"enabled": True}, "swapserver": {"port": 5455}}, + }) + save_settings(config, port=0, fee_millionths=5000, pow_target=30, relays="") + self.assertIs(config.user_config["plugins"]["swapserver_gui"]["enabled"], True) + + def test_none_port_is_rejected_loudly(self) -> None: + """None is the trap. Fail fast in our own code rather than deep in Electrum.""" + config = _make_config({}) + with self.assertRaises(AssertionError): + save_settings(config, port=None, fee_millionths=5000, # type: ignore[arg-type] + pow_target=30, relays="") + + +class UpstreamBugTests(unittest.TestCase): + """Pins down the upstream defect the workaround exists for.""" + + def test_assigning_none_still_crashes_upstream(self) -> None: + config = _make_config({"plugins": {"swapserver_gui": {"enabled": True}}}) + try: + config.SWAPSERVER_PORT = None + except AttributeError: + pass # expected: this is the crash users reported + else: + # Not a failure: upstream fixed delete_key. Storing 0 stays correct + # and behaviourally identical, so the workaround can remain. + self.skipTest("upstream Electrum no longer crashes on dotted-key deletion") + + +class QtWriteGuardTests(unittest.TestCase): + """Static guard: qt.py must not write SWAPSERVER_PORT behind save_settings' back. + + qt.py cannot be imported here (PyQt6 is absent locally and in CI), so this + inspects the source instead -- which is also what keeps the guard cheap. + """ + + @staticmethod + def _port_assignment_lines(source: str, filename: str) -> List[int]: + tree = ast.parse(source, filename=filename) + hits = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Assign): + continue + for target in node.targets: + if isinstance(target, ast.Attribute) and target.attr == "SWAPSERVER_PORT": + hits.append(node.lineno) + return hits + + def test_qt_does_not_assign_swapserver_port_directly(self) -> None: + with open(_QT_PY, encoding="utf-8") as f: + source = f.read() + hits = self._port_assignment_lines(source, _QT_PY) + self.assertEqual( + hits, [], + "qt.py must persist the port via swapserver_gui.save_settings, which " + "enforces the int/0 contract. Direct assignment risks reintroducing " + "the None crash. Offending lines: " + repr(hits)) + + def test_guard_would_catch_the_regression(self) -> None: + """The guard is only worth having if it actually fires.""" + hits = self._port_assignment_lines( + "self.config.SWAPSERVER_PORT = new_port\n", "") + self.assertEqual(len(hits), 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_version.py b/tests/test_version.py new file mode 100644 index 0000000..b64677b --- /dev/null +++ b/tests/test_version.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Unit tests for the plugin version shown in the Swap Server tab header. + +The version is stamped into ``_version.py`` at build time by +``contrib/make_zip.sh`` (from the git tag in CI), so these tests cover both the +runtime formatting and the contract the build script relies on: if the committed +module drifts from what the stamping regex expects, released zips would silently +ship the ``dev`` placeholder instead of the release version. + +Run with: python3 -m pytest tests/test_version.py +""" +import os +import re +import sys +import unittest + +_HERE = os.path.dirname(os.path.abspath(__file__)) +_REPO_ROOT = os.path.dirname(_HERE) # electrum_swapgui/ (our repo) +_PLUGINS_DIR = os.path.join(_REPO_ROOT, "plugins") +if _PLUGINS_DIR not in sys.path: + sys.path.insert(0, _PLUGINS_DIR) + +from swapserver_gui import _version # noqa: E402 + +_VERSION_PY = os.path.join(_PLUGINS_DIR, "swapserver_gui", "_version.py") + +# Must stay in step with the substitution in contrib/make_zip.sh. +_STAMP_RE = re.compile(r'^__version__ = ".*"$', re.MULTILINE) + + +class VersionValueTests(unittest.TestCase): + + def test_version_is_a_non_empty_string(self) -> None: + self.assertIsInstance(_version.__version__, str) + self.assertTrue(_version.__version__.strip()) + + def test_committed_value_is_the_placeholder(self) -> None: + """The repo must not carry a hardcoded release number. + + The tag is the source of truth; committing e.g. "0.2.1" here would go + stale the moment the next tag is pushed and would then misreport the + version to users. + """ + self.assertEqual(_version.__version__, "dev") + + +class FormatVersionTests(unittest.TestCase): + + def test_release_versions_get_a_v_prefix(self) -> None: + # Matches the git tag / GitHub release name users compare against. + self.assertEqual(_version.format_version("0.2.1"), "v0.2.1") + self.assertEqual(_version.format_version("1.0.0"), "v1.0.0") + self.assertEqual(_version.format_version("0.2.1rc1"), "v0.2.1rc1") + + def test_dev_stamps_are_shown_verbatim(self) -> None: + # "vdev" would read as a release; these are not versions in that sense. + self.assertEqual(_version.format_version("dev"), "dev") + self.assertEqual(_version.format_version("dev-g1a2b3c"), "dev-g1a2b3c") + + def test_surrounding_whitespace_is_ignored(self) -> None: + self.assertEqual(_version.format_version(" 0.2.1 "), "v0.2.1") + + def test_missing_version_degrades_gracefully(self) -> None: + # The header label must never render as an empty gap. + for value in ("", " ", None): + with self.subTest(value=value): + self.assertEqual(_version.format_version(value), "unknown") # type: ignore[arg-type] + + +class StampingContractTests(unittest.TestCase): + """_version.py must stay rewritable by contrib/make_zip.sh.""" + + def setUp(self) -> None: + with open(_VERSION_PY, encoding="utf-8") as f: + self.source = f.read() + + def test_exactly_one_stampable_assignment(self) -> None: + matches = _STAMP_RE.findall(self.source) + self.assertEqual( + len(matches), 1, + "contrib/make_zip.sh rewrites the first (and only) __version__ " + "assignment; found: " + repr(matches)) + + def test_stamping_produces_valid_source(self) -> None: + stamped, count = _STAMP_RE.subn('__version__ = "0.2.1"', self.source, count=1) + self.assertEqual(count, 1) + namespace: dict = {} + exec(compile(stamped, _VERSION_PY, "exec"), namespace) + self.assertEqual(namespace["__version__"], "0.2.1") + self.assertEqual(namespace["format_version"]("0.2.1"), "v0.2.1") + + def test_module_has_no_imports(self) -> None: + """Keeps the module trivially safe to rewrite and to exec in isolation.""" + self.assertNotIn("\nimport ", self.source) + self.assertNotIn("\nfrom ", self.source) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_zip_plugin_load.py b/tests/test_zip_plugin_load.py index ad6d447..9031f28 100644 --- a/tests/test_zip_plugin_load.py +++ b/tests/test_zip_plugin_load.py @@ -27,6 +27,7 @@ import tempfile import textwrap import unittest +import zipfile from typing import List _HERE = os.path.dirname(os.path.abspath(__file__)) @@ -90,11 +91,13 @@ def test_ast_guard_would_catch_the_regression(self) -> None: class _ZipBuildMixin: @classmethod - def build_zip(cls) -> str: + def build_zip(cls, version: str = "") -> str: cls._tmpdir = tempfile.TemporaryDirectory() # noqa: SIM115 (closed in tearDownClass) + cmd = ["bash", _MAKE_ZIP, cls._tmpdir.name] + if version: + cmd.append(version) subprocess.run( - ["bash", _MAKE_ZIP, cls._tmpdir.name], - check=True, capture_output=True, timeout=_SUBPROCESS_TIMEOUT, + cmd, check=True, capture_output=True, timeout=_SUBPROCESS_TIMEOUT, ) path = os.path.join(cls._tmpdir.name, "swapserver_gui.zip") assert os.path.exists(path), path @@ -210,6 +213,83 @@ def test_qt_module_is_resolvable(self) -> None: self.assertIn("OK", r.stdout, r.stderr) +class VersionStampingTests(_ZipBuildMixin, unittest.TestCase): + """End-to-end: the version CI injects must reach the running plugin. + + Covers the whole chain the tab header depends on -- make_zip.sh stamps a + staging copy, the stamp lands in the archive, and the module reads back with + that value when loaded the way Electrum loads a zip plugin. + """ + + VERSION = "9.8.7" + + @classmethod + def setUpClass(cls) -> None: + cls.zip_path = cls.build_zip(cls.VERSION) + + @classmethod + def tearDownClass(cls) -> None: + cls._tmpdir.cleanup() + + def test_stamp_lands_in_the_archive(self) -> None: + with zipfile.ZipFile(self.zip_path) as zf: + source = zf.read("swapserver_gui/_version.py").decode("utf-8") + self.assertIn(f'__version__ = "{self.VERSION}"', source) + self.assertNotIn('__version__ = "dev"', source) + + def test_working_tree_is_not_modified(self) -> None: + """Stamping happens in a staging copy; the committed file stays 'dev'.""" + with open(os.path.join(_PLUGIN_DIR, "_version.py"), encoding="utf-8") as f: + self.assertIn('__version__ = "dev"', f.read()) + + def test_version_reads_back_when_loaded_from_zip(self) -> None: + r = self.run_child(f""" + import importlib.util, os, sys, zipfile, zipimport + ZIP = {self.zip_path!r} + BASE = {_BASE_NAME!r} + + with zipfile.ZipFile(ZIP) as zf: + manifest_name = next(n for n in zf.namelist() if n.endswith('manifest.json')) + dirname = os.path.dirname(manifest_name) + + def exec_module_from_spec(spec, path): + module = importlib.util.module_from_spec(spec) + sys.modules[path] = module + spec.loader.exec_module(module) + return module + + init_spec = zipimport.zipimporter(ZIP).find_spec(dirname) + exec_module_from_spec(init_spec, BASE) + + full = BASE + '._version' + mod = exec_module_from_spec(importlib.util.find_spec(full), full) + assert mod.__version__ == {self.VERSION!r}, mod.__version__ + assert mod.format_version(mod.__version__) == 'v{self.VERSION}', mod.__version__ + print('OK') + """) + self.assertIn("OK", r.stdout, r.stderr) + + def test_unstamped_build_keeps_the_placeholder(self) -> None: + tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(tmpdir.cleanup) + subprocess.run(["bash", _MAKE_ZIP, tmpdir.name], + check=True, capture_output=True, timeout=_SUBPROCESS_TIMEOUT) + with zipfile.ZipFile(os.path.join(tmpdir.name, "swapserver_gui.zip")) as zf: + source = zf.read("swapserver_gui/_version.py").decode("utf-8") + self.assertIn('__version__ = "dev"', source) + + def test_unsafe_version_is_rejected(self) -> None: + """A malformed tag must fail the build, not emit a broken _version.py.""" + tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(tmpdir.cleanup) + for bad in ('1.0"; import os', "1.0 $(whoami)", "1.0\nx=1"): + with self.subTest(version=bad): + r = subprocess.run( + ["bash", _MAKE_ZIP, tmpdir.name, bad], + capture_output=True, text=True, timeout=_SUBPROCESS_TIMEOUT) + self.assertNotEqual(r.returncode, 0, f"accepted unsafe version {bad!r}") + + class DirectoryLoadTests(_ZipBuildMixin, unittest.TestCase): """The zip fix must not break the directory layout the rest of the suite uses."""