mirror of
https://github.com/esphome/esphome.git
synced 2025-10-18 09:43:47 +01:00
[substitutions] Fix AttributeError when using packages with substitutions (#11274)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
Jesse Hills
parent
56eb605ec9
commit
a75ccf841c
@@ -1,7 +1,7 @@
|
||||
import logging
|
||||
|
||||
from esphome import core
|
||||
from esphome.config_helpers import Extend, Remove, merge_config
|
||||
from esphome.config_helpers import Extend, Remove, merge_config, merge_dicts_ordered
|
||||
import esphome.config_validation as cv
|
||||
from esphome.const import CONF_SUBSTITUTIONS, VALID_SUBSTITUTIONS_CHARACTERS
|
||||
from esphome.yaml_util import ESPHomeDataBase, ESPLiteralValue, make_data_base
|
||||
@@ -170,10 +170,10 @@ def do_substitution_pass(config, command_line_substitutions, ignore_missing=Fals
|
||||
return
|
||||
|
||||
# Merge substitutions in config, overriding with substitutions coming from command line:
|
||||
substitutions = {
|
||||
**config.get(CONF_SUBSTITUTIONS, {}),
|
||||
**(command_line_substitutions or {}),
|
||||
}
|
||||
# Use merge_dicts_ordered to preserve OrderedDict type for move_to_end()
|
||||
substitutions = merge_dicts_ordered(
|
||||
config.get(CONF_SUBSTITUTIONS, {}), command_line_substitutions or {}
|
||||
)
|
||||
with cv.prepend_path("substitutions"):
|
||||
if not isinstance(substitutions, dict):
|
||||
raise cv.Invalid(
|
||||
|
@@ -12,7 +12,7 @@ from typing import Any
|
||||
import voluptuous as vol
|
||||
|
||||
from esphome import core, loader, pins, yaml_util
|
||||
from esphome.config_helpers import Extend, Remove
|
||||
from esphome.config_helpers import Extend, Remove, merge_dicts_ordered
|
||||
import esphome.config_validation as cv
|
||||
from esphome.const import (
|
||||
CONF_ESPHOME,
|
||||
@@ -922,10 +922,9 @@ def validate_config(
|
||||
if CONF_SUBSTITUTIONS in config or command_line_substitutions:
|
||||
from esphome.components import substitutions
|
||||
|
||||
result[CONF_SUBSTITUTIONS] = {
|
||||
**(config.get(CONF_SUBSTITUTIONS) or {}),
|
||||
**command_line_substitutions,
|
||||
}
|
||||
result[CONF_SUBSTITUTIONS] = merge_dicts_ordered(
|
||||
config.get(CONF_SUBSTITUTIONS) or {}, command_line_substitutions
|
||||
)
|
||||
result.add_output_path([CONF_SUBSTITUTIONS], CONF_SUBSTITUTIONS)
|
||||
try:
|
||||
substitutions.do_substitution_pass(config, command_line_substitutions)
|
||||
|
@@ -10,6 +10,7 @@ from esphome.const import (
|
||||
PlatformFramework,
|
||||
)
|
||||
from esphome.core import CORE
|
||||
from esphome.util import OrderedDict
|
||||
|
||||
# Pre-build lookup map from (platform, framework) tuples to PlatformFramework enum
|
||||
_PLATFORM_FRAMEWORK_LOOKUP = {
|
||||
@@ -17,6 +18,25 @@ _PLATFORM_FRAMEWORK_LOOKUP = {
|
||||
}
|
||||
|
||||
|
||||
def merge_dicts_ordered(*dicts: dict) -> OrderedDict:
|
||||
"""Merge multiple dicts into an OrderedDict, preserving key order.
|
||||
|
||||
This is a helper to ensure that dictionary merging preserves OrderedDict type,
|
||||
which is important for operations like move_to_end().
|
||||
|
||||
Args:
|
||||
*dicts: Variable number of dictionaries to merge (later dicts override earlier ones)
|
||||
|
||||
Returns:
|
||||
OrderedDict with merged contents
|
||||
"""
|
||||
result = OrderedDict()
|
||||
for d in dicts:
|
||||
if d:
|
||||
result.update(d)
|
||||
return result
|
||||
|
||||
|
||||
class Extend:
|
||||
def __init__(self, value):
|
||||
self.value = value
|
||||
@@ -60,7 +80,11 @@ def merge_config(full_old, full_new):
|
||||
if isinstance(new, dict):
|
||||
if not isinstance(old, dict):
|
||||
return new
|
||||
res = old.copy()
|
||||
# Preserve OrderedDict type by copying to OrderedDict if either input is OrderedDict
|
||||
if isinstance(old, OrderedDict) or isinstance(new, OrderedDict):
|
||||
res = OrderedDict(old)
|
||||
else:
|
||||
res = old.copy()
|
||||
for k, v in new.items():
|
||||
if isinstance(v, Remove) and k in old:
|
||||
del res[k]
|
||||
|
@@ -2,9 +2,12 @@ import glob
|
||||
import logging
|
||||
from pathlib import Path
|
||||
|
||||
from esphome import yaml_util
|
||||
from esphome import config as config_module, yaml_util
|
||||
from esphome.components import substitutions
|
||||
from esphome.const import CONF_PACKAGES
|
||||
from esphome.config_helpers import merge_config
|
||||
from esphome.const import CONF_PACKAGES, CONF_SUBSTITUTIONS
|
||||
from esphome.core import CORE
|
||||
from esphome.util import OrderedDict
|
||||
|
||||
_LOGGER = logging.getLogger(__name__)
|
||||
|
||||
@@ -118,3 +121,200 @@ def test_substitutions_fixtures(fixture_path):
|
||||
if DEV_MODE:
|
||||
_LOGGER.error("Tests passed, but Dev mode is enabled.")
|
||||
assert not DEV_MODE # make sure DEV_MODE is disabled after you are finished.
|
||||
|
||||
|
||||
def test_substitutions_with_command_line_maintains_ordered_dict() -> None:
|
||||
"""Test that substitutions remain an OrderedDict when command line substitutions are provided,
|
||||
and that move_to_end() can be called successfully.
|
||||
|
||||
This is a regression test for https://github.com/esphome/esphome/issues/11182
|
||||
where the config would become a regular dict and fail when move_to_end() was called.
|
||||
"""
|
||||
# Create an OrderedDict config with substitutions
|
||||
config = OrderedDict()
|
||||
config["esphome"] = {"name": "test"}
|
||||
config[CONF_SUBSTITUTIONS] = {"var1": "value1", "var2": "value2"}
|
||||
config["other_key"] = "other_value"
|
||||
|
||||
# Command line substitutions that should override
|
||||
command_line_subs = {"var2": "override", "var3": "new_value"}
|
||||
|
||||
# Call do_substitution_pass with command line substitutions
|
||||
substitutions.do_substitution_pass(config, command_line_subs)
|
||||
|
||||
# Verify that config is still an OrderedDict
|
||||
assert isinstance(config, OrderedDict), "Config should remain an OrderedDict"
|
||||
|
||||
# Verify substitutions are at the beginning (move_to_end with last=False)
|
||||
keys = list(config.keys())
|
||||
assert keys[0] == CONF_SUBSTITUTIONS, "Substitutions should be first key"
|
||||
|
||||
# Verify substitutions were properly merged
|
||||
assert config[CONF_SUBSTITUTIONS]["var1"] == "value1"
|
||||
assert config[CONF_SUBSTITUTIONS]["var2"] == "override"
|
||||
assert config[CONF_SUBSTITUTIONS]["var3"] == "new_value"
|
||||
|
||||
# Verify config[CONF_SUBSTITUTIONS] is also an OrderedDict
|
||||
assert isinstance(config[CONF_SUBSTITUTIONS], OrderedDict), (
|
||||
"Substitutions should be an OrderedDict"
|
||||
)
|
||||
|
||||
|
||||
def test_substitutions_without_command_line_maintains_ordered_dict() -> None:
|
||||
"""Test that substitutions work correctly without command line substitutions."""
|
||||
config = OrderedDict()
|
||||
config["esphome"] = {"name": "test"}
|
||||
config[CONF_SUBSTITUTIONS] = {"var1": "value1"}
|
||||
config["other_key"] = "other_value"
|
||||
|
||||
# Call without command line substitutions
|
||||
substitutions.do_substitution_pass(config, None)
|
||||
|
||||
# Verify that config is still an OrderedDict
|
||||
assert isinstance(config, OrderedDict), "Config should remain an OrderedDict"
|
||||
|
||||
# Verify substitutions are at the beginning
|
||||
keys = list(config.keys())
|
||||
assert keys[0] == CONF_SUBSTITUTIONS, "Substitutions should be first key"
|
||||
|
||||
|
||||
def test_substitutions_after_merge_config_maintains_ordered_dict() -> None:
|
||||
"""Test that substitutions work after merge_config (packages scenario).
|
||||
|
||||
This is a regression test for https://github.com/esphome/esphome/issues/11182
|
||||
where using packages would cause config to become a regular dict, breaking move_to_end().
|
||||
"""
|
||||
# Simulate what happens with packages - merge two OrderedDict configs
|
||||
base_config = OrderedDict()
|
||||
base_config["esphome"] = {"name": "base"}
|
||||
base_config[CONF_SUBSTITUTIONS] = {"var1": "value1"}
|
||||
|
||||
package_config = OrderedDict()
|
||||
package_config["sensor"] = [{"platform": "template"}]
|
||||
package_config[CONF_SUBSTITUTIONS] = {"var2": "value2"}
|
||||
|
||||
# Merge configs (simulating package merge)
|
||||
merged_config = merge_config(base_config, package_config)
|
||||
|
||||
# Verify merged config is still an OrderedDict
|
||||
assert isinstance(merged_config, OrderedDict), (
|
||||
"Merged config should be an OrderedDict"
|
||||
)
|
||||
|
||||
# Now try to run substitution pass on the merged config
|
||||
substitutions.do_substitution_pass(merged_config, None)
|
||||
|
||||
# Should not raise AttributeError
|
||||
assert isinstance(merged_config, OrderedDict), (
|
||||
"Config should still be OrderedDict after substitution pass"
|
||||
)
|
||||
keys = list(merged_config.keys())
|
||||
assert keys[0] == CONF_SUBSTITUTIONS, "Substitutions should be first key"
|
||||
|
||||
|
||||
def test_validate_config_with_command_line_substitutions_maintains_ordered_dict(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
"""Test that validate_config preserves OrderedDict when merging command-line substitutions.
|
||||
|
||||
This tests the code path in config.py where result[CONF_SUBSTITUTIONS] is set
|
||||
using merge_dicts_ordered() with command-line substitutions provided.
|
||||
"""
|
||||
# Create a minimal valid config
|
||||
test_config = OrderedDict()
|
||||
test_config["esphome"] = {"name": "test_device", "platform": "ESP32"}
|
||||
test_config[CONF_SUBSTITUTIONS] = OrderedDict({"var1": "value1", "var2": "value2"})
|
||||
test_config["esp32"] = {"board": "esp32dev"}
|
||||
|
||||
# Command line substitutions that should override
|
||||
command_line_subs = {"var2": "override", "var3": "new_value"}
|
||||
|
||||
# Set up CORE for the test with a proper Path object
|
||||
test_yaml = tmp_path / "test.yaml"
|
||||
test_yaml.write_text("# test config")
|
||||
CORE.config_path = test_yaml
|
||||
|
||||
# Call validate_config with command line substitutions
|
||||
result = config_module.validate_config(test_config, command_line_subs)
|
||||
|
||||
# Verify that result[CONF_SUBSTITUTIONS] is an OrderedDict
|
||||
assert isinstance(result.get(CONF_SUBSTITUTIONS), OrderedDict), (
|
||||
"Result substitutions should be an OrderedDict"
|
||||
)
|
||||
|
||||
# Verify substitutions were properly merged
|
||||
assert result[CONF_SUBSTITUTIONS]["var1"] == "value1"
|
||||
assert result[CONF_SUBSTITUTIONS]["var2"] == "override"
|
||||
assert result[CONF_SUBSTITUTIONS]["var3"] == "new_value"
|
||||
|
||||
|
||||
def test_validate_config_without_command_line_substitutions_maintains_ordered_dict(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
"""Test that validate_config preserves OrderedDict without command-line substitutions.
|
||||
|
||||
This tests the code path in config.py where result[CONF_SUBSTITUTIONS] is set
|
||||
using merge_dicts_ordered() when command_line_substitutions is None.
|
||||
"""
|
||||
# Create a minimal valid config
|
||||
test_config = OrderedDict()
|
||||
test_config["esphome"] = {"name": "test_device", "platform": "ESP32"}
|
||||
test_config[CONF_SUBSTITUTIONS] = OrderedDict({"var1": "value1", "var2": "value2"})
|
||||
test_config["esp32"] = {"board": "esp32dev"}
|
||||
|
||||
# Set up CORE for the test with a proper Path object
|
||||
test_yaml = tmp_path / "test.yaml"
|
||||
test_yaml.write_text("# test config")
|
||||
CORE.config_path = test_yaml
|
||||
|
||||
# Call validate_config without command line substitutions
|
||||
result = config_module.validate_config(test_config, None)
|
||||
|
||||
# Verify that result[CONF_SUBSTITUTIONS] is an OrderedDict
|
||||
assert isinstance(result.get(CONF_SUBSTITUTIONS), OrderedDict), (
|
||||
"Result substitutions should be an OrderedDict"
|
||||
)
|
||||
|
||||
# Verify substitutions are unchanged
|
||||
assert result[CONF_SUBSTITUTIONS]["var1"] == "value1"
|
||||
assert result[CONF_SUBSTITUTIONS]["var2"] == "value2"
|
||||
|
||||
|
||||
def test_merge_config_preserves_ordered_dict() -> None:
|
||||
"""Test that merge_config preserves OrderedDict type.
|
||||
|
||||
This is a regression test to ensure merge_config doesn't lose OrderedDict type
|
||||
when merging configs, which causes AttributeError on move_to_end().
|
||||
"""
|
||||
# Test OrderedDict + dict = OrderedDict
|
||||
od = OrderedDict([("a", 1), ("b", 2)])
|
||||
d = {"b": 20, "c": 3}
|
||||
result = merge_config(od, d)
|
||||
assert isinstance(result, OrderedDict), (
|
||||
"OrderedDict + dict should return OrderedDict"
|
||||
)
|
||||
|
||||
# Test dict + OrderedDict = OrderedDict
|
||||
d = {"a": 1, "b": 2}
|
||||
od = OrderedDict([("b", 20), ("c", 3)])
|
||||
result = merge_config(d, od)
|
||||
assert isinstance(result, OrderedDict), (
|
||||
"dict + OrderedDict should return OrderedDict"
|
||||
)
|
||||
|
||||
# Test OrderedDict + OrderedDict = OrderedDict
|
||||
od1 = OrderedDict([("a", 1), ("b", 2)])
|
||||
od2 = OrderedDict([("b", 20), ("c", 3)])
|
||||
result = merge_config(od1, od2)
|
||||
assert isinstance(result, OrderedDict), (
|
||||
"OrderedDict + OrderedDict should return OrderedDict"
|
||||
)
|
||||
|
||||
# Test that dict + dict still returns regular dict (no unnecessary conversion)
|
||||
d1 = {"a": 1, "b": 2}
|
||||
d2 = {"b": 20, "c": 3}
|
||||
result = merge_config(d1, d2)
|
||||
assert isinstance(result, dict), "dict + dict should return dict"
|
||||
assert not isinstance(result, OrderedDict), (
|
||||
"dict + dict should not return OrderedDict"
|
||||
)
|
||||
|
Reference in New Issue
Block a user