From 7fd925c26934425dd42e2efe8d4f144d508923a7 Mon Sep 17 00:00:00 2001 From: Alexander Nguyen Date: Thu, 4 Jun 2026 20:55:10 -0700 Subject: [PATCH] feat(settings): editable NAS rclone remote + live test-connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the Settings → NAS Remote section editable instead of read-only: - Pick the rclone remote from a dropdown of remotes detected via `rclone listremotes` (with a Refresh button that re-lists using the typed config path), and edit base root + optional `--config` path. - All three fields two-way-bind the draft nas block; the remote select converts None <-> "" at the binding boundary so the str field stays valid on Save while satisfying ui.select's option constraint. - The section is now always shown (after Equipment), so the remote can be configured before nas-mode equipment exists. - Test connection now probes the *typed* (unsaved) remote + config path directly via RcloneDriver.about(), independent of deps.equipment_probe (which the /setup/test-equipment endpoint still uses for saved config). Updates the settings/mount unit tests, the e2e UX catalog, and the regenerated docs/UX_INTERACTIONS.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/UX_INTERACTIONS.md | 6 +- src/exlab_wizard/ui/mount.py | 86 +++++++----- src/exlab_wizard/ui/pages/settings.py | 158 +++++++++++++-------- tests/e2e/ux_catalog.py | 13 +- tests/unit/ui/test_mount.py | 96 +++++++------ tests/unit/ui/test_settings_nas_remote.py | 162 ++++++++++++++++++---- 6 files changed, 355 insertions(+), 166 deletions(-) diff --git a/docs/UX_INTERACTIONS.md b/docs/UX_INTERACTIONS.md index 0fde0fa..b92086c 100644 --- a/docs/UX_INTERACTIONS.md +++ b/docs/UX_INTERACTIONS.md @@ -45,9 +45,9 @@ flow test. | Route | Test ID | Element | Action | Outcome | |---|---|---|---|---| -| `/settings` | `settings-nav-nas_remote` | nav row | Click the 'NAS Remote' sidebar row | Shows the configured rclone remote name, base root, and found/not-found badge. | -| `/settings` | `settings-nas-remote-name` | label | View the configured rclone remote name | Displays the remote name from config.nas.remote (or '(not configured)'). | -| `/settings` | `settings-nas-test-connection` | button | Click 'Test connection' | Runs the rclone remote probe and renders the result inline. | +| `/settings` | `settings-nav-nas_remote` | nav row | Click the 'NAS Remote' sidebar row | Opens the NAS Remote section (always present): pick the remote, set base root and optional config path, and test the connection. | +| `/settings` | `settings-nas-remote-name` | select | Pick the rclone remote from the dropdown | Binds config.nas.remote; options are the remotes detected via `rclone listremotes` plus the currently-configured value. | +| `/settings` | `settings-nas-test-connection` | button | Click 'Test connection' | Probes the typed (unsaved) remote + config path via `rclone about` and renders the result inline. | ## New template diff --git a/src/exlab_wizard/ui/mount.py b/src/exlab_wizard/ui/mount.py index ce64ab0..cb35700 100644 --- a/src/exlab_wizard/ui/mount.py +++ b/src/exlab_wizard/ui/mount.py @@ -540,8 +540,8 @@ def _do() -> None: threading.Thread(target=_do, name="exlab-quit", daemon=True).start() - async def _on_test_connection() -> Any: - return await _nas_test_connection(deps) + async def _on_test_connection(remote: str, config_path: str) -> Any: + return await _nas_test_connection(remote, config_path) # ``on_select_section`` is left unset: the settings dialog swaps # sections client-side, so a navigation hook would only reload @@ -556,6 +556,8 @@ async def _on_test_connection() -> Any: lims_password_present=lims_password_present(deps), nas_remote_available=lambda remote: nas_remote_available(deps, remote), on_test_connection=_on_test_connection, + nas_remotes=getattr(deps, "nas_remotes", ()) if deps is not None else (), + list_remotes=_list_nas_remotes, autostart_registered=bool(getattr(deps, "autostart_is_registered", False)), on_set_autostart=_on_set_autostart, on_quit=on_quit, @@ -633,59 +635,71 @@ def _on_clear() -> None: return _on_save, _on_clear -async def _nas_test_connection(deps: Any) -> Any: - """Run the rclone NAS-remote probe and adapt it for the inline panel. +async def _list_nas_remotes(config_path: str) -> tuple[str, ...]: + """Return the rclone remotes (each incl. trailing ``":"``) for ``config_path``. + + Backs the Settings "NAS Remote" dropdown's Refresh affordance: re-runs + ``rclone listremotes`` against the *typed* ``--config`` path so a just-edited + config path is reflected without a tray relaunch. Any failure (no binary, + unreadable config) degrades to ``()`` so the dropdown simply shows nothing + new rather than raising. + """ + from exlab_wizard.sync.transports.rclone import RcloneDriver + + try: + return await RcloneDriver(config_path=config_path or None).listremotes() + except Exception: + return () + + +async def _nas_test_connection(remote: str, config_path: str) -> Any: + """Probe the *typed* rclone NAS remote and adapt it for the inline panel. rclone.conf NAS-sync migration. The Settings "NAS Remote" section's - Test-connection button probes the single configured ``nas:`` remote - (no per-equipment password). It reuses ``deps.equipment_probe`` -- the - same probe the ``POST /setup/test-equipment`` endpoint uses, which now - targets ``nas.remote`` and ignores the per-equipment fields -- passing - the first nas-mode equipment (or any equipment) as the probe argument. - The probe's ``{ok, reason, latency_ms}`` dict is mapped to a + Test-connection button validates the values currently in the form -- + ``remote`` (the rclone remote name) and the optional ``config_path`` + (``rclone --config ``) -- *before* the operator saves, so a fresh + selection can be tested immediately. It runs ``rclone about :`` + through the driver and maps the :class:`AboutResult` to a :class:`TestConnectionResult`. + + This is intentionally independent of ``deps.equipment_probe`` (which the + ``POST /setup/test-equipment`` endpoint still uses against the *saved* + config): the UI probe must reflect unsaved edits. """ import json + import time - from exlab_wizard.constants import SyncMode + from exlab_wizard.sync.transports.rclone import RcloneDriver from exlab_wizard.ui.components.test_connection_panel import TestConnectionResult - config = getattr(deps, "config", None) if deps is not None else None - probe = getattr(deps, "equipment_probe", None) if deps is not None else None - if probe is None or config is None: + if not remote: return TestConnectionResult( success=False, headline="Connection failed", - detail="equipment probe is not available", + detail="no NAS remote configured", raw="", ) - equipment = next( - (e for e in config.equipment if e.sync_mode == SyncMode.NAS), - next(iter(config.equipment), None), - ) + driver = RcloneDriver(config_path=config_path or None) + started = time.monotonic() try: - result = probe(equipment) - if asyncio.iscoroutine(result) or asyncio.isfuture(result): - result = await result + about = await driver.about(f"{remote}:") except Exception as exc: return TestConnectionResult( success=False, headline="Connection failed", detail=str(exc), raw=str(exc) ) - payload = result if isinstance(result, dict) else {"ok": bool(result)} - ok = bool(payload.get("ok")) - reason = payload.get("reason") - latency_ms = payload.get("latency_ms") - if ok: - detail = f"reachable ({latency_ms} ms)" if latency_ms is not None else "reachable" - headline = "Connected" - else: - detail = str(reason) if reason else "connection failed" - headline = "Connection failed" + latency_ms = int((time.monotonic() - started) * 1000) + payload = {"ok": about.ok, "reason": about.reason, "latency_ms": latency_ms, **about.info} + raw = json.dumps(payload, indent=2, sort_keys=True) + if about.ok: + return TestConnectionResult( + success=True, headline="Connected", detail=f"reachable ({latency_ms} ms)", raw=raw + ) return TestConnectionResult( - success=ok, - headline=headline, - detail=detail, - raw=json.dumps(payload, indent=2, sort_keys=True), + success=False, + headline="Connection failed", + detail=str(about.reason) if about.reason else "connection failed", + raw=raw, ) diff --git a/src/exlab_wizard/ui/pages/settings.py b/src/exlab_wizard/ui/pages/settings.py index edcc261..7531138 100644 --- a/src/exlab_wizard/ui/pages/settings.py +++ b/src/exlab_wizard/ui/pages/settings.py @@ -6,7 +6,7 @@ from __future__ import annotations -from collections.abc import Callable +from collections.abc import Awaitable, Callable, Sequence from dataclasses import dataclass, field from typing import Any @@ -38,10 +38,10 @@ # rclone.conf NAS-sync migration. The NAS-remote section is *not* part # of the canonical onboarding-order constant (``SETTINGS_SECTIONS`` stays # at the original eight); it is inserted dynamically after ``equipment`` -# by :func:`settings_sections_for` only when nas-mode equipment exists. -# It shows the single ``nas:`` remote (read-only) plus a Test-connection -# control -- the operator configures the remote with ``rclone config``, -# not by typing a password here. +# by :func:`settings_sections_for`. It lets the operator pick the single +# ``nas:`` remote (a dropdown of remotes detected via ``rclone listremotes``), +# set its base root and optional ``--config`` path, and Test-connection the +# typed values -- no NAS password is ever typed here. NAS_REMOTE_SECTION = "nas_remote" SECTION_TITLES: dict[str, str] = { @@ -58,31 +58,15 @@ } -def _nas_mode_equipment(config: Config | None) -> list[Any]: - """Return the nas-mode equipment for ``config``. - - Drives the NAS-remote section's visibility: the section appears - whenever this device has at least one device syncing directly to the - NAS, so the operator can confirm the configured ``nas:`` remote is - reachable. - """ - if config is None: - return [] - from exlab_wizard.constants import SyncMode - - return [eq for eq in config.equipment if eq.sync_mode == SyncMode.NAS] - - def settings_sections_for(config: Config | None) -> tuple[str, ...]: """Return the visible section ids for ``config``. - The NAS-remote section is inserted right after ``equipment`` only - when at least one nas-mode equipment exists; otherwise the canonical - :data:`SETTINGS_SECTIONS` order is returned unchanged (so a stage-only - / no-equipment install never sees an empty NAS-remote pane). + The NAS-remote section is always inserted right after ``equipment`` so + the operator can configure and test the single ``nas:`` remote even + before (or independent of) adding nas-mode equipment. ``config`` is + accepted for signature symmetry with the other section helpers but no + longer gates visibility. """ - if not _nas_mode_equipment(config): - return SETTINGS_SECTIONS out: list[str] = [] for section in SETTINGS_SECTIONS: out.append(section) @@ -196,7 +180,9 @@ def render_settings_page( on_clear_lims_password: Callable[[], None] | None = None, lims_password_present: bool = False, nas_remote_available: Callable[[str], bool] | None = None, - on_test_connection: Callable[[], Any] | None = None, + on_test_connection: Callable[[str, str], Any] | None = None, + nas_remotes: Sequence[str] = (), + list_remotes: Callable[[str], Awaitable[Sequence[str]]] | None = None, autostart_registered: bool = False, on_set_autostart: Callable[[bool], bool | None] | None = None, on_quit: Callable[[], None] | None = None, @@ -221,14 +207,17 @@ def render_settings_page( than to the draft. ``lims_password_present`` seeds the credential row's resting state from whether the keyring already holds one. - The NAS-remote section (rclone.conf migration) is read-only: the - operator no longer types a NAS password. ``nas_remote_available(name)`` - answers whether the configured ``nas.remote`` is present in the - operator's ``rclone.conf`` (driving a found / not-found badge), and - ``on_test_connection()`` runs the rclone remote probe, returning a - :class:`TestConnectionResult` (or an awaitable of one) for the inline - panel. Both are optional so unit tests can render the section without - a wired rclone driver. + The NAS-remote section (rclone.conf migration) is editable: the + operator picks the remote, base root, and optional ``--config`` path. + ``nas_remotes`` seeds the remote dropdown (the boot-time + ``rclone listremotes`` snapshot) and ``list_remotes(config_path)`` + re-lists on demand from the typed config path. + ``nas_remote_available(name)`` answers whether ``nas.remote`` is present + in the operator's ``rclone.conf`` (found / not-found badge), and + ``on_test_connection(remote, config_path)`` runs the rclone probe + against the *typed* values, returning a :class:`TestConnectionResult` + (or an awaitable of one) for the inline panel. All are optional so unit + tests can render the section without a wired rclone driver. """ s = state or SettingsState() @@ -342,6 +331,8 @@ def _select_section(section: str) -> None: lims_password_present=lims_password_present, nas_remote_available=nas_remote_available, on_test_connection=on_test_connection, + nas_remotes=nas_remotes, + list_remotes=list_remotes, autostart_registered=autostart_registered, on_set_autostart=on_set_autostart, on_quit=on_quit, @@ -474,7 +465,9 @@ def _render_section_body( on_clear_lims_password: Callable[[], None] | None = None, lims_password_present: bool = False, nas_remote_available: Callable[[str], bool] | None = None, - on_test_connection: Callable[[], Any] | None = None, + on_test_connection: Callable[[str, str], Any] | None = None, + nas_remotes: Sequence[str] = (), + list_remotes: Callable[[str], Awaitable[Sequence[str]]] | None = None, autostart_registered: bool = False, on_set_autostart: Callable[[bool], bool | None] | None = None, on_quit: Callable[[], None] | None = None, @@ -564,6 +557,8 @@ def _derived_label(value: str, *, prefix: str = caption) -> str: nas=draft.nas, nas_remote_available=nas_remote_available or (lambda _name: False), on_test_connection=on_test_connection, + nas_remotes=nas_remotes, + list_remotes=list_remotes, ) elif section == "nas_cleanup": ui.checkbox("Cleanup enabled", value=draft.nas_cleanup.enabled).bind_value( @@ -773,22 +768,41 @@ def _add(_evt: Any = None) -> None: ui.button("Add equipment", on_click=_add).props('data-testid="settings-equipment-add"') +def _remote_options(detected: Sequence[str], current: str) -> list[str]: + """Dropdown options for the NAS-remote select. + + ``rclone listremotes`` entries carry a trailing ``":"``; the config + stores the bare name (the probe re-adds it as ``f"{remote}:"``), so + strip it here. The currently-configured ``current`` is unioned in even + when absent from ``detected`` -- a saved remote whose ``rclone.conf`` + is presently unreadable must stay selectable. + """ + names = {name[:-1] if name.endswith(":") else name for name in detected if name} + if current: + names.add(current) + return sorted(names) + + def _render_nas_remote_section( container: Any, *, nas: Any, nas_remote_available: Callable[[str], bool], - on_test_connection: Callable[[], Any] | None, + on_test_connection: Callable[[str, str], Any] | None, + nas_remotes: Sequence[str] = (), + list_remotes: Callable[[str], Awaitable[Sequence[str]]] | None = None, ) -> None: - """Render the read-only NAS-remote status + a Test-connection panel. + """Render the editable NAS-remote fields + a Test-connection panel. - rclone.conf NAS-sync migration. The operator no longer types a NAS + rclone.conf NAS-sync migration. The operator never types a NAS password; the app references a single ``nas:`` remote defined in their ``rclone.conf`` (created out-of-band with ``rclone config``). This - section shows that remote + its base root read-only, a found / - not-found badge derived from ``nas_remote_available(nas.remote)``, and - a single "Test connection" button wired to ``on_test_connection`` (the - rclone remote probe) that renders its result inline. + section lets them choose that remote from a dropdown of remotes + detected via ``rclone listremotes`` (``nas_remotes`` seeds it; + ``list_remotes`` re-lists on demand using the typed config path), set + its base root and optional ``--config`` path, and run the rclone probe + against the *typed* values via ``on_test_connection(remote, config_path)``. + All three fields two-way-bind the draft ``nas`` block. """ import inspect @@ -796,28 +810,61 @@ def _render_nas_remote_section( remote = getattr(nas, "remote", "") or "" base_root = getattr(nas, "base_root", "") or "" - available = bool(remote) and nas_remote_available(remote) + config_path = getattr(nas, "rclone_config_path", "") or "" with container: ui.label( - "NAS sync references a single rclone remote configured in your " - "rclone.conf (run `rclone config` to create it). No password is " - "stored here." + "NAS sync targets a single rclone remote from your rclone.conf " + "(run `rclone config` to create one). Pick the remote, set its base " + "root, and optionally pin a config file. No password is stored here." ).style("font-size: var(--text-sm); color: var(--color-muted);") with ui.row().classes("items-center w-full").style("gap: 0.5rem;"): ui.label("Remote").style("color: var(--color-body); min-width: 6rem;") - ui.label(remote or "(not configured)").props( - 'data-testid="settings-nas-remote-name"' - ).style("font-family: var(--font-mono);") + # ``ui.select`` only accepts a value that is ``None`` or one of its + # options, but ``nas.remote`` is a ``str`` ("" when unset). Display + # the empty state as ``None`` and convert at the binding boundary: + # element ``None`` <-> model "" (a ``None`` reaching the field would + # fail validation on Save). + remote_select = ( + ui.select( + _remote_options(nas_remotes, remote), + value=remote or None, + ) + .props('data-testid="settings-nas-remote-name"') + .style("min-width: 16rem; font-family: var(--font-mono);") + .bind_value(nas, "remote", forward=lambda v: v or "", backward=lambda v: v or None) + ) + + async def _refresh() -> None: + """Re-list remotes from the *typed* config path and rebuild options.""" + if list_remotes is None: + return + current = getattr(nas, "remote", "") or "" + detected = await list_remotes(getattr(nas, "rclone_config_path", "") or "") + remote_select.set_options(_remote_options(detected, current), value=current or None) + + ui.button(icon="refresh", on_click=_refresh).props( + 'flat dense data-testid="settings-nas-remote-refresh"' + ) with ui.row().classes("items-center w-full").style("gap: 0.5rem;"): ui.label("Base root").style("color: var(--color-body); min-width: 6rem;") - ui.label(base_root or "(not configured)").props( + ui.input(value=base_root).props( 'data-testid="settings-nas-remote-base-root"' - ).style("font-family: var(--font-mono);") + ).style("min-width: 16rem; font-family: var(--font-mono);").bind_value( + nas, "base_root" + ) - if available: + with ui.row().classes("items-center w-full").style("gap: 0.5rem;"): + ui.label("Config path").style("color: var(--color-body); min-width: 6rem;") + ui.input(value=config_path, placeholder="(rclone default discovery)").props( + 'data-testid="settings-nas-remote-config-path"' + ).style("min-width: 16rem; font-family: var(--font-mono);").bind_value( + nas, "rclone_config_path" + ) + + if remote and nas_remote_available(remote): badge_text = "Found in rclone.conf" badge_color = "var(--color-success)" else: @@ -833,7 +880,10 @@ async def _test() -> None: panel.clear() if on_test_connection is None: return - result = on_test_connection() + result = on_test_connection( + getattr(nas, "remote", "") or "", + getattr(nas, "rclone_config_path", "") or "", + ) if inspect.isawaitable(result): result = await result with panel: diff --git a/tests/e2e/ux_catalog.py b/tests/e2e/ux_catalog.py index c4b033c..124f093 100644 --- a/tests/e2e/ux_catalog.py +++ b/tests/e2e/ux_catalog.py @@ -153,15 +153,17 @@ class UXInteraction: testid="settings-nav-nas_remote", element="nav row", action="Click the 'NAS Remote' sidebar row", - outcome="Shows the configured rclone remote name, base root, and found/not-found badge.", + outcome="Opens the NAS Remote section (always present): pick the remote, set base root " + "and optional config path, and test the connection.", ), UXInteraction( flow="NAS Remote", route="/settings", testid="settings-nas-remote-name", - element="label", - action="View the configured rclone remote name", - outcome="Displays the remote name from config.nas.remote (or '(not configured)').", + element="select", + action="Pick the rclone remote from the dropdown", + outcome="Binds config.nas.remote; options are the remotes detected via `rclone " + "listremotes` plus the currently-configured value.", ), UXInteraction( flow="NAS Remote", @@ -169,7 +171,8 @@ class UXInteraction: testid="settings-nas-test-connection", element="button", action="Click 'Test connection'", - outcome="Runs the rclone remote probe and renders the result inline.", + outcome="Probes the typed (unsaved) remote + config path via `rclone about` and " + "renders the result inline.", ), UXInteraction( flow="Equipment", diff --git a/tests/unit/ui/test_mount.py b/tests/unit/ui/test_mount.py index af77d5f..7290a4f 100644 --- a/tests/unit/ui/test_mount.py +++ b/tests/unit/ui/test_mount.py @@ -2144,66 +2144,78 @@ def button(self, text: str = "", *, on_click: Any = None, **_kwargs: Any) -> _Fl # --------------------------------------------------------------------------- -# _nas_test_connection +# _nas_test_connection (probes the *typed* remote/config-path directly via +# RcloneDriver -- independent of deps.equipment_probe / the setup endpoint) # --------------------------------------------------------------------------- -async def test_nas_test_connection_unavailable_when_probe_missing() -> None: - """No probe wired -> a failure result rather than a crash.""" - result = await mount._nas_test_connection(_deps(config=_nas_config())) - assert result.success is False - assert "not available" in result.detail +class _StubAbout: + """Stand-in for sync.transports.rclone.AboutResult.""" + def __init__(self, *, ok: bool, reason: str | None = None, info: dict[str, int] | None = None): + self.ok = ok + self.reason = reason + self.info = info or {} -async def test_nas_test_connection_unavailable_when_config_missing() -> None: - result = await mount._nas_test_connection(_deps(config=None, equipment_probe=lambda _e: {})) - assert result.success is False +def _stub_rclone_driver( + monkeypatch: pytest.MonkeyPatch, + *, + about: _StubAbout | None = None, + raises: Exception | None = None, + capture: dict[str, Any] | None = None, +) -> None: + """Patch RcloneDriver so ``_nas_test_connection`` probes a fake remote.""" -async def test_nas_test_connection_success_maps_latency() -> None: - """A reachable probe maps ok/latency into a Connected result.""" - probed: list[Any] = [] + class _Driver: + def __init__(self, *, config_path: str | None = None, **_kw: Any) -> None: + if capture is not None: + capture["config_path"] = config_path - def _probe(equipment: Any) -> dict[str, Any]: - probed.append(equipment) - return {"ok": True, "latency_ms": 42} + async def about(self, remote: str) -> Any: + if capture is not None: + capture["remote"] = remote + if raises is not None: + raise raises + return about - deps = _deps(config=_nas_config(), equipment_probe=_probe) - result = await mount._nas_test_connection(deps) - assert result.success is True - assert result.headline == "Connected" - assert "42 ms" in result.detail - # The nas-mode equipment was chosen as the probe argument. - assert probed[0].id == "EQ1" + monkeypatch.setattr("exlab_wizard.sync.transports.rclone.RcloneDriver", _Driver) -async def test_nas_test_connection_failure_maps_reason() -> None: - deps = _deps( - config=_nas_config(), - equipment_probe=lambda _e: {"ok": False, "reason": "auth denied"}, - ) - result = await mount._nas_test_connection(deps) +async def test_nas_test_connection_empty_remote_short_circuits() -> None: + """An empty remote never spawns rclone -- it reports the gate reason.""" + result = await mount._nas_test_connection("", "") assert result.success is False - assert result.detail == "auth denied" + assert "no NAS remote configured" in result.detail -async def test_nas_test_connection_awaits_coroutine_probe() -> None: - """An async probe is awaited before its dict is mapped.""" - - async def _probe(_equipment: Any) -> dict[str, Any]: - return {"ok": True} - - result = await mount._nas_test_connection(_deps(config=_nas_config(), equipment_probe=_probe)) +async def test_nas_test_connection_success_maps_latency(monkeypatch: pytest.MonkeyPatch) -> None: + """A reachable remote maps ok/latency into a Connected result.""" + capture: dict[str, Any] = {} + _stub_rclone_driver( + monkeypatch, about=_StubAbout(ok=True, info={"free": 10}), capture=capture + ) + result = await mount._nas_test_connection("nas01", "/c.conf") assert result.success is True - assert result.detail == "reachable" + assert result.headline == "Connected" + assert "ms" in result.detail + # Probed the *typed* remote (trailing ':') and config path -- not deps.config. + assert capture["remote"] == "nas01:" + assert capture["config_path"] == "/c.conf" -async def test_nas_test_connection_swallows_probe_exception() -> None: - def _probe(_equipment: Any) -> dict[str, Any]: - msg = "boom" - raise RuntimeError(msg) +async def test_nas_test_connection_failure_maps_reason(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_rclone_driver(monkeypatch, about=_StubAbout(ok=False, reason="auth: denied")) + result = await mount._nas_test_connection("nas01", "") + assert result.success is False + assert result.detail == "auth: denied" + - result = await mount._nas_test_connection(_deps(config=_nas_config(), equipment_probe=_probe)) +async def test_nas_test_connection_swallows_driver_exception( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _stub_rclone_driver(monkeypatch, raises=RuntimeError("boom")) + result = await mount._nas_test_connection("nas01", "") assert result.success is False assert result.detail == "boom" diff --git a/tests/unit/ui/test_settings_nas_remote.py b/tests/unit/ui/test_settings_nas_remote.py index 3c936a8..c75194d 100644 --- a/tests/unit/ui/test_settings_nas_remote.py +++ b/tests/unit/ui/test_settings_nas_remote.py @@ -1,10 +1,12 @@ """Tests for the Settings NAS-remote section (rclone.conf migration). -The section is read-only: it shows the single configured ``nas:`` remote -plus its base root, a found / not-found badge derived from -``nas_remote_available``, and a single Test-connection control. There is -no password input. The section is shown only when nas-mode equipment -exist and is hidden entirely for a stage-only / no-equipment install. +The section is editable: the operator picks the single ``nas:`` remote +from a dropdown of remotes detected via ``rclone listremotes`` (with a +Refresh affordance), sets its base root and optional ``--config`` path, +and Test-connection probes the *typed* values. A found / not-found badge +is derived from ``nas_remote_available``; there is no password input. The +section is always shown (after the equipment section) so the remote can be +configured before nas-mode equipment exists. """ from __future__ import annotations @@ -97,6 +99,14 @@ def _text_of(element: object, testid: str) -> str: return "" +def _value_of(element: object, testid: str) -> Any: + """Return the ``.value`` of the widget carrying ``testid`` (input/select).""" + for child in element.descendants(): # type: ignore[attr-defined] + if child._props.get("data-testid") == testid: + return getattr(child, "value", None) + return None + + # --------------------------------------------------------------------------- # Handler-invocation helpers # @@ -161,12 +171,12 @@ def _draft_of(save_button: object) -> Config: # --------------------------------------------------------------------------- -def test_sections_omit_nas_remote_without_nas_equipment() -> None: - config = _config_with(_stage_equipment("STG1")) - assert "nas_remote" not in settings.settings_sections_for(config) - # No equipment at all -> also omitted. - assert "nas_remote" not in settings.settings_sections_for(_config_with()) - assert "nas_remote" not in settings.settings_sections_for(None) +def test_sections_always_include_nas_remote() -> None: + # Always shown now -- even for a stage-only / no-equipment / no-config + # install -- so the operator can configure the remote up front. + assert "nas_remote" in settings.settings_sections_for(_config_with(_stage_equipment("STG1"))) + assert "nas_remote" in settings.settings_sections_for(_config_with()) + assert "nas_remote" in settings.settings_sections_for(None) def test_sections_insert_nas_remote_after_equipment() -> None: @@ -183,20 +193,59 @@ def test_sections_insert_nas_remote_after_equipment() -> None: # --------------------------------------------------------------------------- -def test_section_renders_configured_remote_and_base_root() -> None: +def test_section_renders_editable_remote_base_root_and_config_path() -> None: config = _config_with(_nas_equipment("EQ1"), remote="nas01") + config.nas.rclone_config_path = "/etc/rclone.conf" out = settings.render_settings_page( config=config, state=settings.SettingsState(active_section="nas_remote"), on_save=lambda s: None, nas_remote_available=lambda _name: True, - on_test_connection=lambda: None, + on_test_connection=lambda _remote, _config_path: None, ) ids = _section_testids(out, "nas_remote") assert "settings-nas-remote-name" in ids assert "settings-nas-remote-base-root" in ids - assert "nas01" in _text_of(out, "settings-nas-remote-name") - assert "/srv/nas" in _text_of(out, "settings-nas-remote-base-root") + assert "settings-nas-remote-config-path" in ids + # Editable widgets carry the configured values (not read-only labels). + assert _value_of(out, "settings-nas-remote-name") == "nas01" + assert _value_of(out, "settings-nas-remote-base-root") == "/srv/nas" + assert _value_of(out, "settings-nas-remote-config-path") == "/etc/rclone.conf" + + +def test_remote_dropdown_lists_detected_remotes_plus_current() -> None: + config = _config_with(_nas_equipment("EQ1"), remote="nas01") + out = settings.render_settings_page( + config=config, + state=settings.SettingsState(active_section="nas_remote"), + on_save=lambda s: None, + nas_remote_available=lambda _name: True, + on_test_connection=lambda _remote, _config_path: None, + # listremotes entries carry a trailing ":" -- the dropdown strips it. + nas_remotes=("nas02:", "backup:"), + ) + select = _find(out, "settings-nas-remote-name") + assert type(select).__name__ == "Select" + options = list(select.options) + # Detected remotes (stripped) and the currently-configured one are all present. + assert set(options) == {"nas01", "nas02", "backup"} + + +def test_remote_dropdown_keeps_undetected_current_remote_selectable() -> None: + # rclone.conf currently unreadable (no detected remotes) -> the saved + # remote must still appear as the selected option. + config = _config_with(_nas_equipment("EQ1"), remote="ghost") + out = settings.render_settings_page( + config=config, + state=settings.SettingsState(active_section="nas_remote"), + on_save=lambda s: None, + nas_remote_available=lambda _name: False, + on_test_connection=lambda _remote, _config_path: None, + nas_remotes=(), + ) + select = _find(out, "settings-nas-remote-name") + assert "ghost" in list(select.options) + assert select.value == "ghost" def test_section_has_no_password_input() -> None: @@ -206,7 +255,7 @@ def test_section_has_no_password_input() -> None: state=settings.SettingsState(active_section="nas_remote"), on_save=lambda s: None, nas_remote_available=lambda _name: True, - on_test_connection=lambda: None, + on_test_connection=lambda _remote, _config_path: None, ) # Scoped to the NAS-remote section body (the page also renders the # LIMS section, which legitimately has a password field). @@ -222,7 +271,7 @@ def test_section_has_test_connection_control() -> None: state=settings.SettingsState(active_section="nas_remote"), on_save=lambda s: None, nas_remote_available=lambda _name: True, - on_test_connection=lambda: None, + on_test_connection=lambda _remote, _config_path: None, ) assert "settings-nas-test-connection" in _section_testids(out, "nas_remote") @@ -234,7 +283,7 @@ def test_status_badge_found_when_remote_available() -> None: state=settings.SettingsState(active_section="nas_remote"), on_save=lambda s: None, nas_remote_available=lambda name: name == "nas01", - on_test_connection=lambda: None, + on_test_connection=lambda _remote, _config_path: None, ) status = _text_of(out, "settings-nas-remote-status") assert "Found" in status @@ -247,7 +296,7 @@ def test_status_badge_not_found_when_remote_absent() -> None: state=settings.SettingsState(active_section="nas_remote"), on_save=lambda s: None, nas_remote_available=lambda _name: False, - on_test_connection=lambda: None, + on_test_connection=lambda _remote, _config_path: None, ) status = _text_of(out, "settings-nas-remote-status") assert "Not found" in status @@ -265,14 +314,16 @@ def test_nav_entry_present_when_nas_equipment_exists() -> None: assert "settings-nav-nas_remote" in _testids(out) -def test_nav_entry_absent_for_stage_only_config() -> None: +def test_nav_entry_present_for_stage_only_config() -> None: + # The NAS-remote nav row is always present now (even with no nas-mode + # equipment), so the remote can be configured before equipment is added. config = _config_with(_stage_equipment("STG1")) out = settings.render_settings_page( config=config, state=settings.SettingsState(active_section="paths"), on_save=lambda s: None, ) - assert "settings-nav-nas_remote" not in _testids(out) + assert "settings-nav-nas_remote" in _testids(out) # --------------------------------------------------------------------------- @@ -282,24 +333,27 @@ def test_nav_entry_absent_for_stage_only_config() -> None: def test_test_connection_handler_invokes_callback_and_renders_panel() -> None: config = _config_with(_nas_equipment("EQ1"), remote="nas01") - calls: list[int] = [] + calls: list[tuple[str, str]] = [] result = ConnResult( success=True, headline="Connected", detail="reached nas01 in 42ms", - raw="rclone lsd nas01:/srv/nas -> ok", + raw="rclone about nas01: -> ok", ) out = settings.render_settings_page( config=config, state=settings.SettingsState(active_section="nas_remote"), on_save=lambda s: None, nas_remote_available=lambda _name: True, - on_test_connection=lambda: (calls.append(1), result)[1], + on_test_connection=lambda remote, config_path: (calls.append((remote, config_path)), result)[ + 1 + ], ) handler = _click_handler(_find(out, "settings-nas-test-connection")) asyncio.new_event_loop().run_until_complete(handler()) - assert calls == [1] + # Probed with the configured (typed) values. + assert calls == [("nas01", "")] # The panel rendered the success headline from the result inline. assert any( getattr(child, "text", "") == "Connected" @@ -307,6 +361,62 @@ def test_test_connection_handler_invokes_callback_and_renders_panel() -> None: ) +def test_test_connection_probes_typed_unsaved_values() -> None: + # The probe must use the *live draft* values (what the operator typed), + # not the originally-loaded config -- so a selection can be tested before + # saving. Mutating the draft directly stands in for editing the widgets. + config = _config_with(_nas_equipment("EQ1"), remote="nas01") + calls: list[tuple[str, str]] = [] + out = settings.render_settings_page( + config=config, + state=settings.SettingsState(active_section="nas_remote"), + on_save=lambda s: None, + nas_remote_available=lambda _name: True, + on_test_connection=lambda remote, config_path: ( + calls.append((remote, config_path)), + None, + )[1], + ) + draft = _draft_of(_find(out, "settings-save")) + draft.nas.remote = "typed99" + draft.nas.rclone_config_path = "/custom/rclone.conf" + + handler = _click_handler(_find(out, "settings-nas-test-connection")) + asyncio.new_event_loop().run_until_complete(handler()) + + assert calls == [("typed99", "/custom/rclone.conf")] + + +def test_refresh_relists_remotes_from_typed_config_path() -> None: + config = _config_with(_nas_equipment("EQ1"), remote="nas01") + seen_paths: list[str] = [] + + async def _list_remotes(config_path: str) -> tuple[str, ...]: + seen_paths.append(config_path) + return ("nas01:", "fresh:") + + out = settings.render_settings_page( + config=config, + state=settings.SettingsState(active_section="nas_remote"), + on_save=lambda s: None, + nas_remote_available=lambda _name: True, + on_test_connection=lambda _remote, _config_path: None, + nas_remotes=("nas01:",), + list_remotes=_list_remotes, + ) + select = _find(out, "settings-nas-remote-name") + assert "fresh" not in list(select.options) + + # The refresh re-lists using the typed config path and rebuilds options. + draft = _draft_of(_find(out, "settings-save")) + draft.nas.rclone_config_path = "/typed.conf" + handler = _click_handler(_find(out, "settings-nas-remote-refresh")) + asyncio.new_event_loop().run_until_complete(handler()) + + assert seen_paths == ["/typed.conf"] + assert set(select.options) == {"nas01", "fresh"} + + def test_test_connection_handler_awaits_coroutine_result() -> None: config = _config_with(_nas_equipment("EQ1"), remote="nas01") result = ConnResult( @@ -316,7 +426,7 @@ def test_test_connection_handler_awaits_coroutine_result() -> None: raw="exit code 1", ) - async def probe() -> ConnResult: + async def probe(_remote: str, _config_path: str) -> ConnResult: return result out = settings.render_settings_page(