Merge remote-tracking branch 'upstream/wifi-esp8266-roam-scan-dwell' into integration

tests/component_tests/packages/test_packages.py

@@ -6,6 +6,7 @@ from unittest.mock import MagicMock, patch
 import pytest
 
 from esphome.components.packages import CONFIG_SCHEMA, do_packages_pass, merge_packages
+from esphome.components.substitutions import do_substitution_pass
 import esphome.config as config_module
 from esphome.config import resolve_extend_remove
 from esphome.config_helpers import Extend, Remove
@@ -71,6 +72,7 @@ def fixture_basic_esphome():
 def packages_pass(config):
     """Wrapper around packages_pass that also resolves Extend and Remove."""
     config = do_packages_pass(config)
+    config = do_substitution_pass(config)
     config = merge_packages(config)
     resolve_extend_remove(config)
     return config

tests/components/number/common.yaml

@@ -0,0 +1,13 @@
+number:
+  - platform: template
+    name: "Test Number"
+    id: test_number
+    optimistic: true
+    min_value: 0
+    max_value: 100
+    step: 1
+
+sensor:
+  - platform: number
+    name: "Test Number Value"
+    source_id: test_number

tests/components/number/test.esp32-idf.yaml

@@ -0,0 +1,2 @@
+packages:
+  common: !include common.yaml

tests/components/number/test.esp8266-ard.yaml

@@ -0,0 +1,2 @@
+packages:
+  common: !include common.yaml

tests/unit_tests/fixtures/substitutions/00-simple_var.approved.yaml

@@ -38,3 +38,20 @@ test_list:
   - '{ 79, 82 }'
   - a: 15 should be 15, overridden from command line
     b: 20 should stay as 20, not overridden
+  - aa:
+      - 1
+      - 2
+      - 3
+      - 4
+      - 5
+      - 6
+    bb:
+      - 7
+      - 8
+      - 9
+  - aa:
+      x: 1
+      y: 3
+      z: 4
+    bb:
+      w: 5

tests/unit_tests/fixtures/substitutions/00-simple_var.input.yaml

@@ -44,3 +44,13 @@ test_list:
   - '{ ${position.x}, ${position.y} }'
   - a: ${a} should be 15, overridden from command line
     b: ${b} should stay as 20, not overridden
+
+  # Test merging lists when substituted keys resolve to an existing key
+  - ${ "aa" }: [1, 2, 3]
+    ${ "a" + "a" }: [4, 5, 6]
+    ${ "bb" }: [7, 8, 9]
+
+  # Test merging dicts when substituted keys resolve to an existing key
+  - ${ "aa" }: {"x": 1, "y": 2}
+    ${ "a" + "a" }: {"y": 3, "z": 4}
+    ${ "bb" }: {"w": 5}

tests/unit_tests/fixtures/substitutions/02-expressions.approved.yaml

@@ -9,6 +9,11 @@ substitutions:
   numberOne: 1
   var1: 79
   double_width: 14
+  double_height: 16
+  y: ${x}
+  x: ${y}
+  b: 79
+  c: 80
 test_list:
   - The area is 56
   - 56
@@ -27,3 +32,4 @@ test_list:
   - chr(97) = a
   - len([1,2,3]) = 3
   - width = 7, double_width = 14
+  - a = ${a}

tests/unit_tests/fixtures/substitutions/02-expressions.input.yaml

@@ -1,4 +1,7 @@
 substitutions:
+  y: ${x}  # Circular reference, expect to pass unresolved.
+  x: ${y}  # Circular reference, expect to pass unresolved.
+  double_height: ${height * 2}
   width: 7
   height: 8
   enabled: true
@@ -9,6 +12,8 @@ substitutions:
   numberOne: 1
   var1: 79
   double_width: ${width * 2}
+  c: ${b+1}
+  b: ${undefined_variable | default(79) }
 
 test_list:
   - "The area is ${width * height}"
@@ -25,3 +30,4 @@ test_list:
   - chr(97) = ${ chr(97) }
   - len([1,2,3]) = ${ len([1,2,3]) }
   - width = ${width}, double_width = ${double_width}
+  - a = ${a}

tests/unit_tests/fixtures/substitutions/07-package_merging.approved.yaml

