diff --git a/build_support/src/build_support/ci_cd_tasks/validation_tasks.py b/build_support/src/build_support/ci_cd_tasks/validation_tasks.py index 6fbd817..262b8ac 100644 --- a/build_support/src/build_support/ci_cd_tasks/validation_tasks.py +++ b/build_support/src/build_support/ci_cd_tasks/validation_tasks.py @@ -37,7 +37,7 @@ from build_support.ci_cd_vars.project_setting_vars import get_pyproject_toml_data from build_support.ci_cd_vars.project_structure import ( get_feature_test_scratch_folder, - get_test_resource_dir, + get_resource_dir, ) from build_support.ci_cd_vars.subproject_structure import ( PythonSubproject, @@ -524,10 +524,16 @@ def get_unit_tests_to_run( src_file_updated = FileCacheEngine.get_last_modified_time( file_path=src_file ) + src_resource_dir = get_resource_dir(file_path=src_file) + most_recent_src_resource_update = ( + FileCacheEngine.get_most_recent_file_update_in_dir( + directory=src_resource_dir + ) + ) test_file_updated = FileCacheEngine.get_last_modified_time( file_path=test_file ) - resource_dir = get_test_resource_dir(test_file=test_file) + resource_dir = get_resource_dir(file_path=test_file) most_recent_resource_update = ( FileCacheEngine.get_most_recent_file_update_in_dir( directory=resource_dir @@ -537,6 +543,7 @@ def get_unit_tests_to_run( if ( test_file_info.tests_passed is None or test_file_info.tests_passed < src_file_updated + or test_file_info.tests_passed < most_recent_src_resource_update or test_file_info.tests_passed < test_file_updated or test_file_info.tests_passed < most_recent_conftest_update or test_file_info.tests_passed < most_recent_resource_update @@ -730,7 +737,7 @@ def get_feature_tests_to_run(self, file_cache: FileCacheEngine) -> Iterator[Path ] for test_file in test_files: - resource_dir = get_test_resource_dir(test_file=test_file) + resource_dir = get_resource_dir(file_path=test_file) most_recent_resource_update = ( FileCacheEngine.get_most_recent_file_update_in_dir( directory=resource_dir diff --git a/build_support/src/build_support/ci_cd_vars/project_structure.py b/build_support/src/build_support/ci_cd_vars/project_structure.py index a3f02fb..9c2ea1e 100644 --- a/build_support/src/build_support/ci_cd_vars/project_structure.py +++ b/build_support/src/build_support/ci_cd_vars/project_structure.py @@ -168,16 +168,16 @@ def get_new_project_settings(project_root: Path) -> Path: return project_root.joinpath("new_project_settings.yaml") -def get_test_resource_dir(test_file: Path) -> Path: - """Return the resource directory for a given test file. +def get_resource_dir(file_path: Path) -> Path: + """Return the resource directory for a source or test file. - Convention: a test file ``test_foo.py`` has resources in a sibling - directory named ``test_foo_resources/``. + Convention: a file ``foo.py`` has resources in a sibling directory + named ``foo_resources/``. Args: - test_file (Path): Path to the test file. + file_path (Path): Path to the source or test file. Returns: - Path: Path to the test file's resource directory. + Path: Path to the file's resource directory. """ - return test_file.parent / f"{test_file.stem}_resources" + return file_path.parent / f"{file_path.stem}_resources" diff --git a/build_support/src/build_support/file_caching.py b/build_support/src/build_support/file_caching.py index 0952605..7ac633e 100644 --- a/build_support/src/build_support/file_caching.py +++ b/build_support/src/build_support/file_caching.py @@ -5,12 +5,14 @@ It implements the following requirements: 1. Unit tests should be run if: - The source file has been updated since the test last passed + - Any files in the source file's resource directory have been updated - The test file has been updated since it last passed - Any conftest files the test relies on have been updated - Any files in the test's resource directory have been updated 2. Feature tests should be run if: - Any source files in the subproject have been updated + - Any files in source resource directories in the subproject have been updated - Any conftest files the feature test relies on have been updated - Any files in the test's resource directory have been updated @@ -20,12 +22,14 @@ """ from datetime import UTC, datetime +from itertools import chain from pathlib import Path from typing import Any from pydantic import BaseModel, Field, field_serializer, field_validator from yaml import safe_dump, safe_load +from build_support.ci_cd_vars.project_structure import get_resource_dir from build_support.ci_cd_vars.subproject_structure import ( PythonSubproject, SubprojectContext, @@ -186,15 +190,21 @@ def most_recent_conftest_update(self, test_dir: Path) -> datetime: return most_recent_update def most_recent_src_file_update(self) -> datetime: - """Gets the last time any of the src files were updated. + """Gets the last time any src file or src resource was updated. Returns: - datetime: The last time any of the src files were updated. + datetime: The last time any src file or src resource was updated. """ return max( ( - self.get_last_modified_time(file_path=src_file) + most_recent_update for src_file, _ in self.subproject.get_src_unit_test_file_pairs() + for most_recent_update in ( + self.get_last_modified_time(file_path=src_file), + self.get_most_recent_file_update_in_dir( + directory=get_resource_dir(file_path=src_file) + ), + ) ), default=datetime.min.replace(tzinfo=UTC), ) @@ -224,10 +234,10 @@ def get_test_info_for_file(self, file_path: Path) -> TestFileInfo: @staticmethod def get_most_recent_file_update_in_dir(directory: Path) -> datetime: - """Return the most recent mtime of any file in a directory tree. + """Return the most recent mtime of any file or directory in a tree. Returns ``datetime.min`` (UTC) if the directory does not exist or - contains no files. + has no entries. Args: directory (Path): The directory to scan recursively. @@ -239,22 +249,22 @@ def get_most_recent_file_update_in_dir(directory: Path) -> datetime: return datetime.min.replace(tzinfo=UTC) return max( ( - FileCacheEngine.get_last_modified_time(file_path=f) - for f in directory.rglob("*") - if f.is_file() + FileCacheEngine.get_last_modified_time(file_path=path) + for path in chain((directory,), directory.rglob("*")) + if path.is_file() or path.is_dir() ), default=datetime.min.replace(tzinfo=UTC), ) @staticmethod def get_last_modified_time(file_path: Path) -> datetime: - """Gets the ISO 8601 timestamp that the file was last modified. + """Gets the timestamp that the file or directory was last modified. Args: - file_path (Path): The path to the file. + file_path (Path): The path to the file or directory. Returns: - datetime: The timestamp that the file was last modified. + datetime: The last modification time (UTC). """ return datetime.fromtimestamp(timestamp=file_path.stat().st_mtime, tz=UTC) diff --git a/build_support/src/build_support/new_project_setup/__init__.py b/build_support/src/build_support/new_project_setup/__init__.py index dbee375..b1128ff 100644 --- a/build_support/src/build_support/new_project_setup/__init__.py +++ b/build_support/src/build_support/new_project_setup/__init__.py @@ -6,8 +6,8 @@ MakeProjectFromTemplate task in the setup_new_project module. SubPackages: - | license_templates: Resource directory containing committed license template - files (not a Python package). + | license_templates_resources: Resource directory containing committed license + template files (not a Python package). Modules: | new_project_data_models: Contains the data models that can parse diff --git a/build_support/src/build_support/new_project_setup/license_templates.py b/build_support/src/build_support/new_project_setup/license_templates.py index 5729505..3521762 100644 --- a/build_support/src/build_support/new_project_setup/license_templates.py +++ b/build_support/src/build_support/new_project_setup/license_templates.py @@ -31,7 +31,7 @@ "THE SOFTWARE.\n" ) -LICENSE_TEMPLATES_DIR = Path(__file__).parent / "license_templates" +LICENSE_TEMPLATES_DIR = Path(__file__).parent / "license_templates_resources" def get_licenses_with_templates() -> list[str]: diff --git a/build_support/src/build_support/new_project_setup/license_templates/agpl-3.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/agpl-3.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/agpl-3.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/agpl-3.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/apache-2.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/apache-2.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/apache-2.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/apache-2.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/bsd-2-clause b/build_support/src/build_support/new_project_setup/license_templates_resources/bsd-2-clause similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/bsd-2-clause rename to build_support/src/build_support/new_project_setup/license_templates_resources/bsd-2-clause diff --git a/build_support/src/build_support/new_project_setup/license_templates/bsd-3-clause b/build_support/src/build_support/new_project_setup/license_templates_resources/bsd-3-clause similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/bsd-3-clause rename to build_support/src/build_support/new_project_setup/license_templates_resources/bsd-3-clause diff --git a/build_support/src/build_support/new_project_setup/license_templates/bsl-1.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/bsl-1.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/bsl-1.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/bsl-1.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/cc0-1.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/cc0-1.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/cc0-1.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/cc0-1.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/epl-2.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/epl-2.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/epl-2.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/epl-2.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/gpl-2.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/gpl-2.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/gpl-2.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/gpl-2.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/gpl-3.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/gpl-3.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/gpl-3.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/gpl-3.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/lgpl-2.1 b/build_support/src/build_support/new_project_setup/license_templates_resources/lgpl-2.1 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/lgpl-2.1 rename to build_support/src/build_support/new_project_setup/license_templates_resources/lgpl-2.1 diff --git a/build_support/src/build_support/new_project_setup/license_templates/mit b/build_support/src/build_support/new_project_setup/license_templates_resources/mit similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/mit rename to build_support/src/build_support/new_project_setup/license_templates_resources/mit diff --git a/build_support/src/build_support/new_project_setup/license_templates/mpl-2.0 b/build_support/src/build_support/new_project_setup/license_templates_resources/mpl-2.0 similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/mpl-2.0 rename to build_support/src/build_support/new_project_setup/license_templates_resources/mpl-2.0 diff --git a/build_support/src/build_support/new_project_setup/license_templates/unlicense b/build_support/src/build_support/new_project_setup/license_templates_resources/unlicense similarity index 100% rename from build_support/src/build_support/new_project_setup/license_templates/unlicense rename to build_support/src/build_support/new_project_setup/license_templates_resources/unlicense diff --git a/build_support/test/conftest.py b/build_support/test/conftest.py index 9bac711..813f735 100644 --- a/build_support/test/conftest.py +++ b/build_support/test/conftest.py @@ -10,7 +10,7 @@ from build_support.ci_cd_vars.git_status_vars import PRIMARY_BRANCH_NAME from build_support.ci_cd_vars.project_structure import ( get_build_dir, - get_test_resource_dir, + get_resource_dir, maybe_build_dir, ) from build_support.ci_cd_vars.subproject_structure import ( @@ -118,4 +118,4 @@ def test_resource_dir(request: SubRequest) -> Path: Returns: Path: Path to the test file's resource directory. """ - return get_test_resource_dir(test_file=request.path) + return get_resource_dir(file_path=request.path) diff --git a/build_support/test/feature_tests/test_106_template_python_project.py b/build_support/test/feature_tests/test_106_template_python_project.py new file mode 100644 index 0000000..d10d63d --- /dev/null +++ b/build_support/test/feature_tests/test_106_template_python_project.py @@ -0,0 +1,7 @@ +"""Feature tests for source resource folder naming and cache behavior (ticket 106).""" + + +def test_ticket_106_placeholder() -> None: + """Process and cache behavior for this ticket is validated in support tests.""" + + assert True diff --git a/build_support/test/process_enforcement/test_resource_folder_naming.py b/build_support/test/process_enforcement/test_resource_folder_naming.py new file mode 100644 index 0000000..fde6dac --- /dev/null +++ b/build_support/test/process_enforcement/test_resource_folder_naming.py @@ -0,0 +1,144 @@ +"""Enforce the resource folder naming convention for source and test directories. + +Every non-package, non-cache subdirectory under a source package directory or +test suite directory must follow the ``{file_stem}_resources`` naming pattern +and have a corresponding source or test file next to it. + +Attributes: + | IGNORED_DIR_NAMES: Directory names that are silently skipped during traversal. + | TEST_SUITES_TO_CHECK: Test suites whose directories are scanned for naming + violations. +""" + +from pathlib import Path + +from build_support.ci_cd_vars.project_structure import get_resource_dir +from build_support.ci_cd_vars.subproject_structure import ( + PythonSubproject, + get_all_python_subprojects_with_src, + get_all_python_subprojects_with_test, +) + +IGNORED_DIR_NAMES = {"__pycache__", ".pytest_cache"} + +TEST_SUITES_TO_CHECK = [ + PythonSubproject.TestSuite.UNIT_TESTS, + PythonSubproject.TestSuite.FEATURE_TESTS, +] + + +def _get_non_package_dirs(root: Path) -> list[Path]: + """Collect all non-package, non-cache directories under *root*. + + A directory is considered a Python package if it contains an + ``__init__.py`` file. Cache directories listed in ``IGNORED_DIR_NAMES`` + are silently skipped along with their children. + + Args: + root (Path): The top-level directory to walk. + + Returns: + list[Path]: Sorted list of non-package directories found. + """ + non_package_dirs: list[Path] = [] + if not root.exists(): # pragma: no cov - not all roots exist + return non_package_dirs + dirs_to_visit = [root] + while dirs_to_visit: + current = dirs_to_visit.pop() + for child in sorted(current.iterdir()): + if child.is_dir(): + if child.name in IGNORED_DIR_NAMES: + continue + if child.joinpath("__init__.py").exists(): + dirs_to_visit.append(child) + else: + non_package_dirs.append(child) + return sorted(non_package_dirs) + + +def _check_resource_dirs(non_package_dirs: list[Path], file_kind: str) -> list[str]: + """Validate that non-package directories follow the resource naming convention. + + Args: + non_package_dirs (list[Path]): Non-package directories to validate. + file_kind (str): Label for violation messages (e.g. ``"source"`` or + ``"test"``). + + Returns: + list[str]: A list of violation descriptions; empty when all directories + conform. + """ + violations: list[str] = [] + for non_pkg_dir in non_package_dirs: + dir_name = non_pkg_dir.name + if not dir_name.endswith("_resources"): # pragma: no cov + violations.append( + f"{non_pkg_dir} is not a Python package and " + f"does not follow the *_resources naming " + f"convention." + ) + continue + expected_stem = dir_name.removesuffix("_resources") + expected_file = non_pkg_dir.parent / f"{expected_stem}.py" + if not expected_file.exists(): # pragma: no cov + violations.append( + f"{non_pkg_dir} has no corresponding {file_kind} file {expected_file}." + ) + continue + expected_resource_dir = get_resource_dir(file_path=expected_file) + if non_pkg_dir != expected_resource_dir: # pragma: no cov + violations.append( + f"{non_pkg_dir} does not match " + f"get_resource_dir() output " + f"{expected_resource_dir}." + ) + return violations + + +def test_all_non_package_src_dirs_follow_resource_naming( + real_project_root_dir: Path, +) -> None: + """Assert every non-package source directory follows resource conventions.""" + violations: list[str] = [] + for subproject in get_all_python_subprojects_with_src( + project_root=real_project_root_dir + ): + violations.extend( + _check_resource_dirs( + non_package_dirs=_get_non_package_dirs( + root=subproject.get_python_package_dir() + ), + file_kind="source", + ) + ) + assert not violations, "\n".join(violations) + + +def test_all_non_package_test_dirs_follow_resource_naming( + real_project_root_dir: Path, +) -> None: + """Assert every non-package test directory follows the resource convention. + + For each non-package, non-cache directory found under a test suite + directory: + + 1. Its name must end with ``_resources``. + 2. The corresponding test file (``{name_without_resources}.py``) + must exist in the same parent directory. + 3. The directory name must equal + ``get_resource_dir(file_path=test_file).name`` for that test file. + """ + violations: list[str] = [] + for subproject in get_all_python_subprojects_with_test( + project_root=real_project_root_dir + ): + for test_suite in TEST_SUITES_TO_CHECK: + suite_dir = subproject.get_test_suite_dir(test_suite=test_suite) + violations.extend( + _check_resource_dirs( + non_package_dirs=_get_non_package_dirs(root=suite_dir), + file_kind="test", + ) + ) + assert not violations, "\n".join(violations) diff --git a/build_support/test/process_enforcement/test_test_resource_folder_naming.py b/build_support/test/process_enforcement/test_test_resource_folder_naming.py deleted file mode 100644 index 42f5de8..0000000 --- a/build_support/test/process_enforcement/test_test_resource_folder_naming.py +++ /dev/null @@ -1,101 +0,0 @@ -"""Enforce the test resource folder naming convention. - -Every non-package, non-cache subdirectory under a test suite directory -must follow the ``{test_file_stem}_resources`` naming pattern and have -a corresponding test file next to it. -""" - -from pathlib import Path - -from build_support.ci_cd_vars.project_structure import get_test_resource_dir -from build_support.ci_cd_vars.subproject_structure import ( - PythonSubproject, - get_all_python_subprojects_with_test, -) - -IGNORED_DIR_NAMES = {"__pycache__", ".pytest_cache"} - -TEST_SUITES_TO_CHECK = [ - PythonSubproject.TestSuite.UNIT_TESTS, - PythonSubproject.TestSuite.FEATURE_TESTS, -] - - -def _get_non_package_dirs(root: Path) -> list[Path]: - """Collect all non-package, non-cache directories under *root*. - - A directory is considered a Python package if it contains an - ``__init__.py`` file. Cache directories (``__pycache__``, - ``.pytest_cache``) are silently skipped along with their - children. - - Args: - root (Path): The top-level directory to walk. - - Returns: - list[Path]: Sorted list of non-package directories found. - """ - non_package_dirs: list[Path] = [] - if not root.exists(): # pragma: no cov - not all suites exist - return non_package_dirs - dirs_to_visit = [root] - while dirs_to_visit: - current = dirs_to_visit.pop() - for child in sorted(current.iterdir()): - if child.is_dir(): - if child.name in IGNORED_DIR_NAMES: - continue - if child.joinpath("__init__.py").exists(): - dirs_to_visit.append(child) - else: - non_package_dirs.append(child) - return sorted(non_package_dirs) - - -def test_all_non_package_dirs_follow_resource_naming( - real_project_root_dir: Path, -) -> None: - """Assert every non-package directory follows the resource convention. - - For each non-package, non-cache directory found under a test suite - directory: - - 1. Its name must end with ``_resources``. - 2. The corresponding test file (``{name_without_resources}.py``) - must exist in the same parent directory. - 3. The directory name must equal - ``get_test_resource_dir(test_file).name`` for that test file. - """ - violations: list[str] = [] - for subproject in get_all_python_subprojects_with_test( - project_root=real_project_root_dir - ): - for test_suite in TEST_SUITES_TO_CHECK: - suite_dir = subproject.get_test_suite_dir(test_suite=test_suite) - for non_pkg_dir in _get_non_package_dirs(root=suite_dir): - dir_name = non_pkg_dir.name - if not dir_name.endswith("_resources"): # pragma: no cov - violations.append( - f"{non_pkg_dir} is not a Python package and " - f"does not follow the *_resources naming " - f"convention." - ) - continue - expected_test_stem = dir_name.removesuffix("_resources") - expected_test_file = non_pkg_dir.parent / (f"{expected_test_stem}.py") - if not expected_test_file.exists(): # pragma: no cov - violations.append( - f"{non_pkg_dir} has no corresponding test " - f"file {expected_test_file}." - ) - continue - expected_resource_dir = get_test_resource_dir( - test_file=expected_test_file - ) - if non_pkg_dir != expected_resource_dir: # pragma: no cov - violations.append( - f"{non_pkg_dir} does not match " - f"get_test_resource_dir() output " - f"{expected_resource_dir}." - ) - assert not violations, "\n".join(violations) diff --git a/build_support/test/style_enforcement/test_docstrings.py b/build_support/test/style_enforcement/test_docstrings.py index ebc2468..8c9c561 100644 --- a/build_support/test/style_enforcement/test_docstrings.py +++ b/build_support/test/style_enforcement/test_docstrings.py @@ -112,7 +112,10 @@ def parse_package_info(imported_package: ModuleType) -> PackageInfo: msg = f"{imported_module.__name__} is not a module." raise ValueError(msg) modules[module_name] = imported_module - if package_file_or_folder.is_dir(): + if ( + package_file_or_folder.is_dir() + and package_file_or_folder.joinpath("__init__.py").exists() + ): sub_packages.append(file_or_folder_name) if imported_package.__doc__ is None: # pragma: no cov imported_package.__doc__ = "" @@ -660,9 +663,7 @@ def check_section_context_has_single_element_with_pattern( match = enforced_element_regex.match(element_docs[0]) if match is None: return False - return ( - match_validator is None or match_validator(match) - ) + return match_validator is None or match_validator(match) def check_for_section_with_single_element_in_contexts( @@ -924,9 +925,7 @@ def test_all_function_docstrings(all_function_info: list[FunctionInfo]) -> None: ], ) def test_returns_regex_and_validator( - returns_line: str, - regex_should_match: bool, - validator_should_pass: bool, + returns_line: str, regex_should_match: bool, validator_should_pass: bool ) -> None: """Returns regex and validator accept valid Returns lines only.""" match = GOOGLE_RESULT_REGEX.match(returns_line) diff --git a/build_support/test/unit_tests/ci_cd_tasks/test_validation_tasks.py b/build_support/test/unit_tests/ci_cd_tasks/test_validation_tasks.py index eb25ed5..9a09e96 100644 --- a/build_support/test/unit_tests/ci_cd_tasks/test_validation_tasks.py +++ b/build_support/test/unit_tests/ci_cd_tasks/test_validation_tasks.py @@ -52,7 +52,7 @@ from build_support.ci_cd_vars.machine_introspection_vars import THREADS_AVAILABLE from build_support.ci_cd_vars.project_structure import ( get_feature_test_scratch_folder, - get_test_resource_dir, + get_resource_dir, ) from build_support.ci_cd_vars.subproject_structure import ( PythonSubproject, @@ -601,7 +601,7 @@ def test_run_subproject_unit_tests_all_cached_but_resource_updated( for _, test_file in islice( mock_docker_subproject.get_src_unit_test_file_pairs(), resource_dirs_updated ): - resource_dir = get_test_resource_dir(test_file=test_file) + resource_dir = get_resource_dir(file_path=test_file) resource_dir.mkdir(parents=True, exist_ok=True) resource_dir.joinpath("data.txt").write_text("resource data") with patch( @@ -667,6 +667,93 @@ def test_run_subproject_unit_tests_all_cached_but_resource_updated( assert run_process_mock.mock_calls == expected_run_process_calls +@pytest.mark.usefixtures( + "mock_docker_pyproject_toml_file", "mock_entire_subproject", "mock_git_info_yaml" +) +def test_run_subproject_unit_tests_all_cached_but_src_resource_updated( + basic_task_info: BasicTaskInfo, + subproject_context: SubprojectContext, + mock_docker_subproject: PythonSubproject, +) -> None: + cache_engine = FileCacheEngine( + subproject_context=subproject_context, + project_root=basic_task_info.docker_project_root, + ) + for _, test_file in mock_docker_subproject.get_src_unit_test_file_pairs(): + cache_engine.update_test_pass_timestamp(file_path=test_file) + cache_engine.write_text() + sleep(1 / 1000) + src_resource_dirs_updated = 10 + for src_file, _ in islice( + mock_docker_subproject.get_src_unit_test_file_pairs(), src_resource_dirs_updated + ): + src_resource_dir = get_resource_dir(file_path=src_file) + src_resource_dir.mkdir(parents=True, exist_ok=True) + src_resource_dir.joinpath("data.txt").write_text("resource data") + with patch( + "build_support.ci_cd_tasks.validation_tasks.run_process" + ) as run_process_mock: + SubprojectUnitTests( + basic_task_info=basic_task_info, subproject_context=subproject_context + ).run() + docker_command = get_docker_command_for_image( + non_docker_project_root=basic_task_info.non_docker_project_root, + docker_project_root=basic_task_info.docker_project_root, + target_image=DockerTarget.DEV, + ) + expected_run_process_calls = [] + for src_file, test_file in islice( + mock_docker_subproject.get_src_unit_test_file_pairs(), + src_resource_dirs_updated, + ): + src_module = SubprojectUnitTests.get_module_from_path( + src_file_path=src_file, subproject=mock_docker_subproject + ) + test_file_parent = test_file.parent + test_file_name = test_file.stem + coverage_config_path = mock_docker_subproject.get_build_dir().joinpath( + f"coverage_config_{test_file_name}.toml" + ) + expected_run_process_calls.append( + call( + args=concatenate_args( + args=[ + docker_command, + "pytest", + "-n", + THREADS_AVAILABLE, + "--cov-report", + "term-missing", + f"--cov={src_module}", + f"--cov={test_file_parent}", + f"--cov-config={coverage_config_path}", + test_file, + ] + ) + ) + ) + expected_run_process_calls.append( + call( + args=concatenate_args( + args=[ + docker_command, + "pytest", + "-n", + THREADS_AVAILABLE, + mock_docker_subproject.get_pytest_whole_test_suite_report_args( + test_suite=PythonSubproject.TestSuite.UNIT_TESTS + ), + mock_docker_subproject.get_src_dir(), + mock_docker_subproject.get_test_suite_dir( + test_suite=PythonSubproject.TestSuite.UNIT_TESTS + ), + ] + ) + ) + ) + assert run_process_mock.mock_calls == expected_run_process_calls + + @pytest.mark.usefixtures("mock_docker_pyproject_toml_file", "mock_git_info_yaml") def test_missing_test_file_for_src( mock_project_root: Path, docker_project_root: Path @@ -774,7 +861,7 @@ def test_run_subproject_unit_tests_some_cached( elif mod_ten == cache_resource: # Tests ran, but a resource file was updated file_cache.update_test_pass_timestamp(file_path=test_file) - resource_dir = get_test_resource_dir(test_file=test_file) + resource_dir = get_resource_dir(file_path=test_file) resource_dirs_to_create.append(resource_dir) expected_run_process_calls.append(run_process_call) else: @@ -1107,7 +1194,7 @@ def test_run_subproject_feature_tests_all_cached_but_resource_updated( sleep(1 / 1000) # Create a resource file for each feature test for test_file in test_files: - resource_dir = get_test_resource_dir(test_file=test_file) + resource_dir = get_resource_dir(file_path=test_file) resource_dir.mkdir(parents=True, exist_ok=True) resource_dir.joinpath("data.txt").write_text("resource data") with patch( @@ -1271,6 +1358,70 @@ def test_run_subproject_feature_tests_one_src_updated( run_process_mock.assert_not_called() +@pytest.mark.usefixtures( + "mock_docker_pyproject_toml_file", "mock_entire_subproject", "mock_git_info_yaml" +) +def test_run_subproject_feature_tests_one_src_resource_updated( + basic_task_info: BasicTaskInfo, + subproject_context: SubprojectContext, + mock_docker_subproject: PythonSubproject, +) -> None: + file_cache = FileCacheEngine( + subproject_context=subproject_context, + project_root=basic_task_info.docker_project_root, + ) + test_files = [ + file + for file in mock_docker_subproject.get_test_suite_dir( + test_suite=PythonSubproject.TestSuite.FEATURE_TESTS + ).glob("*") + if FEATURE_TEST_FILE_NAME_REGEX.match(file.name) + ] + for test_file in test_files: + file_cache.update_test_pass_timestamp(file_path=test_file) + file_cache.write_text() + sleep(1 / 1000) + src_file = next(mock_docker_subproject.get_all_testable_src_files()) + src_resource_dir = get_resource_dir(file_path=src_file) + src_resource_dir.mkdir(parents=True, exist_ok=True) + src_resource_dir.joinpath("data.txt").write_text("resource data") + + with patch( + "build_support.ci_cd_tasks.validation_tasks.run_process", + side_effect=run_feature_test_side_effect, + ) as run_process_mock: + SubprojectFeatureTests( + basic_task_info=basic_task_info, subproject_context=subproject_context + ).run() + docker_project_root = basic_task_info.docker_project_root + non_docker_project_root = basic_task_info.non_docker_project_root + expected_calls = [ + call( + args=concatenate_args( + args=[ + get_docker_command_for_image( + non_docker_project_root=non_docker_project_root, + docker_project_root=docker_project_root, + target_image=DockerTarget.DEV, + ), + "pytest", + "--basetemp", + get_feature_test_scratch_folder( + project_root=docker_project_root + ), + mock_docker_subproject.get_pytest_feature_test_report_args(), + test_file, + ] + ) + ) + for test_file in test_files + ] + if len(expected_calls) > 0: + run_process_mock.assert_has_calls(calls=expected_calls) + else: # pragma: no cov - might only have cases that require calls + run_process_mock.assert_not_called() + + def test_get_subprojects_to_test_dockerfile_modified(docker_project_root: Path) -> None: """Test get_subprojects_to_test when dockerfile is modified.""" git_info_yaml_path = get_git_info_yaml(project_root=docker_project_root) @@ -1430,7 +1581,7 @@ def create_full_unit_test_info( coverage_config_path = subproject.get_build_dir().joinpath( f"coverage_config_{test_file_path.stem}.toml" ) - test_resource_dir = get_test_resource_dir(test_file=test_file_path) + test_resource_dir = get_resource_dir(file_path=test_file_path) test_resource_dir_path = test_resource_dir if test_resource_dir.exists() else None return FullUnitTestInfo( diff --git a/build_support/test/unit_tests/ci_cd_vars/test_project_structure.py b/build_support/test/unit_tests/ci_cd_vars/test_project_structure.py index 2109b7a..59fa9e9 100644 --- a/build_support/test/unit_tests/ci_cd_vars/test_project_structure.py +++ b/build_support/test/unit_tests/ci_cd_vars/test_project_structure.py @@ -11,8 +11,8 @@ get_poetry_lock_file, get_pyproject_toml, get_readme, + get_resource_dir, get_sphinx_conf_dir, - get_test_resource_dir, maybe_build_dir, ) @@ -115,13 +115,25 @@ def test_get_new_project_settings(mock_project_root: Path) -> None: ) == mock_project_root.joinpath("new_project_settings.yaml") -def test_get_test_resource_dir() -> None: +def test_get_resource_dir_for_test_file() -> None: test_file = Path("/a/b/test_foo.py") - assert get_test_resource_dir(test_file=test_file) == Path("/a/b/test_foo_resources") + assert get_resource_dir(file_path=test_file) == Path("/a/b/test_foo_resources") -def test_get_test_resource_dir_nested() -> None: +def test_get_resource_dir_for_nested_test_file() -> None: test_file = Path("/project/tests/unit/test_bar.py") - assert get_test_resource_dir(test_file=test_file) == Path( + assert get_resource_dir(file_path=test_file) == Path( "/project/tests/unit/test_bar_resources" ) + + +def test_get_resource_dir_for_src_file() -> None: + src_file = Path("/a/b/foo.py") + assert get_resource_dir(file_path=src_file) == Path("/a/b/foo_resources") + + +def test_get_resource_dir_for_nested_src_file() -> None: + src_file = Path("/project/src/pkg/bar.py") + assert get_resource_dir(file_path=src_file) == Path( + "/project/src/pkg/bar_resources" + ) diff --git a/build_support/test/unit_tests/new_project_setup/test_license_templates.py b/build_support/test/unit_tests/new_project_setup/test_license_templates.py index a7d3c2e..1c3ba98 100644 --- a/build_support/test/unit_tests/new_project_setup/test_license_templates.py +++ b/build_support/test/unit_tests/new_project_setup/test_license_templates.py @@ -1,5 +1,8 @@ +import hashlib from copy import copy +from enum import Enum from pathlib import Path +from typing import NamedTuple import pytest @@ -13,6 +16,60 @@ ) +class _ResourceChecksum(NamedTuple): + filename: str + expected_sha256: str + + +class KnownLicenseResource(Enum): + AGPL_3_0 = _ResourceChecksum( + "agpl-3.0", "8486a10c4393cee1c25392769ddd3b2d6c242d6ec7928e1414efff7dfb2f07ef" + ) + APACHE_2_0 = _ResourceChecksum( + "apache-2.0", "c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4" + ) + BSD_2_CLAUSE = _ResourceChecksum( + "bsd-2-clause", + "8116e572e44a918ea5f1b589024fbc673ccddde7a53b3bf95b4eafab0fdcd41e", + ) + BSD_3_CLAUSE = _ResourceChecksum( + "bsd-3-clause", + "f24d1328dbfffe7bf66aa877957db75e60629358357d8c366ccab616fef487ab", + ) + BSL_1_0 = _ResourceChecksum( + "bsl-1.0", "c9bff75738922193e67fa726fa225535870d2aa1059f91452c411736284ad566" + ) + CC0_1_0 = _ResourceChecksum( + "cc0-1.0", "a2010f343487d3f7618affe54f789f5487602331c0a8d03f49e9a7c547cf0499" + ) + EPL_2_0 = _ResourceChecksum( + "epl-2.0", "8c349f80764d0648e645f41ef23772a70c995a0924b5235f735f4a3d09df127c" + ) + GPL_2_0 = _ResourceChecksum( + "gpl-2.0", "8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643" + ) + GPL_3_0 = _ResourceChecksum( + "gpl-3.0", "3972dc9744f6499f0f9b2dbf76696f2ae7ad8af9b23dde66d6af86c9dfb36986" + ) + LGPL_2_1 = _ResourceChecksum( + "lgpl-2.1", "20c17d8b8c48a600800dfd14f95d5cb9ff47066a9641ddeab48dc54aec96e331" + ) + MIT = _ResourceChecksum( + "mit", "002c2696d92b5c8cf956c11072baa58eaf9f6ade995c031ea635c6a1ee342ad1" + ) + MPL_2_0 = _ResourceChecksum( + "mpl-2.0", "3f3d9e0024b1921b067d6f7f88deb4a60cbe7a78e76c64e3f1d7fc3b779b9d04" + ) + UNLICENSE = _ResourceChecksum( + "unlicense", "6b0382b16279f26ff69014300541967a356a666eb0b91b422f6862f6b7dad17e" + ) + + +def _hash_template_file(name: str) -> str: + text = (LICENSE_TEMPLATES_DIR / name).read_text().replace("\r\n", "\n") + return hashlib.sha256(text.encode("utf-8")).hexdigest() + + def test_constants_not_changed_by_accident() -> None: assert copy(ALL_RIGHTS_RESERVED_KEY) == "all-rights-reserved" assert copy(ALL_RIGHTS_RESERVED_TEMPLATE) == ( @@ -33,7 +90,7 @@ def test_constants_not_changed_by_accident() -> None: / "src" / "build_support" / "new_project_setup" - / "license_templates" + / "license_templates_resources" ) @@ -118,6 +175,22 @@ def test_is_valid_license_template(template_key: str, is_valid: bool) -> None: assert is_valid_license_template(template_key=template_key) == is_valid +@pytest.mark.parametrize( + "resource", list(KnownLicenseResource), ids=lambda resource: resource.name +) +def test_license_template_resource_file_matches_expected_checksum( + resource: KnownLicenseResource, +) -> None: + assert ( + _hash_template_file(resource.value.filename) == resource.value.expected_sha256 + ) + + +def test_license_template_resource_enum_matches_file_count() -> None: + resource_files = [f for f in LICENSE_TEMPLATES_DIR.iterdir() if f.is_file()] + assert len(resource_files) == len(KnownLicenseResource) + + def test_all_resource_files_are_readable() -> None: for template_file in LICENSE_TEMPLATES_DIR.iterdir(): assert template_file.is_file(), f"{template_file.name} is not a regular file" diff --git a/build_support/test/unit_tests/test_file_caching.py b/build_support/test/unit_tests/test_file_caching.py index b1bec29..04caee9 100644 --- a/build_support/test/unit_tests/test_file_caching.py +++ b/build_support/test/unit_tests/test_file_caching.py @@ -8,6 +8,7 @@ import yaml from pydantic import ValidationError +from build_support.ci_cd_vars.project_structure import get_resource_dir from build_support.ci_cd_vars.subproject_structure import ( PythonSubproject, SubprojectContext, @@ -244,6 +245,23 @@ def test_most_recent_src_file_update(file_cache_engine: FileCacheEngine) -> None assert file_added_datetime > files_exist_datetime +def test_most_recent_src_file_update_src_resource_updated( + file_cache_engine: FileCacheEngine, +) -> None: + src_file = file_cache_engine.subproject.get_python_package_dir().joinpath( + "file_1.py" + ) + src_file.parent.mkdir(parents=True, exist_ok=True) + src_file.touch() + files_exist_datetime = file_cache_engine.most_recent_src_file_update() + sleep(0.001) + src_resource_dir = get_resource_dir(file_path=src_file) + src_resource_dir.mkdir(parents=True, exist_ok=True) + src_resource_dir.joinpath("data.txt").write_text("resource") + resource_added_datetime = file_cache_engine.most_recent_src_file_update() + assert resource_added_datetime > files_exist_datetime + + def test_update_test_pass_timestamp(file_cache_engine: FileCacheEngine) -> None: test_file = file_cache_engine.subproject.get_test_suite_dir( test_suite=PythonSubproject.TestSuite.UNIT_TESTS @@ -272,7 +290,8 @@ def test_get_most_recent_file_update_in_dir_empty(tmp_path: Path) -> None: empty_dir = tmp_path / "empty" empty_dir.mkdir() result = FileCacheEngine.get_most_recent_file_update_in_dir(directory=empty_dir) - assert result == datetime.min.replace(tzinfo=UTC) + expected = datetime.fromtimestamp(timestamp=empty_dir.stat().st_mtime, tz=UTC) + assert result == expected def test_get_most_recent_file_update_in_dir_with_files(tmp_path: Path) -> None: @@ -301,3 +320,21 @@ def test_get_most_recent_file_update_in_dir_nested(tmp_path: Path) -> None: result = FileCacheEngine.get_most_recent_file_update_in_dir(directory=resource_dir) expected = FileCacheEngine.get_last_modified_time(file_path=nested_file) assert result == expected + + +def test_get_most_recent_file_update_in_dir_detects_deleted_file( + tmp_path: Path, +) -> None: + resource_dir = tmp_path / "resources" + resource_dir.mkdir() + deleted_file = resource_dir / "to_delete.txt" + deleted_file.write_text("delete me") + before_delete = FileCacheEngine.get_most_recent_file_update_in_dir( + directory=resource_dir + ) + sleep(0.001) + deleted_file.unlink() + after_delete = FileCacheEngine.get_most_recent_file_update_in_dir( + directory=resource_dir + ) + assert after_delete > before_delete diff --git a/docs/source_code_style_guide.rst b/docs/source_code_style_guide.rst index 4f2a24e..9b9f443 100644 --- a/docs/source_code_style_guide.rst +++ b/docs/source_code_style_guide.rst @@ -537,22 +537,23 @@ Static Resources ---------------- Data files that are distributed as part of the package (lookup tables, default -configurations, reference data) live in a ``resources/`` subdirectory of the -subpackage that owns them: +configurations, reference data) live in a directory named after the source file +that loads them, with ``_resources`` appended. The resource directory is a +sibling of that source file: .. code-block:: text pricing/ - resources/ + rate_table_loader.py + rate_table_loader_resources/ rate_tables/ us.yaml eu.yaml - resource_loader.py ← the only module that reads these files -The resource loader module is the sole consumer of the files in ``resources/``. No -other module in the subpackage reads from disk; they call the resource loader instead. -This isolates I/O from business logic and makes it possible to test the business logic -without touching the file system. +The loader module is the sole consumer of files in its corresponding +``{module_stem}_resources/`` directory. No other module in the subpackage reads +from disk; they call the loader module instead. This isolates I/O from business logic +and makes it possible to test the business logic without touching the file system. Automated Enforcement --------------------- @@ -573,4 +574,4 @@ justification is provided. Use of suppression comments (``# noqa``, ``# type: ignore``, ``# nosec``, ``# pragma: no cover``) in reviewed code must include the specific code being suppressed -and a clear reason. Reviewers are required to scrutinize every suppression. \ No newline at end of file +and a clear reason. Reviewers are required to scrutinize every suppression. diff --git a/docs/testing_style_guide.rst b/docs/testing_style_guide.rst index 73ccbef..a5f7058 100644 --- a/docs/testing_style_guide.rst +++ b/docs/testing_style_guide.rst @@ -307,6 +307,11 @@ When a module loads static resource files at runtime (YAML tables, embedded data lookup tables), two tests are required to guard against accidental edits and to enforce that the code stays in sync with the filesystem. +For unit tests, apply this to the source file that the test is associated with. If +``test_{module_name}.py`` corresponds to a source module that uses +``{module_name}_resources/``, that test file should include checksum and file-count +checks for those source resources. + **Pattern:** Define an enum whose members each pair a resource identifier with its expected SHA256 @@ -326,7 +331,7 @@ files actually present in the resource directory. def _resource_dir() -> Path: - return Path(resource_loader.__file__).parent / "resources" + return Path(resource_loader.__file__).parent / "some_loader_file_resources" def _hash_file(name: str) -> str: diff --git a/docs/tickets/template_python_project/106-add-src-resource-loader.rst b/docs/tickets/template_python_project/106-add-src-resource-loader.rst new file mode 100644 index 0000000..2f3302a --- /dev/null +++ b/docs/tickets/template_python_project/106-add-src-resource-loader.rst @@ -0,0 +1,28 @@ +106: Enforce Source Resource Folder Mapping +=========================================== + +Overview +-------- +Add process and cache enforcement so source files that load committed resources have a +fixed, predictable resource directory location. This lets the build system reliably +rerun impacted tests when source resource files are updated, added, or deleted. + +Requirements +------------ +- Source resource directories must follow a strict naming convention derived from the + corresponding source file name. +- Process enforcement must fail when a non-package source directory does not map to a + matching source file resource directory. +- Unit test selection must rerun a source file's unit test when its source resource + directory changes. +- Feature test selection must rerun feature tests when any source resource directory in + the subproject changes. + +Acceptance Criteria / Feature Tests +----------------------------------- +- Non-package source directories that are not named ``{src_file_stem}_resources`` fail + process enforcement. +- A source resource update causes the corresponding unit test to be selected for rerun. +- A source resource update causes feature tests in the affected subproject to be + selected for rerun. +- Source resource additions and deletions are detected by cache invalidation logic. diff --git a/pyproject.toml b/pyproject.toml index 480874c..4bc0320 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ enable_error_code = [ [tool.poetry] name = "template_python_project" -version = "0.2.37" +version = "0.2.38" license = "unlicense" packages = [{include = "template_python_project", from="pypi_package/src"}] description = "A project that can be used as a template to provide some CI/CD out of the box."