From 8423a0d9734a61bf5b2511b628994abd59f1bc60 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Thu, 18 Sep 2025 18:29:59 +0000 Subject: [PATCH 1/9] Enhance check_config script with JSON output and comprehensive tests - Add JSON output format support to check_config.py - Implement command-line flags for better script control - Add extensive test coverage for flag interactions and JSON output - Improve argument parsing to properly handle script arguments - Refactor test file for better readability and maintainability - Fix ruff formatting and linting issues --- homeassistant/scripts/check_config.py | 38 +- tests/scripts/test_check_config.py | 735 ++++++++++++++++++++++++++ 2 files changed, 769 insertions(+), 4 deletions(-) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 213a45a48e9092..d509f3762c1aad 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -7,6 +7,7 @@ from collections import OrderedDict from collections.abc import Callable, Mapping, Sequence from glob import glob +import json import logging import os from typing import Any @@ -82,17 +83,42 @@ def run(script_args: list) -> int: parser.add_argument( "-s", "--secrets", action="store_true", help="Show secret information" ) + parser.add_argument("--json", action="store_true", help="Output JSON format") + parser.add_argument( + "--fail-on-warnings", + action="store_true", + help="Exit non-zero if warnings are present", + ) - args, unknown = parser.parse_known_args() + args, unknown = parser.parse_known_args(script_args) if unknown: print(color("red", "Unknown arguments:", ", ".join(unknown))) config_dir = os.path.join(os.getcwd(), args.config) - print(color("bold", "Testing configuration at", config_dir)) - res = check(config_dir, args.secrets) + # JSON output branch + if args.json: + json_object = { + "config_dir": config_dir, + "total_errors": sum(len(errors) for errors in res["except"].values()), + "total_warnings": sum(len(warnings) for warnings in res["warn"].values()), + "errors": res["except"], + "warnings": res["warn"], + "components": list(res["components"].keys()), + } + print(json.dumps(json_object, indent=2)) + + # Determine exit code for JSON mode + exit_code = len(res["except"]) + if args.fail_on_warnings and res["warn"]: + exit_code = max(exit_code, 1) + return exit_code + + # Human-readable output starts here + print(color("bold", "Testing configuration at", config_dir)) + domain_info: list[str] = [] if args.info: domain_info = args.info.split(",") @@ -165,7 +191,11 @@ def run(script_args: list) -> int: continue print(" -", skey + ":", sval) - return len(res["except"]) + # Determine final exit code + exit_code = len(res["except"]) + if args.fail_on_warnings and res["warn"]: + exit_code = max(exit_code, 1) + return exit_code def check(config_dir, secrets=False): diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index 2bb58cd4d6842c..9520aa2914db50 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -1,6 +1,8 @@ """Test check_config script.""" +import json import logging +import os from unittest.mock import patch import pytest @@ -180,3 +182,736 @@ def test_bootstrap_error() -> None: assert res["secrets"] == {} assert res["warn"] == {} assert res["yaml_files"] == {} + + +# New tests for JSON and fail-on-warnings functionality + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_backwards_compatibility_no_flags() -> None: + """Test that run() with no flags maintains backwards compatibility.""" + # Test with valid config + exit_code = check_config.run([]) + assert exit_code == 0 + + # Test with config that has warnings + with patch.object(check_config, "check") as mock_check: + mock_check.return_value = { + "except": {}, + "warn": {"light": ["warning message"]}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + # Without --fail-on-warnings, warnings should not affect exit code + exit_code = check_config.run([]) + assert exit_code == 0 + + # Test with config that has errors + with patch.object(check_config, "check") as mock_check: + mock_check.return_value = { + "except": {"homeassistant": ["error message"]}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + exit_code = check_config.run([]) + assert exit_code == 1 # len(res["except"]) = 1 domain with errors + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_json_flag_only() -> None: + """Test that --json flag works independently.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {"domain1": ["error1", "error2"]}, + "warn": {"domain2": ["warning1"]}, + "components": {"homeassistant": {}, "light": {}, "http": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--json"]) + + # Should exit with code 1 (1 domain with errors) + assert exit_code == 1 + + # Should have printed JSON + assert mock_print.call_count == 1 + json_output = mock_print.call_args[0][0] + + # Verify it's valid JSON + parsed_json = json.loads(json_output) + + # Verify JSON structure + assert "config_dir" in parsed_json + assert "total_errors" in parsed_json + assert "total_warnings" in parsed_json + assert "errors" in parsed_json + assert "warnings" in parsed_json + assert "components" in parsed_json + + # Verify JSON content + assert parsed_json["total_errors"] == 2 # 2 error messages + assert parsed_json["total_warnings"] == 1 # 1 warning message + assert parsed_json["errors"] == {"domain1": ["error1", "error2"]} + assert parsed_json["warnings"] == {"domain2": ["warning1"]} + assert set(parsed_json["components"]) == {"homeassistant", "light", "http"} + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_fail_on_warnings_flag_only() -> None: + """Test that --fail-on-warnings flag works independently.""" + # Test with warnings only + with patch.object(check_config, "check") as mock_check: + mock_check.return_value = { + "except": {}, + "warn": {"light": ["warning message"]}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--fail-on-warnings"]) + assert exit_code == 1 # Should exit non-zero due to warnings + + # Test with no warnings or errors + with patch.object(check_config, "check") as mock_check: + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--fail-on-warnings"]) + assert exit_code == 0 # Should exit zero when no warnings/errors + + # Test with both errors and warnings + with patch.object(check_config, "check") as mock_check: + mock_check.return_value = { + "except": {"domain1": ["error"]}, + "warn": {"domain2": ["warning"]}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--fail-on-warnings"]) + assert exit_code == 1 # max(1, 1) = 1 + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_both_flags_combined() -> None: + """Test that both flags work together correctly.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + # Test with warnings only + mock_check.return_value = { + "except": {}, + "warn": {"light": ["warning message"]}, + "components": {"homeassistant": {}, "light": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--json", "--fail-on-warnings"]) + + # Should exit with code 1 due to --fail-on-warnings and warnings present + assert exit_code == 1 + + # Should have printed JSON + assert mock_print.call_count == 1 + json_output = mock_print.call_args[0][0] + parsed_json = json.loads(json_output) + + # Verify JSON content + assert parsed_json["total_errors"] == 0 + assert parsed_json["total_warnings"] == 1 + assert parsed_json["warnings"] == {"light": ["warning message"]} + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_json_output_structure() -> None: + """Test JSON output contains all required fields with correct types.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {"domain1": ["error1", {"config": "bad"}]}, + "warn": {"domain2": ["warning1", {"config": "deprecated"}]}, + "components": {"homeassistant": {}, "light": {}, "automation": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--json", "--config", "/test/path"]) + + json_output = mock_print.call_args[0][0] + parsed_json = json.loads(json_output) + + # Should exit with code 1 due to errors + assert exit_code == 1 + + # Test all required fields are present + required_fields = [ + "config_dir", + "total_errors", + "total_warnings", + "errors", + "warnings", + "components", + ] + for field in required_fields: + assert field in parsed_json, f"Missing required field: {field}" + + # Test field types and values + assert isinstance(parsed_json["config_dir"], str) + assert isinstance(parsed_json["total_errors"], int) + assert isinstance(parsed_json["total_warnings"], int) + assert isinstance(parsed_json["errors"], dict) + assert isinstance(parsed_json["warnings"], dict) + assert isinstance(parsed_json["components"], list) + + # Test counts are correct + assert parsed_json["total_errors"] == 2 # 2 items in domain1 list + assert parsed_json["total_warnings"] == 2 # 2 items in domain2 list + + # Test components is a list of strings + assert all(isinstance(comp, str) for comp in parsed_json["components"]) + assert set(parsed_json["components"]) == { + "homeassistant", + "light", + "automation", + } + + +def test_run_exit_code_logic() -> None: + """Test exit code logic for all flag combinations.""" + test_cases = [ + # (errors, warnings, flags, expected_exit_code) + ({}, {}, [], 0), # No errors, no warnings, no flags + ({}, {}, ["--json"], 0), # No errors, no warnings, json only + ( + {}, + {}, + ["--fail-on-warnings"], + 0, + ), # No errors, no warnings, fail-on-warnings only + ( + {}, + {}, + ["--json", "--fail-on-warnings"], + 0, + ), # No errors, no warnings, both flags + ( + {}, + {"domain": ["warning"]}, + [], + 0, + ), # Warnings only, no flags (backwards compatible) + ({}, {"domain": ["warning"]}, ["--json"], 0), # Warnings only, json only + ( + {}, + {"domain": ["warning"]}, + ["--fail-on-warnings"], + 1, + ), # Warnings only, fail-on-warnings + ( + {}, + {"domain": ["warning"]}, + ["--json", "--fail-on-warnings"], + 1, + ), # Warnings only, both flags + ({"domain": ["error"]}, {}, [], 1), # Errors only, no flags + ({"domain": ["error"]}, {}, ["--json"], 1), # Errors only, json only + ( + {"domain": ["error"]}, + {}, + ["--fail-on-warnings"], + 1, + ), # Errors only, fail-on-warnings + ( + {"domain": ["error"]}, + {}, + ["--json", "--fail-on-warnings"], + 1, + ), # Errors only, both flags + ({"domain": ["error"]}, {"domain2": ["warning"]}, [], 1), # Both, no flags + ( + {"domain": ["error"]}, + {"domain2": ["warning"]}, + ["--json"], + 1, + ), # Both, json only + ( + {"domain": ["error"]}, + {"domain2": ["warning"]}, + ["--fail-on-warnings"], + 1, + ), # Both, fail-on-warnings + ( + {"domain": ["error"]}, + {"domain2": ["warning"]}, + ["--json", "--fail-on-warnings"], + 1, + ), # Both, both flags + ({"d1": ["e1"], "d2": ["e2"]}, {}, [], 2), # Multiple error domains, no flags + ( + {"d1": ["e1"], "d2": ["e2"]}, + {"d3": ["w1"]}, + ["--fail-on-warnings"], + 2, + ), # Multiple errors + warnings + ] + + for errors, warnings, flags, expected_exit in test_cases: + with patch("builtins.print"), patch.object(check_config, "check") as mock_check: + mock_check.return_value = { + "except": errors, + "warn": warnings, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(flags) + assert exit_code == expected_exit, ( + f"Failed for errors={errors}, warnings={warnings}, flags={flags}. " + f"Expected {expected_exit}, got {exit_code}" + ) + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_json_no_human_readable_output() -> None: + """Test that JSON mode doesn't include human-readable messages.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + check_config.run(["--json"]) + + # Should only print once (the JSON output) + assert mock_print.call_count == 1 + + # The output should be valid JSON + json_output = mock_print.call_args[0][0] + json.loads(json_output) # Validate it's valid JSON + + # Should not contain human-readable messages like "Testing configuration at" + assert "Testing configuration at" not in json_output + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_run_human_readable_still_works() -> None: + """Test that human-readable output still works without JSON flag.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + check_config.run([]) + + # Should print the "Testing configuration at" message + printed_outputs = [ + call[0][0] if call[0] else "" for call in mock_print.call_args_list + ] + testing_message_found = any( + "Testing configuration at" in output for output in printed_outputs + ) + assert testing_message_found, ( + "Human-readable 'Testing configuration at' message not found" + ) + + +def test_run_with_config_path() -> None: + """Test that config path is correctly included in JSON output.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + test_config_path = "/custom/config/path" + check_config.run(["--json", "--config", test_config_path]) + + json_output = mock_print.call_args[0][0] + parsed_json = json.loads(json_output) + + # The config_dir should include the full path + expected_path = os.path.join(os.getcwd(), test_config_path) + assert parsed_json["config_dir"] == expected_path + + +# Flag Interaction Tests + + +def test_flag_order_independence() -> None: + """Test that flag order doesn't affect behavior.""" + with ( + patch("builtins.print") as mock_print1, + patch.object(check_config, "check") as mock_check1, + ): + mock_check1.return_value = { + "except": {"domain1": ["error1"]}, + "warn": {"domain2": ["warning1"]}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code1 = check_config.run(["--json", "--fail-on-warnings"]) + + with ( + patch("builtins.print") as mock_print2, + patch.object(check_config, "check") as mock_check2, + ): + mock_check2.return_value = { + "except": {"domain1": ["error1"]}, + "warn": {"domain2": ["warning1"]}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code2 = check_config.run(["--fail-on-warnings", "--json"]) + + # Both should have same exit code and JSON output + assert exit_code1 == exit_code2 == 1 + assert mock_print1.call_count == mock_print2.call_count == 1 + + json_output1 = json.loads(mock_print1.call_args[0][0]) + json_output2 = json.loads(mock_print2.call_args[0][0]) + assert json_output1 == json_output2 + + +def test_unknown_arguments_with_json() -> None: + """Test that unknown arguments are handled properly with JSON flag.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + check_config.run(["--json", "--unknown-flag", "value"]) + + # Should still print unknown argument warning AND JSON + assert mock_print.call_count == 2 + + # First call should be the unknown argument warning + unknown_warning = mock_print.call_args_list[0][0][0] + assert "Unknown arguments" in unknown_warning + assert "unknown-flag" in unknown_warning + + # Second call should be valid JSON + json_output = mock_print.call_args_list[1][0][0] + parsed_json = json.loads(json_output) + assert "config_dir" in parsed_json + + +def test_empty_script_args() -> None: + """Test that empty arguments don't crash the script.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + # Should not crash and should use default behavior (human-readable) + exit_code = check_config.run([]) + assert exit_code == 0 + + # Should print at least the "Testing configuration at..." message + assert mock_print.call_count >= 1 + + # First call should be the header message + first_call = mock_print.call_args_list[0][0][0] + assert "Testing configuration at" in first_call + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_all_flags_together() -> None: + """Test behavior when multiple flags are used together.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {"light": ["warning message"]}, + "components": {"homeassistant": {}, "light": {}}, + "secrets": {"test_secret": "test_value"}, + "secret_cache": {"secrets.yaml": {"test_secret": "test_value"}}, + "yaml_files": {"/config/configuration.yaml": True}, + } + + # Test with --json, --fail-on-warnings, --secrets, and --files together + exit_code = check_config.run( + ["--json", "--fail-on-warnings", "--secrets", "--files"] + ) + + # Should exit with code 1 due to warnings + fail-on-warnings + assert exit_code == 1 + + # Should only print JSON (secrets and files should be ignored in JSON mode) + assert mock_print.call_count == 1 + + json_output = json.loads(mock_print.call_args[0][0]) + assert json_output["total_warnings"] == 1 + assert "light" in json_output["warnings"] + + +@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) +@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") +def test_info_flag_with_json() -> None: + """Test how --info flag interacts with --json.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}, "light": {"platform": "demo"}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + # Test --json with --info - JSON should take precedence + exit_code = check_config.run(["--json", "--info", "light"]) + + assert exit_code == 0 + assert mock_print.call_count == 1 + + # Should be JSON output, not info output + json_output = json.loads(mock_print.call_args[0][0]) + assert "config_dir" in json_output + assert "components" in json_output + assert "light" in json_output["components"] + + +def test_config_flag_variations() -> None: + """Test different ways to specify config directory.""" + test_cases = [ + (["-c", "/test/path"], "/test/path"), + (["--config", "/test/path"], "/test/path"), + (["--json", "-c", "relative/path"], "relative/path"), + (["--config", ".", "--json"], "."), + ] + + for flags, expected_config_part in test_cases: + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + check_config.run(flags) + + if "--json" in flags: + json_output = json.loads(mock_print.call_args[0][0]) + expected_full_path = os.path.join(os.getcwd(), expected_config_part) + assert json_output["config_dir"] == expected_full_path + + +def test_flag_case_sensitivity() -> None: + """Test that flags are case sensitive (negative test).""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + # Uppercase flags should be treated as unknown arguments + check_config.run(["--JSON", "--FAIL-ON-WARNINGS"]) + + # Should print unknown arguments warning and then human-readable output + assert mock_print.call_count > 1 + + # First call should contain unknown argument warning + first_call = mock_print.call_args_list[0][0][0] + assert "Unknown arguments" in first_call + assert "JSON" in first_call + + +def test_multiple_config_flags() -> None: + """Test behavior with multiple config directory specifications.""" + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": {}, + "warn": {}, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + # Last config flag should win + check_config.run( + ["--json", "--config", "/first/path", "--config", "/second/path"] + ) + + json_output = json.loads(mock_print.call_args[0][0]) + expected_path = os.path.join(os.getcwd(), "/second/path") + assert json_output["config_dir"] == expected_path + + +def test_json_with_errors_and_warnings_combinations() -> None: + """Test JSON output with various error/warning combinations.""" + test_scenarios = [ + # (errors, warnings, expected_exit_code) + ({}, {}, 0), + ({"domain1": ["error"]}, {}, 1), + ({}, {"domain1": ["warning"]}, 0), # Without --fail-on-warnings + ({"d1": ["e1"]}, {"d2": ["w1"]}, 1), # Errors take precedence + ({"d1": ["e1"], "d2": ["e2"]}, {}, 2), # Multiple error domains + ( + {"d1": ["e1", "e2"]}, + {"d2": ["w1", "w2"]}, + 1, + ), # Multiple errors in one domain = 1 + ] + + for errors, warnings, expected_exit in test_scenarios: + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": errors, + "warn": warnings, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--json"]) + assert exit_code == expected_exit + + json_output = json.loads(mock_print.call_args[0][0]) + assert json_output["total_errors"] == sum(len(e) for e in errors.values()) + assert json_output["total_warnings"] == sum( + len(w) for w in warnings.values() + ) + assert json_output["errors"] == errors + assert json_output["warnings"] == warnings + + +def test_fail_on_warnings_with_json_combinations() -> None: + """Test --fail-on-warnings with --json in various scenarios.""" + test_scenarios = [ + # (errors, warnings, expected_exit_code) + ({}, {}, 0), + ({"domain1": ["error"]}, {}, 1), + ({}, {"domain1": ["warning"]}, 1), # With --fail-on-warnings + ({"d1": ["e1"]}, {"d2": ["w1"]}, 1), # Errors still take precedence + ({"d1": ["e1"], "d2": ["e2"]}, {"d3": ["w1"]}, 2), # Multiple errors > warnings + ] + + for errors, warnings, expected_exit in test_scenarios: + with ( + patch("builtins.print") as mock_print, + patch.object(check_config, "check") as mock_check, + ): + mock_check.return_value = { + "except": errors, + "warn": warnings, + "components": {"homeassistant": {}}, + "secrets": {}, + "secret_cache": {}, + "yaml_files": {}, + } + + exit_code = check_config.run(["--json", "--fail-on-warnings"]) + assert exit_code == expected_exit + + # Should still output valid JSON + json_output = json.loads(mock_print.call_args[0][0]) + assert json_output["total_errors"] == sum(len(e) for e in errors.values()) + assert json_output["total_warnings"] == sum( + len(w) for w in warnings.values() + ) From 0f83d8f44de73b0c263e864ba57ea8c6922db599 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Thu, 18 Sep 2025 19:12:48 +0000 Subject: [PATCH 2/9] Remove redundant print statements from check_config script and clean up test file --- homeassistant/scripts/check_config.py | 2 +- tests/scripts/test_check_config.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index d509f3762c1aad..8d3547898bc967 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -116,7 +116,7 @@ def run(script_args: list) -> int: exit_code = max(exit_code, 1) return exit_code - # Human-readable output starts here + # Human-readable output starts here print(color("bold", "Testing configuration at", config_dir)) domain_info: list[str] = [] diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index 9520aa2914db50..574f1f903734d0 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -184,9 +184,6 @@ def test_bootstrap_error() -> None: assert res["yaml_files"] == {} -# New tests for JSON and fail-on-warnings functionality - - @pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) @pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") def test_run_backwards_compatibility_no_flags() -> None: From 25be7d16c8230c9a658ec234d0be4d1f3438a6a4 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Thu, 18 Sep 2025 19:25:55 +0000 Subject: [PATCH 3/9] refactor: Remove redundant exploratory tests for check_config script --- homeassistant/scripts/check_config.py | 1 - tests/scripts/test_check_config.py | 234 -------------------------- 2 files changed, 235 deletions(-) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 8d3547898bc967..907e33be1cc1c9 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -116,7 +116,6 @@ def run(script_args: list) -> int: exit_code = max(exit_code, 1) return exit_code - # Human-readable output starts here print(color("bold", "Testing configuration at", config_dir)) domain_info: list[str] = [] diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index 574f1f903734d0..ccde9ec0f7a490 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -312,40 +312,6 @@ def test_run_fail_on_warnings_flag_only() -> None: assert exit_code == 1 # max(1, 1) = 1 -@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) -@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") -def test_run_both_flags_combined() -> None: - """Test that both flags work together correctly.""" - with ( - patch("builtins.print") as mock_print, - patch.object(check_config, "check") as mock_check, - ): - # Test with warnings only - mock_check.return_value = { - "except": {}, - "warn": {"light": ["warning message"]}, - "components": {"homeassistant": {}, "light": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - exit_code = check_config.run(["--json", "--fail-on-warnings"]) - - # Should exit with code 1 due to --fail-on-warnings and warnings present - assert exit_code == 1 - - # Should have printed JSON - assert mock_print.call_count == 1 - json_output = mock_print.call_args[0][0] - parsed_json = json.loads(json_output) - - # Verify JSON content - assert parsed_json["total_errors"] == 0 - assert parsed_json["total_warnings"] == 1 - assert parsed_json["warnings"] == {"light": ["warning message"]} - - @pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) @pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") def test_run_json_output_structure() -> None: @@ -501,36 +467,6 @@ def test_run_exit_code_logic() -> None: ) -@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) -@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") -def test_run_json_no_human_readable_output() -> None: - """Test that JSON mode doesn't include human-readable messages.""" - with ( - patch("builtins.print") as mock_print, - patch.object(check_config, "check") as mock_check, - ): - mock_check.return_value = { - "except": {}, - "warn": {}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - check_config.run(["--json"]) - - # Should only print once (the JSON output) - assert mock_print.call_count == 1 - - # The output should be valid JSON - json_output = mock_print.call_args[0][0] - json.loads(json_output) # Validate it's valid JSON - - # Should not contain human-readable messages like "Testing configuration at" - assert "Testing configuration at" not in json_output - - @pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) @pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") def test_run_human_readable_still_works() -> None: @@ -591,47 +527,6 @@ def test_run_with_config_path() -> None: # Flag Interaction Tests -def test_flag_order_independence() -> None: - """Test that flag order doesn't affect behavior.""" - with ( - patch("builtins.print") as mock_print1, - patch.object(check_config, "check") as mock_check1, - ): - mock_check1.return_value = { - "except": {"domain1": ["error1"]}, - "warn": {"domain2": ["warning1"]}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - exit_code1 = check_config.run(["--json", "--fail-on-warnings"]) - - with ( - patch("builtins.print") as mock_print2, - patch.object(check_config, "check") as mock_check2, - ): - mock_check2.return_value = { - "except": {"domain1": ["error1"]}, - "warn": {"domain2": ["warning1"]}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - exit_code2 = check_config.run(["--fail-on-warnings", "--json"]) - - # Both should have same exit code and JSON output - assert exit_code1 == exit_code2 == 1 - assert mock_print1.call_count == mock_print2.call_count == 1 - - json_output1 = json.loads(mock_print1.call_args[0][0]) - json_output2 = json.loads(mock_print2.call_args[0][0]) - assert json_output1 == json_output2 - - def test_unknown_arguments_with_json() -> None: """Test that unknown arguments are handled properly with JSON flag.""" with ( @@ -663,66 +558,6 @@ def test_unknown_arguments_with_json() -> None: assert "config_dir" in parsed_json -def test_empty_script_args() -> None: - """Test that empty arguments don't crash the script.""" - with ( - patch("builtins.print") as mock_print, - patch.object(check_config, "check") as mock_check, - ): - mock_check.return_value = { - "except": {}, - "warn": {}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - # Should not crash and should use default behavior (human-readable) - exit_code = check_config.run([]) - assert exit_code == 0 - - # Should print at least the "Testing configuration at..." message - assert mock_print.call_count >= 1 - - # First call should be the header message - first_call = mock_print.call_args_list[0][0][0] - assert "Testing configuration at" in first_call - - -@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) -@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") -def test_all_flags_together() -> None: - """Test behavior when multiple flags are used together.""" - with ( - patch("builtins.print") as mock_print, - patch.object(check_config, "check") as mock_check, - ): - mock_check.return_value = { - "except": {}, - "warn": {"light": ["warning message"]}, - "components": {"homeassistant": {}, "light": {}}, - "secrets": {"test_secret": "test_value"}, - "secret_cache": {"secrets.yaml": {"test_secret": "test_value"}}, - "yaml_files": {"/config/configuration.yaml": True}, - } - - # Test with --json, --fail-on-warnings, --secrets, and --files together - exit_code = check_config.run( - ["--json", "--fail-on-warnings", "--secrets", "--files"] - ) - - # Should exit with code 1 due to warnings + fail-on-warnings - assert exit_code == 1 - - # Should only print JSON (secrets and files should be ignored in JSON mode) - assert mock_print.call_count == 1 - - json_output = json.loads(mock_print.call_args[0][0]) - assert json_output["total_warnings"] == 1 - assert "light" in json_output["warnings"] - - @pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) @pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") def test_info_flag_with_json() -> None: @@ -784,33 +619,6 @@ def test_config_flag_variations() -> None: assert json_output["config_dir"] == expected_full_path -def test_flag_case_sensitivity() -> None: - """Test that flags are case sensitive (negative test).""" - with ( - patch("builtins.print") as mock_print, - patch.object(check_config, "check") as mock_check, - ): - mock_check.return_value = { - "except": {}, - "warn": {}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - # Uppercase flags should be treated as unknown arguments - check_config.run(["--JSON", "--FAIL-ON-WARNINGS"]) - - # Should print unknown arguments warning and then human-readable output - assert mock_print.call_count > 1 - - # First call should contain unknown argument warning - first_call = mock_print.call_args_list[0][0][0] - assert "Unknown arguments" in first_call - assert "JSON" in first_call - - def test_multiple_config_flags() -> None: """Test behavior with multiple config directory specifications.""" with ( @@ -836,48 +644,6 @@ def test_multiple_config_flags() -> None: assert json_output["config_dir"] == expected_path -def test_json_with_errors_and_warnings_combinations() -> None: - """Test JSON output with various error/warning combinations.""" - test_scenarios = [ - # (errors, warnings, expected_exit_code) - ({}, {}, 0), - ({"domain1": ["error"]}, {}, 1), - ({}, {"domain1": ["warning"]}, 0), # Without --fail-on-warnings - ({"d1": ["e1"]}, {"d2": ["w1"]}, 1), # Errors take precedence - ({"d1": ["e1"], "d2": ["e2"]}, {}, 2), # Multiple error domains - ( - {"d1": ["e1", "e2"]}, - {"d2": ["w1", "w2"]}, - 1, - ), # Multiple errors in one domain = 1 - ] - - for errors, warnings, expected_exit in test_scenarios: - with ( - patch("builtins.print") as mock_print, - patch.object(check_config, "check") as mock_check, - ): - mock_check.return_value = { - "except": errors, - "warn": warnings, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - - exit_code = check_config.run(["--json"]) - assert exit_code == expected_exit - - json_output = json.loads(mock_print.call_args[0][0]) - assert json_output["total_errors"] == sum(len(e) for e in errors.values()) - assert json_output["total_warnings"] == sum( - len(w) for w in warnings.values() - ) - assert json_output["errors"] == errors - assert json_output["warnings"] == warnings - - def test_fail_on_warnings_with_json_combinations() -> None: """Test --fail-on-warnings with --json in various scenarios.""" test_scenarios = [ From be36c886a2b74b4bc39a0bc7716bb1675354ccc8 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Fri, 26 Sep 2025 13:03:53 -0700 Subject: [PATCH 4/9] refactor: Simplify exit code calculation in run function of check_config script --- homeassistant/scripts/check_config.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 907e33be1cc1c9..03d165ac54891c 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -111,10 +111,7 @@ def run(script_args: list) -> int: print(json.dumps(json_object, indent=2)) # Determine exit code for JSON mode - exit_code = len(res["except"]) - if args.fail_on_warnings and res["warn"]: - exit_code = max(exit_code, 1) - return exit_code + return len(res["except"]) + int(args.fail_on_warnings and res["warn"]) print(color("bold", "Testing configuration at", config_dir)) @@ -191,10 +188,7 @@ def run(script_args: list) -> int: print(" -", skey + ":", sval) # Determine final exit code - exit_code = len(res["except"]) - if args.fail_on_warnings and res["warn"]: - exit_code = max(exit_code, 1) - return exit_code + return len(res["except"]) + int(args.fail_on_warnings and res["warn"]) def check(config_dir, secrets=False): From 1d845d72733fe444c5ee652c69c8376e7bc0a7af Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Fri, 26 Sep 2025 13:17:32 -0700 Subject: [PATCH 5/9] refactor: Improve code structure and readability in check_config script --- homeassistant/scripts/check_config.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 03d165ac54891c..58cb2b5e9ba45c 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -96,6 +96,9 @@ def run(script_args: list) -> int: config_dir = os.path.join(os.getcwd(), args.config) + if not args.json: + print(color("bold", "Testing configuration at", config_dir)) + res = check(config_dir, args.secrets) # JSON output branch @@ -113,8 +116,6 @@ def run(script_args: list) -> int: # Determine exit code for JSON mode return len(res["except"]) + int(args.fail_on_warnings and res["warn"]) - print(color("bold", "Testing configuration at", config_dir)) - domain_info: list[str] = [] if args.info: domain_info = args.info.split(",") From ddbabc7b9638f7819efc1d3fd7253caac29831eb Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Fri, 26 Sep 2025 13:27:13 -0700 Subject: [PATCH 6/9] refactor: notify if --secrets is used --- homeassistant/scripts/check_config.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 58cb2b5e9ba45c..77cd1e595fa555 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -94,6 +94,13 @@ def run(script_args: list) -> int: if unknown: print(color("red", "Unknown arguments:", ", ".join(unknown))) + if args.json and args.secrets: + print( + color( + "yellow", "Warning: --secrets flag is ignored when using --json output" + ) + ) + config_dir = os.path.join(os.getcwd(), args.config) if not args.json: From beb6f16065033a167d1bd390b6d7dcc6173b5d02 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Fri, 26 Sep 2025 13:29:39 -0700 Subject: [PATCH 7/9] remove test --- tests/scripts/test_check_config.py | 36 ------------------------------ 1 file changed, 36 deletions(-) diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index ccde9ec0f7a490..4a9c65682f655c 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -184,42 +184,6 @@ def test_bootstrap_error() -> None: assert res["yaml_files"] == {} -@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) -@pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") -def test_run_backwards_compatibility_no_flags() -> None: - """Test that run() with no flags maintains backwards compatibility.""" - # Test with valid config - exit_code = check_config.run([]) - assert exit_code == 0 - - # Test with config that has warnings - with patch.object(check_config, "check") as mock_check: - mock_check.return_value = { - "except": {}, - "warn": {"light": ["warning message"]}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - # Without --fail-on-warnings, warnings should not affect exit code - exit_code = check_config.run([]) - assert exit_code == 0 - - # Test with config that has errors - with patch.object(check_config, "check") as mock_check: - mock_check.return_value = { - "except": {"homeassistant": ["error message"]}, - "warn": {}, - "components": {"homeassistant": {}}, - "secrets": {}, - "secret_cache": {}, - "yaml_files": {}, - } - exit_code = check_config.run([]) - assert exit_code == 1 # len(res["except"]) = 1 domain with errors - - @pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG]) @pytest.mark.usefixtures("mock_is_file", "mock_hass_config_yaml") def test_run_json_flag_only() -> None: From f6fc56a42ecc0d0ac21ec3f720b8fa5dfe09d539 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sun, 12 Oct 2025 11:25:14 -0700 Subject: [PATCH 8/9] refactor: Enhance secrets handling and exit code logic in check_config script --- homeassistant/scripts/check_config.py | 32 +++++++++++++++++++-------- tests/scripts/test_check_config.py | 4 ++-- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 77cd1e595fa555..0081ca96dd8b2f 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -94,13 +94,6 @@ def run(script_args: list) -> int: if unknown: print(color("red", "Unknown arguments:", ", ".join(unknown))) - if args.json and args.secrets: - print( - color( - "yellow", "Warning: --secrets flag is ignored when using --json output" - ) - ) - config_dir = os.path.join(os.getcwd(), args.config) if not args.json: @@ -118,10 +111,31 @@ def run(script_args: list) -> int: "warnings": res["warn"], "components": list(res["components"].keys()), } + + # Include secrets information if requested + if args.secrets: + # Build list of missing secrets (referenced but not found) + missing_secrets = [ + key for key, val in res["secrets"].items() if val is None + ] + + # Build list of used secrets (found and used) + used_secrets = [ + key for key, val in res["secrets"].items() if val is not None + ] + + json_object["secrets"] = { + "secret_files": res["secret_cache"], + "used_secrets": used_secrets, + "missing_secrets": missing_secrets, + "total_secrets": len(res["secrets"]), + "total_missing": len(missing_secrets), + } + print(json.dumps(json_object, indent=2)) # Determine exit code for JSON mode - return len(res["except"]) + int(args.fail_on_warnings and res["warn"]) + return 1 if res["except"] or (args.fail_on_warnings and res["warn"]) else 0 domain_info: list[str] = [] if args.info: @@ -196,7 +210,7 @@ def run(script_args: list) -> int: print(" -", skey + ":", sval) # Determine final exit code - return len(res["except"]) + int(args.fail_on_warnings and res["warn"]) + return 1 if res["except"] or (args.fail_on_warnings and res["warn"]) else 0 def check(config_dir, secrets=False): diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index 4a9c65682f655c..4270967826e538 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -404,12 +404,12 @@ def test_run_exit_code_logic() -> None: ["--json", "--fail-on-warnings"], 1, ), # Both, both flags - ({"d1": ["e1"], "d2": ["e2"]}, {}, [], 2), # Multiple error domains, no flags + ({"d1": ["e1"], "d2": ["e2"]}, {}, [], 1), # Multiple error domains, no flags ( {"d1": ["e1"], "d2": ["e2"]}, {"d3": ["w1"]}, ["--fail-on-warnings"], - 2, + 1, ), # Multiple errors + warnings ] From 2860c69010d45a1450d5568ac2fa529b15f38e47 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sun, 12 Oct 2025 14:20:31 -0700 Subject: [PATCH 9/9] fix: Correct expected exit code for multiple errors in test_fail_on_warnings_with_json_combinations --- tests/scripts/test_check_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index 4270967826e538..b2dcb004bcbba8 100644 --- a/tests/scripts/test_check_config.py +++ b/tests/scripts/test_check_config.py @@ -616,7 +616,7 @@ def test_fail_on_warnings_with_json_combinations() -> None: ({"domain1": ["error"]}, {}, 1), ({}, {"domain1": ["warning"]}, 1), # With --fail-on-warnings ({"d1": ["e1"]}, {"d2": ["w1"]}, 1), # Errors still take precedence - ({"d1": ["e1"], "d2": ["e2"]}, {"d3": ["w1"]}, 2), # Multiple errors > warnings + ({"d1": ["e1"], "d2": ["e2"]}, {"d3": ["w1"]}, 1), # Multiple errors > warnings ] for errors, warnings, expected_exit in test_scenarios: