From 3353886481ae699520f8a835347cdc1793dd5b34 Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Tue, 19 Sep 2023 14:46:33 -0400 Subject: [PATCH] Add pyproject encoding tests and validation tests --- poetry.lock | 2 +- pyproject.toml | 1 + snakebids/admin.py | 11 +- snakebids/jinja2_ext/toml_encode.py | 13 ++ snakebids/project_template/copier.yaml | 20 +- ...'poetry' %}pyproject.toml{% endif %}.jinja | 12 +- ...'poetry' %}pyproject.toml{% endif %}.jinja | 10 +- snakebids/tests/conftest.py | 2 + snakebids/tests/test_admin.py | 47 ++++ snakebids/tests/test_template.py | 214 ++++++++++++++++-- typings/copier.pyi | 4 +- 11 files changed, 297 insertions(+), 39 deletions(-) create mode 100644 snakebids/jinja2_ext/toml_encode.py diff --git a/poetry.lock b/poetry.lock index 4b0e5899..6f178220 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3082,4 +3082,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = ">=3.8,<3.12" -content-hash = "e4e7282a1d70bef9acd4e368fc5202fcd21975803d03a1974cc1378920dd6b8a" +content-hash = "cf50e37d6560e400dfed5ecddd180e28ce5c8b067289b88f650fd26ddc362d5c" diff --git a/pyproject.toml b/pyproject.toml index e24d8cc5..670d60ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,6 +85,7 @@ pathvalidate = "^3.0.0" # As of pyright==1.1.327 only 1.1.324 is bug free pyright = "==1.1.324" ruff = "^0.0.285" +tomli = "^2.0.1" [tool.poetry.scripts] snakebids = "snakebids.admin:main" diff --git a/snakebids/admin.py b/snakebids/admin.py index 872e2b26..602e9e23 100644 --- a/snakebids/admin.py +++ b/snakebids/admin.py @@ -1,6 +1,7 @@ """Script to generate a Snakebids project.""" import argparse +import re import sys from pathlib import Path @@ -22,6 +23,13 @@ def create_app(args: argparse.Namespace) -> None: file=sys.stderr, ) sys.exit(1) + if not re.match(r"^[a-zA-Z_][a-zA-Z_0-9]*$", output.name): + print( + f"{Fore.RED}Output directory name {Style.BRIGHT}{output.name}" + f"{Style.RESET_ALL}{Fore.RED} is not a valid python module name", + file=sys.stderr, + ) + sys.exit(1) print( f"Creating Snakebids app at {Fore.GREEN}{output}{Fore.RESET}", file=sys.stderr ) @@ -39,7 +47,6 @@ def create_app(args: argparse.Namespace) -> None: def create_descriptor(args: argparse.Namespace) -> None: - # pylint: disable=unsubscriptable-object app = SnakeBidsApp(args.app_dir.resolve()) add_dynamic_args(app.parser, app.config["parse_args"], app.config["pybids_inputs"]) app.create_descriptor(args.out_path) @@ -77,7 +84,7 @@ def gen_parser() -> argparse.ArgumentParser: def main() -> None: - """Invoke Cookiecutter on the Snakebids project template.""" + """Invoke snakebids cli.""" parser = gen_parser() args = parser.parse_args() diff --git a/snakebids/jinja2_ext/toml_encode.py b/snakebids/jinja2_ext/toml_encode.py new file mode 100644 index 00000000..c76e475c --- /dev/null +++ b/snakebids/jinja2_ext/toml_encode.py @@ -0,0 +1,13 @@ +import json + +import jinja2 +from jinja2.ext import Extension + + +def toml_string(item: str): + return json.dumps(item, ensure_ascii=False).replace("\x7F", "\\u007f") + + +class TomlEncodeExtension(Extension): + def __init__(self, env: jinja2.Environment): + env.filters["toml_string"] = toml_string # type: ignore diff --git a/snakebids/project_template/copier.yaml b/snakebids/project_template/copier.yaml index ef6ac10e..1add8f44 100644 --- a/snakebids/project_template/copier.yaml +++ b/snakebids/project_template/copier.yaml @@ -1,10 +1,20 @@ +app_full_name: + type: str + help: What is the name of your app? + validator: >- + {% if not (app_full_name | regex_search("^[a-zA-Z_][a-zA-Z_0-9]*$")) -%} + Name must be a valid python module name + {%- elif not app_full_name -%} + Required + {%- endif %} + full_name: type: str help: What is your name? default: '{% gitconfig "user.name" %}' email_regex: - default: (?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\]) + default: ^(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$ when: false email: @@ -12,7 +22,7 @@ email: help: What is your email? default: '{% gitconfig "user.email" %}' validator: >- - {% if email and not (email | regex_search(email_regex)) %} + {% if email and not (email | regex_search(email_regex, ignorecase=True)) %} Must be a valid email {% endif %} @@ -20,11 +30,6 @@ github: type: str help: What is your github username? -app_full_name: - type: str - help: What is the name of your app? - validator: '{% if not app_full_name %}Required{% endif %}' - app_description: type: str help: Provide a brief description of your app @@ -132,3 +137,4 @@ _jinja_extensions: - jinja2_time.TimeExtension - snakebids.jinja2_ext.vcs.GitConfigExtension - snakebids.jinja2_ext.colorama.ColoramaExtension + - snakebids.jinja2_ext.toml_encode.TomlEncodeExtension diff --git a/snakebids/project_template/{% if build_system != 'poetry' %}pyproject.toml{% endif %}.jinja b/snakebids/project_template/{% if build_system != 'poetry' %}pyproject.toml{% endif %}.jinja index a70448d8..dc666104 100644 --- a/snakebids/project_template/{% if build_system != 'poetry' %}pyproject.toml{% endif %}.jinja +++ b/snakebids/project_template/{% if build_system != 'poetry' %}pyproject.toml{% endif %}.jinja @@ -1,22 +1,22 @@ [project] -name = "{{ app_full_name }}" -version = "{{ app_version }}" -description = "{{ app_description }}" +name = {{ app_full_name | toml_string }} +version = {{ app_version | toml_string }} +description = {{ app_description | toml_string }} readme = "README.md" {% if license -%} -license = "{{ license }}" +license = {{ license | toml_string }} {%- endif %} {% if full_name or email -%} authors = [ { {{ '' }} {%- if full_name -%} - name = "{{ full_name }}" + name = {{ full_name | toml_string }} {%- endif -%} {%- if full_name and email -%} ,{{ ' ' }} {%- endif -%} {%- if email -%} - email = "{{ email }}" + email = {{ email | toml_string }} {%- endif -%} {{ ' ' }}} ] diff --git a/snakebids/project_template/{% if build_system == 'poetry' %}pyproject.toml{% endif %}.jinja b/snakebids/project_template/{% if build_system == 'poetry' %}pyproject.toml{% endif %}.jinja index 4c4b11b7..729511d3 100644 --- a/snakebids/project_template/{% if build_system == 'poetry' %}pyproject.toml{% endif %}.jinja +++ b/snakebids/project_template/{% if build_system == 'poetry' %}pyproject.toml{% endif %}.jinja @@ -1,17 +1,17 @@ [tool.poetry] name = "{{ app_full_name }}" -version = "{{ app_version }}" -description = "{{ app_description }}" +version = {{ app_version | toml_string }} +description = {{ app_description | toml_string }} readme = "README.md" {% if license -%} -license = "{{ license }}" +license = {{ license | toml_string }} {%- endif %} {% if full_name -%} authors = [ {% if email -%} - "{{ full_name }} <{{email}}>" + {{ (full_name + " <" + email + ">") | toml_string }} {%- else -%} - "{{ full_name }}" + {{ full_name | toml_string }} {%- endif %} ] {%- endif %} diff --git a/snakebids/tests/conftest.py b/snakebids/tests/conftest.py index d748890d..f8822f06 100644 --- a/snakebids/tests/conftest.py +++ b/snakebids/tests/conftest.py @@ -5,6 +5,7 @@ from pathlib import Path import bids.layout +import hypothesis.vendor import pytest from hypothesis import settings from pyfakefs.fake_filesystem import FakeFilesystem @@ -66,4 +67,5 @@ def bids_fs(fakefs: FakeFilesystem | None) -> FakeFilesystem | None: fakefs.add_real_file(f / "bids.json") fakefs.add_real_file(f / "derivatives.json") fakefs.add_real_file(Path(*resources.__path__) / "bids_tags.json") + print(Path(*hypothesis.vendor.__path__)) return fakefs diff --git a/snakebids/tests/test_admin.py b/snakebids/tests/test_admin.py index 44e0a407..9425ac9c 100644 --- a/snakebids/tests/test_admin.py +++ b/snakebids/tests/test_admin.py @@ -1,10 +1,16 @@ +import re import sys from argparse import ArgumentParser, Namespace +from pathlib import Path import pytest +from hypothesis import given +from hypothesis import strategies as st +from pathvalidate import Platform, is_valid_filename from pytest_mock.plugin import MockerFixture from snakebids.admin import gen_parser +from snakebids.tests.helpers import allow_function_scoped @pytest.fixture @@ -27,6 +33,47 @@ def test_fails_if_invalid_subcommand( with pytest.raises(SystemExit): parser.parse_args() + @given( + name=st.text() + .filter(lambda s: not re.match(r"^[a-zA-Z_][a-zA-Z_0-9]*$", s)) + .filter(lambda s: is_valid_filename(s, Platform.LINUX)) + ) + @allow_function_scoped + def test_create_fails_with_invalid_filename( + self, + parser: ArgumentParser, + mocker: MockerFixture, + name: str, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ): + mocker.patch.object(sys, "argv", ["snakebids", "create", str(tmp_path / name)]) + args = parser.parse_args() + with pytest.raises(SystemExit): + args.func(args) + capture = capsys.readouterr() + assert "valid python module" in capture.err + assert name in capture.err + + @given(name=st.text().filter(lambda s: is_valid_filename(s, Platform.LINUX))) + @allow_function_scoped + def test_create_fails_missing_parent_dir( + self, + parser: ArgumentParser, + mocker: MockerFixture, + name: str, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ): + path = tmp_path / name / "sub" + mocker.patch.object(sys, "argv", ["snakebids", "create", str(path)]) + args = parser.parse_args() + with pytest.raises(SystemExit): + args.func(args) + capture = capsys.readouterr() + assert "does not exist" in capture.err + assert str(path.parent) in capture.err + def test_create_succeeds(self, parser: ArgumentParser, mocker: MockerFixture): mocker.patch.object(sys, "argv", ["snakebids", "create"]) assert isinstance(parser.parse_args(), Namespace) diff --git a/snakebids/tests/test_template.py b/snakebids/tests/test_template.py index 8139b93c..1c5cef32 100644 --- a/snakebids/tests/test_template.py +++ b/snakebids/tests/test_template.py @@ -1,30 +1,47 @@ from __future__ import annotations import platform +import re import subprocess as sp +import sys +import tempfile from pathlib import Path +from typing import Literal, TypedDict -import copier # type: ignore +import copier import more_itertools as itx +import pathvalidate +import prompt_toolkit.validation import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st +from typing_extensions import Unpack + +if sys.version_info < (3, 11): + import tomli as tomllib +else: + import tomllib import snakebids -from snakebids.tests.helpers import needs_docker +from snakebids.tests.helpers import allow_function_scoped, needs_docker +BuildSystems = Literal["poetry", "hatch", "flit", "setuptools"] -@needs_docker(f"snakebids/test-template:{platform.python_version()}") -@pytest.mark.parametrize( - ["build", "venv"], - [ - ("setuptools", "setuptools"), - ("poetry", "poetry"), - ("hatch", "hatch"), - ("flit", "pdm"), - ], -) -def test_template_dry_runs_successfully(tmp_path: Path, build: str, venv: str): - app_name = "snakebids_app" - data: dict[str, str | bool] = { + +class DataFields(TypedDict): + full_name: str + email: str + app_full_name: str + github: str + app_description: str + build_system: BuildSystems + app_version: str + create_doc_template: bool + license: str + + +def get_empty_data(app_name: str, build: BuildSystems) -> DataFields: + data: DataFields = { "full_name": "", "email": "", "app_full_name": app_name, @@ -39,8 +56,173 @@ def test_template_dry_runs_successfully(tmp_path: Path, build: str, venv: str): if build == "poetry": data["full_name"] = "John Doe" data["email"] = "example@email.com" + return data + + +# emails complements of https://gist.github.com/cjaoude/fd9910626629b53c4d25 +def invalid_emails(): + return st.sampled_from( + [ + "plainaddress", + "#@%^%#$@#$@#.com", + "@example.com", + "Joe Smith ", + "email.example.com", + "email@example@example.com", + ".email@example.com", + "email.@example.com", + "email..email@example.com", + "あいうえお@example.com", + "email@example.com (Joe Smith)", + "email@example", + "email@-example.com", + "email@example.web-", + "email@[111.123.123.4444]", + "email@example..com", + "Abc..123@example.com", + r"”(),:;<>[\]@example.com", + "just”not”right@example.com", + r'this\ is"really"not\allowed@example.com', + ] + ) + + +@given(email=invalid_emails()) +@allow_function_scoped +def test_invalid_email_raises_error(email: str, tmp_path: Path): + data = get_empty_data("testapp", "setuptools") + data["email"] = email + with pytest.raises(prompt_toolkit.validation.ValidationError): + copier.run_copy( + str(Path(itx.first(snakebids.__path__), "project_template")), + tmp_path / data["app_full_name"], + data=data, + unsafe=True, + ) + + +@given( + name=st.text() + .filter(lambda s: not re.match(r"^[a-zA-Z_][a-zA-Z_0-9]*$", s)) + .filter(lambda s: pathvalidate.is_valid_filename(s, pathvalidate.Platform.LINUX)) +) +@allow_function_scoped +def test_invalid_app_name_raises_error(name: str, tmp_path: Path): + data = get_empty_data(name, "setuptools") + with pytest.raises(prompt_toolkit.validation.ValidationError): + copier.run_copy( + str(Path(itx.first(snakebids.__path__), "project_template")), + tmp_path / data["app_full_name"], + data=data, + unsafe=True, + ) + + +@pytest.mark.parametrize( + ["build", "build_backend"], + [ + ("poetry", "poetry.core.masonry.api"), + ("hatch", "hatchling.build"), + ("flit", "flit_core.buildapi"), + ("setuptools", "setuptools.build_meta"), + ], +) +def test_correct_build_system_used( + tmp_path: Path, build: BuildSystems, build_backend: str +): + tmpdir = Path(tempfile.mkdtemp(dir=tmp_path)) + data = get_empty_data("testapp", build) + copier.run_copy( + str(Path(itx.first(snakebids.__path__), "project_template")), + tmpdir / data["app_full_name"], + data=data, + unsafe=True, + ) + with open(tmpdir / data["app_full_name"] / "pyproject.toml", "rb") as f: + pyproject = tomllib.load(f) + assert pyproject["build-system"]["build-backend"] == build_backend + + +@given( + full_name=st.text(), + email=st.emails() | st.just(""), + app_full_name=st.from_regex(r"[a-zA-Z_][a-zA-Z_0-9]*", fullmatch=True), + github=st.text(), + app_description=st.text(), + app_version=st.text(min_size=1), + create_doc_template=st.just(False), + license=st.text(), +) +@settings(suppress_health_check=[HealthCheck.function_scoped_fixture], deadline=1000) +@pytest.mark.parametrize( + ["build"], [("poetry",), ("hatch",), ("flit",), ("setuptools",)] +) +def test_pyproject_correctly_formatted( + tmp_path: Path, build: BuildSystems, **kwargs: Unpack[DataFields] +): + tmpdir = Path(tempfile.mkdtemp(dir=tmp_path)) + kwargs["build_system"] = build + copier.run_copy( + str(Path(itx.first(snakebids.__path__), "project_template")), + tmpdir / kwargs["app_full_name"], + data=kwargs, + unsafe=True, + ) + with open(tmpdir / kwargs["app_full_name"] / "pyproject.toml", "rb") as f: + pyproject = tomllib.load(f) + if build == "poetry": + assert kwargs["app_full_name"] == pyproject["tool"]["poetry"]["name"] + assert kwargs["app_version"] == pyproject["tool"]["poetry"]["version"] + assert kwargs["app_description"] == pyproject["tool"]["poetry"]["description"] + if kwargs["license"]: + assert kwargs["license"] == pyproject["tool"]["poetry"]["license"] + else: + assert "license" not in pyproject["tool"]["poetry"] + if kwargs["full_name"]: + assert len(pyproject["tool"]["poetry"]["authors"]) == 1 + email_tag = f" <{kwargs['email']}>" if kwargs["email"] else "" + assert ( + f'{kwargs["full_name"]}{email_tag}' + == pyproject["tool"]["poetry"]["authors"][0] + ) + else: + assert "authors" not in pyproject["tool"]["poetry"] + return + + assert kwargs["app_full_name"] == pyproject["project"]["name"] + assert kwargs["app_version"] == pyproject["project"]["version"] + assert kwargs["app_description"] == pyproject["project"]["description"] + if kwargs["license"]: + assert kwargs["license"] == pyproject["project"]["license"] + else: + assert "license" not in pyproject["project"] + if kwargs["full_name"] or kwargs["email"]: + assert len(pyproject["project"]["authors"]) == 1 + author_obj: dict[str, str] = {} + if kwargs["full_name"]: + author_obj["name"] = kwargs["full_name"] + if kwargs["email"]: + author_obj["email"] = kwargs["email"] + assert author_obj == pyproject["project"]["authors"][0] + else: + assert "authors" not in pyproject["project"] + + +@needs_docker(f"snakebids/test-template:{platform.python_version()}") +@pytest.mark.parametrize( + ["build", "venv"], + [ + ("setuptools", "setuptools"), + ("poetry", "poetry"), + ("hatch", "hatch"), + ("flit", "pdm"), + ], +) +def test_template_dry_runs_successfully(tmp_path: Path, build: BuildSystems, venv: str): + app_name = "snakebids_app" + data = get_empty_data(app_name, build) - copier.run_copy( # type: ignore + copier.run_copy( str(Path(itx.first(snakebids.__path__), "project_template")), tmp_path / app_name, data=data, diff --git a/typings/copier.pyi b/typings/copier.pyi index 200e6852..7bcf5ae8 100644 --- a/typings/copier.pyi +++ b/typings/copier.pyi @@ -1,5 +1,5 @@ from os import PathLike -from typing import Any, AnyStr, Literal, TypedDict +from typing import Any, AnyStr, Literal, Mapping, TypedDict from git import Sequence from typing_extensions import TypeAlias, Unpack @@ -26,6 +26,6 @@ class CopierArgs(TypedDict, total=False): def run_copy( src_path: str, dst_path: AnyPath[str] | AnyPath[bytes] = ..., - data: dict[str, Any] | None = ..., + data: Mapping[str, Any] | None = ..., **kwargs: Unpack[CopierArgs] ) -> None: ...