From 769a0742f53ed572d80ccd6389903d1eb8305bbc Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 04:28:14 -0500 Subject: [PATCH 1/7] test(git): Pass git remote directly --- tests/test_git.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_git.py b/tests/test_git.py index 8d0967504..66cd214c9 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -8,7 +8,12 @@ import pytest from libvcs import exc -from libvcs.git import GitRemote, convert_pip_url as git_convert_pip_url, extract_status +from libvcs.git import ( + GitRemote, + GitRepo, + convert_pip_url as git_convert_pip_url, + extract_status, +) from libvcs.shortcuts import create_repo_from_pip_url from libvcs.util import run, which @@ -167,9 +172,9 @@ def test_remotes_preserves_git_ssh(repos_path, git_remote): remote_name = "myremote" remote_url = "git+ssh://git@github.com/tony/AlgoXY.git" - git_repo = create_repo_from_pip_url( - pip_url=f"git+file://{git_remote}", - repo_dir=repo_dir, + git_repo = GitRepo( + url=f"file://{git_remote}", + repo_dir=str(repo_dir), ) git_repo.obtain() git_repo.set_remote(name=remote_name, url=remote_url) From 7a1c5fb6025e1b3c6d442c5b2e8f9d43abeac3b3 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 04:51:01 -0500 Subject: [PATCH 2/7] test(git): Use both constructors via parametrize --- tests/test_git.py | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/tests/test_git.py b/tests/test_git.py index 66cd214c9..ee46d2981 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -165,17 +165,36 @@ def test_git_get_url_and_rev_from_pip_url(): assert rev == "eucalyptus" -def test_remotes_preserves_git_ssh(repos_path, git_remote): +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda git_remote, repo_dir, **kwargs: { + "url": f"file://{git_remote}", + "repo_dir": str(repo_dir), + }, + ], + [ + create_repo_from_pip_url, + lambda git_remote, repo_dir, **kwargs: { + "pip_url": f"git+file://{git_remote}", + "repo_dir": repo_dir, + }, + ], + ], +) +def test_remotes_preserves_git_ssh( + repos_path, git_remote, constructor, lazy_constructor_options +): # Regression test for #14 repo_name = "myexamplegit" repo_dir = repos_path / repo_name remote_name = "myremote" remote_url = "git+ssh://git@github.com/tony/AlgoXY.git" + git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) - git_repo = GitRepo( - url=f"file://{git_remote}", - repo_dir=str(repo_dir), - ) git_repo.obtain() git_repo.set_remote(name=remote_name, url=remote_url) From 0845655b1c1cae62b8ab8aaa086a828550da5b4f Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 05:08:56 -0500 Subject: [PATCH 3/7] test(test_remotes): Parametrize --- tests/test_git.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/test_git.py b/tests/test_git.py index ee46d2981..e29d41a9c 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -127,15 +127,32 @@ def progress_callback_spy(output, timestamp): assert progress_callback.called -def test_remotes(repos_path, git_remote): +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda git_remote, repos_path, repo_name, **kwargs: { + "url": f"file://{git_remote}", + "repo_dir": repos_path / repo_name, + }, + ], + [ + create_repo_from_pip_url, + lambda git_remote, repos_path, repo_name, **kwargs: { + "pip_url": f"git+file://{git_remote}", + "repo_dir": repos_path / repo_name, + }, + ], + ], +) +def test_remotes(repos_path, git_remote, constructor, lazy_constructor_options): repo_name = "myrepo" remote_name = "myremote" remote_url = "https://localhost/my/git/repo.git" - git_repo = create_repo_from_pip_url( - pip_url=f"git+file://{git_remote}", - repo_dir=repos_path / repo_name, - ) + git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() git_repo.set_remote(name=remote_name, url=remote_url) From 58f6f421dd9ecd297ddd80b047a49530465e8f47 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 05:15:34 -0500 Subject: [PATCH 4/7] tests: More parametrization --- tests/test_git.py | 66 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/tests/test_git.py b/tests/test_git.py index e29d41a9c..5a75e4df7 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -81,14 +81,30 @@ def test_repo_git_obtain_full(tmp_path: pathlib.Path, git_remote): assert os.path.exists(tmp_path / "myrepo") -def test_repo_update_handle_cases(tmp_path: pathlib.Path, git_remote, mocker): - git_repo = create_repo_from_pip_url( - **{ - "pip_url": f"git+file://{git_remote}", - "repo_dir": tmp_path / "myrepo", - } - ) - +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda git_remote, tmp_path, **kwargs: { + "url": f"file://{git_remote}", + "repo_dir": tmp_path / "myrepo", + }, + ], + [ + create_repo_from_pip_url, + lambda git_remote, tmp_path, **kwargs: { + "pip_url": f"git+file://{git_remote}", + "repo_dir": tmp_path / "myrepo", + }, + ], + ], +) +def test_repo_update_handle_cases( + tmp_path: pathlib.Path, git_remote, mocker, constructor, lazy_constructor_options +): + git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() # clone initial repo mocka = mocker.spy(git_repo, "run") git_repo.update_repo() @@ -103,7 +119,31 @@ def test_repo_update_handle_cases(tmp_path: pathlib.Path, git_remote, mocker): assert mocker.call(["symbolic-ref", "--short", "HEAD"]) not in mocka.mock_calls -def test_progress_callback(tmp_path: pathlib.Path, git_remote, mocker): +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda git_remote, tmp_path, progress_callback, **kwargs: { + "url": f"file://{git_remote}", + "repo_dir": tmp_path / "myrepo", + "progress_callback": progress_callback, + }, + ], + [ + create_repo_from_pip_url, + lambda git_remote, tmp_path, progress_callback, **kwargs: { + "pip_url": f"git+file://{git_remote}", + "repo_dir": tmp_path / "myrepo", + "progress_callback": progress_callback, + }, + ], + ], +) +def test_progress_callback( + tmp_path: pathlib.Path, git_remote, mocker, constructor, lazy_constructor_options +): def progress_callback_spy(output, timestamp): assert isinstance(output, str) assert isinstance(timestamp, datetime.datetime) @@ -115,13 +155,7 @@ def progress_callback_spy(output, timestamp): run(["git", "rev-parse", "HEAD"], cwd=git_remote) # create a new repo with the repo as a remote - git_repo = create_repo_from_pip_url( - **{ - "pip_url": f"git+file://{git_remote}", - "repo_dir": tmp_path / "myrepo", - "progress_callback": progress_callback, - } - ) + git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() assert progress_callback.called From d8c91b816fa337ba4cfea8cdb3e89b9ce2d0a9f5 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 05:18:23 -0500 Subject: [PATCH 5/7] tweak(tests): Annotation for pytest-mock fixture --- tests/test_git.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_git.py b/tests/test_git.py index 5a75e4df7..ef713e32e 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -7,6 +7,8 @@ import pytest +from pytest_mock import MockerFixture + from libvcs import exc from libvcs.git import ( GitRemote, @@ -102,7 +104,11 @@ def test_repo_git_obtain_full(tmp_path: pathlib.Path, git_remote): ], ) def test_repo_update_handle_cases( - tmp_path: pathlib.Path, git_remote, mocker, constructor, lazy_constructor_options + tmp_path: pathlib.Path, + git_remote, + mocker: MockerFixture, + constructor, + lazy_constructor_options, ): git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() # clone initial repo From 9f4ceb3fd175e9eb06c09c869d6db966f30c4efc Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 06:09:19 -0500 Subject: [PATCH 6/7] test(git): Parametrize test_repo_git_obtain_full --- tests/test_git.py | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/tests/test_git.py b/tests/test_git.py index ef713e32e..b7b8a4e0c 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -67,14 +67,33 @@ def test_repo_git_obtain_initial_commit_repo(tmp_path: pathlib.Path): assert git_repo.get_revision() == "initial" -def test_repo_git_obtain_full(tmp_path: pathlib.Path, git_remote): - git_repo = create_repo_from_pip_url( - **{ - "pip_url": f"git+file://{git_remote}", - "repo_dir": tmp_path / "myrepo", - } - ) - +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda git_remote, tmp_path, **kwargs: { + "url": f"file://{git_remote}", + "repo_dir": tmp_path / "myrepo", + }, + ], + [ + create_repo_from_pip_url, + lambda git_remote, tmp_path, **kwargs: { + "pip_url": f"git+file://{git_remote}", + "repo_dir": tmp_path / "myrepo", + }, + ], + ], +) +def test_repo_git_obtain_full( + tmp_path: pathlib.Path, + git_remote, + constructor, + lazy_constructor_options, +): + git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() test_repo_revision = run(["git", "rev-parse", "HEAD"], cwd=git_remote) From 5fe95a7489b233afc1e11b4c3140c3363c612276 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 27 Mar 2022 06:11:30 -0500 Subject: [PATCH 7/7] test(git): Update test_repo_git_obtain_initial_commit_repo --- tests/test_git.py | 107 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 83 insertions(+), 24 deletions(-) diff --git a/tests/test_git.py b/tests/test_git.py index b7b8a4e0c..36f911ccf 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -4,6 +4,7 @@ import os import pathlib import textwrap +from typing import Callable import pytest @@ -23,6 +24,10 @@ pytestmark = pytest.mark.skip(reason="git is not available") +RepoTestFactory = Callable[..., GitRepo] +RepoTestFactoryLazyKwargs = Callable[..., dict] + + @pytest.fixture(autouse=True, scope="module") def gitconfig(user_path: pathlib.Path): gitconfig = user_path / ".gitconfig" @@ -44,7 +49,31 @@ def gitconfig_default(monkeypatch: pytest.MonkeyPatch, user_path: pathlib.Path): monkeypatch.setenv("HOME", str(user_path)) -def test_repo_git_obtain_initial_commit_repo(tmp_path: pathlib.Path): +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda bare_repo_dir, tmp_path, **kwargs: { + "url": f"file://{bare_repo_dir}", + "repo_dir": tmp_path / "obtaining a bare repo", + }, + ], + [ + create_repo_from_pip_url, + lambda bare_repo_dir, tmp_path, **kwargs: { + "pip_url": f"git+file://{bare_repo_dir}", + "repo_dir": tmp_path / "obtaining a bare repo", + }, + ], + ], +) +def test_repo_git_obtain_initial_commit_repo( + tmp_path: pathlib.Path, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, +): """initial commit repos return 'initial'. note: this behaviors differently from git(1)'s use of the word "bare". @@ -55,13 +84,7 @@ def test_repo_git_obtain_initial_commit_repo(tmp_path: pathlib.Path): run(["git", "init", repo_name], cwd=tmp_path) bare_repo_dir = tmp_path / repo_name - - git_repo = create_repo_from_pip_url( - **{ - "pip_url": f"git+file://{bare_repo_dir}", - "repo_dir": tmp_path / "obtaining a bare repo", - } - ) + git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() assert git_repo.get_revision() == "initial" @@ -90,8 +113,8 @@ def test_repo_git_obtain_initial_commit_repo(tmp_path: pathlib.Path): def test_repo_git_obtain_full( tmp_path: pathlib.Path, git_remote, - constructor, - lazy_constructor_options, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, ): git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() @@ -124,10 +147,10 @@ def test_repo_git_obtain_full( ) def test_repo_update_handle_cases( tmp_path: pathlib.Path, - git_remote, + git_remote: pathlib.Path, mocker: MockerFixture, - constructor, - lazy_constructor_options, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, ): git_repo: GitRepo = constructor(**lazy_constructor_options(**locals())) git_repo.obtain() # clone initial repo @@ -167,7 +190,11 @@ def test_repo_update_handle_cases( ], ) def test_progress_callback( - tmp_path: pathlib.Path, git_remote, mocker, constructor, lazy_constructor_options + tmp_path: pathlib.Path, + git_remote: pathlib.Path, + mocker: MockerFixture, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, ): def progress_callback_spy(output, timestamp): assert isinstance(output, str) @@ -206,7 +233,12 @@ def progress_callback_spy(output, timestamp): ], ], ) -def test_remotes(repos_path, git_remote, constructor, lazy_constructor_options): +def test_remotes( + repos_path: pathlib.Path, + git_remote: pathlib.Path, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, +): repo_name = "myrepo" remote_name = "myremote" remote_url = "https://localhost/my/git/repo.git" @@ -262,7 +294,10 @@ def test_git_get_url_and_rev_from_pip_url(): ], ) def test_remotes_preserves_git_ssh( - repos_path, git_remote, constructor, lazy_constructor_options + repos_path: pathlib.Path, + git_remote: pathlib.Path, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, ): # Regression test for #14 repo_name = "myexamplegit" @@ -280,7 +315,31 @@ def test_remotes_preserves_git_ssh( ) -def test_private_ssh_format(pip_url_kwargs): +@pytest.mark.parametrize( + # Postpone evaluation of options so fixture variables can interpolate + "constructor,lazy_constructor_options", + [ + [ + GitRepo, + lambda bare_repo_dir, tmp_path, **kwargs: { + "url": f"file://{bare_repo_dir}", + "repo_dir": tmp_path / "obtaining a bare repo", + }, + ], + [ + create_repo_from_pip_url, + lambda bare_repo_dir, tmp_path, **kwargs: { + "pip_url": f"git+file://{bare_repo_dir}", + "repo_dir": tmp_path / "obtaining a bare repo", + }, + ], + ], +) +def test_private_ssh_format( + pip_url_kwargs: dict, + constructor: RepoTestFactory, + lazy_constructor_options: RepoTestFactoryLazyKwargs, +): pip_url_kwargs.update( **{"pip_url": "git+ssh://github.com:/tmp/omg/private_ssh_repo"} ) @@ -290,14 +349,14 @@ def test_private_ssh_format(pip_url_kwargs): excinfo.match(r"is malformatted") -def test_ls_remotes(git_repo): +def test_ls_remotes(git_repo: GitRepo): remotes = git_repo.remotes() assert "origin" in remotes assert "origin" in git_repo.remotes(flat=True) -def test_get_remotes(git_repo): +def test_get_remotes(git_repo: GitRepo): assert "origin" in git_repo.remotes() @@ -307,7 +366,7 @@ def test_get_remotes(git_repo): ["myrepo", "file:///apples"], ], ) -def test_set_remote(git_repo, repo_name, new_repo_url): +def test_set_remote(git_repo: GitRepo, repo_name: str, new_repo_url: str): mynewremote = git_repo.set_remote(name=repo_name, url="file:///") assert "file:///" in mynewremote, "set_remote returns remote" @@ -329,13 +388,13 @@ def test_set_remote(git_repo, repo_name, new_repo_url): ), "Running remove_set should overwrite previous remote" -def test_get_git_version(git_repo): +def test_get_git_version(git_repo: GitRepo): expected_version = git_repo.run(["--version"]).replace("git version ", "") assert git_repo.get_git_version() assert expected_version == git_repo.get_git_version() -def test_get_current_remote_name(git_repo): +def test_get_current_remote_name(git_repo: GitRepo): assert git_repo.get_current_remote_name() == "origin" new_branch = "another-branch-with-no-upstream" @@ -428,7 +487,7 @@ def test_extract_status(): ], ], ) -def test_extract_status_b(fixture, expected_result): +def test_extract_status_b(fixture: str, expected_result: dict): assert ( extract_status(textwrap.dedent(fixture))._asdict().items() >= expected_result.items() @@ -478,7 +537,7 @@ def test_extract_status_b(fixture, expected_result): ], ], ) -def test_extract_status_c(fixture, expected_result): +def test_extract_status_c(fixture: str, expected_result: dict): assert ( expected_result.items() <= extract_status(textwrap.dedent(fixture))._asdict().items()