From f335e3ff177642bd0a58867435022929f7eb7a46 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Wed, 24 Aug 2022 20:14:57 +0100 Subject: [PATCH 1/5] Further enhance `check_consistent.py` --- requirements-tests.txt | 2 ++ tests/check_consistent.py | 71 ++++++++++++++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/requirements-tests.txt b/requirements-tests.txt index a29d4c5d9256..4edde78e7002 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -12,4 +12,6 @@ tomli==1.2.2 # must match .pre-commit-config.yaml pycln==2.1.1 packaging==21.3 +pyyaml==6.0 termcolor +types-pyyaml diff --git a/tests/check_consistent.py b/tests/check_consistent.py index 082277746587..4622d66ec525 100755 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -11,15 +11,18 @@ from pathlib import Path import tomli +import yaml from packaging.requirements import Requirement +from packaging.specifiers import SpecifierSet from packaging.version import Version metadata_keys = {"version", "requires", "extra_description", "obsolete_since", "no_longer_updated", "tool"} tool_keys = {"stubtest": {"skip", "apt_dependencies", "extras", "ignore_missing_stub"}} +extension_descriptions = {".pyi": "stub", ".py": ".py"} -def assert_stubs_only(directory: Path, allowed: set[str]) -> None: - """Check that given directory contains only valid stub files.""" +def assert_consistent_filetypes(directory: Path, *, kind: str, allowed: set[str]) -> None: + """Check that given directory contains only valid Python files of a certain kind.""" allowed_paths = {Path(f) for f in allowed} contents = list(directory.iterdir()) while contents: @@ -29,14 +32,17 @@ def assert_stubs_only(directory: Path, allowed: set[str]) -> None: continue if entry.is_file(): assert entry.stem.isidentifier(), f"Files must be valid modules, got: {entry}" - assert entry.suffix == ".pyi", f"Only stub files allowed, got: {entry}" + bad_filetype = ( + f'Only {extension_descriptions[kind]} files allowed in the "{directory}" directory; ' f"got: {entry.suffix!r}" + ) + assert entry.suffix == kind, bad_filetype else: assert entry.name.isidentifier(), f"Directories must be valid packages, got: {entry}" contents.extend(entry.iterdir()) def check_stdlib() -> None: - assert_stubs_only(Path("stdlib"), allowed={"_typeshed/README.md", "VERSIONS"}) + assert_consistent_filetypes(Path("stdlib"), kind=".pyi", allowed={"_typeshed/README.md", "VERSIONS"}) def check_stubs() -> None: @@ -50,10 +56,17 @@ def check_stubs() -> None: assert not dist.name.startswith("types-"), f"Directory name not allowed to start with 'types-': {dist}" allowed = {"METADATA.toml", "README", "README.md", "README.rst", "@tests"} - assert_stubs_only(dist, allowed) + assert_consistent_filetypes(dist, kind=".pyi", allowed=allowed) -def check_same_files() -> None: +def check_test_cases() -> None: + assert_consistent_filetypes(Path("test_cases"), kind=".py", allowed={"README.md"}) + bad_test_case_filename = 'Files in the `test_cases` directory must have names starting with "test_"' + for file in Path("test_cases").rglob("*.py"): + assert file.stem.startswith("test_"), bad_test_case_filename + + +def check_no_symlinks() -> None: files = [os.path.join(root, file) for root, _, files in os.walk(".") for file in files] no_symlink = "You cannot use symlinks in typeshed, please copy {} to its link." for file in files: @@ -65,12 +78,16 @@ def check_same_files() -> None: _VERSIONS_RE = re.compile(r"^([a-zA-Z_][a-zA-Z0-9_.]*): [23]\.\d{1,2}-(?:[23]\.\d{1,2})?$") +def strip_comments(text: str) -> str: + return text.split("#")[0].strip() + + def check_versions() -> None: versions = set() with open("stdlib/VERSIONS") as f: data = f.read().splitlines() for line in data: - line = line.split("#")[0].strip() + line = strip_comments(line) if line == "": continue m = _VERSIONS_RE.match(line) @@ -126,10 +143,48 @@ def check_metadata() -> None: assert key in tk, f"Unrecognised {tool} key {key} for {distribution}" +def get_txt_requirements() -> dict[str, SpecifierSet]: + with open("requirements-tests.txt") as requirements_file: + stripped_lines = map(strip_comments, requirements_file) + reqs = [Requirement(stripped_line) for stripped_line in stripped_lines if stripped_line != ""] + return {req.name: req.specifier for req in reqs} + + +def get_precommit_requirements() -> dict[str, SpecifierSet]: + with open(".pre-commit-config.yaml") as precommit_file: + precommit = precommit_file.read() + yam = yaml.load(precommit, Loader=yaml.Loader) + precommit_requirements = {} + for repo in yam["repos"]: + hook = repo["hooks"][0] + package_name, package_rev = hook["id"], repo["rev"] + package_specifier = SpecifierSet(f"=={package_rev.removeprefix('v')}") + precommit_requirements[package_name] = package_specifier + for additional_req in hook.get("additional_dependencies", []): + req = Requirement(additional_req) + precommit_requirements[req.name] = req.specifier + return precommit_requirements + + +def check_requirements() -> None: + requirements_txt_requirements = get_txt_requirements() + precommit_requirements = get_precommit_requirements() + for requirement, specifier in precommit_requirements.items(): + no_txt_entry_msg = "All pre-commit requirements must also be listed in `requirements-tests.txt`" + assert requirement in requirements_txt_requirements, no_txt_entry_msg + specifier_mismatch = ( + f"Specifier for {requirement!r} in `.pre-commit-config.yaml` " + "does not match the specifier in `requirements-tests.txt`" + ) + assert specifier == requirements_txt_requirements[requirement], specifier_mismatch + + if __name__ == "__main__": assert sys.version_info >= (3, 9), "Python 3.9+ is required to run this test" check_stdlib() check_versions() check_stubs() check_metadata() - check_same_files() + check_no_symlinks() + check_test_cases() + check_requirements() From da6f6b7f21d21cedc2e31f58d716b2e5089654bf Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Thu, 25 Aug 2022 09:39:44 +0100 Subject: [PATCH 2/5] Fix weirdness introduced by black --- tests/check_consistent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/check_consistent.py b/tests/check_consistent.py index 4622d66ec525..c443b94be2f7 100755 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -33,7 +33,7 @@ def assert_consistent_filetypes(directory: Path, *, kind: str, allowed: set[str] if entry.is_file(): assert entry.stem.isidentifier(), f"Files must be valid modules, got: {entry}" bad_filetype = ( - f'Only {extension_descriptions[kind]} files allowed in the "{directory}" directory; ' f"got: {entry.suffix!r}" + f'Only {extension_descriptions[kind]} files allowed in the "{directory}" directory; got: {entry.suffix!r}' ) assert entry.suffix == kind, bad_filetype else: From a14710474b09d2f7422f55fdd2d9668d62860ef8 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 26 Aug 2022 11:13:22 +0100 Subject: [PATCH 3/5] Address review, make a few other tweaks --- tests/check_consistent.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/check_consistent.py b/tests/check_consistent.py index c443b94be2f7..e224752b95b3 100755 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -31,9 +31,9 @@ def assert_consistent_filetypes(directory: Path, *, kind: str, allowed: set[str] # Note if a subdirectory is allowed, we will not check its contents continue if entry.is_file(): - assert entry.stem.isidentifier(), f"Files must be valid modules, got: {entry}" + assert entry.stem.isidentifier(), f'Files must be valid modules, got: "{entry}"' bad_filetype = ( - f'Only {extension_descriptions[kind]} files allowed in the "{directory}" directory; got: {entry.suffix!r}' + f'Only {extension_descriptions[kind]!r} files allowed in the "{directory}" directory; got: {entry}' ) assert entry.suffix == kind, bad_filetype else: @@ -52,7 +52,7 @@ def check_stubs() -> None: valid_dist_name = "^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$" # courtesy of PEP 426 assert re.fullmatch( valid_dist_name, dist.name, re.IGNORECASE - ), f"Directory name must have valid distribution name: {dist}" + ), f"Directory name must be a valid distribution name: {dist}" assert not dist.name.startswith("types-"), f"Directory name not allowed to start with 'types-': {dist}" allowed = {"METADATA.toml", "README", "README.md", "README.rst", "@tests"} @@ -61,9 +61,9 @@ def check_stubs() -> None: def check_test_cases() -> None: assert_consistent_filetypes(Path("test_cases"), kind=".py", allowed={"README.md"}) - bad_test_case_filename = 'Files in the `test_cases` directory must have names starting with "test_"' + bad_test_case_filename = 'Files in the `test_cases` directory must have names starting with "test_"; got "{}"' for file in Path("test_cases").rglob("*.py"): - assert file.stem.startswith("test_"), bad_test_case_filename + assert file.stem.startswith("test_"), bad_test_case_filename.format(file) def check_no_symlinks() -> None: @@ -146,8 +146,8 @@ def check_metadata() -> None: def get_txt_requirements() -> dict[str, SpecifierSet]: with open("requirements-tests.txt") as requirements_file: stripped_lines = map(strip_comments, requirements_file) - reqs = [Requirement(stripped_line) for stripped_line in stripped_lines if stripped_line != ""] - return {req.name: req.specifier for req in reqs} + requirements = map(Requirement, filter(None, stripped_lines)) + return {requirement.name: requirement.specifier for requirement in requirements} def get_precommit_requirements() -> dict[str, SpecifierSet]: @@ -170,11 +170,13 @@ def check_requirements() -> None: requirements_txt_requirements = get_txt_requirements() precommit_requirements = get_precommit_requirements() for requirement, specifier in precommit_requirements.items(): - no_txt_entry_msg = "All pre-commit requirements must also be listed in `requirements-tests.txt`" + no_txt_entry_msg = ( + f"All pre-commit requirements must also be listed in `requirements-tests.txt` (missing {requirement!r})" + ) assert requirement in requirements_txt_requirements, no_txt_entry_msg specifier_mismatch = ( - f"Specifier for {requirement!r} in `.pre-commit-config.yaml` " - "does not match the specifier in `requirements-tests.txt`" + f'Specifier "{specifier}" for {requirement!r} in `.pre-commit-config.yaml` ' + f'does not match specifier "{requirements_txt_requirements[requirement]}" in `requirements-tests.txt`' ) assert specifier == requirements_txt_requirements[requirement], specifier_mismatch From 51e9b6fb8be7de6e35f9f494e4eee93b4b214d1c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Aug 2022 10:14:43 +0000 Subject: [PATCH 4/5] [pre-commit.ci] auto fixes from pre-commit.com hooks --- tests/check_consistent.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/check_consistent.py b/tests/check_consistent.py index e224752b95b3..9c7c3edf3f00 100755 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -32,9 +32,7 @@ def assert_consistent_filetypes(directory: Path, *, kind: str, allowed: set[str] continue if entry.is_file(): assert entry.stem.isidentifier(), f'Files must be valid modules, got: "{entry}"' - bad_filetype = ( - f'Only {extension_descriptions[kind]!r} files allowed in the "{directory}" directory; got: {entry}' - ) + bad_filetype = f'Only {extension_descriptions[kind]!r} files allowed in the "{directory}" directory; got: {entry}' assert entry.suffix == kind, bad_filetype else: assert entry.name.isidentifier(), f"Directories must be valid packages, got: {entry}" From 443b29e3e5342c9e2dbba215ca41c69455886c98 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 26 Aug 2022 11:21:23 +0100 Subject: [PATCH 5/5] More tweaking --- tests/check_consistent.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/check_consistent.py b/tests/check_consistent.py index 9c7c3edf3f00..9fc27b8d1fae 100755 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -167,11 +167,9 @@ def get_precommit_requirements() -> dict[str, SpecifierSet]: def check_requirements() -> None: requirements_txt_requirements = get_txt_requirements() precommit_requirements = get_precommit_requirements() + no_txt_entry_msg = "All pre-commit requirements must also be listed in `requirements-tests.txt` (missing {requirement!r})" for requirement, specifier in precommit_requirements.items(): - no_txt_entry_msg = ( - f"All pre-commit requirements must also be listed in `requirements-tests.txt` (missing {requirement!r})" - ) - assert requirement in requirements_txt_requirements, no_txt_entry_msg + assert requirement in requirements_txt_requirements, no_txt_entry_msg.format(requirement) specifier_mismatch = ( f'Specifier "{specifier}" for {requirement!r} in `.pre-commit-config.yaml` ' f'does not match specifier "{requirements_txt_requirements[requirement]}" in `requirements-tests.txt`'