diff --git a/esphome/__main__.py b/esphome/__main__.py index 01ca8ca5d9..49fc7a3020 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -18,7 +18,6 @@ from typing import TYPE_CHECKING, Protocol from esphome import const, platform_hooks from esphome.build_helpers.native import analysis_backend, native_backend from esphome.const import ( - ALLOWED_NAME_CHARS, ARGUMENT_HELP_DEVICE, BUNDLE_EXTENSION, CONF_API, @@ -36,7 +35,6 @@ from esphome.const import ( CONF_LOGGER, CONF_MDNS, CONF_MQTT, - CONF_NAME, CONF_NAME_ADD_MAC_SUFFIX, CONF_OTA, CONF_PASSWORD, @@ -62,6 +60,7 @@ from esphome.stacktrace import LogLineProcessor from esphome.types import ConfigType from esphome.upload_targets import PortType, get_port_type from esphome.util import ( + ESPHOME_COMMAND, PICOTOOL_PACKAGE, FlashImage, detect_rp2040_bootsel, @@ -85,7 +84,6 @@ if TYPE_CHECKING: _LOGGER = logging.getLogger(__name__) -ESPHOME_COMMAND = [sys.executable, "-m", "esphome"] # Maximum buffer size for serial log reading to prevent unbounded memory growth SERIAL_BUFFER_MAX_SIZE = 65536 @@ -2104,155 +2102,9 @@ def command_analyze_memory(args: ArgsProtocol, config: ConfigType) -> int: def command_rename(args: ArgsProtocol, config: ConfigType) -> int | None: - from esphome import yaml_util + from esphome.cli.rename import command_rename as run - new_name = args.name - for c in new_name: - if c not in ALLOWED_NAME_CHARS: - safe_print( - color( - AnsiFore.BOLD_RED, - f"'{c}' is an invalid character for names. Valid characters are: " - f"{ALLOWED_NAME_CHARS} (lowercase, no spaces)", - ) - ) - return 1 - # Load existing yaml file - raw_contents = CORE.config_path.read_text(encoding="utf-8") - - yaml = yaml_util.load_yaml(CORE.config_path) - if CONF_ESPHOME not in yaml or CONF_NAME not in yaml[CONF_ESPHOME]: - safe_print( - color( - AnsiFore.BOLD_RED, "Complex YAML files cannot be automatically renamed." - ) - ) - return 1 - old_name = yaml[CONF_ESPHOME][CONF_NAME] - match = re.match(r"^\$\{?([a-zA-Z0-9_]+)\}?$", old_name) - if match is None: - # Only swap the ``name:`` line that sits directly under the - # top-level ``esphome:`` block. A naked ``re.sub`` would - # also clobber any other ``name:`` line whose value happens - # to match (e.g. a sensor / output / wifi entry sharing the - # device's hostname), silently rewriting unrelated user - # configuration. The pattern anchors: - # - at the start of the line so ``friendly_name:``, - # ``device_name:`` etc. don't match the trailing ``name:`` - # substring; and - # - at the end of the value (lookahead for whitespace + - # comment + EOL) so ``old_name`` doesn't match as a - # prefix of a longer value (``kitchen`` vs ``kitchen2``). - name_pattern = re.compile( - rf"^(\s*)name:\s+[\"']?{re.escape(old_name)}[\"']?(?=\s*(?:#|$))" - ) - out_lines: list[str] = [] - in_esphome_block = False - for line in raw_contents.splitlines(keepends=True): - if line and not line[0].isspace() and line.strip(): - in_esphome_block = line.lstrip().startswith("esphome:") - out_lines.append(line) - continue - if in_esphome_block: - line = name_pattern.sub(rf'\1name: "{new_name}"', line, count=1) - out_lines.append(line) - new_raw = "".join(out_lines) - else: - old_name = yaml[CONF_SUBSTITUTIONS][match.group(1)] - if ( - len( - re.findall( - rf"^\s+{match.group(1)}:\s+[\"']?{old_name}[\"']?", - raw_contents, - flags=re.MULTILINE, - ) - ) - > 1 - ): - safe_print( - color(AnsiFore.BOLD_RED, "Too many matches in YAML to safely rename") - ) - return 1 - - new_raw = re.sub( - rf"^(\s+{match.group(1)}):\s+[\"']?{old_name}[\"']?", - f'\\1: "{new_name}"', - raw_contents, - flags=re.MULTILINE, - ) - - # ``new_name == old_name`` (after substitution resolution) is - # a no-op rewrite that would still queue a pointless re-flash. - # Catch it before the path-equality check below — covers the - # case where the config filename doesn't match the device name - # (e.g. ``weird-file.yaml`` whose ``esphome.name`` is - # ``kitchen``; running ``esphome rename weird-file.yaml kitchen`` - # would otherwise just re-flash the same hostname). - if new_name == old_name: - safe_print( - color( - AnsiFore.BOLD_RED, - f"'{new_name}' is already the device's name.", - ) - ) - return 1 - - new_path: Path = CORE.config_dir / (new_name + ".yaml") - if new_path.resolve() == CORE.config_path.resolve(): - safe_print( - color( - AnsiFore.BOLD_RED, - f"'{new_name}' is already the device's name.", - ) - ) - return 1 - if new_path.exists(): - safe_print( - color( - AnsiFore.BOLD_RED, - f"Cannot rename: {new_path} already exists. " - "Refusing to overwrite an existing configuration.", - ) - ) - return 1 - safe_print( - f"Updating {color(AnsiFore.CYAN, str(CORE.config_path))} to {color(AnsiFore.CYAN, str(new_path))}" - ) - print() - - new_path.write_text(new_raw, encoding="utf-8") - - rc = run_external_process(*ESPHOME_COMMAND, "config", str(new_path)) - if rc != 0: - safe_print(color(AnsiFore.BOLD_RED, "Rename failed. Reverting changes.")) - new_path.unlink() - return 1 - - cli_args = [ - "run", - str(new_path), - "--no-logs", - "--device", - CORE.address, - ] - - if args.dashboard: - cli_args.insert(0, "--dashboard") - - try: - rc = run_external_process(*ESPHOME_COMMAND, *cli_args) - except KeyboardInterrupt: - rc = 1 - if rc != 0: - new_path.unlink() - return 1 - - if CORE.config_path != new_path: - CORE.config_path.unlink() - - safe_print(color(AnsiFore.BOLD_GREEN, "SUCCESS")) - print() - return 0 + return run(args, config) PRE_CONFIG_ACTIONS = { diff --git a/esphome/cli/__init__.py b/esphome/cli/__init__.py new file mode 100644 index 0000000000..fa6e1fa266 --- /dev/null +++ b/esphome/cli/__init__.py @@ -0,0 +1,2 @@ +"""Commands of the esphome command line, one module each, imported by +__main__ only when they run so that startup stays light.""" diff --git a/esphome/cli/rename.py b/esphome/cli/rename.py new file mode 100644 index 0000000000..cbef68fb70 --- /dev/null +++ b/esphome/cli/rename.py @@ -0,0 +1,164 @@ +"""``esphome rename``.""" + +from __future__ import annotations + +import argparse +from pathlib import Path +import re + +from esphome import yaml_edit, yaml_util +from esphome.const import ( + ALLOWED_NAME_CHARS, + CONF_ESPHOME, + CONF_NAME, + CONF_SUBSTITUTIONS, +) +from esphome.core import CORE, EsphomeError +from esphome.log import AnsiFore, color +from esphome.types import ConfigType +from esphome.util import ESPHOME_COMMAND, run_external_process, safe_print + + +def _revert(new_path: Path, why: str) -> int: + """Say why the rename stopped and take the new file back; an orphan the + next attempt would trip over is reported.""" + safe_print(color(AnsiFore.BOLD_RED, f"Rename failed: {why}")) + try: + new_path.unlink(missing_ok=True) + except OSError as err: + safe_print(color(AnsiFore.BOLD_RED, f"Could not remove {new_path}: {err}")) + return 1 + + +def command_rename(args: argparse.Namespace, config: ConfigType) -> int | None: + """Rename the device: a new file with the name line rewritten, validated + and installed, then the old file removed.""" + new_name = args.name + for c in new_name: + if c not in ALLOWED_NAME_CHARS: + safe_print( + color( + AnsiFore.BOLD_RED, + f"'{c}' is an invalid character for names. Valid characters are: " + f"{ALLOWED_NAME_CHARS} (lowercase, no spaces)", + ) + ) + return 1 + + yaml = yaml_util.load_yaml(CORE.config_path) + + def name_edit() -> tuple[str, yaml_edit.LineEdit]: + """The name and the line to rewrite: the name's own line, or the + substitution's line it comes from, as a plain value in this file.""" + esphome_conf = yaml.get(CONF_ESPHOME) + if not isinstance(esphome_conf, dict) or CONF_NAME not in esphome_conf: + raise EsphomeError(f"no '{CONF_ESPHOME}: {CONF_NAME}:' in the file") + old_name = str(esphome_conf[CONF_NAME]) + mapping, field = esphome_conf, CONF_NAME + if match := re.match(r"^\$\{?([a-zA-Z0-9_]+)\}?$", old_name): + mapping, field = yaml.get(CONF_SUBSTITUTIONS), match.group(1) + if not isinstance(mapping, dict) or field not in mapping: + raise EsphomeError(f"the substitution '{field}' is not in the file") + old_name = str(mapping[field]) + # Only read here; the rewritten text goes to a new file, so the + # source may live anywhere the config path points to + source = yaml_edit.source_of(mapping, field) + if source is None or source[0].resolve() != CORE.config_path.resolve(): + raise EsphomeError(f"'{field}' was not read from {CORE.config_path}") + doc, line_no = source + text = yaml_edit.line_at(doc, line_no) + if (line_match := yaml_edit.field_line_re(field, old_name).match(text)) is None: + raise EsphomeError(f"'{field}' is not a plain value on {doc}:{line_no + 1}") + # The new value is always quoted, whatever the old line had + return old_name, yaml_edit.LineEdit( + doc, line_no, text, yaml_edit.rewrite(line_match, new_name, '"') + ) + + try: + old_name, edit = name_edit() + except EsphomeError as err: + safe_print( + color( + AnsiFore.BOLD_RED, + f"Complex YAML files cannot be automatically renamed: {err}", + ) + ) + return 1 + + # ``new_name == old_name`` (after substitution resolution) is + # a no-op rewrite that would still queue a pointless re-flash. + # Catch it before the path-equality check below — covers the + # case where the config filename doesn't match the device name + # (e.g. ``weird-file.yaml`` whose ``esphome.name`` is + # ``kitchen``; running ``esphome rename weird-file.yaml kitchen`` + # would otherwise just re-flash the same hostname). + if new_name == old_name: + safe_print( + color( + AnsiFore.BOLD_RED, + f"'{new_name}' is already the device's name.", + ) + ) + return 1 + + new_path: Path = CORE.config_dir / (new_name + ".yaml") + if new_path.resolve() == CORE.config_path.resolve(): + safe_print( + color( + AnsiFore.BOLD_RED, + f"'{new_name}' is already the device's name.", + ) + ) + return 1 + if new_path.exists(): + safe_print( + color( + AnsiFore.BOLD_RED, + f"Cannot rename: {new_path} already exists. " + "Refusing to overwrite an existing configuration.", + ) + ) + return 1 + safe_print( + f"Updating {color(AnsiFore.CYAN, str(CORE.config_path))} to {color(AnsiFore.CYAN, str(new_path))}" + ) + print() + + try: + yaml_edit.write_keeping_mode( + new_path, + yaml_edit.rewritten_text(yaml_edit.read_text(CORE.config_path), [edit]), + like=CORE.config_path, + ) + except EsphomeError as err: + return _revert(new_path, str(err)) + + if run_external_process(*ESPHOME_COMMAND, "config", str(new_path)) != 0: + return _revert(new_path, "the new configuration does not validate") + + cli_args = [ + "run", + str(new_path), + "--no-logs", + "--device", + CORE.address, + ] + + if args.dashboard: + cli_args.insert(0, "--dashboard") + + try: + rc = run_external_process(*ESPHOME_COMMAND, *cli_args) + except KeyboardInterrupt: + rc = 1 + if rc != 0: + return _revert( + new_path, + "the install did not finish; the device may already run the new name", + ) + + CORE.config_path.unlink() + + safe_print(color(AnsiFore.BOLD_GREEN, "SUCCESS")) + print() + return 0 diff --git a/esphome/util.py b/esphome/util.py index 8aa321d908..dd1998a7c2 100644 --- a/esphome/util.py +++ b/esphome/util.py @@ -352,6 +352,10 @@ def run_external_command( return retval +# How a command starts another esphome, as a child of this one +ESPHOME_COMMAND = [sys.executable, "-m", "esphome"] + + def run_external_process(*cmd: str, **kwargs: Any) -> int | str: # Deferred: an OTA upload/logs run never spawns an external process. import subprocess diff --git a/esphome/yaml_edit.py b/esphome/yaml_edit.py new file mode 100644 index 0000000000..da66fa088f --- /dev/null +++ b/esphome/yaml_edit.py @@ -0,0 +1,114 @@ +"""Rewrite single lines of a yaml file in place, located by the source +ranges the loader keeps, so quotes, comments, indentation and line endings +around them survive and nothing else in the file is touched.""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +import re +import stat + +from esphome.core import EsphomeError +from esphome.helpers import write_file +from esphome.types import ConfigType + +# A plain scalar with no yaml indicator, so an empty value, a block scalar +# (`>-`, `|`) or a flow collection never counts; then optional matching +# quotes and the trailer, where a comment needs whitespace before its `#` +PLAIN_SCALAR = r"[^\s#\"'>|&*!%@`\[\]{},]+" +TRAILER = r"(?P[\"']?){value}(?P=quote)(?P(?:\s+#.*)?\s*)$" + + +def read_text(path: Path) -> str: + """The file as written, line endings included; read_file would fold them.""" + try: + return path.read_bytes().decode("utf-8") + except (OSError, UnicodeDecodeError) as err: + raise EsphomeError(f"Error reading file {path}: {err}") from err + + +@dataclass +class LineEdit: + """One line to rewrite; ``old_line`` is what it held when located.""" + + path: Path + line: int + old_line: str + new_line: str + + +def field_line_re( + name: str, value: str | None = None, indent: str = r"\s*" +) -> re.Pattern[str]: + """Match ``name: value``, or ``name:`` with any plain scalar, at + ``indent``; the name may be quoted. Keeps the prefix, the quotes and the + trailer for rewrite.""" + scalar = PLAIN_SCALAR if value is None else re.escape(value) + prefix = rf"{indent}[\"']?{re.escape(name)}[\"']?\s*:\s*" + return re.compile(rf"^(?P{prefix}){TRAILER.format(value=scalar)}") + + +def rewrite(match: re.Match[str], value: str, quote: str | None = None) -> str: + """The matched line with ``value`` in place of the scalar; the source + quotes stay unless ``quote`` is given.""" + quote = match["quote"] if quote is None else quote + return f"{match['prefix']}{quote}{value}{quote}{match['trail']}" + + +def line_at(doc: Path, line_no: int) -> str: + lines = read_text(doc).splitlines() + if line_no >= len(lines): + raise EsphomeError(f"{doc}:{line_no + 1} changed since it was read") + return lines[line_no] + + +def source_of(mapping: ConfigType, name: str) -> tuple[Path, int] | None: + """The file and line ``name:`` was read from, None when validation added + it or a merge key brought it in from an anchor elsewhere. Mapping keys + keep their range through validation, values may not.""" + rng = getattr(next((k for k in mapping if k == name), None), "esp_range", None) + if rng is None: + return None + # An included mapping carries the `!include` line of its parent, so only + # a key in the same document can be placed against the mapping + if (own := getattr(mapping, "esp_range", None)) is None: + return None # a mapping built in code, its keys are not on its lines + if ( + rng.start_mark.document == own.start_mark.document + and not own.start_mark.line <= rng.start_mark.line <= own.end_mark.line + ): + return None + return Path(rng.start_mark.document), rng.start_mark.line + + +def write_keeping_mode(path: Path, text: str, like: Path | None = None) -> None: + """Write with the mode of ``like`` (default: the file itself) rather + than write_file's 0644; a 0600 secrets file stays 0600.""" + try: + mode = stat.S_IMODE((like or path).stat().st_mode) + except OSError as err: + raise EsphomeError(f"Could not read the mode of {like or path}: {err}") from err + try: + write_file(path, text, private=True) + except EsphomeError as err: + # write_file keeps the reason in the cause only + raise EsphomeError(f"{err}: {err.__cause__}") from err + try: + path.chmod(mode) + except OSError as err: + raise EsphomeError( + f"{path} was written but could not get its mode back: {err}" + ) from err + + +def rewritten_text(original: str, edits: list[LineEdit]) -> str: + """``original`` with the edits applied; every edit must still find the + line it was located on.""" + lines = original.splitlines(keepends=True) + for edit in edits: + text = lines[edit.line].rstrip("\r\n") if edit.line < len(lines) else None + if text != edit.old_line: + raise EsphomeError(f"{edit.path}:{edit.line + 1} changed since it was read") + lines[edit.line] = edit.new_line + lines[edit.line][len(text) :] + return "".join(lines) diff --git a/tests/unit_tests/cli/__init__.py b/tests/unit_tests/cli/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit_tests/cli/test_rename.py b/tests/unit_tests/cli/test_rename.py new file mode 100644 index 0000000000..944d1e37ca --- /dev/null +++ b/tests/unit_tests/cli/test_rename.py @@ -0,0 +1,839 @@ +"""Tests for ``esphome rename``.""" + +from __future__ import annotations + +from collections.abc import Generator +from dataclasses import dataclass +from pathlib import Path +import sys +from typing import Any +from unittest.mock import Mock, patch + +import pytest +from pytest import CaptureFixture + +from esphome.cli.rename import command_rename +from esphome.const import CONF_ESPHOME, CONF_NAME, CONF_SUBSTITUTIONS +from esphome.core import CORE + + +@dataclass +class MockArgs: + name: str | None = None + dashboard: bool = False + + +def setup_core(tmp_path: Path, config: dict[str, Any] | None = None) -> None: + """Point CORE at a config in ``tmp_path``; the tests override the path.""" + CORE.config = config or {} + CORE.config_path = tmp_path / "test.yaml" + CORE.name = "test" + + +@pytest.fixture +def mock_run_external_process() -> Generator[Mock]: + """The child esphome the command starts to validate and install.""" + with patch("esphome.cli.rename.run_external_process") as mock: + mock.return_value = 0 + yield mock + + +def test_command_rename_invalid_characters( + tmp_path: Path, capfd: CaptureFixture[str] +) -> None: + """Test command_rename with invalid characters in name.""" + setup_core(tmp_path=tmp_path) + + # Test with invalid character (space) + args = MockArgs(name="invalid name") + result = command_rename(args, {}) + + assert result == 1 + captured = capfd.readouterr() + assert "invalid character" in captured.out.lower() + + +def test_command_rename_complex_yaml( + tmp_path: Path, capfd: CaptureFixture[str] +) -> None: + """Test command_rename with complex YAML that cannot be renamed.""" + config_file = tmp_path / "test.yaml" + config_file.write_text("# Complex YAML without esphome section\nsome_key: value\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + + args = MockArgs(name="newname") + result = command_rename(args, {}) + + assert result == 1 + captured = capfd.readouterr() + assert "complex yaml" in captured.out.lower() + + +def test_command_rename_success( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test successful rename of a simple configuration.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + +esp32: + board: nodemcu-32s + +wifi: + ssid: "test" + password: "test1234" +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + + # Set up CORE.config to avoid ValueError when accessing CORE.address + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + + # Simulate successful validation and upload + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + + # Verify new file was created + new_file = tmp_path / "newname.yaml" + assert new_file.exists() + + # Verify old file was removed + assert not config_file.exists() + + # Verify content was updated + content = new_file.read_text() + assert ( + 'name: "newname"' in content + or "name: 'newname'" in content + or "name: newname" in content + ) + + captured = capfd.readouterr() + assert "SUCCESS" in captured.out + + +def test_command_rename_with_substitutions( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename with substitutions in YAML.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +substitutions: + device_name: oldname + +esphome: + name: ${device_name} + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + + # Set up CORE.config to avoid ValueError when accessing CORE.address + CORE.config = { + CONF_ESPHOME: {CONF_NAME: "oldname"}, + CONF_SUBSTITUTIONS: {"device_name": "oldname"}, + } + + args = MockArgs(name="newname", dashboard=False) + + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + + # Verify substitution was updated + new_file = tmp_path / "newname.yaml" + content = new_file.read_text() + assert 'device_name: "newname"' in content + + +def test_command_rename_validation_failure( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename when validation fails.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + + args = MockArgs(name="newname", dashboard=False) + + # First call for validation fails + mock_run_external_process.return_value = 1 + + result = command_rename(args, {}) + + assert result == 1 + + # Verify new file was created but then removed due to failure + new_file = tmp_path / "newname.yaml" + assert not new_file.exists() + + # Verify old file still exists (not removed on failure) + assert config_file.exists() + + captured = capfd.readouterr() + assert "Rename failed" in captured.out + + +def test_command_rename_install_failure_reverts( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename when the install (esphome run) step fails.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + + # First call (config validation) succeeds; second (esphome run) fails. + mock_run_external_process.side_effect = [0, 1] + + result = command_rename(args, {}) + + assert result == 1 + + # New file was unlinked when install failed. + new_file = tmp_path / "newname.yaml" + assert not new_file.exists() + + # Old file is preserved so the device stays reachable under the + # original hostname. + assert config_file.exists() + + +def test_command_rename_target_exists_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when the target filename already exists. + + Without this guard, the rename would overwrite the unrelated + device's YAML and OTA-install our firmware to the wrong device. + """ + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + +esp32: + board: nodemcu-32s +""") + target_file = tmp_path / "newname.yaml" + target_file.write_text(""" +esphome: + name: someoneelse + +esp32: + board: nodemcu-32s +""") + target_original = target_file.read_text() + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + # No subprocess work happened — refusal is up-front. + mock_run_external_process.assert_not_called() + # Target file untouched: same content, still on disk. + assert target_file.exists() + assert target_file.read_text() == target_original + # Source file untouched. + assert config_file.exists() + + captured = capfd.readouterr() + assert "already exists" in captured.out + + +def test_command_rename_same_name_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when the new name matches the current name. + + A same-name rename would otherwise re-write the YAML and queue + a redundant compile + install — wasted work the user almost + certainly didn't intend. + """ + config_file = tmp_path / "samename.yaml" + config_file.write_text(""" +esphome: + name: samename + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "samename"}} + + args = MockArgs(name="samename", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # File preserved verbatim — no rewrite happened. + assert config_file.exists() + + captured = capfd.readouterr() + assert "already" in captured.out.lower() + + +def test_command_rename_does_not_touch_friendly_name_substring( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + r"""Test rename does not match the ``name:`` substring of ``friendly_name:``. + + Without anchoring the regex at line start, the pattern + ``\s*name:\s+`` could match the trailing ``name:`` + substring inside ``friendly_name: ``. The rewrite would + flip both lines to the new name, leaving the user with a + silently corrupted ``friendly_name``. + """ + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + friendly_name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "newname.yaml" + content = new_file.read_text() + # esphome.name swapped. + assert 'name: "newname"' in content + # friendly_name kept verbatim. + assert "friendly_name: oldname" in content + + +def test_command_rename_does_not_match_old_name_as_value_prefix( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + r"""Test rename does not match ``old_name`` as a prefix of a longer value. + + With ``old_name = kitchen`` the value ``kitchen2`` (a sensor + or wifi entry) would otherwise match the unanchored + ``["']?kitchen["']?`` pattern at the prefix and get + rewritten to the new name. The end-of-value lookahead keeps + the match restricted to whole tokens. + """ + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: kitchen + +esp32: + board: nodemcu-32s + +wifi: + ap: + ssid: kitchen2 +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "garage.yaml" + content = new_file.read_text() + assert 'name: "garage"' in content + # The wifi ssid value is unrelated and stays intact. + assert "ssid: kitchen2" in content + + +def test_command_rename_same_resolved_name_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when ``new_name`` matches the resolved device name. + + The path-equality check only catches the case where the + config filename matches the device name. For a config whose + filename and ``esphome.name`` differ (here ``weird-file.yaml`` + holds ``esphome.name: kitchen``), running + ``esphome rename weird-file.yaml kitchen`` would otherwise + fall through to the rewrite + install: the YAML's name stays + ``kitchen``, the file is renamed to ``kitchen.yaml``, and the + device gets a redundant flash. Refuse up-front so the + "already the device's name" message matches reality. + """ + config_file = tmp_path / "weird-file.yaml" + config_file.write_text(""" +esphome: + name: kitchen + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="kitchen", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # Source file untouched, no derived target written. + assert config_file.exists() + assert not (tmp_path / "kitchen.yaml").exists() + + captured = capfd.readouterr() + assert "already" in captured.out.lower() + + +def test_command_rename_target_path_equals_source_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when the new path resolves to the source file. + + Reachable only when the YAML's filename and ``esphome.name`` + disagree — here ``kitchen.yaml`` holds ``esphome.name: garage`` + and the user runs ``esphome rename kitchen.yaml kitchen``. The + name-equality check above passes (``garage != kitchen``), but + ``/kitchen.yaml`` resolves to the source file + itself, so the rewrite would clobber the source mid-rename. + Refuse rather than silently overwriting. + """ + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: garage + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "garage"}} + + args = MockArgs(name="kitchen", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # Source file still present and unmodified. + assert config_file.exists() + assert "name: garage" in config_file.read_text() + + captured = capfd.readouterr() + assert "already" in captured.out.lower() + + +def test_command_rename_does_not_touch_lookalike_name_in_other_blocks( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename only swaps the esphome.name line. + + A device whose name happens to match a sensor's / output's + ``name:`` value must not have those other names rewritten — + they're independent. Without an anchor for the esphome block + a naive regex would clobber every line whose value matches. + """ + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: kitchen + +esp32: + board: nodemcu-32s + +sensor: + - platform: template + name: kitchen + lambda: 'return 0;' +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + + new_file = tmp_path / "garage.yaml" + content = new_file.read_text() + # esphome.name renamed. + assert 'name: "garage"' in content + # Sensor's name is the user's entity name — must not be touched. + assert " name: kitchen\n" in content + + +def test_command_rename_preserves_trailing_comment( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename preserves a trailing ``# comment`` on the name line.""" + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: kitchen # primary device + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + + new_file = tmp_path / "garage.yaml" + content = new_file.read_text() + assert "# primary device" in content + + +def test_command_rename_handles_double_quoted_value( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename matches when the existing value is double-quoted.""" + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: "kitchen" + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "garage.yaml" + assert 'name: "garage"' in new_file.read_text() + + +def test_command_rename_handles_single_quoted_value( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename matches when the existing value is single-quoted.""" + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: 'kitchen' + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "garage.yaml" + assert 'name: "garage"' in new_file.read_text() + + +def test_command_rename_leaves_a_lookalike_substitution_line_alone( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Only the substitution's own line changes; another block's field of + the same name and value is not it.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +substitutions: + device_name: oldname + +esphome: + name: ${device_name} + +example: + device_name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = { + CONF_ESPHOME: {CONF_NAME: "oldname"}, + CONF_SUBSTITUTIONS: {"device_name": "oldname"}, + } + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 0 + content = (tmp_path / "newname.yaml").read_text() + assert 'device_name: "newname"' in content + assert "example:\n device_name: oldname\n" in content + + +def test_command_rename_keeps_line_endings_and_mode( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """A CRLF file stays CRLF and the new file gets the old one's mode.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_bytes( + b"esphome:\r\n name: oldname # device\r\n\r\nesp32:\r\n board: nodemcu-32s\r\n" + ) + if sys.platform != "win32": + config_file.chmod(0o600) + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 0 + new_file = tmp_path / "newname.yaml" + assert new_file.read_bytes() == ( + b'esphome:\r\n name: "newname" # device\r\n\r\nesp32:\r\n board: nodemcu-32s\r\n' + ) + if sys.platform != "win32": + assert new_file.stat().st_mode & 0o777 == 0o600 + + +@pytest.mark.parametrize( + ("yaml_text", "extra"), + [ + ("esphome:\n name: ${missing}\n", {}), + ("esphome: {name: oldname}\n", {}), + ("esphome: !include base.yaml\n", {"base.yaml": "name: oldname\n"}), + ( + ( + "named: &named\n name: oldname\n\nesphome:\n <<: *named\n\n" + "sensor:\n - platform: template\n <<: *named\n" + ), + {}, + ), + ], + ids=["missing_substitution", "flow_mapping", "included_name", "merged_name"], +) +def test_command_rename_refuses_shapes_without_a_plain_name_line( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, + yaml_text: str, + extra: dict[str, str], +) -> None: + """The name line must be a plain value in the file being renamed.""" + for name, text in extra.items(): + (tmp_path / name).write_text(text) + config_file = tmp_path / "oldname.yaml" + config_file.write_text(yaml_text) + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + mock_run_external_process.assert_not_called() + assert "complex yaml" in capfd.readouterr().out.lower() + + +def test_command_rename_removes_the_new_file_when_its_mode_cannot_be_set( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """No orphan is left for the next attempt to trip over.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + with patch("pathlib.Path.chmod", side_effect=OSError("read-only share")): + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + assert not (tmp_path / "newname.yaml").exists() + mock_run_external_process.assert_not_called() + assert "Rename failed" in capfd.readouterr().out + + +def test_command_rename_refuses_a_name_without_a_source_line( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """A key the loader did not read from a file cannot be located.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + with patch("esphome.yaml_edit.source_of", return_value=None): + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + mock_run_external_process.assert_not_called() + assert "was not read from" in capfd.readouterr().out + + +def test_command_rename_passes_dashboard_to_the_install( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + assert command_rename(MockArgs(name="newname", dashboard=True), {}) == 0 + install = mock_run_external_process.call_args_list[-1].args + assert install[-6:-4] == ("--dashboard", "run") + + +def test_command_rename_interrupted_install_reverts( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + mock_run_external_process.side_effect = [0, KeyboardInterrupt] + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + assert not (tmp_path / "newname.yaml").exists() + assert config_file.exists() + + +def test_command_rename_reads_a_config_linked_from_outside( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """The source is only read; the new file lands in the config directory.""" + outside = tmp_path / "elsewhere.yaml" + outside.write_text("esphome:\n name: oldname\n") + config_dir = tmp_path / "config" + config_dir.mkdir() + config_file = config_dir / "oldname.yaml" + config_file.symlink_to(outside) + setup_core(tmp_path=config_dir) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 0 + assert (config_dir / "newname.yaml").read_text() == 'esphome:\n name: "newname"\n' + assert not config_file.exists() + assert outside.exists() + + +def test_command_rename_reports_an_orphan_it_could_not_remove( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """The write failure is the message; a cleanup failure is added to it.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + with ( + patch("pathlib.Path.chmod", side_effect=OSError("read-only share")), + patch("pathlib.Path.unlink", side_effect=OSError("busy")), + ): + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + out = capfd.readouterr().out + assert "Rename failed" in out + assert "Could not remove" in out and "newname.yaml" in out + + +def test_command_rename_install_failure_says_so( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """The device may already carry the new name; the user is told.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + mock_run_external_process.side_effect = [0, 1] + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + out = capfd.readouterr().out + assert "Rename failed: the install did not finish" in out + assert "may already run the new name" in out + + +def test_command_rename_validation_revert_reports_an_orphan( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + config_file = tmp_path / "oldname.yaml" + config_file.write_text("esphome:\n name: oldname\n") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + mock_run_external_process.return_value = 1 + with patch("pathlib.Path.unlink", side_effect=OSError("busy")): + assert command_rename(MockArgs(name="newname", dashboard=False), {}) == 1 + out = capfd.readouterr().out + assert "does not validate" in out and "Could not remove" in out diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index bf5d1566c0..837643417d 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -42,7 +42,6 @@ from esphome.__main__ import ( command_config_hash, command_dashboard, command_idedata, - command_rename, command_run, command_update_all, command_wizard, @@ -101,7 +100,6 @@ from esphome.const import ( CONF_PASSWORD, CONF_PLATFORM, CONF_PORT, - CONF_SUBSTITUTIONS, CONF_TOPIC, CONF_USE_ADDRESS, CONF_USERNAME, @@ -4448,627 +4446,6 @@ def test_command_config_hash( assert output == f"0x{CORE.config_hash:08x}" -def test_command_rename_invalid_characters( - tmp_path: Path, capfd: CaptureFixture[str] -) -> None: - """Test command_rename with invalid characters in name.""" - setup_core(tmp_path=tmp_path) - - # Test with invalid character (space) - args = MockArgs(name="invalid name") - result = command_rename(args, {}) - - assert result == 1 - captured = capfd.readouterr() - assert "invalid character" in captured.out.lower() - - -def test_command_rename_complex_yaml( - tmp_path: Path, capfd: CaptureFixture[str] -) -> None: - """Test command_rename with complex YAML that cannot be renamed.""" - config_file = tmp_path / "test.yaml" - config_file.write_text("# Complex YAML without esphome section\nsome_key: value\n") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - - args = MockArgs(name="newname") - result = command_rename(args, {}) - - assert result == 1 - captured = capfd.readouterr() - assert "complex yaml" in captured.out.lower() - - -def test_command_rename_success( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test successful rename of a simple configuration.""" - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -esphome: - name: oldname - -esp32: - board: nodemcu-32s - -wifi: - ssid: "test" - password: "test1234" -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - - # Set up CORE.config to avoid ValueError when accessing CORE.address - CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} - - args = MockArgs(name="newname", dashboard=False) - - # Simulate successful validation and upload - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - - # Verify new file was created - new_file = tmp_path / "newname.yaml" - assert new_file.exists() - - # Verify old file was removed - assert not config_file.exists() - - # Verify content was updated - content = new_file.read_text() - assert ( - 'name: "newname"' in content - or "name: 'newname'" in content - or "name: newname" in content - ) - - captured = capfd.readouterr() - assert "SUCCESS" in captured.out - - -def test_command_rename_with_substitutions( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - """Test rename with substitutions in YAML.""" - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -substitutions: - device_name: oldname - -esphome: - name: ${device_name} - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - - # Set up CORE.config to avoid ValueError when accessing CORE.address - CORE.config = { - CONF_ESPHOME: {CONF_NAME: "oldname"}, - CONF_SUBSTITUTIONS: {"device_name": "oldname"}, - } - - args = MockArgs(name="newname", dashboard=False) - - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - - # Verify substitution was updated - new_file = tmp_path / "newname.yaml" - content = new_file.read_text() - assert 'device_name: "newname"' in content - - -def test_command_rename_validation_failure( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename when validation fails.""" - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -esphome: - name: oldname - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - - args = MockArgs(name="newname", dashboard=False) - - # First call for validation fails - mock_run_external_process.return_value = 1 - - result = command_rename(args, {}) - - assert result == 1 - - # Verify new file was created but then removed due to failure - new_file = tmp_path / "newname.yaml" - assert not new_file.exists() - - # Verify old file still exists (not removed on failure) - assert config_file.exists() - - captured = capfd.readouterr() - assert "Rename failed" in captured.out - - -def test_command_rename_install_failure_reverts( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename when the install (esphome run) step fails.""" - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -esphome: - name: oldname - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} - - args = MockArgs(name="newname", dashboard=False) - - # First call (config validation) succeeds; second (esphome run) fails. - mock_run_external_process.side_effect = [0, 1] - - result = command_rename(args, {}) - - assert result == 1 - - # New file was unlinked when install failed. - new_file = tmp_path / "newname.yaml" - assert not new_file.exists() - - # Old file is preserved so the device stays reachable under the - # original hostname. - assert config_file.exists() - - -def test_command_rename_target_exists_refuses( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename refuses when the target filename already exists. - - Without this guard, the rename would overwrite the unrelated - device's YAML and OTA-install our firmware to the wrong device. - """ - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -esphome: - name: oldname - -esp32: - board: nodemcu-32s -""") - target_file = tmp_path / "newname.yaml" - target_file.write_text(""" -esphome: - name: someoneelse - -esp32: - board: nodemcu-32s -""") - target_original = target_file.read_text() - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} - - args = MockArgs(name="newname", dashboard=False) - - result = command_rename(args, {}) - - assert result == 1 - # No subprocess work happened — refusal is up-front. - mock_run_external_process.assert_not_called() - # Target file untouched: same content, still on disk. - assert target_file.exists() - assert target_file.read_text() == target_original - # Source file untouched. - assert config_file.exists() - - captured = capfd.readouterr() - assert "already exists" in captured.out - - -def test_command_rename_same_name_refuses( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename refuses when the new name matches the current name. - - A same-name rename would otherwise re-write the YAML and queue - a redundant compile + install — wasted work the user almost - certainly didn't intend. - """ - config_file = tmp_path / "samename.yaml" - config_file.write_text(""" -esphome: - name: samename - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "samename"}} - - args = MockArgs(name="samename", dashboard=False) - - result = command_rename(args, {}) - - assert result == 1 - mock_run_external_process.assert_not_called() - # File preserved verbatim — no rewrite happened. - assert config_file.exists() - - captured = capfd.readouterr() - assert "already" in captured.out.lower() - - -def test_command_rename_does_not_touch_friendly_name_substring( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - r"""Test rename does not match the ``name:`` substring of ``friendly_name:``. - - Without anchoring the regex at line start, the pattern - ``\s*name:\s+`` could match the trailing ``name:`` - substring inside ``friendly_name: ``. The rewrite would - flip both lines to the new name, leaving the user with a - silently corrupted ``friendly_name``. - """ - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -esphome: - name: oldname - friendly_name: oldname - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} - - args = MockArgs(name="newname", dashboard=False) - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - new_file = tmp_path / "newname.yaml" - content = new_file.read_text() - # esphome.name swapped. - assert 'name: "newname"' in content - # friendly_name kept verbatim. - assert "friendly_name: oldname" in content - - -def test_command_rename_does_not_match_old_name_as_value_prefix( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - r"""Test rename does not match ``old_name`` as a prefix of a longer value. - - With ``old_name = kitchen`` the value ``kitchen2`` (a sensor - or wifi entry) would otherwise match the unanchored - ``["']?kitchen["']?`` pattern at the prefix and get - rewritten to the new name. The end-of-value lookahead keeps - the match restricted to whole tokens. - """ - config_file = tmp_path / "kitchen.yaml" - config_file.write_text(""" -esphome: - name: kitchen - -esp32: - board: nodemcu-32s - -wifi: - ap: - ssid: kitchen2 -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} - - args = MockArgs(name="garage", dashboard=False) - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - new_file = tmp_path / "garage.yaml" - content = new_file.read_text() - assert 'name: "garage"' in content - # The wifi ssid value is unrelated and stays intact. - assert "ssid: kitchen2" in content - - -def test_command_rename_same_resolved_name_refuses( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename refuses when ``new_name`` matches the resolved device name. - - The path-equality check only catches the case where the - config filename matches the device name. For a config whose - filename and ``esphome.name`` differ (here ``weird-file.yaml`` - holds ``esphome.name: kitchen``), running - ``esphome rename weird-file.yaml kitchen`` would otherwise - fall through to the rewrite + install: the YAML's name stays - ``kitchen``, the file is renamed to ``kitchen.yaml``, and the - device gets a redundant flash. Refuse up-front so the - "already the device's name" message matches reality. - """ - config_file = tmp_path / "weird-file.yaml" - config_file.write_text(""" -esphome: - name: kitchen - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} - - args = MockArgs(name="kitchen", dashboard=False) - - result = command_rename(args, {}) - - assert result == 1 - mock_run_external_process.assert_not_called() - # Source file untouched, no derived target written. - assert config_file.exists() - assert not (tmp_path / "kitchen.yaml").exists() - - captured = capfd.readouterr() - assert "already" in captured.out.lower() - - -def test_command_rename_target_path_equals_source_refuses( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename refuses when the new path resolves to the source file. - - Reachable only when the YAML's filename and ``esphome.name`` - disagree — here ``kitchen.yaml`` holds ``esphome.name: garage`` - and the user runs ``esphome rename kitchen.yaml kitchen``. The - name-equality check above passes (``garage != kitchen``), but - ``/kitchen.yaml`` resolves to the source file - itself, so the rewrite would clobber the source mid-rename. - Refuse rather than silently overwriting. - """ - config_file = tmp_path / "kitchen.yaml" - config_file.write_text(""" -esphome: - name: garage - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "garage"}} - - args = MockArgs(name="kitchen", dashboard=False) - - result = command_rename(args, {}) - - assert result == 1 - mock_run_external_process.assert_not_called() - # Source file still present and unmodified. - assert config_file.exists() - assert "name: garage" in config_file.read_text() - - captured = capfd.readouterr() - assert "already" in captured.out.lower() - - -def test_command_rename_does_not_touch_lookalike_name_in_other_blocks( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - """Test rename only swaps the esphome.name line. - - A device whose name happens to match a sensor's / output's - ``name:`` value must not have those other names rewritten — - they're independent. Without an anchor for the esphome block - a naive regex would clobber every line whose value matches. - """ - config_file = tmp_path / "kitchen.yaml" - config_file.write_text(""" -esphome: - name: kitchen - -esp32: - board: nodemcu-32s - -sensor: - - platform: template - name: kitchen - lambda: 'return 0;' -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} - - args = MockArgs(name="garage", dashboard=False) - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - - new_file = tmp_path / "garage.yaml" - content = new_file.read_text() - # esphome.name renamed. - assert 'name: "garage"' in content - # Sensor's name is the user's entity name — must not be touched. - assert " name: kitchen\n" in content - - -def test_command_rename_preserves_trailing_comment( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - """Test rename preserves a trailing ``# comment`` on the name line.""" - config_file = tmp_path / "kitchen.yaml" - config_file.write_text(""" -esphome: - name: kitchen # primary device - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} - - args = MockArgs(name="garage", dashboard=False) - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - - new_file = tmp_path / "garage.yaml" - content = new_file.read_text() - assert "# primary device" in content - - -def test_command_rename_handles_double_quoted_value( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - """Test rename matches when the existing value is double-quoted.""" - config_file = tmp_path / "kitchen.yaml" - config_file.write_text(""" -esphome: - name: "kitchen" - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} - - args = MockArgs(name="garage", dashboard=False) - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - new_file = tmp_path / "garage.yaml" - assert 'name: "garage"' in new_file.read_text() - - -def test_command_rename_handles_single_quoted_value( - tmp_path: Path, - mock_run_external_process: Mock, -) -> None: - """Test rename matches when the existing value is single-quoted.""" - config_file = tmp_path / "kitchen.yaml" - config_file.write_text(""" -esphome: - name: 'kitchen' - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} - - args = MockArgs(name="garage", dashboard=False) - mock_run_external_process.return_value = 0 - - result = command_rename(args, {}) - - assert result == 0 - new_file = tmp_path / "garage.yaml" - assert 'name: "garage"' in new_file.read_text() - - -def test_command_rename_too_many_substitution_matches_refuses( - tmp_path: Path, - capfd: CaptureFixture[str], - mock_run_external_process: Mock, -) -> None: - """Test rename refuses when ``${var}`` resolves to multiple matches. - - When ``esphome.name: ${device_name}`` and the substitution - definition ``device_name: foo`` appears more than once in the - YAML (e.g. inside multiple included blocks), the regex rewrite - can't tell which one to flip. Rather than silently picking one - or rewriting both, the command refuses. - """ - config_file = tmp_path / "oldname.yaml" - config_file.write_text(""" -substitutions: - device_name: oldname - -esphome: - name: ${device_name} - -# A copy-pasted block that re-declares the substitution at the -# same indent level - happens when users splice in a packaged -# fragment without renaming the variable. -example: - device_name: oldname - -esp32: - board: nodemcu-32s -""") - setup_core(tmp_path=tmp_path) - CORE.config_path = config_file - CORE.config = { - CONF_ESPHOME: {CONF_NAME: "oldname"}, - CONF_SUBSTITUTIONS: {"device_name": "oldname"}, - } - - args = MockArgs(name="newname", dashboard=False) - - result = command_rename(args, {}) - - assert result == 1 - mock_run_external_process.assert_not_called() - # File untouched. - assert config_file.exists() - assert "device_name: oldname" in config_file.read_text() - - captured = capfd.readouterr() - assert "Too many matches" in captured.out - - def test_command_update_all_path_string_conversion( tmp_path: Path, mock_run_external_process: Mock, @@ -8028,3 +7405,11 @@ def test_command_analyze_memory_host_refuses_before_compiling( command_analyze_memory(MockArgs(), {}) mock_write_cpp.assert_not_called() mock_compile_program.assert_not_called() + + +def test_command_rename_is_dispatched_to_the_cli_module() -> None: + """__main__ keeps a thin wrapper and imports the command when it runs.""" + args = MockArgs(name="newname") + with patch("esphome.cli.rename.command_rename", return_value=7) as run: + assert main.command_rename(args, {}) == 7 + run.assert_called_once_with(args, {}) diff --git a/tests/unit_tests/test_yaml_edit.py b/tests/unit_tests/test_yaml_edit.py new file mode 100644 index 0000000000..31df23dc46 --- /dev/null +++ b/tests/unit_tests/test_yaml_edit.py @@ -0,0 +1,182 @@ +"""Tests for rewriting single lines of a yaml file.""" + +from __future__ import annotations + +from pathlib import Path +import sys + +import pytest + +from esphome import yaml_util +from esphome.config import do_substitution_pass +from esphome.const import CONF_ESPHOME, CONF_NAME +from esphome.core import CORE, EsphomeError +from esphome.yaml_edit import ( + LineEdit, + field_line_re, + line_at, + read_text, + rewrite, + rewritten_text, + source_of, + write_keeping_mode, +) + +YAML = """esphome: + name: kitchen # the device + +wifi: + ssid: kitchen +""" + + +def _setup(tmp_path: Path, yaml_text: str) -> Path: + """Write the yaml, point CORE at it and load it the way read_config does, + so every node carries its source range.""" + CORE.reset() + CORE.config_path = tmp_path / "test.yaml" + # Bytes, so Windows does not turn the newlines into CRLF on the way in + CORE.config_path.write_bytes(yaml_text.encode()) + CORE.raw_config = do_substitution_pass(yaml_util.load_yaml(CORE.config_path), None) + return CORE.config_path + + +def _name_edit(new_name: str) -> LineEdit: + doc, line_no = source_of(CORE.raw_config[CONF_ESPHOME], CONF_NAME) + text = line_at(doc, line_no) + match = field_line_re(CONF_NAME, "kitchen").match(text) + return LineEdit(doc, line_no, text, rewrite(match, new_name)) + + +def test_rewrite_keeps_the_rest_of_the_line(tmp_path: Path) -> None: + """The value changes; indentation, quotes and the comment stay.""" + path = _setup(tmp_path, YAML.replace("name: kitchen", "name: 'kitchen'")) + edit = _name_edit("garage") + assert (edit.line, edit.new_line) == (1, " name: 'garage' # the device") + assert rewritten_text(read_text(path), [edit]) == YAML.replace( + "name: kitchen", "name: 'garage'" + ) + + +def test_rewrite_can_force_quotes() -> None: + match = field_line_re(CONF_NAME, "kitchen").match(" name: kitchen # x") + assert rewrite(match, "garage", quote='"') == ' name: "garage" # x' + + +def test_only_the_located_line_changes(tmp_path: Path) -> None: + """A lookalike `name:` under another block has its own range.""" + yaml_text = YAML + "sensor:\n - platform: template\n name: kitchen\n" + path = _setup(tmp_path, yaml_text) + text = rewritten_text(read_text(path), [_name_edit("garage")]) + assert text.endswith(" name: kitchen\n") + assert " name: garage # the device" in text + + +def test_line_endings_are_kept(tmp_path: Path) -> None: + path = _setup(tmp_path, YAML.replace("\n", "\r\n")) + assert rewritten_text(read_text(path), [_name_edit("garage")]) == YAML.replace( + "\n", "\r\n" + ).replace("name: kitchen", "name: garage") + + +def test_stale_line_is_refused(tmp_path: Path) -> None: + path = _setup(tmp_path, YAML) + edit = _name_edit("garage") + with pytest.raises(EsphomeError, match="changed since it was read"): + rewritten_text(YAML.replace("kitchen #", "pantry #"), [edit]) + with pytest.raises(EsphomeError, match="changed since it was read"): + rewritten_text("esphome:\n", [edit]) + with pytest.raises(EsphomeError, match="changed since it was read"): + line_at(path, 5) + + +def test_a_comment_needs_whitespace_and_a_scalar_is_not_empty() -> None: + """`abc#def` is one value to the loader, and a bare `key:` heads a block.""" + assert field_line_re("key", "abc").match("key: abc#def") is None + assert field_line_re("key").match("key:") is None + assert field_line_re("key").match("key: abc # c")["trail"] == " # c" + assert field_line_re("key", "abc#def").match("key: abc#def") is not None + + +def test_source_of_is_none_for_a_value_validation_added(tmp_path: Path) -> None: + """Only a key read from a file carries a range, and only a mapping read + from a file can place its keys; a mapping built in code cannot.""" + assert source_of({"name": "kitchen"}, "name") is None + _setup(tmp_path, YAML) + loaded_key = next(iter(CORE.raw_config[CONF_ESPHOME])) + assert source_of({loaded_key: "kitchen"}, CONF_NAME) is None + + +def test_source_of_is_none_for_a_merged_key(tmp_path: Path) -> None: + """A key a merge brought in points at the anchor, which other mappings + may merge as well; it is not this mapping's own line.""" + _setup( + tmp_path, + "named: &named\n name: kitchen\n\nesphome:\n <<: *named\n friendly_name: x\n", + ) + assert source_of(CORE.raw_config[CONF_ESPHOME], CONF_NAME) is None + assert source_of(CORE.raw_config[CONF_ESPHOME], "friendly_name") == ( + tmp_path / "test.yaml", + 5, + ) + + +def test_source_of_names_the_file_the_loader_read(tmp_path: Path) -> None: + """An include has its own document; a symlink is reported as given.""" + (tmp_path / "base.yaml").write_bytes(b"name: kitchen\n") + _setup(tmp_path, "esphome: !include base.yaml\n") + assert source_of(CORE.raw_config[CONF_ESPHOME], CONF_NAME) == ( + tmp_path / "base.yaml", + 0, + ) + target = tmp_path / "shared" / "test.yaml" + target.parent.mkdir() + target.write_bytes(YAML.encode()) + CORE.config_path.unlink() + CORE.config_path.symlink_to(target) + CORE.raw_config = yaml_util.load_yaml(CORE.config_path) + assert _name_edit("garage").path == CORE.config_path + + +@pytest.mark.skipif(sys.platform == "win32", reason="posix file modes") +def test_write_keeps_the_mode_of_the_file_or_another(tmp_path: Path) -> None: + path = _setup(tmp_path, YAML) + path.chmod(0o600) + write_keeping_mode(path, YAML) + assert path.stat().st_mode & 0o777 == 0o600 + other = tmp_path / "other.yaml" + write_keeping_mode(other, YAML, like=path) + assert other.stat().st_mode & 0o777 == 0o600 + + +def test_write_failures_say_which_step_and_why(tmp_path: Path) -> None: + """A missing mode source, a write that fails, and a mode that cannot be + put back after the write are three different situations.""" + from unittest.mock import patch + + path = _setup(tmp_path, YAML) + with pytest.raises(EsphomeError, match="Could not read the mode of .*gone.yaml"): + write_keeping_mode(path, YAML, like=tmp_path / "gone.yaml") + with ( + patch("pathlib.Path.chmod", side_effect=OSError("denied")), + pytest.raises( + EsphomeError, match="was written but could not get its mode back: denied" + ), + ): + write_keeping_mode(path, YAML) + + def refuse(*_args: object, **_kwargs: object) -> None: + raise EsphomeError(f"Could not write file at {path}") from OSError("disk full") + + with ( + patch("esphome.yaml_edit.write_file", side_effect=refuse), + pytest.raises(EsphomeError, match="Could not write file at .*: disk full"), + ): + write_keeping_mode(path, YAML) + + +def test_read_text_reports_a_file_it_cannot_decode(tmp_path: Path) -> None: + path = tmp_path / "latin1.yaml" + path.write_bytes(b"caf\xe9: 1\n") + with pytest.raises(EsphomeError, match="Error reading file"): + read_text(path)