From 58438815326fee389e4d0225b4bc727e3cd6914d Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Tue, 11 Aug 2026 12:50:08 -0400 Subject: [PATCH] Fix #68 - fail tests that omit the # do not pre-load pragma (as might commonly be authored by a naive LLM --- src/dayamlchecker/messages.py | 11 +++ src/dayamlchecker/yaml_structure.py | 116 ++++++++++++++++++++++++++-- tests/test_yaml_structure_cli.py | 69 ++++++++++++++++- 3 files changed, 190 insertions(+), 6 deletions(-) diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index 7bb9431..8f96ca4 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -33,6 +33,7 @@ class MessageId(StrEnum): PYTHON_CODE_TYPE = "python_code_type" PYTHON_SYNTAX_ERROR = "python_syntax_error" PYTHON_CODE_FUNCTION_DEF = "python_code_function_def" + PYTHON_TEST_MODULE_MISSING_NO_PRELOAD = "python_test_module_missing_no_preload" VALIDATION_CODE_MISSING_VALIDATION_ERROR = ( "validation_code_missing_validation_error" ) @@ -291,6 +292,16 @@ class MessageDefinition: "helper functions to a Python module instead, or use inline code for small snippets that are only used once" ), ), + MessageId.PYTHON_TEST_MODULE_MISSING_NO_PRELOAD: MessageDefinition( + code="EG105", + severity=Severity.ERROR, + finding_class=FindingClass.GENERAL, + summary="Test module may be pre-loaded by docassemble", + template=( + "test modules must start with `# do not pre-load` so docassemble " + "does not import them during server startup" + ), + ), MessageId.VALIDATION_CODE_MISSING_VALIDATION_ERROR: MessageDefinition( code="WG101", severity=Severity.WARNING, diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index 7eb5bd9..74d1dac 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/src/dayamlchecker/yaml_structure.py @@ -2,6 +2,7 @@ import ast import argparse from dataclasses import dataclass, field, replace +import os from pathlib import Path from pyexpat import features import re @@ -47,7 +48,12 @@ # https://docassemble.org/docs/interviews.html#jinja2 -__all__ = ["find_errors_from_string", "find_errors", "_collect_yaml_files"] +__all__ = [ + "find_errors_from_string", + "find_errors", + "_collect_yaml_files", + "_collect_test_python_modules", +] DEFAULT_LINT_MODE = "default" ACCESSIBILITY_LINT_MODE = "accessibility" @@ -2397,6 +2403,96 @@ def _collect_yaml_files( return _formatter_collect(paths, include_default_ignores=include_default_ignores) +_DO_NOT_PRELOAD_DIRECTIVE = b"# do not pre-load" + + +def _is_default_ignored_python_dir(dirname: str) -> bool: + return ( + dirname.startswith(".git") + or dirname.startswith(".github") + or dirname.startswith(".venv") + or dirname in {"build", "dist", "node_modules"} + ) + + +def _is_docassemble_python_module(path: Path) -> bool: + return "docassemble" in path.resolve().parts + + +def _collect_test_python_modules( + paths: list[Path], + yaml_files: list[Path] | None = None, + include_default_ignores: bool = True, +) -> list[Path]: + """Find test_*.py modules under the docassemble namespace. + + Package roots inferred from selected question files are included so that a + command targeting an individual YAML file still checks sibling modules. + Ordinary pytest files outside ``docassemble`` are intentionally excluded. + """ + + candidates: list[Path] = [] + search_roots = [path for path in paths if path.is_dir()] + if yaml_files: + search_roots.extend(infer_package_dirs(yaml_files)) + + for path in paths: + if ( + path.is_file() + and path.match("test_*.py") + and _is_docassemble_python_module(path) + ): + candidates.append(path) + + for search_root in search_roots: + for root, dirnames, filenames in os.walk(search_root, topdown=True): + if include_default_ignores and _is_default_ignored_python_dir( + Path(root).name + ): + dirnames[:] = [] + continue + if include_default_ignores: + dirnames[:] = [ + dirname + for dirname in dirnames + if not _is_default_ignored_python_dir(dirname) + ] + + root_path = Path(root) + if not _is_docassemble_python_module(root_path): + continue + candidates.extend( + root_path / filename + for filename in filenames + if filename.startswith("test_") and filename.endswith(".py") + ) + + unique_modules: dict[Path, Path] = {} + for candidate in candidates: + unique_modules.setdefault(candidate.resolve(), candidate) + return sorted(unique_modules.values()) + + +def _find_test_module_preload_findings(paths: list[Path]) -> list[Finding]: + findings: list[Finding] = [] + for path in paths: + try: + with path.open("rb") as test_module: + first_line = test_module.readline().rstrip(b"\r\n") + except OSError: + first_line = b"" + + if not first_line.startswith(_DO_NOT_PRELOAD_DIRECTIVE): + findings.append( + make_finding( + MessageId.PYTHON_TEST_MODULE_MISSING_NO_PRELOAD, + file_name=str(path), + line_number=1, + ) + ) + return findings + + def process_file( input_file, lint_mode: str = DEFAULT_LINT_MODE, @@ -2431,7 +2527,10 @@ def main(argv: Optional[list[str]] = None) -> int: "files", nargs="+", type=Path, - help="YAML files or directories to validate (directories are searched recursively)", + help=( + "YAML/Python files or directories to validate " + "(directories are searched recursively)" + ), ) parser.add_argument( "--suppress", @@ -2595,8 +2694,13 @@ def main(argv: Optional[list[str]] = None) -> int: yaml_files = _collect_yaml_files( args.files, include_default_ignores=not args.check_all ) - if not yaml_files: - print("No YAML files found.", file=sys.stderr) + test_python_modules = _collect_test_python_modules( + args.files, + yaml_files=yaml_files, + include_default_ignores=not args.check_all, + ) + if not yaml_files and not test_python_modules: + print("No YAML files or test Python modules found.", file=sys.stderr) return 1 from dayamlchecker.messages import print_github_annotation @@ -2610,7 +2714,9 @@ def main(argv: Optional[list[str]] = None) -> int: ) all_findings.extend(findings) - if args.url_check: + all_findings.extend(_find_test_module_preload_findings(test_python_modules)) + + if args.url_check and yaml_files: url_check_root = ( args.url_check_root.resolve() if args.url_check_root is not None diff --git a/tests/test_yaml_structure_cli.py b/tests/test_yaml_structure_cli.py index 6c7c011..fabc00e 100644 --- a/tests/test_yaml_structure_cli.py +++ b/tests/test_yaml_structure_cli.py @@ -7,7 +7,11 @@ import dayamlchecker.yaml_structure as yaml_structure from dayamlchecker.check_questions_urls import URLCheckResult, URLIssue from dayamlchecker.messages import MessageId, make_finding, print_github_annotation -from dayamlchecker.yaml_structure import _collect_yaml_files, main +from dayamlchecker.yaml_structure import ( + _collect_test_python_modules, + _collect_yaml_files, + main, +) def _write_valid_question(path: Path) -> None: @@ -167,6 +171,69 @@ def test_collect_yaml_files_can_disable_default_ignores(): ) +def test_collect_test_python_modules_only_in_docassemble_namespace(tmp_path): + package = tmp_path / "docassemble" / "Demo" + interview = package / "data" / "questions" / "interview.yml" + module = package / "data" / "sources" / "test_helpers.py" + ordinary_pytest_file = tmp_path / "tests" / "test_helpers.py" + non_test_module = package / "data" / "sources" / "helpers.py" + + _write_valid_question(interview) + module.parent.mkdir(parents=True) + ordinary_pytest_file.parent.mkdir(parents=True) + module.write_text("# do not pre-load\n", encoding="utf-8") + ordinary_pytest_file.write_text("def test_example(): pass\n", encoding="utf-8") + non_test_module.write_text("class Helper: pass\n", encoding="utf-8") + + assert _collect_test_python_modules([interview], yaml_files=[interview]) == [module] + + +def test_main_rejects_each_test_module_missing_do_not_preload(tmp_path, capsys): + package = tmp_path / "docassemble" / "Demo" + interview = package / "data" / "questions" / "interview.yml" + sources = package / "data" / "sources" + first_bad_module = sources / "test_empty.py" + second_bad_module = sources / "test_mocked.py" + + _write_valid_question(interview) + sources.mkdir(parents=True) + first_bad_module.write_text("", encoding="utf-8") + second_bad_module.write_text("from unittest.mock import patch\n", encoding="utf-8") + + assert main(["--no-url-check", str(tmp_path)]) == 1 + + output = capsys.readouterr().out + assert output.count("[EG105]") == 2 + assert str(first_bad_module) in output + assert str(second_bad_module) in output + assert "must start with `# do not pre-load`" in output + + +def test_main_accepts_test_module_with_do_not_preload_on_first_line(tmp_path, capsys): + package = tmp_path / "docassemble" / "Demo" + interview = package / "data" / "questions" / "interview.yml" + module = package / "data" / "sources" / "test_helpers.py" + + _write_valid_question(interview) + module.parent.mkdir(parents=True) + module.write_text( + "# do not pre-load \nfrom unittest.mock import patch\n", + encoding="utf-8", + ) + + assert main(["--no-url-check", str(interview)]) == 0 + assert capsys.readouterr().out == "No issues found.\n" + + +def test_main_requires_do_not_preload_to_be_the_first_line(tmp_path, capsys): + module = tmp_path / "docassemble" / "Demo" / "test_helpers.py" + module.parent.mkdir(parents=True) + module.write_text("\n# do not pre-load\n", encoding="utf-8") + + assert main(["--no-url-check", str(module)]) == 1 + assert "[EG105]" in capsys.readouterr().out + + def test_main_default_wcag_reports_failures(): with TemporaryDirectory() as tmp: root = Path(tmp)