From a1b1b4dbf21e76e241e906b6f439155278368fa9 Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Tue, 28 Sep 2021 21:01:00 -0500 Subject: [PATCH 1/9] test: add tests using git (#347) git is isolated using the following environment, which are set globally for the duration of that the new pytest fixture "tmp_git" is used: GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM HOME GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME GIT_AUTHOR_DATE GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_DATE GIT_DIR and GIT_WORK_TREE could be set so that git can be called using any method, but instead a git() function is added that uses git's -C command-line option. These were taken from one of git's test scripts: https://github.com/git/git/blob/cefe983a320c03d7843ac78e73bd513a27806845/t/test-lib.sh#L454-L461 There are probably other ways git can be isolated. The repository is initialized with an empty commit, but this isn't strictly necessary, it just makes some of the possible tests require less setup. The new tmp_project fixture copies the sample module from ./test/samples/module1_toml to the project and commits the files. A test is added for #345 as an example of how this can be used. A pytest marker is added so that tests with either "needgit" or "needsgit" in the name are skipped if python can't find an executable named "git". The tox configuration is changed and another pytest marker is added so that those tests can be run by themselves: tox -- -m needgit or skipped: tox -- -m "not needgit" Type hints were added to help with development, but aren't necessary to keep. --- pyproject.toml | 6 ++++ tests/conftest.py | 86 +++++++++++++++++++++++++++++++++++++++++++-- tests/test_build.py | 9 +++++ tox.ini | 2 +- 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7a613581..6ac8dbb9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,3 +42,9 @@ Source = "https://github.com/takluyver/flit" [project.scripts] flit = "flit:main" + +[tool.pytest.ini_options] +markers = [ + "needgit: needs git to be installed in order to run", + "needsgit: needs git to be installed in order to run", +] diff --git a/tests/conftest.py b/tests/conftest.py index 4c5ecef1..0a7e7de2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,15 +1,97 @@ from pathlib import Path +from shutil import copy, copytree, which +from subprocess import check_output +from typing import TYPE_CHECKING +from unittest.mock import patch + import pytest -from shutil import copytree -samples_dir = Path(__file__).parent / 'samples' +if TYPE_CHECKING: + from typing import Iterator, List, Union + +samples_dir = Path(__file__).parent / "samples" + +skip_if_no_git = pytest.mark.skipif( + (not which("git")), + reason="needs git to be installed and findable through the PATH environment variable", +) + + +def pytest_collection_modifyitems(items): + for item in items: + if "needgit" in item.nodeid or "needsgit" in item.nodeid: + item.add_marker(skip_if_no_git) + item.add_marker(pytest.mark.needgit) + item.add_marker(pytest.mark.needsgit) + @pytest.fixture def copy_sample(tmp_path): """Copy a subdirectory from the samples dir to a temp dir""" + def copy(dirname): dst = tmp_path / dirname copytree(str(samples_dir / dirname), str(dst)) return dst return copy + + +def git(repo: Path, command: "Union[List[str], str]") -> bytes: + if isinstance(command, str): + args = command.split() + else: + args = command + + return check_output( + ["git", "-C", str(repo), *args], + ) + + +@pytest.fixture +def tmp_git(tmp_path: Path) -> "Iterator[Path]": + """ + Make a git repository in a temporary folder + + The path returned is what should be passed to git's -C command, or what cwd + should be set to in subprocess calls + """ + git_global_config = tmp_path / "git_global_config" + git_global_config.touch(exist_ok=False) + repository = tmp_path / "repository" + repository.mkdir(exist_ok=False) + with patch.dict( + "os.environ", + { + # https://git-scm.com/docs/git#Documentation/git.txt-codeGITCONFIGGLOBALcode + "GIT_CONFIG_GLOBAL": str(git_global_config), + # https://git-scm.com/docs/git#Documentation/git.txt-codeGITCONFIGNOSYSTEMcode + "GIT_CONFIG_NOSYSTEM": "true", + "HOME": str(tmp_path), + # tox by default only passes the PATH environment variable, so + # XDG_CONFIG_HOME is already unset + # https://github.com/git/git/blob/cefe983a320c03d7843ac78e73bd513a27806845/t/test-lib.sh#L454-L461 + "GIT_AUTHOR_EMAIL": "author@example.com", + "GIT_AUTHOR_NAME": "A U Thor", + "GIT_AUTHOR_DATE": "1112354055 +0200", + "GIT_COMMITTER_EMAIL": "committer@example.com", + "GIT_COMMITTER_NAME": "committer", + "GIT_COMMITTER_DATE": "1112354055 +0200", + }, + ): + git(repository, "config --global init.defaultBranch main") + git(repository, ["init"]) + git(repository, "commit --allow-empty --allow-empty-message --no-edit") + + yield repository + + +@pytest.fixture +def tmp_project(tmp_git: Path) -> "Iterator[Path]": + "return a path to the root of a git repository containing a sample package" + for file in (samples_dir / "module1_toml").glob("*"): + copy(str(file), str(tmp_git / file.name)) + git(tmp_git, "add -A :/") + git(tmp_git, "commit --allow-empty --allow-empty-message --no-edit") + + yield tmp_git diff --git a/tests/test_build.py b/tests/test_build.py index c17ba649..2ee485eb 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -5,6 +5,7 @@ from tempfile import TemporaryDirectory from testpath import assert_isdir, MockCommand +from .conftest import git from flit_core import common from flit import build @@ -69,3 +70,11 @@ def test_build_module_no_docstring(): with pytest.raises(common.NoDocstringError) as exc_info: build.main(pyproject) assert 'no_docstring.py' in str(exc_info.value) + +def test_build_needgit_unicode_filenames(tmp_project: Path) -> None: + "does a package build if it includes a unicode filename?" + noel_file = tmp_project / "No\N{LATIN SMALL LETTER E WITH DIAERESIS}l" + noel_file.touch() + git(tmp_project, "add -A :/") + git(tmp_project, "commit --allow-empty --allow-empty-message --no-edit") + build.main(tmp_project / "pyproject.toml") diff --git a/tox.ini b/tox.ini index 59ddc05d..7f807b2b 100644 --- a/tox.ini +++ b/tox.ini @@ -27,7 +27,7 @@ setenv = PYTHONPATH = flit_core commands = - python -m pytest --cov=flit --cov=flit_core/flit_core + python -m pytest --cov=flit --cov=flit_core/flit_core {posargs} [testenv:bootstrap] skip_install = true From 872cf57ffb9b1d60659bf6e757dc43319ffb084a Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 01:01:39 -0500 Subject: [PATCH 2/9] test(conftest): str.split() -> shlex.split() --- tests/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0a7e7de2..b87c53b6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import shlex from pathlib import Path from shutil import copy, copytree, which from subprocess import check_output @@ -39,7 +40,7 @@ def copy(dirname): def git(repo: Path, command: "Union[List[str], str]") -> bytes: if isinstance(command, str): - args = command.split() + args = shlex.split(command) else: args = command From b245dcfef29535b03e1e90f6378276617ff59a38 Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 01:04:01 -0500 Subject: [PATCH 3/9] test(conftest): tmp_git -> tmp_git_repo --- tests/conftest.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index b87c53b6..a53aa2be 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -50,7 +50,7 @@ def git(repo: Path, command: "Union[List[str], str]") -> bytes: @pytest.fixture -def tmp_git(tmp_path: Path) -> "Iterator[Path]": +def tmp_git_repo(tmp_path: Path) -> "Iterator[Path]": """ Make a git repository in a temporary folder @@ -88,11 +88,11 @@ def tmp_git(tmp_path: Path) -> "Iterator[Path]": @pytest.fixture -def tmp_project(tmp_git: Path) -> "Iterator[Path]": +def tmp_project(tmp_git_repo: Path) -> "Iterator[Path]": "return a path to the root of a git repository containing a sample package" for file in (samples_dir / "module1_toml").glob("*"): - copy(str(file), str(tmp_git / file.name)) - git(tmp_git, "add -A :/") - git(tmp_git, "commit --allow-empty --allow-empty-message --no-edit") + copy(str(file), str(tmp_git_repo / file.name)) + git(tmp_git_repo, "add -A :/") + git(tmp_git_repo, "commit --allow-empty --allow-empty-message --no-edit") - yield tmp_git + yield tmp_git_repo From ed360ad52daf524955f040396f3bf676ad7e52c1 Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 01:37:18 -0500 Subject: [PATCH 4/9] test: rename git_cmd -> git and make closure --- tests/conftest.py | 30 +++++++++++++++++++++--------- tests/test_build.py | 7 +++---- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a53aa2be..9daf2e77 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import shlex +from functools import wraps from pathlib import Path from shutil import copy, copytree, which from subprocess import check_output @@ -8,7 +9,9 @@ import pytest if TYPE_CHECKING: - from typing import Iterator, List, Union + from typing import Callable, Iterator, List, Union + + GitCmd = Callable[Union[str, List[str]], bytes] samples_dir = Path(__file__).parent / "samples" @@ -38,14 +41,14 @@ def copy(dirname): return copy -def git(repo: Path, command: "Union[List[str], str]") -> bytes: +def git_cmd(repository_path: Path, command: "Union[List[str], str]") -> bytes: if isinstance(command, str): args = shlex.split(command) else: args = command return check_output( - ["git", "-C", str(repo), *args], + ["git", "-C", str(repository_path), *args], ) @@ -80,19 +83,28 @@ def tmp_git_repo(tmp_path: Path) -> "Iterator[Path]": "GIT_COMMITTER_DATE": "1112354055 +0200", }, ): - git(repository, "config --global init.defaultBranch main") - git(repository, ["init"]) - git(repository, "commit --allow-empty --allow-empty-message --no-edit") + git_cmd(repository, "config --global init.defaultBranch main") + git_cmd(repository, ["init"]) + git_cmd(repository, "commit --allow-empty --allow-empty-message --no-edit") yield repository @pytest.fixture -def tmp_project(tmp_git_repo: Path) -> "Iterator[Path]": +def git(tmp_git_repo: Path) -> "Iterator[GitCmd]": + @wraps(git_cmd) + def wrapper(command: "Union[str, List[str]]") -> bytes: + return git_cmd(repository_path=tmp_git_repo, command=command) + + return wrapper + + +@pytest.fixture +def tmp_project(tmp_git_repo: Path, git: "GitCmd") -> "Iterator[Path]": "return a path to the root of a git repository containing a sample package" for file in (samples_dir / "module1_toml").glob("*"): copy(str(file), str(tmp_git_repo / file.name)) - git(tmp_git_repo, "add -A :/") - git(tmp_git_repo, "commit --allow-empty --allow-empty-message --no-edit") + git("add -A :/") + git("commit --allow-empty --allow-empty-message --no-edit") yield tmp_git_repo diff --git a/tests/test_build.py b/tests/test_build.py index 2ee485eb..ead24721 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -5,7 +5,6 @@ from tempfile import TemporaryDirectory from testpath import assert_isdir, MockCommand -from .conftest import git from flit_core import common from flit import build @@ -71,10 +70,10 @@ def test_build_module_no_docstring(): build.main(pyproject) assert 'no_docstring.py' in str(exc_info.value) -def test_build_needgit_unicode_filenames(tmp_project: Path) -> None: +def test_build_needgit_unicode_filenames(tmp_project: Path, git: "GitCmd") -> None: "does a package build if it includes a unicode filename?" noel_file = tmp_project / "No\N{LATIN SMALL LETTER E WITH DIAERESIS}l" noel_file.touch() - git(tmp_project, "add -A :/") - git(tmp_project, "commit --allow-empty --allow-empty-message --no-edit") + git("add -A :/") + git("commit --allow-empty --allow-empty-message --no-edit") build.main(tmp_project / "pyproject.toml") From ea1e051bffc134c2fc168735046f6c54b4a50045 Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 02:16:21 -0500 Subject: [PATCH 5/9] test: remove type annotations --- tests/conftest.py | 16 +++++----------- tests/test_build.py | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 9daf2e77..bf4c5da7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,16 +3,10 @@ from pathlib import Path from shutil import copy, copytree, which from subprocess import check_output -from typing import TYPE_CHECKING from unittest.mock import patch import pytest -if TYPE_CHECKING: - from typing import Callable, Iterator, List, Union - - GitCmd = Callable[Union[str, List[str]], bytes] - samples_dir = Path(__file__).parent / "samples" skip_if_no_git = pytest.mark.skipif( @@ -41,7 +35,7 @@ def copy(dirname): return copy -def git_cmd(repository_path: Path, command: "Union[List[str], str]") -> bytes: +def git_cmd(repository_path, command): if isinstance(command, str): args = shlex.split(command) else: @@ -53,7 +47,7 @@ def git_cmd(repository_path: Path, command: "Union[List[str], str]") -> bytes: @pytest.fixture -def tmp_git_repo(tmp_path: Path) -> "Iterator[Path]": +def tmp_git_repo(tmp_path): """ Make a git repository in a temporary folder @@ -91,16 +85,16 @@ def tmp_git_repo(tmp_path: Path) -> "Iterator[Path]": @pytest.fixture -def git(tmp_git_repo: Path) -> "Iterator[GitCmd]": +def git(tmp_git_repo): @wraps(git_cmd) - def wrapper(command: "Union[str, List[str]]") -> bytes: + def wrapper(command): return git_cmd(repository_path=tmp_git_repo, command=command) return wrapper @pytest.fixture -def tmp_project(tmp_git_repo: Path, git: "GitCmd") -> "Iterator[Path]": +def tmp_project(tmp_git_repo, git): "return a path to the root of a git repository containing a sample package" for file in (samples_dir / "module1_toml").glob("*"): copy(str(file), str(tmp_git_repo / file.name)) diff --git a/tests/test_build.py b/tests/test_build.py index ead24721..89cf8bc4 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -70,7 +70,7 @@ def test_build_module_no_docstring(): build.main(pyproject) assert 'no_docstring.py' in str(exc_info.value) -def test_build_needgit_unicode_filenames(tmp_project: Path, git: "GitCmd") -> None: +def test_build_needgit_unicode_filenames(tmp_project, git): "does a package build if it includes a unicode filename?" noel_file = tmp_project / "No\N{LATIN SMALL LETTER E WITH DIAERESIS}l" noel_file.touch() From 7430aee7e4f60a76a269e09e234136245e26bb63 Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 18:51:15 -0500 Subject: [PATCH 6/9] test: explicitly use only needsgit marker --- pyproject.toml | 1 - tests/conftest.py | 4 +--- tests/test_build.py | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6ac8dbb9..b55c6e68 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,5 @@ flit = "flit:main" [tool.pytest.ini_options] markers = [ - "needgit: needs git to be installed in order to run", "needsgit: needs git to be installed in order to run", ] diff --git a/tests/conftest.py b/tests/conftest.py index bf4c5da7..7832e727 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,10 +17,8 @@ def pytest_collection_modifyitems(items): for item in items: - if "needgit" in item.nodeid or "needsgit" in item.nodeid: + if item.get_closest_marker("needsgit"): item.add_marker(skip_if_no_git) - item.add_marker(pytest.mark.needgit) - item.add_marker(pytest.mark.needsgit) @pytest.fixture diff --git a/tests/test_build.py b/tests/test_build.py index 89cf8bc4..559ecfda 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -70,6 +70,7 @@ def test_build_module_no_docstring(): build.main(pyproject) assert 'no_docstring.py' in str(exc_info.value) +@pytest.mark.needsgit def test_build_needgit_unicode_filenames(tmp_project, git): "does a package build if it includes a unicode filename?" noel_file = tmp_project / "No\N{LATIN SMALL LETTER E WITH DIAERESIS}l" From a47ae163096a63cf003372eedfbd9d2c613dafc4 Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 19:14:27 -0500 Subject: [PATCH 7/9] test: ensure env vars are removed --- tests/conftest.py | 54 ++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 7832e727..346e3130 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,6 @@ from pathlib import Path from shutil import copy, copytree, which from subprocess import check_output -from unittest.mock import patch import pytest @@ -45,7 +44,7 @@ def git_cmd(repository_path, command): @pytest.fixture -def tmp_git_repo(tmp_path): +def tmp_git_repo(tmp_path, monkeypatch): """ Make a git repository in a temporary folder @@ -56,30 +55,33 @@ def tmp_git_repo(tmp_path): git_global_config.touch(exist_ok=False) repository = tmp_path / "repository" repository.mkdir(exist_ok=False) - with patch.dict( - "os.environ", - { - # https://git-scm.com/docs/git#Documentation/git.txt-codeGITCONFIGGLOBALcode - "GIT_CONFIG_GLOBAL": str(git_global_config), - # https://git-scm.com/docs/git#Documentation/git.txt-codeGITCONFIGNOSYSTEMcode - "GIT_CONFIG_NOSYSTEM": "true", - "HOME": str(tmp_path), - # tox by default only passes the PATH environment variable, so - # XDG_CONFIG_HOME is already unset - # https://github.com/git/git/blob/cefe983a320c03d7843ac78e73bd513a27806845/t/test-lib.sh#L454-L461 - "GIT_AUTHOR_EMAIL": "author@example.com", - "GIT_AUTHOR_NAME": "A U Thor", - "GIT_AUTHOR_DATE": "1112354055 +0200", - "GIT_COMMITTER_EMAIL": "committer@example.com", - "GIT_COMMITTER_NAME": "committer", - "GIT_COMMITTER_DATE": "1112354055 +0200", - }, - ): - git_cmd(repository, "config --global init.defaultBranch main") - git_cmd(repository, ["init"]) - git_cmd(repository, "commit --allow-empty --allow-empty-message --no-edit") - - yield repository + + git_environment_variables = { + # https://git-scm.com/docs/git#Documentation/git.txt-codeGITCONFIGGLOBALcode + "GIT_CONFIG_GLOBAL": str(git_global_config), + # https://git-scm.com/docs/git#Documentation/git.txt-codeGITCONFIGNOSYSTEMcode + "GIT_CONFIG_NOSYSTEM": "true", + "HOME": str(tmp_path), + # https://github.com/git/git/blob/cefe983a320c03d7843ac78e73bd513a27806845/t/test-lib.sh#L454-L461 + "GIT_AUTHOR_EMAIL": "author@example.com", + "GIT_AUTHOR_NAME": "A U Thor", + "GIT_AUTHOR_DATE": "1112354055 +0200", + "GIT_COMMITTER_EMAIL": "committer@example.com", + "GIT_COMMITTER_NAME": "committer", + "GIT_COMMITTER_DATE": "1112354055 +0200", + } + for name, value in git_environment_variables.items(): + monkeypatch.setenv(name, value) + + # https://github.com/git/git/blob/cefe983a320c03d7843ac78e73bd513a27806845/t/test-lib.sh#L454-L461 + for name in ["XDG_CONFIG_HOME", "XDG_CACHE_HOME"]: + monkeypatch.delenv(name, raising=False) + + git_cmd(repository, "config --global init.defaultBranch main") + git_cmd(repository, ["init"]) + git_cmd(repository, "commit --allow-empty --allow-empty-message --no-edit") + + yield repository @pytest.fixture From 8696c3a3c535fb8bcd4ebbb67832677428a9e03a Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 19:16:52 -0500 Subject: [PATCH 8/9] test: make marker descriptions match --- pyproject.toml | 2 +- tests/conftest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b55c6e68..5ff06cf4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,5 +45,5 @@ flit = "flit:main" [tool.pytest.ini_options] markers = [ - "needsgit: needs git to be installed in order to run", + "needsgit: needs git to be installed and findable through the PATH environment variable in order to run these tests", ] diff --git a/tests/conftest.py b/tests/conftest.py index 346e3130..a97f4dbb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,7 +10,7 @@ skip_if_no_git = pytest.mark.skipif( (not which("git")), - reason="needs git to be installed and findable through the PATH environment variable", + reason="needs git to be installed and findable through the PATH environment variable in order to run these tests", ) From 4d9fe7451f99d07131a3c85a4d78cf81ff615a6b Mon Sep 17 00:00:00 2001 From: Matthew Willcockson Date: Wed, 29 Sep 2021 20:03:35 -0500 Subject: [PATCH 9/9] test: remove needgit from func name --- tests/test_build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_build.py b/tests/test_build.py index 559ecfda..d0a8f382 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -71,7 +71,7 @@ def test_build_module_no_docstring(): assert 'no_docstring.py' in str(exc_info.value) @pytest.mark.needsgit -def test_build_needgit_unicode_filenames(tmp_project, git): +def test_build_unicode_filenames(tmp_project, git): "does a package build if it includes a unicode filename?" noel_file = tmp_project / "No\N{LATIN SMALL LETTER E WITH DIAERESIS}l" noel_file.touch()