@@ -0,0 +1,46 @@
+fancy_component: &id001
+  - id: component9
+    value: 9
+some_component:
+  - id: component1
+    value: 1
+  - id: component2
+    value: 2
+  - id: component3
+    value: 3
+  - id: component4
+    value: 4
+  - id: component5
+    value: 79
+    power: 200
+  - id: component6
+    value: 6
+  - id: component7
+    value: 7
+switch: &id002
+  - platform: gpio
+    id: switch1
+    pin: 12
+  - platform: gpio
+    id: switch2
+    pin: 13
+display:
+  - platform: ili9xxx
+    dimensions:
+      width: 100
+      height: 480
+substitutions:
+  extended_component: component5
+  package_options:
+    alternative_package:
+      alternative_component:
+        - id: component8
+          value: 8
+    fancy_package:
+      substitutions:
+        fancy_subst: 42
+      fancy_component: *id001
+  pin: 12
+  some_switches: *id002
+  package_selection: fancy_package
+  fancy_subst: 42

tests/unit_tests/fixtures/substitutions/07-package_merging.input.yaml

@@ -0,0 +1,63 @@
+substitutions:
+  package_options:
+    alternative_package:
+      alternative_component:
+        - id: component8
+          value: 8
+    fancy_package:
+      substitutions:
+        fancy_subst: 42
+      fancy_component:
+        - id: component9
+          value: 9
+
+  pin: 12
+  some_switches:
+    - platform: gpio
+      id: switch1
+      pin: ${pin}
+    - platform: gpio
+      id: switch2
+      pin: ${pin+1}
+
+  package_selection: fancy_package
+
+packages:
+  - ${ package_options[package_selection] }
+  - some_component:
+      - id: component1
+        value: 1
+  - some_component:
+      - id: component2
+        value: 2
+  - switch: ${ some_switches }
+  - packages:
+      package_with_defaults: !include
+        file: display.yaml
+        vars:
+          native_width: 100
+          high_dpi: false
+      my_package:
+        packages:
+          - packages:
+              special_package:
+                substitutions:
+                  extended_component: component5
+                some_component:
+                  - id: component3
+                    value: 3
+        some_component:
+          - id: component4
+            value: 4
+          - id: !extend ${ extended_component }
+            power: 200
+            value: 79
+    some_component:
+      - id: component5
+        value: 5
+
+some_component:
+  - id: component6
+    value: 6
+  - id: component7
+    value: 7

tests/unit_tests/fixtures/substitutions/09-include_vars_without_substs.approved.yaml

@@ -0,0 +1,5 @@
+values:
+  - var1: $var1
+  - a: 10
+  - b: B-default
+  - c: The value of C is 79

tests/unit_tests/fixtures/substitutions/09-include_vars_without_substs.input.yaml

@@ -0,0 +1,7 @@
+# Test that include_vars with vars works even when there are no substitutions key defined.
+packages:
+  - !include
+    file: inc1.yaml
+    vars:
+      a: 10
+      c: 79

tests/unit_tests/test_substitutions.py

@@ -10,9 +10,10 @@ from esphome import config as config_module, yaml_util
 from esphome.components import substitutions
 from esphome.components.packages import do_packages_pass, merge_packages
 from esphome.config import resolve_extend_remove
-from esphome.config_helpers import merge_config
+from esphome.config_helpers import Extend, merge_config
+import esphome.config_validation as cv
 from esphome.const import CONF_SUBSTITUTIONS
-from esphome.core import CORE
+from esphome.core import CORE, Lambda
 from esphome.util import OrderedDict
 
 _LOGGER = logging.getLogger(__name__)
@@ -144,7 +145,7 @@ def test_substitutions_fixtures(
 
     config = do_packages_pass(config)
 
-    substitutions.do_substitution_pass(config, command_line_substitutions)
+    config = substitutions.do_substitution_pass(config, command_line_substitutions)
 
     config = merge_packages(config)
 
@@ -206,7 +207,7 @@ def test_substitutions_with_command_line_maintains_ordered_dict() -> None:
     command_line_subs = {"var2": "override", "var3": "new_value"}
 
     # Call do_substitution_pass with command line substitutions
-    substitutions.do_substitution_pass(config, command_line_subs)
+    config = substitutions.do_substitution_pass(config, command_line_subs)
 
     # Verify that config is still an OrderedDict
     assert isinstance(config, OrderedDict), "Config should remain an OrderedDict"
@@ -234,7 +235,7 @@ def test_substitutions_without_command_line_maintains_ordered_dict() -> None:
     config["other_key"] = "other_value"
 
     # Call without command line substitutions
-    substitutions.do_substitution_pass(config, None)
+    config = substitutions.do_substitution_pass(config, None)
 
     # Verify that config is still an OrderedDict
     assert isinstance(config, OrderedDict), "Config should remain an OrderedDict"
@@ -268,7 +269,7 @@ def test_substitutions_after_merge_config_maintains_ordered_dict() -> None:
     )
 
     # Now try to run substitution pass on the merged config
