From 12b016e3d27b03f88e87c301ca2c29a193be128a Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Fri, 2 Oct 2026 05:27:29 -0700 Subject: [PATCH] [substitutions] Support !include with a condition (#19455) Co-authored-by: J. Nick Koston --- esphome/components/substitutions/__init__.py | 19 +- esphome/yaml_util.py | 114 +++++++-- .../substitutions/01-include.approved.yaml | 7 + .../substitutions/01-include.input.yaml | 19 ++ .../substitutions/12-yaml-merge.input.yaml | 6 + .../fixtures/yaml_util/includes/true.yaml | 2 + .../fixtures/yaml_util/includetest.yaml | 6 +- .../includetest_invalid_condition_type.yaml | 3 + .../yaml_util/includetest_no_file.yaml | 3 + tests/unit_tests/test_bundle.py | 2 +- tests/unit_tests/test_substitutions.py | 95 +++++++- tests/unit_tests/test_yaml_util.py | 220 +++++++++++++++--- 12 files changed, 420 insertions(+), 76 deletions(-) create mode 100644 tests/unit_tests/fixtures/yaml_util/includes/true.yaml create mode 100644 tests/unit_tests/fixtures/yaml_util/includetest_invalid_condition_type.yaml create mode 100644 tests/unit_tests/fixtures/yaml_util/includetest_no_file.yaml diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index 5ef7a699eb..d59f39b9c2 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -353,7 +353,7 @@ def resolve_include( strict_undefined: bool = True, errors: ErrList | None = None, ) -> Any: - """Resolve an include, substituting the filename if needed. + """Resolve an include, substituting the condition and filename if needed. Note: no path-traversal validation is performed on the resolved filename. A substitution that resolves to an absolute path will bypass the parent @@ -362,6 +362,19 @@ def resolve_include( values (including command-line substitutions), so path restrictions are an explicit non-goal here. """ + if isinstance(original_condition := include.condition, str): + condition = str( + _expand_substitutions( + original_condition, + path + ["condition"], + context_vars, + strict_undefined, + errors, + ) + ) + if condition != original_condition: + include = include.with_condition(condition) + original = include.file filename = str( _expand_substitutions( @@ -372,8 +385,8 @@ def resolve_include( if substituted: include = include.with_file(filename) try: - return include.load() - except esphome.core.EsphomeError as err: + return include.load() if include.should_load() else {} + except (esphome.core.EsphomeError, cv.Invalid) as err: resolved = f" (expanded from '{original}')" if substituted else "" raise cv.Invalid( f"Error including file '{filename}'{resolved}: {err}" diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index 7c6cf691b9..e892664724 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -35,7 +35,7 @@ from esphome.core import ( TimePeriod, ) from esphome.expression import has_substitution_or_expression -from esphome.helpers import add_class_to_obj +from esphome.helpers import FALSY_BOOL_STRINGS, TRUTHY_BOOL_STRINGS, add_class_to_obj from esphome.util import OrderedDict, filter_yaml_files _LOGGER = logging.getLogger(__name__) @@ -75,6 +75,12 @@ def take_dropped_merge_keys() -> list[tuple[str, str]]: return CORE.data.pop(_MERGE_WARNINGS_KEY, []) +def _raise_invalid(message: str) -> None: + from voluptuous import Invalid + + raise Invalid(message) + + class SensitiveStr(str): """Marker subclass for validated strings that should be masked in user-visible YAML output. ``cv.sensitive`` wraps validated values in this @@ -224,16 +230,18 @@ class IncludeFile: Created during YAML parsing instead of loading the file immediately, allowing substitution variables to appear in the filename path - (e.g. ``!include device-${platform}.yaml``). The actual file is - loaded on the first call to ``load()``, and the result is cached. + (e.g. ``!include device-${platform}.yaml``) and in an optional condition. + The actual file is loaded on the first call to ``load()``, and the result + is cached. The client is responsible for testing the condition if needed. """ def __init__( self, parent_file: Path, file: str, - vars: dict[str, Any] | None, yaml_loader: Callable[[Path], Any], + vars: dict[str, Any] | None = None, + condition: bool | str | None = None, ) -> None: self.parent_file = parent_file # The raw include text may be a substitution/Jinja expression, so it @@ -241,8 +249,9 @@ class IncludeFile: # rewrites "/" to "\", which Jinja then decodes as escapes like # "\b" -> backspace (issue #18545). self.file = file - self.vars = vars self.yaml_loader = yaml_loader + self.vars = vars + self.condition = condition self._content: Any = _UNSET def __repr__(self) -> str: @@ -256,23 +265,62 @@ class IncludeFile: """ if self._content is not _UNSET: return self._content - if self.has_unresolved_expressions(): - from voluptuous import Invalid - - raise Invalid( + if self.has_unresolved_file(): + _raise_invalid( f"Cannot load include with unresolved substitutions: {self.file}" ) self._content = self.yaml_loader(self.parent_file.parent / self.file) self._content = add_context(self._content, self.vars) return self._content - def has_unresolved_expressions(self) -> bool: + def has_unresolved_file(self) -> bool: """Check if the filename contains substitution variables or Jinja expressions.""" return has_substitution_or_expression(self.file) + def should_load(self) -> bool: + """Evaluates the condition and returns True if the file should be loaded.""" + if self.condition is None: + return True + if isinstance(self.condition, bool): + return self.condition + if self.has_unresolved_condition(): + _raise_invalid( + f"Cannot evaluate include condition for '{self.file}' with unresolved substitutions: {self.condition}" + ) + value = self.condition.lower() + if value in TRUTHY_BOOL_STRINGS: + return True + if value not in FALSY_BOOL_STRINGS: + _raise_invalid( + f"Cannot convert include condition for '{self.file}' to a boolean, please use 'true' or 'false': {self.condition}" + ) + return False + + def has_unresolved_condition(self) -> bool: + """Check if the condition contains substitution variables or Jinja expressions.""" + return isinstance(self.condition, str) and has_substitution_or_expression( + self.condition + ) + def with_file(self, file: str) -> IncludeFile: """Clone this include with *file* as the filename.""" - return IncludeFile(self.parent_file, file, self.vars, self.yaml_loader) + return IncludeFile( + self.parent_file, + file, + self.yaml_loader, + vars=self.vars, + condition=self.condition, + ) + + def with_condition(self, condition: bool | str | None) -> IncludeFile: + """Clone this include with *condition* as the condition.""" + return IncludeFile( + self.parent_file, + self.file, + self.yaml_loader, + vars=self.vars, + condition=condition, + ) def _is_visible_path(rel: Path) -> bool: @@ -434,7 +482,7 @@ def force_load_include_files( if id(obj) in _seen: return _seen.add(id(obj)) - if obj.has_unresolved_expressions(): + if obj.has_unresolved_file(): _load_include_candidates( obj, warn_on_unresolved=warn_on_unresolved, @@ -578,14 +626,21 @@ def _resolve_merge_include(value: Any, node: yaml.Node, value_node: yaml.Node) - for _ in range(_MAX_MERGE_INCLUDE_DEPTH): if not isinstance(value, IncludeFile): break - if value.has_unresolved_expressions(): + if value.has_unresolved_file(): raise yaml.constructor.ConstructorError( "While constructing a mapping", node.start_mark, "Substitution in include filename with merge keys is not supported yet.", value_node.start_mark, ) - value = value.load() + if value.has_unresolved_condition(): + raise yaml.constructor.ConstructorError( + "While constructing a mapping", + node.start_mark, + "Substitution in include condition with merge keys is not supported yet.", + value_node.start_mark, + ) + value = value.load() if value.should_load() else {} else: raise yaml.constructor.ConstructorError( "While constructing a mapping", @@ -791,11 +846,11 @@ class ESPHomeLoaderMixin: @_add_data_ref def construct_include(self, node: yaml.Node) -> Any: - from esphome.const import CONF_VARS + from esphome.const import CONF_CONDITION, CONF_FILE, CONF_VARS - def extract_file_vars(node): + def extract_fields(node): fields = self.construct_yaml_map(node) - file = fields.get("file") + file = fields.get(CONF_FILE) if file is None: raise yaml.MarkedYAMLError("Must include 'file'", node.start_mark) if not isinstance(file, str): @@ -803,14 +858,21 @@ class ESPHomeLoaderMixin: "Include 'file' must be a string", node.start_mark ) vars = fields.get(CONF_VARS) - return file, vars + condition = fields.get(CONF_CONDITION) + if condition is not None and not isinstance(condition, (bool, str)): + raise yaml.MarkedYAMLError( + "Include 'condition' must be a boolean or string", node.start_mark + ) + return file, vars, condition if isinstance(node, yaml.nodes.MappingNode): - file, vars = extract_file_vars(node) + file, vars, condition = extract_fields(node) else: - file, vars = node.value, None + file, vars, condition = node.value, None, None - return IncludeFile(self.name, file, vars, self.yaml_loader) + return IncludeFile( + self.name, file, self.yaml_loader, vars=vars, condition=condition + ) # Directory includes (!include_dir_*) load eagerly during YAML parsing # because their paths are directory names, not individual files, and @@ -967,7 +1029,7 @@ def _load_yaml_internal(fname: Path, *, track_document_range: bool = True) -> An # Top-level !include returns a deferred IncludeFile; resolve it so # callers always receive the final content. if isinstance(res, IncludeFile): - res = res.load() + res = res.load() if res.should_load() else {} return res @@ -1358,8 +1420,12 @@ class ESPHomeDumper(yaml.SafeDumper): return self.represent_scalar(tag="!remove", value=value.value) def represent_include_file(self, value): - if value.vars: - mapping = {"file": value.file, "vars": value.vars} + if value.vars or value.condition is not None: + mapping = {"file": value.file} + if value.vars: + mapping["vars"] = value.vars + if value.condition is not None: + mapping["condition"] = value.condition return self.represent_mapping( tag="!include", mapping=mapping, flow_style=False ) diff --git a/tests/unit_tests/fixtures/substitutions/01-include.approved.yaml b/tests/unit_tests/fixtures/substitutions/01-include.approved.yaml index a812fedcfd..e13db65ef7 100644 --- a/tests/unit_tests/fixtures/substitutions/01-include.approved.yaml +++ b/tests/unit_tests/fixtures/substitutions/01-include.approved.yaml @@ -13,3 +13,10 @@ test_list: - a: alpha - b: beta - c: The value of C is $c + - {} + - values: + - var1: '1' + - a: alpha + - b: delta + - c: The value of C is saw + - {} diff --git a/tests/unit_tests/fixtures/substitutions/01-include.input.yaml b/tests/unit_tests/fixtures/substitutions/01-include.input.yaml index d3daa681a4..b30ae5f99e 100644 --- a/tests/unit_tests/fixtures/substitutions/01-include.input.yaml +++ b/tests/unit_tests/fixtures/substitutions/01-include.input.yaml @@ -13,3 +13,22 @@ test_list: file: inc1.yaml vars: b: "beta" + condition: true + - !include + file: inc1.yaml + vars: + b: "gamma" + c: "saw" + condition: false + - !include + file: inc1.yaml + vars: + b: "delta" + c: "saw" + condition: ${var1 == "1"} + - !include + file: inc1.yaml + vars: + b: "epsilon" + c: "saw" + condition: ${var1 == "2"} diff --git a/tests/unit_tests/fixtures/substitutions/12-yaml-merge.input.yaml b/tests/unit_tests/fixtures/substitutions/12-yaml-merge.input.yaml index a03e66e393..3152f8ddf0 100644 --- a/tests/unit_tests/fixtures/substitutions/12-yaml-merge.input.yaml +++ b/tests/unit_tests/fixtures/substitutions/12-yaml-merge.input.yaml @@ -7,4 +7,10 @@ test_list: file: inc3.yaml vars: y: 8 + condition: true + <<: !include + file: inc3.yaml + vars: + y: 10 + condition: false after: Content after diff --git a/tests/unit_tests/fixtures/yaml_util/includes/true.yaml b/tests/unit_tests/fixtures/yaml_util/includes/true.yaml new file mode 100644 index 0000000000..abd17b22f7 --- /dev/null +++ b/tests/unit_tests/fixtures/yaml_util/includes/true.yaml @@ -0,0 +1,2 @@ +--- +true diff --git a/tests/unit_tests/fixtures/yaml_util/includetest.yaml b/tests/unit_tests/fixtures/yaml_util/includetest.yaml index 7b01da6a17..6db5c294f4 100644 --- a/tests/unit_tests/fixtures/yaml_util/includetest.yaml +++ b/tests/unit_tests/fixtures/yaml_util/includetest.yaml @@ -11,8 +11,8 @@ esphome: # should be substituted as 'original', # not overwritten by vars in the !include above name: ${name} - name_add_mac_suffix: true - libraries: !include {file: includes/list.yaml, vars: {var1: Wire}} + name_add_mac_suffix: !include includes/true.yaml + libraries: !include {file: includes/list.yaml, vars: {var1: Wire}, condition: "true"} esp8266: - board: !include {file: includes/scalar.yaml, vars: {var1: nodemcu}} + board: !include {file: includes/scalar.yaml, vars: {var1: nodemcu}, condition: true} diff --git a/tests/unit_tests/fixtures/yaml_util/includetest_invalid_condition_type.yaml b/tests/unit_tests/fixtures/yaml_util/includetest_invalid_condition_type.yaml new file mode 100644 index 0000000000..94eb7d65ab --- /dev/null +++ b/tests/unit_tests/fixtures/yaml_util/includetest_invalid_condition_type.yaml @@ -0,0 +1,3 @@ +--- +esphome: + libraries: !include {file: includes/list.yaml, vars: {var1: Wire}, condition: 1} diff --git a/tests/unit_tests/fixtures/yaml_util/includetest_no_file.yaml b/tests/unit_tests/fixtures/yaml_util/includetest_no_file.yaml new file mode 100644 index 0000000000..7724bb301f --- /dev/null +++ b/tests/unit_tests/fixtures/yaml_util/includetest_no_file.yaml @@ -0,0 +1,3 @@ +--- +esphome: + libraries: !include {vars: {var1: Wire}} diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index 1abc7a3ab8..8a2510cf0a 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -1384,7 +1384,7 @@ def test_force_load_skips_duplicate_include_file() -> None: self.parent_file = Path("root.yaml") self.load_calls = 0 - def has_unresolved_expressions(self) -> bool: + def has_unresolved_file(self) -> bool: return False def load(self) -> dict[str, Any]: diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index b94a70443c..198b002d43 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -573,6 +573,65 @@ def test_undefined_variable_warning( assert "'undefined_var' is undefined" in caplog.text +def test_undefined_variable_warning_avoided_by_include_condition( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Compare two different ways to conditionally enable an !include file + that defines and uses new variables. One way generates a undefined + variable warning when disabled and the other does not.""" + include_file = tmp_path / "include.yaml" + include_file.write_text("substitutions:\n var: 1\nresult: ${var}\n") + method_a_file = tmp_path / "method_a.yaml" + method_a_file.write_text( + "packages:\n" + " - !include\n" + " file: include.yaml\n" + " condition: ${enable == 'y'}\n" + ) + method_b_file = tmp_path / "method_b.yaml" + method_b_file.write_text( + "substitutions:\n" + " content: !include include.yaml\n" + "packages:\n" + " - ${content if enable == 'y' else {}}\n" + ) + + def process_file(file, enable): + with caplog.at_level(logging.WARNING): + args = {"enable": enable} + config = yaml_util.load_yaml(file) + config = do_packages_pass(config, command_line_substitutions=args) + return substitutions.do_substitution_pass( + config, command_line_substitutions=args + ) + + # Case: method A with include enabled produces a result and no warnings + config = process_file(method_a_file, "y") + assert caplog.text == "" + assert config["packages"][0]["result"] == 1 + + # Case: method A with include disabled produces no result and no warnings + # because the file was not even loaded + config = process_file(method_a_file, "n") + assert caplog.text == "" + assert config["packages"][0] == {} + + # Case: method B with include enabled produces a result and no warnings (same as method A) + config = process_file(method_b_file, "y") + assert caplog.text == "" + assert config["packages"][0]["result"] == 1 + + # Case: method B with include disabled produces no result and an undefined variable warning + # because the file is loaded into a variable and its own variables cannot be expanded + # (different from method A) + config = process_file(method_b_file, "n") + assert ( + "Could not resolve substitution variable 'content': 'var' is undefined" + in caplog.text + ) + assert config["packages"][0] == {} + + def test_password_field_warnings_suppressed( caplog: pytest.LogCaptureFixture, ) -> None: @@ -717,11 +776,9 @@ def test_resolve_package_max_depth_exceeded(tmp_path: Path) -> None: # Each call to the loader returns a fresh IncludeFile pointing at itself, # so PACKAGE_SCHEMA always sees an IncludeFile and never a dict. def always_returns_include(path: Path) -> yaml_util.IncludeFile: - return yaml_util.IncludeFile(parent, path.name, None, always_returns_include) + return yaml_util.IncludeFile(parent, path.name, always_returns_include) - package_config = yaml_util.IncludeFile( - parent, "test.yaml", None, always_returns_include - ) + package_config = yaml_util.IncludeFile(parent, "test.yaml", always_returns_include) processor = _PackageProcessor({}, None) with pytest.raises( cv.Invalid, @@ -791,6 +848,22 @@ def test_include_filename_jinja_expression_with_path_separator( assert config["result"] == {"value": 42} +def test_include_condition_substitution_undefined_var(tmp_path: Path) -> None: + """!include with an undefined substitution variable raises cv.Invalid. + + The error message must reference the unresolved condition template so the + user knows which include failed, rather than seeing a bare file-not-found. + """ + main_file = tmp_path / "main.yaml" + main_file.write_text( + "result: !include\n file: file.yaml\n condition: ${undefined_var}\n" + ) + + config = yaml_util.load_yaml(main_file) + with pytest.raises(cv.Invalid, match=r"\$\{undefined_var\}"): + substitutions.do_substitution_pass(config) + + def test_raise_first_undefined_logs_extras_at_debug( caplog: pytest.LogCaptureFixture, ) -> None: @@ -838,7 +911,7 @@ def test_do_substitution_pass_included_substitutions_must_be_mapping( def loader(path: Path): return ["not", "a", "mapping"] - include = yaml_util.IncludeFile(parent, "subs.yaml", None, loader) + include = yaml_util.IncludeFile(parent, "subs.yaml", loader) config = OrderedDict({CONF_SUBSTITUTIONS: include}) with pytest.raises( @@ -861,7 +934,7 @@ def test_do_packages_pass_included_substitutions_must_be_mapping( def loader(path: Path): return ["not", "a", "mapping"] - include = yaml_util.IncludeFile(parent, "subs.yaml", None, loader) + include = yaml_util.IncludeFile(parent, "subs.yaml", loader) config = OrderedDict( { CONF_SUBSTITUTIONS: include, @@ -888,9 +961,7 @@ def test_resolve_package_undefined_var_in_include_filename(tmp_path: Path) -> No def loader(path: Path): raise EsphomeError(f"Error reading file {path}: No such file") - package_config = yaml_util.IncludeFile( - parent, "${undefined_var}.yaml", None, loader - ) + package_config = yaml_util.IncludeFile(parent, "${undefined_var}.yaml", loader) processor = _PackageProcessor({}, None) with pytest.raises(cv.Invalid, match="unresolved substitutions"): processor.resolve_package(package_config, substitutions.ContextVars(), []) @@ -906,7 +977,7 @@ def test_resolve_include_error_shows_expanded_from_when_substituted( def failing_loader(_path: Path) -> None: raise EsphomeError("File not found") - include = yaml_util.IncludeFile(parent, "${device}.yaml", None, failing_loader) + include = yaml_util.IncludeFile(parent, "${device}.yaml", failing_loader) context = substitutions.ContextVars({"device": "my_device"}) with pytest.raises(cv.Invalid) as exc_info: @@ -927,7 +998,7 @@ def test_resolve_include_error_no_expanded_from_for_literal_filename( def failing_loader(_path: Path) -> None: raise EsphomeError("File not found") - include = yaml_util.IncludeFile(parent, "literal.yaml", None, failing_loader) + include = yaml_util.IncludeFile(parent, "literal.yaml", failing_loader) with pytest.raises(cv.Invalid) as exc_info: substitutions.resolve_include(include, [], substitutions.ContextVars()) @@ -946,7 +1017,7 @@ def test_include_vars_applied_to_lambda_value(tmp_path: Path) -> None: included.write_text('!lambda |-\n return "${foo}";\n') include = yaml_util.IncludeFile( - tmp_path / "main.yaml", "lambda.yaml", {"foo": "bar"}, yaml_util.load_yaml + tmp_path / "main.yaml", "lambda.yaml", yaml_util.load_yaml, vars={"foo": "bar"} ) config = OrderedDict({"value": include.load()}) result = substitutions.do_substitution_pass(config) diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index f3f73ea4dc..e1c3080e29 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -43,17 +43,37 @@ def clear_core_frontmatter() -> None: core.CORE.frontmatter = {} -def test_include_with_vars(fixture_path: Path) -> None: +def test_include(fixture_path: Path) -> None: + """Test !include with and without vars, with and without conditions""" yaml_file = fixture_path / "yaml_util" / "includetest.yaml" actual = yaml_util.load_yaml(yaml_file) actual = substitutions.do_substitution_pass(actual, None) assert actual["esphome"]["name"] == "original" + assert actual["esphome"]["name_add_mac_suffix"] assert actual["esphome"]["libraries"][0] == "Wire" assert actual["esp8266"]["board"] == "nodemcu" assert actual["wifi"]["ssid"] == "my_custom_ssid" +def test_include_with_no_file(fixture_path: Path) -> None: + """Ensure that an error is emitted when the file field is missing.""" + yaml_file = fixture_path / "yaml_util" / "includetest_no_file.yaml" + + with pytest.raises(EsphomeError, match=r"Must include 'file'"): + yaml_util.load_yaml(yaml_file) + + +def test_include_with_invalid_condition_type(fixture_path: Path) -> None: + """Ensure that an error is emitted when the condition field is an invalid type.""" + yaml_file = fixture_path / "yaml_util" / "includetest_invalid_condition_type.yaml" + + with pytest.raises( + EsphomeError, match=r"Include 'condition' must be a boolean or string" + ): + yaml_util.load_yaml(yaml_file) + + def test_loading_a_missing_file(fixture_path): """We throw EsphomeError when loading a missing file.""" yaml_file = fixture_path / "yaml_util" / "missing.yaml" @@ -64,7 +84,7 @@ def test_loading_a_missing_file(fixture_path): def test_parsing_with_custom_loader(fixture_path): """Test custom loader used for vscode connection - Default loader is tested in test_include_with_vars + Default loader is tested in test_include """ yaml_file = fixture_path / "yaml_util" / "includetest.yaml" @@ -78,10 +98,11 @@ def test_parsing_with_custom_loader(fixture_path): # substitute config to expand includes: substitutions.substitute(config, [], substitutions.ContextVars(), False) - assert len(loader_calls) == 3 + assert len(loader_calls) == 4 assert loader_calls[0].parts[-2:] == ("includes", "included.yaml") - assert loader_calls[1].parts[-2:] == ("includes", "list.yaml") - assert loader_calls[2].parts[-2:] == ("includes", "scalar.yaml") + assert loader_calls[1].parts[-2:] == ("includes", "true.yaml") + assert loader_calls[2].parts[-2:] == ("includes", "list.yaml") + assert loader_calls[3].parts[-2:] == ("includes", "scalar.yaml") def test_construct_secret_simple(fixture_path: Path) -> None: @@ -570,7 +591,7 @@ def test_represent_remove() -> None: def test_represent_include_file() -> None: """Test that IncludeFile objects are dumped as !include scalars.""" include = yaml_util.IncludeFile( - Path("/fake/main.yaml"), "path/to/file.yaml", None, lambda _: {} + Path("/fake/main.yaml"), "path/to/file.yaml", lambda _: {} ) assert yaml_util.dump({"key": include}) == "key: !include 'path/to/file.yaml'\n" @@ -580,13 +601,27 @@ def test_represent_include_file_with_vars() -> None: include = yaml_util.IncludeFile( Path("/fake/main.yaml"), "path/to/file.yaml", - {"key": "value"}, lambda _: {}, + vars={"key": "value"}, + ) + assert ( + yaml_util.dump({"key": include}) + == "key: !include\n file: path/to/file.yaml\n vars:\n key: value\n" + ) + + +def test_represent_include_file_with_condition() -> None: + """Test that IncludeFile with condition is dumped as !include mapping form.""" + include = yaml_util.IncludeFile( + Path("/fake/main.yaml"), + "path/to/file.yaml", + lambda _: {}, + condition="true", + ) + assert ( + yaml_util.dump({"key": include}) + == "key: !include\n file: path/to/file.yaml\n condition: 'true'\n" ) - result = yaml_util.dump({"key": include}) - assert "!include" in result - assert "file: path/to/file.yaml" in result - assert "key: value" in result def test_represent_include_file_with_data_base_mixin() -> None: @@ -596,7 +631,7 @@ def test_represent_include_file_with_data_base_mixin() -> None: subclass. add_multi_representer must match this subclass through the MRO. """ include = yaml_util.IncludeFile( - Path("/fake/main.yaml"), "common/spi.yaml", None, lambda _: {} + Path("/fake/main.yaml"), "common/spi.yaml", lambda _: {} ) wrapped = yaml_util.make_data_base(include) assert isinstance(wrapped, yaml_util.ESPHomeDataBase) @@ -609,7 +644,7 @@ def test_represent_include_file_with_data_base_mixin() -> None: def test_include_file_repr(tmp_path: Path) -> None: """repr() includes the filename so it appears usefully in error messages.""" parent = tmp_path / "main.yaml" - include = yaml_util.IncludeFile(parent, "some/nested.yaml", None, lambda _: {}) + include = yaml_util.IncludeFile(parent, "some/nested.yaml", lambda _: {}) assert repr(include) == "IncludeFile(some/nested.yaml)" @@ -624,7 +659,7 @@ def test_include_file_load_caches_result(tmp_path: Path) -> None: call_count += 1 return content - include = yaml_util.IncludeFile(parent, "child.yaml", None, counting_loader) + include = yaml_util.IncludeFile(parent, "child.yaml", counting_loader) first = include.load() second = include.load() @@ -641,7 +676,7 @@ def test_include_file_load_caches_none_result(tmp_path: Path) -> None: nonlocal call_count call_count += 1 - include = yaml_util.IncludeFile(parent, "empty.yaml", None, counting_loader) + include = yaml_util.IncludeFile(parent, "empty.yaml", counting_loader) first = include.load() second = include.load() @@ -653,7 +688,7 @@ def test_include_file_load_caches_none_result(tmp_path: Path) -> None: def test_include_file_load_raises_on_unresolved_expressions(tmp_path: Path) -> None: """load() raises if the filename contains unresolved substitutions or expressions.""" parent = tmp_path / "main.yaml" - include = yaml_util.IncludeFile(parent, "${undefined_var}.yaml", None, lambda _: {}) + include = yaml_util.IncludeFile(parent, "${undefined_var}.yaml", lambda _: {}) with pytest.raises(cv.Invalid, match="unresolved"): include.load() @@ -670,13 +705,83 @@ def test_include_file_load_raises_on_unresolved_expressions(tmp_path: Path) -> N ("price-100$.yaml", False), # $ at end, not followed by valid substitution ], ) -def test_include_file_has_unresolved_expressions( +def test_include_file_has_unresolved_file( tmp_path: Path, filename: str, expected: bool ) -> None: - """has_unresolved_expressions() detects substitution patterns in the filename.""" + """has_unresolved_file() detects substitution patterns in the filename.""" parent = tmp_path / "main.yaml" - include = yaml_util.IncludeFile(parent, filename, None, lambda _: {}) - assert include.has_unresolved_expressions() == expected + include = yaml_util.IncludeFile(parent, filename, lambda _: {}) + assert include.has_unresolved_file() == expected + + +@pytest.mark.parametrize( + ("condition", "expected"), + [ + (None, False), + (True, False), + (False, False), + ("true", False), + ("false", False), + ("$has_feature", True), # whole substitution + ("${has_feature}", True), # whole substitution + ("tr$ue", True), # partial substitution + ("$.", False), # malformed substitution + ("${1 == 1}", True), # Jinja expression + ("${", False), # malformed expression + ], +) +def test_include_file_has_unresolved_condition( + tmp_path: Path, condition: bool | str | None, expected: bool +) -> None: + """has_unresolved_condition() detects substitution patterns in the condition.""" + parent = tmp_path / "main.yaml" + include = yaml_util.IncludeFile( + parent, "device.yaml", lambda _: {}, condition=condition + ) + assert include.has_unresolved_condition() == expected + + +@pytest.mark.parametrize( + ("condition", "expected_result_or_error"), + [ + (None, True), + (True, True), + (False, False), + ("true", True), + ("True", True), + ("TRUE", True), + ("yes", True), + ("on", True), + ("enable", True), + ("false", False), + ("False", False), + ("FALSE", False), + ("no", False), + ("off", False), + ("disable", False), + ( + "$sub", + "Cannot evaluate include condition for 'device.yaml' with unresolved substitutions", + ), + ("", "Cannot convert include condition for 'device.yaml' to a boolean"), + ("trues", "Cannot convert include condition for 'device.yaml' to a boolean"), + ], +) +def test_include_file_should_load( + tmp_path: Path, + condition: bool | str | None, + expected_result_or_error: bool | str, +) -> None: + """should_load() evaluates the condition and raises an error if it is malformed.""" + parent = tmp_path / "main.yaml" + include = yaml_util.IncludeFile( + parent, "device.yaml", lambda _: {}, condition=condition + ) + if isinstance(expected_result_or_error, bool): + assert include.should_load() == expected_result_or_error + else: + with pytest.raises(cv.Invalid, match=expected_result_or_error): + include.should_load() def test_mapping_include_non_string_file_rejected(tmp_path: Path) -> None: @@ -691,16 +796,16 @@ def test_include_file_templated_filename_stays_raw_string(tmp_path: Path) -> Non """A templated filename keeps its verbatim text (issue #18545).""" parent = tmp_path / "main.yaml" expr = '${ "bluetooth/proxy.yaml" if enable_bluetooth_proxy else "../empty.yaml" }' - include = yaml_util.IncludeFile(parent, expr, None, lambda _: {}) + include = yaml_util.IncludeFile(parent, expr, lambda _: {}) assert include.file == expr - assert include.has_unresolved_expressions() + assert include.has_unresolved_file() assert repr(include) == f"IncludeFile({expr})" def test_represent_include_file_templated() -> None: """Dumping a templated IncludeFile emits the raw expression unchanged.""" expr = '${ "a/b.yaml" if flag else "../c.yaml" }' - include = yaml_util.IncludeFile(Path("/fake/main.yaml"), expr, None, lambda _: {}) + include = yaml_util.IncludeFile(Path("/fake/main.yaml"), expr, lambda _: {}) assert yaml_util.dump({"key": include}) == f"key: !include '{expr}'\n" @@ -710,12 +815,12 @@ def test_include_in_list_context() -> None: parent = Path("/fake/main.yaml") # The nested IncludeFile resolves to a plain string value - inner = yaml_util.IncludeFile(parent, "inner.yaml", None, lambda _: "gamma") + inner = yaml_util.IncludeFile(parent, "inner.yaml", lambda _: "gamma") # The outer IncludeFile returns a list whose last element is itself an IncludeFile, # exercising the substitution pass's ability to recurse into loaded content. outer = yaml_util.IncludeFile( - parent, "items.yaml", None, lambda _: ["alpha", "beta", inner] + parent, "items.yaml", lambda _: ["alpha", "beta", inner] ) config = OrderedDict({"values": outer}) @@ -736,15 +841,48 @@ def test_top_level_include_resolved_by_load_yaml(tmp_path: Path) -> None: assert result["key"] == "value" +@pytest.mark.parametrize( + ("condition", "expected_result_or_error"), + [ + ("true", True), + ("false", False), + ('"TRUE"', True), + ('"FALSE"', False), + ('"x"', "Cannot convert include condition for 'child.yaml' to a boolean"), + ( + "$sub", + "Cannot evaluate include condition for 'child.yaml' with unresolved substitutions", + ), + ], +) +def test_top_level_include_with_condition_resolved_by_load_yaml( + tmp_path: Path, condition: bool | str | None, expected_result_or_error: bool | str +) -> None: + """load_yaml evaluates the condition for a top-level !include.""" + child = tmp_path / "child.yaml" + child.write_text("key: value\n") + main = tmp_path / "main.yaml" + main.write_text(f"!include {{ file: child.yaml, condition: {condition} }}\n") + + if isinstance(expected_result_or_error, bool): + result = yaml_util.load_yaml(main) + assert isinstance(result, dict) + if expected_result_or_error: + assert result["key"] == "value" + else: + assert result == {} + else: + with pytest.raises(cv.Invalid, match=expected_result_or_error): + result = yaml_util.load_yaml(main) + + def test_include_plain_filename_loads_after_deferred_refactor() -> None: """!include with a plain filename (no $ expressions) still loads correctly. Regression guard: the deferred-loading refactor must not break the simple case. """ parent = Path("/fake/main.yaml") - include = yaml_util.IncludeFile( - parent, "child.yaml", None, lambda _: {"answer": 42} - ) + include = yaml_util.IncludeFile(parent, "child.yaml", lambda _: {"answer": 42}) config = OrderedDict({"result": include}) config = substitutions.do_substitution_pass(config) @@ -756,7 +894,23 @@ def test_yaml_merge_include_with_filename_substitution_raises() -> None: """<<: !include ${expr} raises a clear error — substitutions in merge-key filenames are not yet supported, and the error message must say so.""" yaml_text = "base:\n existing: value\n <<: !include ${filename}.yaml\n" - with pytest.raises(EsphomeError, match="not supported yet"): + with pytest.raises( + EsphomeError, + match="Substitution in include filename with merge keys is not supported yet", + ): + yaml_util.parse_yaml( + Path("/fake/main.yaml"), io.StringIO(yaml_text), lambda _: {} + ) + + +def test_yaml_merge_include_with_condition_substitution_raises() -> None: + """<<: !include { file: ${expr}, condition: {} } raises a clear error — substitutions in merge-key conditions + are not yet supported, and the error message must say so.""" + yaml_text = "base:\n existing: value\n <<: !include\n file: filename.yaml\n condition: ${expr}\n" + with pytest.raises( + EsphomeError, + match="Substitution in include condition with merge keys is not supported yet", + ): yaml_util.parse_yaml( Path("/fake/main.yaml"), io.StringIO(yaml_text), lambda _: {} ) @@ -775,8 +929,8 @@ def test_yaml_merge_chain_include_resolves() -> None: """Chained includes in merge keys resolve through multiple IncludeFile layers.""" parent = Path("/fake/main.yaml") - inner = yaml_util.IncludeFile(parent, "inner.yaml", None, lambda _: {"x": 1}) - outer = yaml_util.IncludeFile(parent, "outer.yaml", None, lambda _: inner) + inner = yaml_util.IncludeFile(parent, "inner.yaml", lambda _: {"x": 1}) + outer = yaml_util.IncludeFile(parent, "outer.yaml", lambda _: inner) yaml_text = "base:\n existing: value\n <<: !include outer.yaml\n" config = yaml_util.parse_yaml(parent, io.StringIO(yaml_text), lambda _: outer) @@ -791,7 +945,7 @@ def test_yaml_merge_chain_include_depth_exceeded() -> None: parent = Path("/fake/main.yaml") def self_referencing_loader(path: Path) -> yaml_util.IncludeFile: - return yaml_util.IncludeFile(parent, path.name, None, self_referencing_loader) + return yaml_util.IncludeFile(parent, path.name, self_referencing_loader) yaml_text = "base:\n <<: !include loop.yaml\n" with pytest.raises(EsphomeError, match="Maximum include chain depth"): @@ -1061,7 +1215,7 @@ class _StubInclude: self._raise = raise_on_load self.load_calls = 0 - def has_unresolved_expressions(self) -> bool: + def has_unresolved_file(self) -> bool: return self._unresolved def load(self) -> object: