diff --git a/script/hassfest/requirements.py b/script/hassfest/requirements.py index d8aa383cfec..a2d305f76ef 100644 --- a/script/hassfest/requirements.py +++ b/script/hassfest/requirements.py @@ -11,7 +11,7 @@ import os import re import subprocess import sys -from typing import Any +from typing import Any, TypedDict from awesomeversion import AwesomeVersion, AwesomeVersionStrategy from tqdm import tqdm @@ -295,6 +295,9 @@ FORBIDDEN_PACKAGE_EXCEPTIONS: dict[str, dict[str, set[str]]] = { }, } +FORBIDDEN_FILE_NAMES: set[str] = { + "py.typed", # should be placed inside a package +} FORBIDDEN_PACKAGE_NAMES: set[str] = { "doc", "docs", @@ -364,7 +367,15 @@ PYTHON_VERSION_CHECK_EXCEPTIONS: dict[str, dict[str, set[str]]] = { }, } -_packages_checked_files_cache: dict[str, set[str]] = {} + +class _PackageFilesCheckResult(TypedDict): + """Data structure to store results of package files check.""" + + top_level: set[str] + file_names: set[str] + + +_packages_checked_files_cache: dict[str, _PackageFilesCheckResult] = {} def validate(integrations: dict[str, Integration], config: Config) -> None: @@ -733,24 +744,33 @@ def check_dependency_files( pkg: str, package_exceptions: Collection[str], ) -> bool: - """Check dependency files for forbidden package names.""" + """Check dependency files for forbidden files and forbidden package names.""" if (results := _packages_checked_files_cache.get(pkg)) is None: top_level: set[str] = set() + file_names: set[str] = set() for file in files(pkg) or (): - top = file.parts[0].lower() - if top.endswith((".dist-info", ".py")): - continue - top_level.add(top) - results = FORBIDDEN_PACKAGE_NAMES & top_level + if not (top := file.parts[0].lower()).endswith((".dist-info", ".py")): + top_level.add(top) + if (name := str(file)).lower() in FORBIDDEN_FILE_NAMES: + file_names.add(name) + results = _PackageFilesCheckResult( + top_level=FORBIDDEN_PACKAGE_NAMES & top_level, + file_names=file_names, + ) _packages_checked_files_cache[pkg] = results - if not results: + if not (results["top_level"] or results["file_names"]): return True - for dir_name in results: + for dir_name in results["top_level"]: integration.add_warning_or_error( pkg in package_exceptions, "requirements", - f"Package {pkg} has a forbidden top level directory {dir_name} in {package}", + f"Package {pkg} has a forbidden top level directory '{dir_name}' in {package}", + ) + for file_name in results["file_names"]: + integration.add_error( + "requirements", + f"Package {pkg} has a forbidden file '{file_name}' in {package}", ) return False diff --git a/tests/hassfest/test_requirements.py b/tests/hassfest/test_requirements.py index 944b06d3c90..329357bfca4 100644 --- a/tests/hassfest/test_requirements.py +++ b/tests/hassfest/test_requirements.py @@ -170,8 +170,8 @@ def test_dependency_version_range_prepare_update( @pytest.mark.usefixtures("mock_forbidden_package_names") -def test_check_dependency_files(integration: Integration) -> None: - """Test dependency files check for forbidden package names is working correctly.""" +def test_check_dependency_package_names(integration: Integration) -> None: + """Test dependency package names check for forbidden package names is working correctly.""" package = "homeassistant" pkg = "my_package" @@ -190,17 +190,15 @@ def test_check_dependency_files(integration: Integration) -> None: ): assert not _packages_checked_files_cache assert check_dependency_files(integration, package, pkg, ()) is False - assert _packages_checked_files_cache[pkg] == {"tests", "test"} + assert _packages_checked_files_cache[pkg]["top_level"] == {"tests", "test"} assert len(integration.errors) == 2 assert ( - f"Package {pkg} has a forbidden top level directory tests in {package}" - in x.error - for x in integration.errors + f"Package {pkg} has a forbidden top level directory 'tests' in {package}" + in [x.error for x in integration.errors] ) assert ( - f"Package {pkg} has a forbidden top level directory test in {package}" - in x.error - for x in integration.errors + f"Package {pkg} has a forbidden top level directory 'test' in {package}" + in [x.error for x in integration.errors] ) integration.errors.clear() @@ -227,13 +225,12 @@ def test_check_dependency_files(integration: Integration) -> None: check_dependency_files(integration, package, pkg, package_exceptions={pkg}) is False ) - assert _packages_checked_files_cache[pkg] == {"tests"} + assert _packages_checked_files_cache[pkg]["top_level"] == {"tests"} assert len(integration.errors) == 0 assert len(integration.warnings) == 1 assert ( - f"Package {pkg} has a forbidden top level directory tests in {package}" - in x.error - for x in integration.warnings + f"Package {pkg} has a forbidden top level directory 'tests' in {package}" + in [x.error for x in integration.warnings] ) integration.warnings.clear() @@ -260,7 +257,62 @@ def test_check_dependency_files(integration: Integration) -> None: ): assert not _packages_checked_files_cache assert check_dependency_files(integration, package, pkg, ()) is True - assert _packages_checked_files_cache[pkg] == set() + assert _packages_checked_files_cache[pkg]["top_level"] == set() + assert len(integration.errors) == 0 + + # Repeated call should use cache + assert check_dependency_files(integration, package, pkg, ()) is True + assert mock_files.call_count == 1 + assert len(integration.errors) == 0 + + +def test_check_dependency_file_names(integration: Integration) -> None: + """Test dependency file name check for forbidden files is working correctly.""" + package = "homeassistant" + pkg = "my_package" + + # Forbidden file: 'py.typed' at top level + pkg_files = [ + PackagePath("py.typed"), + PackagePath("my_package.py"), + PackagePath("my_package-1.0.0.dist-info/METADATA"), + ] + with ( + patch( + "script.hassfest.requirements.files", return_value=pkg_files + ) as mock_files, + patch.dict(_packages_checked_files_cache, {}, clear=True), + ): + assert not _packages_checked_files_cache + assert check_dependency_files(integration, package, pkg, ()) is False + assert _packages_checked_files_cache[pkg]["file_names"] == {"py.typed"} + assert len(integration.errors) == 1 + assert f"Package {pkg} has a forbidden file 'py.typed' in {package}" in [ + x.error for x in integration.errors + ] + integration.errors.clear() + + # Repeated call should use cache + assert check_dependency_files(integration, package, pkg, ()) is False + assert mock_files.call_count == 1 + assert len(integration.errors) == 1 + integration.errors.clear() + + # All good + pkg_files = [ + PackagePath("my_package/__init__.py"), + PackagePath("my_package/py.typed"), + PackagePath("my_package.dist-info/METADATA"), + ] + with ( + patch( + "script.hassfest.requirements.files", return_value=pkg_files + ) as mock_files, + patch.dict(_packages_checked_files_cache, {}, clear=True), + ): + assert not _packages_checked_files_cache + assert check_dependency_files(integration, package, pkg, ()) is True + assert _packages_checked_files_cache[pkg]["file_names"] == set() assert len(integration.errors) == 0 # Repeated call should use cache