-    substitutions.do_substitution_pass(merged_config, None)
+    merged_config = substitutions.do_substitution_pass(merged_config, None)
 
     # Should not raise AttributeError
     assert isinstance(merged_config, OrderedDict), (
@@ -279,7 +280,7 @@ def test_substitutions_after_merge_config_maintains_ordered_dict() -> None:
 
 
 def test_validate_config_with_command_line_substitutions_maintains_ordered_dict(
-    tmp_path,
+    tmp_path: Path,
 ) -> None:
     """Test that validate_config preserves OrderedDict when merging command-line substitutions.
 
@@ -288,7 +289,7 @@ def test_validate_config_with_command_line_substitutions_maintains_ordered_dict(
     """
     # Create a minimal valid config
     test_config = OrderedDict()
-    test_config["esphome"] = {"name": "test_device", "platform": "ESP32"}
+    test_config["esphome"] = {"name": "test_device"}
     test_config[CONF_SUBSTITUTIONS] = OrderedDict({"var1": "value1", "var2": "value2"})
     test_config["esp32"] = {"board": "esp32dev"}
 
@@ -314,17 +315,11 @@ def test_validate_config_with_command_line_substitutions_maintains_ordered_dict(
     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.
-    """
+def _get_test_minimal_valid_config(tmp_path: Path) -> OrderedDict:
+    """Helper to create a minimal valid config for testing."""
     # Create a minimal valid config
     test_config = OrderedDict()
-    test_config["esphome"] = {"name": "test_device", "platform": "ESP32"}
+    test_config["esphome"] = {"name": "test_device"}
     test_config[CONF_SUBSTITUTIONS] = OrderedDict({"var1": "value1", "var2": "value2"})
     test_config["esp32"] = {"board": "esp32dev"}
 
@@ -332,6 +327,19 @@ def test_validate_config_without_command_line_substitutions_maintains_ordered_di
     test_yaml = tmp_path / "test.yaml"
     test_yaml.write_text("# test config")
     CORE.config_path = test_yaml
+    return test_config
+
+
+def test_validate_config_without_command_line_substitutions_maintains_ordered_dict(
+    tmp_path: 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.
+    """
+
+    test_config = _get_test_minimal_valid_config(tmp_path)
 
     # Call validate_config without command line substitutions
     result = config_module.validate_config(test_config, None)
@@ -384,3 +392,205 @@ def test_merge_config_preserves_ordered_dict() -> None:
     assert not isinstance(result, OrderedDict), (
         "dict + dict should not return OrderedDict"
     )
+
+
+def test_substitution_pass_error_gets_captured(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """vol.Invalid from do_substitution_pass is captured by validate_config."""
+
+    # Patch the target: in config_module.do_substitution_pass (NOT where it's defined)
+    def fake_do_substitution_pass(*args, **kwargs):
+        raise cv.Invalid("Error in do_substitutions_pass!!")
+
+    monkeypatch.setattr(
+        config_module, "do_substitution_pass", fake_do_substitution_pass
+    )
+
+    # Prepare minimal config + no CLI substitutions
+    config = _get_test_minimal_valid_config(tmp_path)
+
+    # Call the function under test
+    result = config_module.validate_config(config, None)
+
+    # Now assert that add_error was called with the vol.Invalid
+
+    assert "Error in do_substitutions_pass!!" in str(result.get_error_for_path([]))
+
+
+@pytest.mark.parametrize(
+    "value", ["", "   ", "1foo", "9VAR", "0abc", "$1foo", "$9VAR", "$0abc"]
+)
+def test_validate_substitution_key_empty_raises(value: str) -> None:
+    """Empty (or all-whitespace) substitution keys are rejected."""
+    with pytest.raises(cv.Invalid):
+        substitutions.validate_substitution_key(value)
+
+
+@pytest.mark.parametrize(
+    "input_value, expected_output",
+    [
+        ("$FOO_bar9", "FOO_bar9"),  # Valid key with leading '$'
+        ("Foo_bar9", "Foo_bar9"),  # Normal valid key
+    ],
+)
+def test_validate_substitution_key_valid(
+    input_value: str, expected_output: str
+) -> None:
+    """Valid substitution keys are accepted with optional leading '$'."""
+    result = substitutions.validate_substitution_key(input_value)
+    assert result == expected_output
+
+
+def test_circular_dependency_warnings(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Circular substitution references produce warnings naming the cause."""
+    config = OrderedDict(
+        {
+            CONF_SUBSTITUTIONS: OrderedDict({"x": "${y}", "y": "${x}"}),
+            "key": "value",
+        }
+    )
+    with caplog.at_level(logging.WARNING):
+        substitutions.do_substitution_pass(config)
+
+    assert "Could not resolve substitution variable 'x'" in caplog.text
+    assert "'y' is undefined" in caplog.text
+    assert "Could not resolve substitution variable 'y'" in caplog.text
+    assert "'x' is undefined" in caplog.text
+    # Verify path includes location
+    assert "substitutions->x" in caplog.text
+    assert "substitutions->y" in caplog.text
+
+
+def test_missing_dependency_warning(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """A substitution referencing an undefined variable warns with the cause."""
+    config = OrderedDict(
+        {
+            CONF_SUBSTITUTIONS: OrderedDict({"a": "${missing}"}),
+            "key": "value",
+        }
+    )
+    with caplog.at_level(logging.WARNING):
+        substitutions.do_substitution_pass(config)
+
+    assert "Could not resolve substitution variable 'a'" in caplog.text
+    assert "'missing' is undefined" in caplog.text
+    assert "substitutions->a" in caplog.text
+
+
+def test_undefined_variable_warning(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """A reference to an undefined variable in config values produces a warning."""
+    config = OrderedDict(
+        {
+            "key": "${undefined_var}",
+        }
+    )
+    with caplog.at_level(logging.WARNING):
+        substitutions.do_substitution_pass(config)
+
+    assert "'undefined_var' is undefined" in caplog.text
+
+
+def test_password_field_warnings_suppressed(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Undefined variables in password fields should not produce warnings."""
+    config = OrderedDict(
+        {
+            "password": "${undefined_var}",
+        }
+    )
+    with caplog.at_level(logging.WARNING):
+        substitutions.do_substitution_pass(config)
+
+    assert caplog.text == ""
+
+
+def test_config_context_unresolvable_warns(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Unresolvable vars in a ConfigContext produce warnings via push_context."""
+    inner = OrderedDict({"key": "${a}"})
+    yaml_util.add_context(inner, {"a": "${undefined}"})
+    config = OrderedDict({"items": [inner]})
+    with caplog.at_level(logging.WARNING):
+        substitutions.do_substitution_pass(config)
+
+    assert "Could not resolve substitution variable 'a'" in caplog.text
+    assert "'undefined' is undefined" in caplog.text
+
+
+def test_non_string_substitution_value_warning(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Undefined vars in non-string contexts (e.g. dict keys) produce warnings."""
+    config = OrderedDict(
+        {
+            "items": {"${undefined_key}": "value"},
+        }
+    )
+    with caplog.at_level(logging.WARNING):
+        substitutions.do_substitution_pass(config)
+
+    assert "'undefined_key' is undefined" in caplog.text
+
+
+def test_lambda_substitution() -> None:
+    """Substitution inside a Lambda value should be expanded."""
+    lam = Lambda("return ${var};")
+    config = OrderedDict(
+        {
+            CONF_SUBSTITUTIONS: OrderedDict({"var": "42"}),
+            "lambda": lam,
+        }
+    )
+    substitutions.do_substitution_pass(config)
+    assert lam.value == "return 42;"
+
+
+def test_lambda_no_substitution_unchanged() -> None:
+    """A Lambda with no variable references should not be mutated."""
+    lam = Lambda("return 1;")
+    original_value = lam.value
+    config = OrderedDict(
+        {
+            CONF_SUBSTITUTIONS: OrderedDict({"var": "42"}),
+            "lambda": lam,
+        }
+    )
+    substitutions.do_substitution_pass(config)
+    assert lam.value is original_value
+
+
+def test_extend_substitution() -> None:
+    """Substitution inside an Extend value should be expanded."""
+    ext = Extend("${component_id}")
+    config = OrderedDict(
+        {
+            CONF_SUBSTITUTIONS: OrderedDict({"component_id": "my_sensor"}),
+            "sensor": ext,
+        }
+    )
+    substitutions.do_substitution_pass(config)
+    assert ext.value == "my_sensor"
+
+
+def test_do_substitution_pass_substitutions_must_be_mapping_from_config() -> None:
+    """Non-mapping substitutions raises cv.Invalid."""
+    config = OrderedDict(
+        {
+            CONF_SUBSTITUTIONS: ["not", "a", "mapping"],
+            "other": "value",
+        }
+    )
+
+    with pytest.raises(
+        cv.Invalid, match="Substitutions must be a key to value mapping"
+    ):
+        substitutions.do_substitution_pass(config)

tests/unit_tests/test_yaml_util.py

@@ -25,7 +25,7 @@ def test_include_with_vars(fixture_path: Path) -> None:
     yaml_file = fixture_path / "yaml_util" / "includetest.yaml"
 
     actual = yaml_util.load_yaml(yaml_file)
-    substitutions.do_substitution_pass(actual, None)
+    actual = substitutions.do_substitution_pass(actual, None)
     assert actual["esphome"]["name"] == "original"
     assert actual["esphome"]["libraries"][0] == "Wire"
     assert actual["esp8266"]["board"] == "nodemcu"