[dashboard] Stabilize device-builder dashboard backend's API surface (#16206)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tests/unit_tests/test_dashboard_import.py

@@ -0,0 +1,203 @@
+"""Unit tests for ``esphome.components.dashboard_import.import_config``.
+
+Locks the YAML shape that ``import_config`` materialises on disk for
+adopted factory firmware. Both the legacy dashboard and the new
+device-builder backend (esphome/device-builder) call this function
+during the adoption flow and depend on the output's ``esphome.name``
+/ ``packages:`` keys to route subsequent compile + flash operations.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+import yaml as pyyaml
+
+from esphome.components.dashboard_import import import_config
+
+
+def _load_plain_yaml(path: Path) -> dict:
+    """Load YAML without invoking ESPHome's ``CORE``-aware loader.
+
+    ``esphome.yaml_util.load_yaml`` resolves ``!include`` /
+    ``!secret`` against ``CORE.config_path`` which isn't set in
+    these tests. We're only asserting on plain key/value structure,
+    so ``pyyaml.load`` with a custom loader subclassing
+    ``pyyaml.SafeLoader`` (and empty fallbacks for the secret/include
+    tags) is enough.
+    """
+
+    class _Loader(pyyaml.SafeLoader):
+        pass
+
+    _Loader.add_constructor("!secret", lambda loader, node: f"!secret {node.value}")
+    _Loader.add_constructor("!include", lambda loader, node: f"!include {node.value}")
+
+    return pyyaml.load(path.read_text(encoding="utf-8"), Loader=_Loader)
+
+
+def test_basic_import_writes_expected_yaml_shape(tmp_path: Path) -> None:
+    """A minimal Wi-Fi import emits the substitutions / packages / esphome triad.
+
+    These three top-level blocks are the contract: substitutions
+    holds the device-specific name, packages pulls in the upstream
+    firmware via the import URL, and esphome.name interpolates from
+    substitutions. Anything that depends on this output (frontend
+    config viewer, follow-up edits, version checks) reads those
+    keys directly.
+    """
+    yaml_path = tmp_path / "kitchen.yaml"
+
+    import_config(
+        path=str(yaml_path),
+        name="kitchen",
+        friendly_name="Kitchen",
+        project_name="acme.kitchen-light",
+        import_url="github://acme/firmware/kitchen.yaml@main",
+    )
+
+    assert yaml_path.exists()
+    config = _load_plain_yaml(yaml_path)
+
+    assert config["substitutions"] == {
+        "name": "kitchen",
+        "friendly_name": "Kitchen",
+    }
+    assert config["packages"] == {
+        "acme.kitchen-light": "github://acme/firmware/kitchen.yaml@main"
+    }
+    assert config["esphome"] == {
+        "name": "${name}",
+        "name_add_mac_suffix": False,
+        "friendly_name": "${friendly_name}",
+    }
+
+
+def test_import_appends_wifi_config_when_network_is_wifi(tmp_path: Path) -> None:
+    """Wi-Fi devices get a ``wifi:`` block templated with secrets references.
+
+    Adopted Wi-Fi devices need a ``wifi:`` section so they can
+    actually connect on the user's LAN — the boilerplate references
+    ``!secret wifi_ssid`` / ``!secret wifi_password`` so the
+    user's existing secrets file plugs in. Devices on other
+    networks (Ethernet) shouldn't get the Wi-Fi block.
+    """
+    yaml_path = tmp_path / "kitchen.yaml"
+    import_config(
+        path=str(yaml_path),
+        name="kitchen",
+        friendly_name=None,
+        project_name="acme.kitchen-light",
+        import_url="github://acme/firmware/kitchen.yaml@main",
+    )
+    contents = yaml_path.read_text()
+    assert "wifi:" in contents
+    assert "!secret wifi_ssid" in contents
+    assert "!secret wifi_password" in contents
+
+
+def test_import_omits_wifi_block_for_ethernet_network(tmp_path: Path) -> None:
+    """Ethernet devices get no ``wifi:`` block — caller wires Ethernet separately.
+
+    The ``network`` parameter exists specifically so non-Wi-Fi
+    devices (PoE / Ethernet, etc.) skip the Wi-Fi templating —
+    otherwise their generated YAML would carry an unused ``wifi:``
+    section the user has to clean up by hand.
+    """
+    yaml_path = tmp_path / "olimex-poe.yaml"
+    import_config(
+        path=str(yaml_path),
+        name="olimex-poe",
+        friendly_name=None,
+        project_name="acme.poe-monitor",
+        import_url="github://acme/firmware/poe.yaml@main",
+        network="ethernet",
+    )
+    contents = yaml_path.read_text()
+    assert "wifi:" not in contents
+
+
+def test_import_with_encryption_writes_api_key(tmp_path: Path) -> None:
+    """``encryption=True`` generates a fresh Noise PSK in the api block.
+
+    Used during the adoption flow when the device-builder UI
+    explicitly opts the new device into encrypted API. Each
+    invocation must produce a fresh 32-byte PSK base64-encoded into
+    the YAML; subsequent compiles and the dashboard's encryption
+    indicator both read it from there.
+    """
+    yaml_path_1 = tmp_path / "a.yaml"
+    yaml_path_2 = tmp_path / "b.yaml"
+
+    import_config(
+        path=str(yaml_path_1),
+        name="a",
+        friendly_name=None,
+        project_name="acme.dev",
+        import_url="github://acme/firmware/dev.yaml@main",
+        encryption=True,
+    )
+    import_config(
+        path=str(yaml_path_2),
+        name="b",
+        friendly_name=None,
+        project_name="acme.dev",
+        import_url="github://acme/firmware/dev.yaml@main",
+        encryption=True,
+    )
+
+    config_1 = _load_plain_yaml(yaml_path_1)
+    config_2 = _load_plain_yaml(yaml_path_2)
+    assert "api" in config_1 and "encryption" in config_1["api"]
+    key_1 = config_1["api"]["encryption"]["key"]
+    key_2 = config_2["api"]["encryption"]["key"]
+    # Fresh per-call PSK, not a hardcoded value.
+    assert key_1 != key_2
+    # Base64-encoded 32 bytes → length 44 with one trailing `=`.
+    assert len(key_1) == 44
+
+
+def test_import_without_friendly_name_omits_friendly_substitution(
+    tmp_path: Path,
+) -> None:
+    """``friendly_name=None`` skips the friendly_name substitution.
+
+    Some imported configs don't carry a friendly name. The output
+    shouldn't pretend they do — the substitutions block must omit
+    ``friendly_name`` so the dashboard renders blank rather than
+    the literal substitution token.
+    """
+    yaml_path = tmp_path / "noname.yaml"
+    import_config(
+        path=str(yaml_path),
+        name="noname",
+        friendly_name=None,
+        project_name="acme.dev",
+        import_url="github://acme/firmware/dev.yaml@main",
+    )
+    config = _load_plain_yaml(yaml_path)
+    assert config["substitutions"] == {"name": "noname"}
+    assert "friendly_name" not in config["esphome"]
+
+
+def test_import_refuses_to_overwrite_existing_yaml(tmp_path: Path) -> None:
+    """An already-present file raises rather than clobbering the user's edits.
+
+    Both the legacy dashboard and device-builder rely on the
+    ``FileExistsError`` to surface a "config already exists" message
+    instead of silently destroying user data.
+    """
+    yaml_path = tmp_path / "existing.yaml"
+    yaml_path.write_text("# user's hand-edited config\n", encoding="utf-8")
+
+    with pytest.raises(FileExistsError):
+        import_config(
+            path=str(yaml_path),
+            name="existing",
+            friendly_name=None,
+            project_name="acme.dev",
+            import_url="github://acme/firmware/dev.yaml@main",
+        )
+    # Original content survives unchanged.
+    assert yaml_path.read_text() == "# user's hand-edited config\n"

tests/unit_tests/test_helpers.py

@@ -90,6 +90,51 @@ def test_cpp_string_escape(string, expected):
     assert actual == expected
 
 
+@pytest.mark.parametrize(
+    "value, expected",
+    (
+        # Basic underscore→dash conversion.
+        ("Living Room Sensor", "living-room-sensor"),
+        # Already-slugified input passes through with dash output.
+        ("kitchen_light", "kitchen-light"),
+        # Accents are stripped (matches the underlying ``slugify``).
+        ("Café Caché", "cafe-cache"),
+        # Mixed casing + multiple separators collapse correctly.
+        ("Foo  Bar__Baz", "foo-bar-baz"),
+        # Empty input yields empty output.
+        ("", ""),
+        # Numbers survive intact.
+        ("Sensor 42", "sensor-42"),
+    ),
+)
+def test_friendly_name_slugify(value, expected):
+    """Friendly-name → URL-safe dash-slug.
+
+    Stable mapping is part of the cross-tool contract
+    (legacy dashboard + device-builder both depend on it for
+    filename → device-name routing). Lock the cases here so a
+    refactor can't accidentally change a slug shape and break
+    on-disk filenames in already-deployed installs.
+    """
+    assert helpers.friendly_name_slugify(value) == expected
+
+
+def test_friendly_name_slugify_back_compat_shim():
+    """``esphome.dashboard.util.text`` keeps re-exporting for back-compat.
+
+    The function moved to ``esphome.helpers`` so the new
+    device-builder dashboard backend can import it without depending
+    on the legacy dashboard package, but downstream code that still
+    imports from the old path keeps working until the dashboard
+    module is removed.
+    """
+    from esphome.dashboard.util.text import (
+        friendly_name_slugify as legacy_friendly_name_slugify,
+    )
+
+    assert legacy_friendly_name_slugify is helpers.friendly_name_slugify
+
+
 @pytest.mark.parametrize(
     "host",
     (

tests/unit_tests/test_zeroconf.py

@@ -0,0 +1,237 @@
+"""Unit tests for ``esphome.zeroconf`` device-discovery primitives.
+
+Covers ``DashboardImportDiscovery`` (state transitions for adoption /
+import flows) and ``DiscoveredImport`` (TXT-record parse shape). Both
+are part of the cross-tool contract between the legacy dashboard and
+the new device-builder backend (esphome/device-builder); changes to
+the callback signature, the ``import_state`` dict shape, or the
+``DiscoveredImport`` field set will break downstream consumers.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+from zeroconf import ServiceStateChange
+
+from esphome.zeroconf import (
+    ESPHOME_SERVICE_TYPE,
+    DashboardImportDiscovery,
+    DiscoveredImport,
+)
+
+
+def _make_service_info(
+    package_import_url: str = "github://esphome/example/example.yaml",
+    project_name: str = "esphome.example",
+    project_version: str = "1.0.0",
+    network: str | None = "wifi",
+    friendly_name: str | None = "Living Room",
+    version: str | None = "2025.1.0",
+) -> MagicMock:
+    """Build a fake ``AsyncServiceInfo`` with the TXT records we care about.
+
+    The real callback path resolves a service via zeroconf and then
+    reads ``info.properties`` (a ``dict[bytes, bytes | None]``). Mock
+    that shape so we can drive ``_process_service_info`` directly
+    without spinning up a real zeroconf instance.
+    """
+    info = MagicMock()
+    properties: dict[bytes, bytes | None] = {
+        b"package_import_url": package_import_url.encode(),
+        b"project_name": project_name.encode(),
+        b"project_version": project_version.encode(),
+    }
+    if network is not None:
+        properties[b"network"] = network.encode()
+    if friendly_name is not None:
+        properties[b"friendly_name"] = friendly_name.encode()
+    if version is not None:
+        properties[b"version"] = version.encode()
+    info.properties = properties
+    info.load_from_cache.return_value = True
+    return info
+
+
+def test_added_service_populates_import_state_and_fires_callback() -> None:
+    """An ADD with the required TXT records lands a ``DiscoveredImport`` and notifies.
+
+    Mirrors what both the legacy dashboard and device-builder rely
+    on — the callback is the only signal that an importable device
+    has appeared on the LAN, and ``import_state`` is the snapshot
+    they read on demand.
+    """
+    on_update = MagicMock()
+    discovery = DashboardImportDiscovery(on_update=on_update)
+
+    info = _make_service_info()
+    name = f"living-room.{ESPHOME_SERVICE_TYPE}"
+    discovery._process_service_info(name, info)
+
+    assert name in discovery.import_state
+    entry = discovery.import_state[name]
+    assert isinstance(entry, DiscoveredImport)
+    assert entry.device_name == "living-room"
+    assert entry.package_import_url == "github://esphome/example/example.yaml"
+    assert entry.project_name == "esphome.example"
+    assert entry.project_version == "1.0.0"
+    assert entry.network == "wifi"
+    assert entry.friendly_name == "Living Room"
+    on_update.assert_called_once_with(name, entry)
+
+
+def test_added_service_without_required_txt_is_ignored() -> None:
+    """A device that doesn't carry ``package_import_url`` etc. isn't importable.
+
+    The dashboard browser also fires for plain ``_esphomelib._tcp``
+    services that happen to match the type but aren't dashboard
+    imports. Those must not land in ``import_state`` or fire the
+    update callback — otherwise the dashboard would surface every
+    API-enabled device on the LAN as "ready to adopt".
+    """
+    on_update = MagicMock()
+    discovery = DashboardImportDiscovery(on_update=on_update)
+
+    info = MagicMock()
+    # Empty TXT records — no import URL, no version. ``version``-only
+    # services hit a separate ``update_device_mdns`` path that talks
+    # to ``StorageJSON``; that's covered elsewhere.
+    info.properties = {}
+    info.load_from_cache.return_value = True
+
+    discovery._process_service_info(f"plain.{ESPHOME_SERVICE_TYPE}", info)
+
+    assert discovery.import_state == {}
+    on_update.assert_not_called()
+
+
+def test_repeated_add_does_not_re_fire_callback() -> None:
+    """Re-resolving the same service doesn't spam the on_update callback.
+
+    The dashboard re-resolves periodically; without the ``is_new``
+    guard, every refresh would fire ``IMPORTABLE_DEVICE_ADDED`` and
+    the dashboard's UI would re-render endlessly.
+    """
+    on_update = MagicMock()
+    discovery = DashboardImportDiscovery(on_update=on_update)
+
+    info = _make_service_info()
+    name = f"living-room.{ESPHOME_SERVICE_TYPE}"
+    discovery._process_service_info(name, info)
+    discovery._process_service_info(name, info)
+
+    on_update.assert_called_once()
+
+
+def test_removed_service_clears_state_and_fires_none_callback() -> None:
+    """A ServiceStateChange.Removed pops the entry and notifies with ``None``.
+
+    Both consumers rely on the ``(name, None)`` callback shape to
+    distinguish "device gone" from "device updated". Coordinate
+    before changing the second-arg semantics.
+    """
+    on_update = MagicMock()
+    discovery = DashboardImportDiscovery(on_update=on_update)
+
+    info = _make_service_info()
+    name = f"living-room.{ESPHOME_SERVICE_TYPE}"
+    discovery._process_service_info(name, info)
+    on_update.reset_mock()
+
+    discovery.browser_callback(
+        zeroconf=MagicMock(),
+        service_type=ESPHOME_SERVICE_TYPE,
+        name=name,
+        state_change=ServiceStateChange.Removed,
+    )
+
+    assert name not in discovery.import_state
+    on_update.assert_called_once_with(name, None)
+
+
+def test_remove_for_unknown_service_does_not_fire_callback() -> None:
+    """A spurious Removed for a service we never tracked is a silent no-op.
+
+    The browser can fire Removed for any matching service type,
+    not just the importable ones we're tracking. Don't let those
+    confuse the callback consumer.
+    """
+    on_update = MagicMock()
+    discovery = DashboardImportDiscovery(on_update=on_update)
+
+    discovery.browser_callback(
+        zeroconf=MagicMock(),
+        service_type=ESPHOME_SERVICE_TYPE,
+        name=f"never-seen.{ESPHOME_SERVICE_TYPE}",
+        state_change=ServiceStateChange.Removed,
+    )
+
+    on_update.assert_not_called()
+
+
+def test_updated_service_for_unknown_name_is_ignored() -> None:
+    """Updates without a prior Add don't seed ``import_state``.
+
+    The dashboard counts on Add to introduce the device and Update
+    to refresh it. Letting Update silently introduce new state would
+    let an unrelated TXT change bypass the Add-time validation.
+    """
+    on_update = MagicMock()
+    discovery = DashboardImportDiscovery(on_update=on_update)
+
+    discovery.browser_callback(
+        zeroconf=MagicMock(),
+        service_type=ESPHOME_SERVICE_TYPE,
+        name=f"living-room.{ESPHOME_SERVICE_TYPE}",
+        state_change=ServiceStateChange.Updated,
+    )
+
+    assert discovery.import_state == {}
+    on_update.assert_not_called()
+
+
+def test_network_defaults_to_wifi_when_txt_absent() -> None:
+    """Older firmware that doesn't broadcast ``network`` defaults to ``wifi``.
+
+    The TXT record was added in a later release; pre-existing
+    factory firmwares advertise without it. ``DiscoveredImport``
+    has to default cleanly so adoption flows can still produce a
+    valid YAML for those devices.
+    """
+    discovery = DashboardImportDiscovery()
+    info = _make_service_info(network=None)
+    name = f"older.{ESPHOME_SERVICE_TYPE}"
+    discovery._process_service_info(name, info)
+
+    assert discovery.import_state[name].network == "wifi"
+
+
+def test_friendly_name_optional() -> None:
+    """``friendly_name`` may be ``None`` if the device doesn't broadcast it.
+
+    Both consumers handle the ``None`` case (rendering the device
+    name as fallback in the UI). Locking this in keeps the
+    optionality explicit so a future refactor doesn't accidentally
+    coerce it into an empty string.
+    """
+    discovery = DashboardImportDiscovery()
+    info = _make_service_info(friendly_name=None)
+    name = f"no-friendly.{ESPHOME_SERVICE_TYPE}"
+    discovery._process_service_info(name, info)
+
+    assert discovery.import_state[name].friendly_name is None
+
+
+def test_callback_is_optional() -> None:
+    """``on_update=None`` lets ``import_state`` track silently.
+
+    Used by callers that read the dict directly rather than
+    subscribing to events.
+    """
+    discovery = DashboardImportDiscovery(on_update=None)
+    info = _make_service_info()
+    name = f"silent.{ESPHOME_SERVICE_TYPE}"
+    discovery._process_service_info(name, info)
+
+    # No callback to assert against; just verify state landed.
+    assert name in discovery.import_state