From ea4206b5de989608c86dbd339465f22681f6e18d Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Tue, 4 Jul 2023 16:50:08 +1000 Subject: [PATCH 01/24] add initial function for parsing show status output --- src/repository.py | 99 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/src/repository.py b/src/repository.py index bbaf598e..a2c1d70f 100644 --- a/src/repository.py +++ b/src/repository.py @@ -8,8 +8,9 @@ import re from collections.abc import Iterator, Sequence from contextlib import contextmanager +from enum import Enum from functools import cached_property -from itertools import chain +from itertools import chain, takewhile from pathlib import Path from typing import Any, NamedTuple @@ -630,3 +631,99 @@ def create_repository_client(access_token: str | None, base_path: Path) -> Clien repository_fullname = _get_repository_name_from_git_url(remote_url=remote_url) remote_repo = github_client.get_repo(repository_fullname) return Client(repository=local_repo, github_repository=remote_repo) + + +class _CommitFileAdded(NamedTuple): + """File that was added or copied in a commit. + + Attributes: + path: The location of the file on disk. + content: The content of the file. + """ + + path: Path + content: str + + +class _CommitFileModified(NamedTuple): + """File that was modified in a commit. + + Attributes: + path: The location of the file on disk. + content: The content of the file. + """ + + path: Path + content: str + + +class _CommitFileDeleted(NamedTuple): + """File that was deleted in a commit. + + Attributes: + path: The location of the file on disk. + """ + + path: Path + + +# Copied will be mapped to added and renamed will be mapped to be a delete and add +_CommitFile = _CommitFileAdded | _CommitFileModified | _CommitFileDeleted +_ADDED_PATTERN = re.compile(r"A\s*(\S*)") +_MODIFIED_PATTERN = re.compile(r"M\s*(\S*)") +_DELETED_PATTERN = re.compile(r"D\s*(\S*)") +_RENAMED_PATTERN = re.compile(r"R\d+\s*(\S*)\s*(\S*)") +_COPIED_PATTERN = re.compile(r"C\d+\s*(\S*)\s*(\S*)") + + +def _parse_git_show(output: str) -> Iterator[_CommitFile]: + """Parse the output of a git show with --name-status intmanageable files. + + Args: + output: The output of the git show command. + + Yields: + Information about each of the files that changed in the commit. + """ + # Processing in reverse up to empty line to detect end of file changes as an empty line. + # Example output: + # git show --name-status + # commit (HEAD -> ) + # Author: + # Date: + + # + + # A add-file.text + # M change-file.text + # D delete-file.txt + # R100 renamed-file.text is-renamed-file.text + # C100 to-be-copied-file.text copied-file.text + lines = takewhile(lambda line: bool(line), reversed(output.splitlines())) + for line in lines: + if (added_match := _ADDED_PATTERN.match(line)) is not None: + path = Path(added_match.group(1)) + yield _CommitFileAdded(path, path.read_text(encoding="utf-8")) + continue + + if (copied_match := _COPIED_PATTERN.match(line)) is not None: + path = Path(copied_match.group(2)) + yield _CommitFileAdded(path, path.read_text(encoding="utf-8")) + continue + + if (modified_match := _MODIFIED_PATTERN.match(line)) is not None: + path = Path(modified_match.group(1)) + yield _CommitFileModified(path, path.read_text(encoding="utf-8")) + continue + + if (delete_match := _DELETED_PATTERN.match(line)) is not None: + path = Path(delete_match.group(1)) + yield _CommitFileDeleted(path) + continue + + if (renamed_match := _RENAMED_PATTERN.match(line)) is not None: + old_path = Path(renamed_match.group(1)) + path = Path(renamed_match.group(2)) + yield _CommitFileDeleted(old_path) + yield _CommitFileAdded(path, path.read_text(encoding="utf-8")) + continue From 096c3d06d845b9939ec8fdffbccd26c2b402a8f1 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 00:39:15 +1000 Subject: [PATCH 02/24] add tests for parsing git show output --- src/repository.py | 11 +- tests/unit/test_repository.py | 195 ++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 5 deletions(-) diff --git a/src/repository.py b/src/repository.py index a2c1d70f..9fee9320 100644 --- a/src/repository.py +++ b/src/repository.py @@ -676,11 +676,12 @@ class _CommitFileDeleted(NamedTuple): _COPIED_PATTERN = re.compile(r"C\d+\s*(\S*)\s*(\S*)") -def _parse_git_show(output: str) -> Iterator[_CommitFile]: +def _parse_git_show(output: str, repository_path: Path) -> Iterator[_CommitFile]: """Parse the output of a git show with --name-status intmanageable files. Args: output: The output of the git show command. + repository_path: The path to the git repository. Yields: Information about each of the files that changed in the commit. @@ -703,17 +704,17 @@ def _parse_git_show(output: str) -> Iterator[_CommitFile]: for line in lines: if (added_match := _ADDED_PATTERN.match(line)) is not None: path = Path(added_match.group(1)) - yield _CommitFileAdded(path, path.read_text(encoding="utf-8")) + yield _CommitFileAdded(path, (repository_path / path).read_text(encoding="utf-8")) continue if (copied_match := _COPIED_PATTERN.match(line)) is not None: path = Path(copied_match.group(2)) - yield _CommitFileAdded(path, path.read_text(encoding="utf-8")) + yield _CommitFileAdded(path, (repository_path / path).read_text(encoding="utf-8")) continue if (modified_match := _MODIFIED_PATTERN.match(line)) is not None: path = Path(modified_match.group(1)) - yield _CommitFileModified(path, path.read_text(encoding="utf-8")) + yield _CommitFileModified(path, (repository_path / path).read_text(encoding="utf-8")) continue if (delete_match := _DELETED_PATTERN.match(line)) is not None: @@ -725,5 +726,5 @@ def _parse_git_show(output: str) -> Iterator[_CommitFile]: old_path = Path(renamed_match.group(1)) path = Path(renamed_match.group(2)) yield _CommitFileDeleted(old_path) - yield _CommitFileAdded(path, path.read_text(encoding="utf-8")) + yield _CommitFileAdded(path, (repository_path / path).read_text(encoding="utf-8")) continue diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index 4c7e051c..866d3823 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -1021,3 +1021,198 @@ def test_create_pull_request_function( assert ( upstream_repository_path / DOCUMENTATION_FOLDER_NAME / filler_file ).read_text() == filler_text + + +def test__parse_git_show_empty(repository_client: Client): + """ + arrange: given empty show output + act: when output is passed to _parse_git_show + assert: then no files are returned. + """ + show_output = "" + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=Path())) + + assert len(commit_files) == 0 + + +def test__parse_git_show_unsupported(repository_client: Client): + """ + arrange: given show output that includes a line that is unknown + act: when output is passed to _parse_git_show + assert: then no files are returned. + """ + show_output = "X file.text" + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=Path())) + + assert len(commit_files) == 0 + + +def test__parse_git_show_added(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and show is called and the output passed to _parse_git_show + assert: then _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text( + contents := "content 1", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == file + assert isinstance(commit_file, repository._CommitFileAdded) + assert commit_file.content == contents + + +def test__parse_git_show_modified(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then modified and show is called and the output passed to + _parse_git_show + assert: then _CommitFileModified is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).write_text(contents := "content 2", encoding="utf-8") + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == file + assert isinstance(commit_file, repository._CommitFileModified) + assert commit_file.content == contents + + +def test__parse_git_show_deleted(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then deleted and show is called and the output passed to + _parse_git_show + assert: then _CommitFileDeleted is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).unlink() + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == file + assert isinstance(commit_file, repository._CommitFileDeleted) + + +def test__parse_git_show_renamed(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then renamed and show is called and the output passed to + _parse_git_show + assert: then _CommitFileDeleted and _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text( + contents := "content 1", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).rename(repository_path / (new_file := Path("other_file.text"))) + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 2 + commit_file_delete = commit_files[0] + assert commit_file_delete.path == file + assert isinstance(commit_file_delete, repository._CommitFileDeleted) + commit_file_add = commit_files[1] + assert commit_file_add.path == new_file + assert isinstance(commit_file_add, repository._CommitFileAdded) + assert commit_file_add.content == contents + + +def test__parse_git_show_copied(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then renamed and show is called and the output modified to copied + and passed to _parse_git_show + assert: then _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text( + contents := "content 1", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).rename(repository_path / (new_file := Path("other_file.text"))) + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output: str = repository_client._git_repo.git.show("--name-status") + # Change renamed to copied, it isn't clear how to make git think a file was copied + show_output = show_output.replace("R100", "C100") + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == new_file + assert isinstance(commit_file, repository._CommitFileAdded) + assert commit_file.content == contents + + +def test__parse_git_show_multiple(repository_client: Client): + """ + arrange: given git repository + act: when multiple files are added and show is called and the output passed to _parse_git_show + assert: then multiple _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file_1 := Path("file_1.text"))).write_text( + contents_1 := "content 1", encoding="utf-8" + ) + (repository_path / (file_2 := Path("file_2.text"))).write_text( + contents_2 := "content 2", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 2 + commit_file_1 = commit_files[1] + assert commit_file_1.path == file_1 + assert isinstance(commit_file_1, repository._CommitFileAdded) + assert commit_file_1.content == contents_1 + commit_file_2 = commit_files[0] + assert commit_file_2.path == file_2 + assert isinstance(commit_file_2, repository._CommitFileAdded) + assert commit_file_2.content == contents_2 From f6135550c3df0f270d5323dd94674f28421e8516 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 00:50:33 +1000 Subject: [PATCH 03/24] fix linting issue --- src/repository.py | 3 +-- tests/unit/test_repository.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/repository.py b/src/repository.py index 9fee9320..ee4bc730 100644 --- a/src/repository.py +++ b/src/repository.py @@ -8,7 +8,6 @@ import re from collections.abc import Iterator, Sequence from contextlib import contextmanager -from enum import Enum from functools import cached_property from itertools import chain, takewhile from pathlib import Path @@ -700,7 +699,7 @@ def _parse_git_show(output: str, repository_path: Path) -> Iterator[_CommitFile] # D delete-file.txt # R100 renamed-file.text is-renamed-file.text # C100 to-be-copied-file.text copied-file.text - lines = takewhile(lambda line: bool(line), reversed(output.splitlines())) + lines = takewhile(bool, reversed(output.splitlines())) for line in lines: if (added_match := _ADDED_PATTERN.match(line)) is not None: path = Path(added_match.group(1)) diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index 866d3823..ff45078a 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -1023,7 +1023,7 @@ def test_create_pull_request_function( ).read_text() == filler_text -def test__parse_git_show_empty(repository_client: Client): +def test__parse_git_show_empty(): """ arrange: given empty show output act: when output is passed to _parse_git_show @@ -1036,7 +1036,7 @@ def test__parse_git_show_empty(repository_client: Client): assert len(commit_files) == 0 -def test__parse_git_show_unsupported(repository_client: Client): +def test__parse_git_show_unsupported(): """ arrange: given show output that includes a line that is unknown act: when output is passed to _parse_git_show From 2a11139728c2c4b6895d0a79cca51b12995368bd Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 15:39:18 +1000 Subject: [PATCH 04/24] add function that converts files in a commit to PyGithub actions --- src/commit.py | 107 ++++++++++++ src/repository.py | 159 +++++++----------- tests/unit/test_commit.py | 208 +++++++++++++++++++++++ tests/unit/test_repository.py | 303 ++++++++++++---------------------- 4 files changed, 481 insertions(+), 296 deletions(-) create mode 100644 src/commit.py create mode 100644 tests/unit/test_commit.py diff --git a/src/commit.py b/src/commit.py new file mode 100644 index 00000000..dfc60e2c --- /dev/null +++ b/src/commit.py @@ -0,0 +1,107 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Module for handling interactions with git commit.""" + +import re +from collections.abc import Iterator +from itertools import takewhile +from pathlib import Path +from typing import NamedTuple + + +class FileAdded(NamedTuple): + """File that was added or copied in a commit. + + Attributes: + path: The location of the file on disk. + content: The content of the file. + """ + + path: Path + content: str + + +class FileModified(NamedTuple): + """File that was modified in a commit. + + Attributes: + path: The location of the file on disk. + content: The content of the file. + """ + + path: Path + content: str + + +class FileDeleted(NamedTuple): + """File that was deleted in a commit. + + Attributes: + path: The location of the file on disk. + """ + + path: Path + + +# Copied will be mapped to added and renamed will be mapped to be a delete and add +FileAction = FileAdded | FileModified | FileDeleted +_ADDED_PATTERN = re.compile(r"A\s*(\S*)") +_MODIFIED_PATTERN = re.compile(r"M\s*(\S*)") +_DELETED_PATTERN = re.compile(r"D\s*(\S*)") +_RENAMED_PATTERN = re.compile(r"R\d+\s*(\S*)\s*(\S*)") +_COPIED_PATTERN = re.compile(r"C\d+\s*(\S*)\s*(\S*)") + + +def parse_git_show(output: str, repository_path: Path) -> Iterator[FileAction]: + """Parse the output of a git show with --name-status intmanageable files. + + Args: + output: The output of the git show command. + repository_path: The path to the git repository. + + Yields: + Information about each of the files that changed in the commit. + """ + # Processing in reverse up to empty line to detect end of file changes as an empty line. + # Example output: + # git show --name-status + # commit (HEAD -> ) + # Author: + # Date: + + # + + # A add-file.text + # M change-file.text + # D delete-file.txt + # R100 renamed-file.text is-renamed-file.text + # C100 to-be-copied-file.text copied-file.text + lines = takewhile(bool, reversed(output.splitlines())) + for line in lines: + if (added_match := _ADDED_PATTERN.match(line)) is not None: + path = Path(added_match.group(1)) + yield FileAdded(path, (repository_path / path).read_text(encoding="utf-8")) + continue + + if (copied_match := _COPIED_PATTERN.match(line)) is not None: + path = Path(copied_match.group(2)) + yield FileAdded(path, (repository_path / path).read_text(encoding="utf-8")) + continue + + if (modified_match := _MODIFIED_PATTERN.match(line)) is not None: + path = Path(modified_match.group(1)) + yield FileModified(path, (repository_path / path).read_text(encoding="utf-8")) + continue + + if (delete_match := _DELETED_PATTERN.match(line)) is not None: + path = Path(delete_match.group(1)) + yield FileDeleted(path) + continue + + if (renamed_match := _RENAMED_PATTERN.match(line)) is not None: + old_path = Path(renamed_match.group(1)) + path = Path(renamed_match.group(2)) + yield FileDeleted(old_path) + yield FileAdded(path, (repository_path / path).read_text(encoding="utf-8")) + continue diff --git a/src/repository.py b/src/repository.py index ee4bc730..fea3cb2b 100644 --- a/src/repository.py +++ b/src/repository.py @@ -6,17 +6,18 @@ import base64 import logging import re -from collections.abc import Iterator, Sequence +from collections.abc import Iterable, Iterator, Sequence from contextlib import contextmanager from functools import cached_property -from itertools import chain, takewhile +from itertools import chain from pathlib import Path -from typing import Any, NamedTuple +from typing import Any, NamedTuple, cast from git import GitCommandError from git.diff import Diff from git.repo import Repo from github import Github +from github.ContentFile import ContentFile from github.GithubException import GithubException, UnknownObjectException from github.Repository import Repository @@ -24,6 +25,7 @@ from src.metadata import get as get_metadata from src.types_ import Metadata +from . import commit as commit_module from .constants import DOCUMENTATION_FOLDER_NAME from .exceptions import ( InputError, @@ -319,6 +321,60 @@ def create_branch(self, branch_name: str, base: str | None = None) -> "Client": return self + def _github_client_push( + self, commit_files: Iterable[commit_module.FileAction], commit_msg: str + ) -> None: + """Push files from a commit to GitHub using PyGithub. + + Args: + commit_files: The files that were added, modified or deleted in a commit. + commit_msg: The message to use for commits. + """ + for commit_file in commit_files: + file_commit_msg = f"'{commit_msg} path {commit_file.path}'" + + match type(commit_file): + case commit_module.FileAdded: + commit_file = cast(commit_module.FileAdded, commit_file) + self._github_repo.create_file( + path=str(commit_file.path), + message=file_commit_msg, + content=commit_file.content, + branch=self.current_branch, + ) + case commit_module.FileModified: + commit_file = cast(commit_module.FileModified, commit_file) + git_contents = cast( + ContentFile, + self._github_repo.get_contents( + path=str(commit_file.path), ref=self.current_branch + ), + ) + self._github_repo.update_file( + path=git_contents.path, + message=file_commit_msg, + content=commit_file.content, + sha=git_contents.sha, + branch=self.current_branch, + ) + case commit_module.FileDeleted: + commit_file = cast(commit_module.FileDeleted, commit_file) + git_contents = cast( + ContentFile, + self._github_repo.get_contents( + path=str(commit_file.path), ref=self.current_branch + ), + ) + self._github_repo.delete_file( + path=git_contents.path, + message=file_commit_msg, + sha=git_contents.sha, + branch=self.current_branch, + ) + # Here just in case, should not occur in production + case _: ## pragma: no cover + raise NotImplementedError(f"unsupported file in commit, {commit_file}") + def update_branch( self, commit_msg: str, @@ -630,100 +686,3 @@ def create_repository_client(access_token: str | None, base_path: Path) -> Clien repository_fullname = _get_repository_name_from_git_url(remote_url=remote_url) remote_repo = github_client.get_repo(repository_fullname) return Client(repository=local_repo, github_repository=remote_repo) - - -class _CommitFileAdded(NamedTuple): - """File that was added or copied in a commit. - - Attributes: - path: The location of the file on disk. - content: The content of the file. - """ - - path: Path - content: str - - -class _CommitFileModified(NamedTuple): - """File that was modified in a commit. - - Attributes: - path: The location of the file on disk. - content: The content of the file. - """ - - path: Path - content: str - - -class _CommitFileDeleted(NamedTuple): - """File that was deleted in a commit. - - Attributes: - path: The location of the file on disk. - """ - - path: Path - - -# Copied will be mapped to added and renamed will be mapped to be a delete and add -_CommitFile = _CommitFileAdded | _CommitFileModified | _CommitFileDeleted -_ADDED_PATTERN = re.compile(r"A\s*(\S*)") -_MODIFIED_PATTERN = re.compile(r"M\s*(\S*)") -_DELETED_PATTERN = re.compile(r"D\s*(\S*)") -_RENAMED_PATTERN = re.compile(r"R\d+\s*(\S*)\s*(\S*)") -_COPIED_PATTERN = re.compile(r"C\d+\s*(\S*)\s*(\S*)") - - -def _parse_git_show(output: str, repository_path: Path) -> Iterator[_CommitFile]: - """Parse the output of a git show with --name-status intmanageable files. - - Args: - output: The output of the git show command. - repository_path: The path to the git repository. - - Yields: - Information about each of the files that changed in the commit. - """ - # Processing in reverse up to empty line to detect end of file changes as an empty line. - # Example output: - # git show --name-status - # commit (HEAD -> ) - # Author: - # Date: - - # - - # A add-file.text - # M change-file.text - # D delete-file.txt - # R100 renamed-file.text is-renamed-file.text - # C100 to-be-copied-file.text copied-file.text - lines = takewhile(bool, reversed(output.splitlines())) - for line in lines: - if (added_match := _ADDED_PATTERN.match(line)) is not None: - path = Path(added_match.group(1)) - yield _CommitFileAdded(path, (repository_path / path).read_text(encoding="utf-8")) - continue - - if (copied_match := _COPIED_PATTERN.match(line)) is not None: - path = Path(copied_match.group(2)) - yield _CommitFileAdded(path, (repository_path / path).read_text(encoding="utf-8")) - continue - - if (modified_match := _MODIFIED_PATTERN.match(line)) is not None: - path = Path(modified_match.group(1)) - yield _CommitFileModified(path, (repository_path / path).read_text(encoding="utf-8")) - continue - - if (delete_match := _DELETED_PATTERN.match(line)) is not None: - path = Path(delete_match.group(1)) - yield _CommitFileDeleted(path) - continue - - if (renamed_match := _RENAMED_PATTERN.match(line)) is not None: - old_path = Path(renamed_match.group(1)) - path = Path(renamed_match.group(2)) - yield _CommitFileDeleted(old_path) - yield _CommitFileAdded(path, (repository_path / path).read_text(encoding="utf-8")) - continue diff --git a/tests/unit/test_commit.py b/tests/unit/test_commit.py new file mode 100644 index 00000000..4144e4c6 --- /dev/null +++ b/tests/unit/test_commit.py @@ -0,0 +1,208 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for git.""" + +# Need access to protected functions for testing +# pylint: disable=protected-access + +from pathlib import Path + +from src import commit +from src.constants import DEFAULT_BRANCH +from src.repository import Client + + +def test__parse_git_show_empty(): + """ + arrange: given empty show output + act: when output is passed to _parse_git_show + assert: then no files are returned. + """ + show_output = "" + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=Path())) + + assert len(commit_files) == 0 + + +def test__parse_git_show_unsupported(): + """ + arrange: given show output that includes a line that is unknown + act: when output is passed to _parse_git_show + assert: then no files are returned. + """ + show_output = "X file.text" + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=Path())) + + assert len(commit_files) == 0 + + +def test__parse_git_show_added(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and show is called and the output passed to _parse_git_show + assert: then _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text( + contents := "content 1", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == file + assert isinstance(commit_file, commit.FileAdded) + assert commit_file.content == contents + + +def test__parse_git_show_modified(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then modified and show is called and the output passed to + _parse_git_show + assert: then _CommitFileModified is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).write_text(contents := "content 2", encoding="utf-8") + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == file + assert isinstance(commit_file, commit.FileModified) + assert commit_file.content == contents + + +def test__parse_git_show_deleted(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then deleted and show is called and the output passed to + _parse_git_show + assert: then _CommitFileDeleted is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).unlink() + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == file + assert isinstance(commit_file, commit.FileDeleted) + + +def test__parse_git_show_renamed(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then renamed and show is called and the output passed to + _parse_git_show + assert: then _CommitFileDeleted and _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text( + contents := "content 1", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).rename(repository_path / (new_file := Path("other_file.text"))) + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 2 + commit_file_delete = commit_files[0] + assert commit_file_delete.path == file + assert isinstance(commit_file_delete, commit.FileDeleted) + commit_file_add = commit_files[1] + assert commit_file_add.path == new_file + assert isinstance(commit_file_add, commit.FileAdded) + assert commit_file_add.content == contents + + +def test__parse_git_show_copied(repository_client: Client): + """ + arrange: given git repository + act: when a file is added and then renamed and show is called and the output modified to copied + and passed to _parse_git_show + assert: then _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file := Path("file.text"))).write_text( + contents := "content 1", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + (repository_path / file).rename(repository_path / (new_file := Path("other_file.text"))) + repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) + show_output: str = repository_client._git_repo.git.show("--name-status") + # Change renamed to copied, it isn't clear how to make git think a file was copied + show_output = show_output.replace("R100", "C100") + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 1 + commit_file = commit_files[0] + assert commit_file.path == new_file + assert isinstance(commit_file, commit.FileAdded) + assert commit_file.content == contents + + +def test__parse_git_show_multiple(repository_client: Client): + """ + arrange: given git repository + act: when multiple files are added and show is called and the output passed to _parse_git_show + assert: then multiple _CommitFileAdded is returned. + """ + repository_path = repository_client.base_path + (repository_path / (file_1 := Path("file_1.text"))).write_text( + contents_1 := "content 1", encoding="utf-8" + ) + (repository_path / (file_2 := Path("file_2.text"))).write_text( + contents_2 := "content 2", encoding="utf-8" + ) + branch_name = "test-add-branch" + repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( + branch_name + ).update_branch(commit_msg="commit-1", push=False, directory=None) + show_output = repository_client._git_repo.git.show("--name-status") + + commit_files = tuple(commit.parse_git_show(show_output, repository_path=repository_path)) + + assert len(commit_files) == 2 + commit_file_1 = commit_files[1] + assert commit_file_1.path == file_1 + assert isinstance(commit_file_1, commit.FileAdded) + assert commit_file_1.content == contents_1 + commit_file_2 = commit_files[0] + assert commit_file_2.path == file_2 + assert isinstance(commit_file_2, commit.FileAdded) + assert commit_file_2.content == contents_2 diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index ff45078a..7c541bdf 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -21,7 +21,7 @@ from github.PullRequest import PullRequest from github.Repository import Repository -from src import repository +from src import commit, repository from src.constants import DEFAULT_BRANCH, DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_TAG from src.exceptions import ( InputError, @@ -838,6 +838,112 @@ def side_effect(*args): ) +def test__github_client_push_added(repository_client: Client, mock_github_repo): + """ + arrange: given Client with a mocked github client + act: when _github_client_push is called with an added file + assert: then create_file is called on the github client with the added file. + """ + repository_client.switch(DEFAULT_BRANCH) + path = Path("test.text") + content = "content 1" + commit_file = commit.FileAdded(path=path, content=content) + commit_msg = "commit-1" + + repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) + + mock_github_repo.create_file.assert_called_once_with( + path=str(path), + message=f"'{commit_msg} path {path}'", + content=content, + branch=DEFAULT_BRANCH, + ) + + +def test__github_client_push_modified(repository_client: Client, mock_github_repo): + """ + arrange: given Client with a mocked github client + act: when _github_client_push is called with a modified file + assert: then create_file is called on the github client with the modified file. + """ + repository_client.switch(DEFAULT_BRANCH) + path = Path("test.text") + content = "content 1" + commit_file = commit.FileModified(path=path, content=content) + commit_msg = "commit-1" + mock_contents = mock.MagicMock() + mock_github_repo.get_contents.return_value = mock_contents + + repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) + + mock_github_repo.get_contents.assert_called_once_with(path=str(path), ref=DEFAULT_BRANCH) + mock_github_repo.update_file.assert_called_once_with( + path=mock_contents.path, + message=f"'{commit_msg} path {path}'", + content=content, + sha=mock_contents.sha, + branch=DEFAULT_BRANCH, + ) + + +def test__github_client_push_deleted(repository_client: Client, mock_github_repo): + """ + arrange: given Client with a mocked github client + act: when _github_client_push is called with a deleted file + assert: then create_file is called on the github client with the deleted file. + """ + repository_client.switch(DEFAULT_BRANCH) + path = Path("test.text") + commit_file = commit.FileDeleted(path=path) + commit_msg = "commit-1" + mock_contents = mock.MagicMock() + mock_github_repo.get_contents.return_value = mock_contents + + repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) + + mock_github_repo.get_contents.assert_called_once_with(path=str(path), ref=DEFAULT_BRANCH) + mock_github_repo.delete_file.assert_called_once_with( + path=mock_contents.path, + message=f"'{commit_msg} path {path}'", + sha=mock_contents.sha, + branch=DEFAULT_BRANCH, + ) + + +def test__github_client_push_multiple(repository_client: Client, mock_github_repo): + """ + arrange: given Client with a mocked github client + act: when _github_client_push is called with multiple added files + assert: then create_file is called on the github client with the added files. + """ + repository_client.switch(DEFAULT_BRANCH) + path_1 = Path("test_1.text") + content_1 = "content 1" + commit_file_1 = commit.FileAdded(path=path_1, content=content_1) + path_2 = Path("test_2.text") + content_2 = "content 2" + commit_file_2 = commit.FileAdded(path=path_2, content=content_2) + commit_msg = "commit-1" + + repository_client._github_client_push( + commit_files=(commit_file_1, commit_file_2), commit_msg=commit_msg + ) + + assert mock_github_repo.create_file.call_count == 2 + mock_github_repo.create_file.assert_any_call( + path=str(path_1), + message=f"'{commit_msg} path {path_1}'", + content=content_1, + branch=DEFAULT_BRANCH, + ) + mock_github_repo.create_file.assert_any_call( + path=str(path_2), + message=f"'{commit_msg} path {path_2}'", + content=content_2, + branch=DEFAULT_BRANCH, + ) + + def test_update_branch_unknown_error(monkeypatch, repository_client: Client): """ arrange: given Client with a mocked local git repository client that raises an @@ -1021,198 +1127,3 @@ def test_create_pull_request_function( assert ( upstream_repository_path / DOCUMENTATION_FOLDER_NAME / filler_file ).read_text() == filler_text - - -def test__parse_git_show_empty(): - """ - arrange: given empty show output - act: when output is passed to _parse_git_show - assert: then no files are returned. - """ - show_output = "" - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=Path())) - - assert len(commit_files) == 0 - - -def test__parse_git_show_unsupported(): - """ - arrange: given show output that includes a line that is unknown - act: when output is passed to _parse_git_show - assert: then no files are returned. - """ - show_output = "X file.text" - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=Path())) - - assert len(commit_files) == 0 - - -def test__parse_git_show_added(repository_client: Client): - """ - arrange: given git repository - act: when a file is added and show is called and the output passed to _parse_git_show - assert: then _CommitFileAdded is returned. - """ - repository_path = repository_client.base_path - (repository_path / (file := Path("file.text"))).write_text( - contents := "content 1", encoding="utf-8" - ) - branch_name = "test-add-branch" - repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( - branch_name - ).update_branch(commit_msg="commit-1", push=False, directory=None) - show_output = repository_client._git_repo.git.show("--name-status") - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) - - assert len(commit_files) == 1 - commit_file = commit_files[0] - assert commit_file.path == file - assert isinstance(commit_file, repository._CommitFileAdded) - assert commit_file.content == contents - - -def test__parse_git_show_modified(repository_client: Client): - """ - arrange: given git repository - act: when a file is added and then modified and show is called and the output passed to - _parse_git_show - assert: then _CommitFileModified is returned. - """ - repository_path = repository_client.base_path - (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") - branch_name = "test-add-branch" - repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( - branch_name - ).update_branch(commit_msg="commit-1", push=False, directory=None) - (repository_path / file).write_text(contents := "content 2", encoding="utf-8") - repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) - show_output = repository_client._git_repo.git.show("--name-status") - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) - - assert len(commit_files) == 1 - commit_file = commit_files[0] - assert commit_file.path == file - assert isinstance(commit_file, repository._CommitFileModified) - assert commit_file.content == contents - - -def test__parse_git_show_deleted(repository_client: Client): - """ - arrange: given git repository - act: when a file is added and then deleted and show is called and the output passed to - _parse_git_show - assert: then _CommitFileDeleted is returned. - """ - repository_path = repository_client.base_path - (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") - branch_name = "test-add-branch" - repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( - branch_name - ).update_branch(commit_msg="commit-1", push=False, directory=None) - (repository_path / file).unlink() - repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) - show_output = repository_client._git_repo.git.show("--name-status") - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) - - assert len(commit_files) == 1 - commit_file = commit_files[0] - assert commit_file.path == file - assert isinstance(commit_file, repository._CommitFileDeleted) - - -def test__parse_git_show_renamed(repository_client: Client): - """ - arrange: given git repository - act: when a file is added and then renamed and show is called and the output passed to - _parse_git_show - assert: then _CommitFileDeleted and _CommitFileAdded is returned. - """ - repository_path = repository_client.base_path - (repository_path / (file := Path("file.text"))).write_text( - contents := "content 1", encoding="utf-8" - ) - branch_name = "test-add-branch" - repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( - branch_name - ).update_branch(commit_msg="commit-1", push=False, directory=None) - (repository_path / file).rename(repository_path / (new_file := Path("other_file.text"))) - repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) - show_output = repository_client._git_repo.git.show("--name-status") - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) - - assert len(commit_files) == 2 - commit_file_delete = commit_files[0] - assert commit_file_delete.path == file - assert isinstance(commit_file_delete, repository._CommitFileDeleted) - commit_file_add = commit_files[1] - assert commit_file_add.path == new_file - assert isinstance(commit_file_add, repository._CommitFileAdded) - assert commit_file_add.content == contents - - -def test__parse_git_show_copied(repository_client: Client): - """ - arrange: given git repository - act: when a file is added and then renamed and show is called and the output modified to copied - and passed to _parse_git_show - assert: then _CommitFileAdded is returned. - """ - repository_path = repository_client.base_path - (repository_path / (file := Path("file.text"))).write_text( - contents := "content 1", encoding="utf-8" - ) - branch_name = "test-add-branch" - repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( - branch_name - ).update_branch(commit_msg="commit-1", push=False, directory=None) - (repository_path / file).rename(repository_path / (new_file := Path("other_file.text"))) - repository_client.update_branch(commit_msg="commit-2", push=False, directory=None) - show_output: str = repository_client._git_repo.git.show("--name-status") - # Change renamed to copied, it isn't clear how to make git think a file was copied - show_output = show_output.replace("R100", "C100") - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) - - assert len(commit_files) == 1 - commit_file = commit_files[0] - assert commit_file.path == new_file - assert isinstance(commit_file, repository._CommitFileAdded) - assert commit_file.content == contents - - -def test__parse_git_show_multiple(repository_client: Client): - """ - arrange: given git repository - act: when multiple files are added and show is called and the output passed to _parse_git_show - assert: then multiple _CommitFileAdded is returned. - """ - repository_path = repository_client.base_path - (repository_path / (file_1 := Path("file_1.text"))).write_text( - contents_1 := "content 1", encoding="utf-8" - ) - (repository_path / (file_2 := Path("file_2.text"))).write_text( - contents_2 := "content 2", encoding="utf-8" - ) - branch_name = "test-add-branch" - repository_client.switch(DEFAULT_BRANCH).create_branch(branch_name=branch_name).switch( - branch_name - ).update_branch(commit_msg="commit-1", push=False, directory=None) - show_output = repository_client._git_repo.git.show("--name-status") - - commit_files = tuple(repository._parse_git_show(show_output, repository_path=repository_path)) - - assert len(commit_files) == 2 - commit_file_1 = commit_files[1] - assert commit_file_1.path == file_1 - assert isinstance(commit_file_1, repository._CommitFileAdded) - assert commit_file_1.content == contents_1 - commit_file_2 = commit_files[0] - assert commit_file_2.path == file_2 - assert isinstance(commit_file_2, repository._CommitFileAdded) - assert commit_file_2.content == contents_2 From 8e48bc42d889d058d491b9f22954859f7e7ef504 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 16:11:36 +1000 Subject: [PATCH 05/24] add retry if push fails using PyGithub --- src/repository.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/repository.py b/src/repository.py index fea3cb2b..5f338f45 100644 --- a/src/repository.py +++ b/src/repository.py @@ -7,7 +7,7 @@ import logging import re from collections.abc import Iterable, Iterator, Sequence -from contextlib import contextmanager +from contextlib import contextmanager, suppress from functools import cached_property from itertools import chain from pathlib import Path @@ -405,7 +405,23 @@ def update_branch( if force: args.append("-f") args.extend([ORIGIN_NAME, self.current_branch]) - self._git_repo.git.push(*args) + try: + self._git_repo.git.push(*args) + except GitCommandError as exc: + # Try with the PyGithub client, suppress any errors and report the original + # problem on failure + try: + logging.info( + "encountered error with push, try to use GutHub API to sign commits" + ) + show_output = self._git_repo.git.show("--name-status") + commit_files = commit_module.parse_git_show( + output=show_output, repository_path=self.base_path + ) + self._github_client_push(commit_files=commit_files, commit_msg=commit_msg) + except (GitCommandError, GithubException): + # Raise original exception + raise exc except GitCommandError as exc: raise RepositoryClientError( f"Unexpected error updating branch {self.current_branch}. {exc=!r}" From b117fcb24056cc1cc81daa4f892fa8fba4ba6d80 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 16:55:03 +1000 Subject: [PATCH 06/24] add branch creation if it doesn't exist --- src/repository.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/repository.py b/src/repository.py index 5f338f45..1fafbf4d 100644 --- a/src/repository.py +++ b/src/repository.py @@ -7,7 +7,7 @@ import logging import re from collections.abc import Iterable, Iterator, Sequence -from contextlib import contextmanager, suppress +from contextlib import contextmanager from functools import cached_property from itertools import chain from pathlib import Path @@ -26,7 +26,7 @@ from src.types_ import Metadata from . import commit as commit_module -from .constants import DOCUMENTATION_FOLDER_NAME +from .constants import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_TAG from .exceptions import ( InputError, RepositoryClientError, @@ -330,6 +330,15 @@ def _github_client_push( commit_files: The files that were added, modified or deleted in a commit. commit_msg: The message to use for commits. """ + # Create branch if it doesn't exist + try: + self._github_repo.get_branch(self.current_branch) + except GithubException: + # The branch probably doesn't exist, try to create it + self._github_repo.create_git_ref( + ref=f"refs/heads/{DEFAULT_BRANCH_NAME}", sha=DOCUMENTATION_TAG + ) + for commit_file in commit_files: file_commit_msg = f"'{commit_msg} path {commit_file.path}'" From eb9bfa24def82967b70c45273c02cfbdd26e1c20 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 17:17:40 +1000 Subject: [PATCH 07/24] retrieve git tag --- src/repository.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/repository.py b/src/repository.py index 1fafbf4d..90360fa8 100644 --- a/src/repository.py +++ b/src/repository.py @@ -335,8 +335,9 @@ def _github_client_push( self._github_repo.get_branch(self.current_branch) except GithubException: # The branch probably doesn't exist, try to create it + tag_ref = self._github_repo.get_git_ref(f"tags/{DOCUMENTATION_TAG}") self._github_repo.create_git_ref( - ref=f"refs/heads/{DEFAULT_BRANCH_NAME}", sha=DOCUMENTATION_TAG + ref=f"refs/heads/{DEFAULT_BRANCH_NAME}", sha=tag_ref.object.sha ) for commit_file in commit_files: From 6de622ad27ad1597e453083bb021ee4a74ba5fed Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 23:26:11 +1000 Subject: [PATCH 08/24] deliberately raise error --- src/repository.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/repository.py b/src/repository.py index 90360fa8..152f77ce 100644 --- a/src/repository.py +++ b/src/repository.py @@ -416,6 +416,7 @@ def update_branch( args.append("-f") args.extend([ORIGIN_NAME, self.current_branch]) try: + raise GitCommandError("testing") self._git_repo.git.push(*args) except GitCommandError as exc: # Try with the PyGithub client, suppress any errors and report the original From 64018cc71301283edbecd705e7cfab48c28fdc94 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 23:49:32 +1000 Subject: [PATCH 09/24] ensure the branch exists before pushing changes --- src/repository.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/repository.py b/src/repository.py index 152f77ce..9f27ff29 100644 --- a/src/repository.py +++ b/src/repository.py @@ -330,16 +330,6 @@ def _github_client_push( commit_files: The files that were added, modified or deleted in a commit. commit_msg: The message to use for commits. """ - # Create branch if it doesn't exist - try: - self._github_repo.get_branch(self.current_branch) - except GithubException: - # The branch probably doesn't exist, try to create it - tag_ref = self._github_repo.get_git_ref(f"tags/{DOCUMENTATION_TAG}") - self._github_repo.create_git_ref( - ref=f"refs/heads/{DEFAULT_BRANCH_NAME}", sha=tag_ref.object.sha - ) - for commit_file in commit_files: file_commit_msg = f"'{commit_msg} path {commit_file.path}'" @@ -408,6 +398,14 @@ def update_branch( Repository client with the updated branch """ try: + # Create the branch if it doesn't exist + if push: + args = ["-u"] + if force: + args.append("-f") + args.extend([ORIGIN_NAME, self.current_branch]) + self._git_repo.git.push(*args) + self._git_repo.git.add("-A", directory or ".") self._git_repo.git.commit("-m", f"'{commit_msg}'") if push: @@ -416,7 +414,6 @@ def update_branch( args.append("-f") args.extend([ORIGIN_NAME, self.current_branch]) try: - raise GitCommandError("testing") self._git_repo.git.push(*args) except GitCommandError as exc: # Try with the PyGithub client, suppress any errors and report the original @@ -710,6 +707,6 @@ def create_repository_client(access_token: str | None, base_path: Path) -> Clien logging.info("executing in git repository in the directory: %s", local_repo.working_dir) github_client = Github(login_or_token=access_token) remote_url = local_repo.remote().url - repository_fullname = _get_repository_name_from_git_url(remote_url=remote_url) + repository_fullname = "canonical/wordpress-k8s-operator" remote_repo = github_client.get_repo(repository_fullname) return Client(repository=local_repo, github_repository=remote_repo) From 4f2ccb4d756cd086d524702e329c09414413dfa9 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Wed, 5 Jul 2023 23:59:58 +1000 Subject: [PATCH 10/24] only handle added files --- src/repository.py | 81 +++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/src/repository.py b/src/repository.py index 9f27ff29..1c4ec18c 100644 --- a/src/repository.py +++ b/src/repository.py @@ -342,38 +342,38 @@ def _github_client_push( content=commit_file.content, branch=self.current_branch, ) - case commit_module.FileModified: - commit_file = cast(commit_module.FileModified, commit_file) - git_contents = cast( - ContentFile, - self._github_repo.get_contents( - path=str(commit_file.path), ref=self.current_branch - ), - ) - self._github_repo.update_file( - path=git_contents.path, - message=file_commit_msg, - content=commit_file.content, - sha=git_contents.sha, - branch=self.current_branch, - ) - case commit_module.FileDeleted: - commit_file = cast(commit_module.FileDeleted, commit_file) - git_contents = cast( - ContentFile, - self._github_repo.get_contents( - path=str(commit_file.path), ref=self.current_branch - ), - ) - self._github_repo.delete_file( - path=git_contents.path, - message=file_commit_msg, - sha=git_contents.sha, - branch=self.current_branch, - ) - # Here just in case, should not occur in production - case _: ## pragma: no cover - raise NotImplementedError(f"unsupported file in commit, {commit_file}") + # case commit_module.FileModified: + # commit_file = cast(commit_module.FileModified, commit_file) + # git_contents = cast( + # ContentFile, + # self._github_repo.get_contents( + # path=str(commit_file.path), ref=self.current_branch + # ), + # ) + # self._github_repo.update_file( + # path=git_contents.path, + # message=file_commit_msg, + # content=commit_file.content, + # sha=git_contents.sha, + # branch=self.current_branch, + # ) + # case commit_module.FileDeleted: + # commit_file = cast(commit_module.FileDeleted, commit_file) + # git_contents = cast( + # ContentFile, + # self._github_repo.get_contents( + # path=str(commit_file.path), ref=self.current_branch + # ), + # ) + # self._github_repo.delete_file( + # path=git_contents.path, + # message=file_commit_msg, + # sha=git_contents.sha, + # branch=self.current_branch, + # ) + # # Here just in case, should not occur in production + # case _: ## pragma: no cover + # raise NotImplementedError(f"unsupported file in commit, {commit_file}") def update_branch( self, @@ -397,24 +397,21 @@ def update_branch( Returns: Repository client with the updated branch """ + push_args = ["-u"] + if force: + push_args.append("-f") + push_args.extend([ORIGIN_NAME, self.current_branch]) + try: # Create the branch if it doesn't exist if push: - args = ["-u"] - if force: - args.append("-f") - args.extend([ORIGIN_NAME, self.current_branch]) - self._git_repo.git.push(*args) + self._git_repo.git.push(*push_args) self._git_repo.git.add("-A", directory or ".") self._git_repo.git.commit("-m", f"'{commit_msg}'") if push: - args = ["-u"] - if force: - args.append("-f") - args.extend([ORIGIN_NAME, self.current_branch]) try: - self._git_repo.git.push(*args) + self._git_repo.git.push(*push_args) except GitCommandError as exc: # Try with the PyGithub client, suppress any errors and report the original # problem on failure From 0c55d15082e24a9557e9c69f5a1402cb0f7f16df Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 00:07:25 +1000 Subject: [PATCH 11/24] add in a file --- src/repository.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/repository.py b/src/repository.py index 1c4ec18c..9359f825 100644 --- a/src/repository.py +++ b/src/repository.py @@ -330,6 +330,10 @@ def _github_client_push( commit_files: The files that were added, modified or deleted in a commit. commit_msg: The message to use for commits. """ + commit_files = chain( + commit_files, + (commit_module.FileAdded(path=Path("test.text"), content="test content"),), + ) for commit_file in commit_files: file_commit_msg = f"'{commit_msg} path {commit_file.path}'" From c17902489b5e35c3450046c93edbc9bca4c08dad Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 13:35:30 +1000 Subject: [PATCH 12/24] switch to creating a git tree --- src/repository.py | 84 ++++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 45 deletions(-) diff --git a/src/repository.py b/src/repository.py index 9359f825..17460788 100644 --- a/src/repository.py +++ b/src/repository.py @@ -19,6 +19,7 @@ from github import Github from github.ContentFile import ContentFile from github.GithubException import GithubException, UnknownObjectException +from github.InputGitTreeElement import InputGitTreeElement from github.Repository import Repository from src.docs_directory import has_docs_directory @@ -134,6 +135,36 @@ def __str__(self) -> str: return " // ".join(chain(modified_str, new_str, removed_str)) +def _commit_file_to_tree_element(commit_file: commit_module.FileAction) -> InputGitTreeElement: + """Convert a file with an action to a tree element. + + Args: + commit_file: The file action to convert. + + Returns: + The git tree element. + """ + match type(commit_file): + case commit_module.FileAdded: + commit_file = cast(commit_module.FileAdded, commit_file) + return InputGitTreeElement( + path=str(commit_file.path), mode="100644", type="blob", content=commit_file.content + ) + case commit_module.FileModified: + commit_file = cast(commit_module.FileModified, commit_file) + return InputGitTreeElement( + path=str(commit_file.path), mode="100644", type="blob", content=commit_file.content + ) + case commit_module.FileDeleted: + commit_file = cast(commit_module.FileDeleted, commit_file) + return InputGitTreeElement( + path=str(commit_file.path), mode="100644", type="blob", sha=None + ) + # Here just in case, should not occur in production + case _: ## pragma: no cover + raise NotImplementedError(f"unsupported file in commit, {commit_file}") + + class Client: """Wrapper for git/git-server related functionalities. @@ -330,54 +361,17 @@ def _github_client_push( commit_files: The files that were added, modified or deleted in a commit. commit_msg: The message to use for commits. """ + branch = self._github_repo.get_branch(self.current_branch) + current_tree = self._github_repo.get_git_tree(sha=branch.commit.sha) commit_files = chain( commit_files, (commit_module.FileAdded(path=Path("test.text"), content="test content"),), ) - for commit_file in commit_files: - file_commit_msg = f"'{commit_msg} path {commit_file.path}'" - - match type(commit_file): - case commit_module.FileAdded: - commit_file = cast(commit_module.FileAdded, commit_file) - self._github_repo.create_file( - path=str(commit_file.path), - message=file_commit_msg, - content=commit_file.content, - branch=self.current_branch, - ) - # case commit_module.FileModified: - # commit_file = cast(commit_module.FileModified, commit_file) - # git_contents = cast( - # ContentFile, - # self._github_repo.get_contents( - # path=str(commit_file.path), ref=self.current_branch - # ), - # ) - # self._github_repo.update_file( - # path=git_contents.path, - # message=file_commit_msg, - # content=commit_file.content, - # sha=git_contents.sha, - # branch=self.current_branch, - # ) - # case commit_module.FileDeleted: - # commit_file = cast(commit_module.FileDeleted, commit_file) - # git_contents = cast( - # ContentFile, - # self._github_repo.get_contents( - # path=str(commit_file.path), ref=self.current_branch - # ), - # ) - # self._github_repo.delete_file( - # path=git_contents.path, - # message=file_commit_msg, - # sha=git_contents.sha, - # branch=self.current_branch, - # ) - # # Here just in case, should not occur in production - # case _: ## pragma: no cover - # raise NotImplementedError(f"unsupported file in commit, {commit_file}") + tree_elements = [_commit_file_to_tree_element(commit_file) for commit_file in commit_files] + tree = self._github_repo.create_git_tree(tree_elements, current_tree) + self._github_repo.create_git_commit( + message=commit_msg, tree=tree, parents=[branch.commit.commit] + ) def update_branch( self, @@ -708,6 +702,6 @@ def create_repository_client(access_token: str | None, base_path: Path) -> Clien logging.info("executing in git repository in the directory: %s", local_repo.working_dir) github_client = Github(login_or_token=access_token) remote_url = local_repo.remote().url - repository_fullname = "canonical/wordpress-k8s-operator" + repository_fullname = _get_repository_name_from_git_url(remote_url) remote_repo = github_client.get_repo(repository_fullname) return Client(repository=local_repo, github_repository=remote_repo) From 31df743be5b7812a6dad7ecb997bb62a280e1f9e Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 14:08:37 +1000 Subject: [PATCH 13/24] add commit to branch --- src/repository.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/repository.py b/src/repository.py index 17460788..9ecaee33 100644 --- a/src/repository.py +++ b/src/repository.py @@ -369,9 +369,11 @@ def _github_client_push( ) tree_elements = [_commit_file_to_tree_element(commit_file) for commit_file in commit_files] tree = self._github_repo.create_git_tree(tree_elements, current_tree) - self._github_repo.create_git_commit( + commit = self._github_repo.create_git_commit( message=commit_msg, tree=tree, parents=[branch.commit.commit] ) + branch_git_ref = self._github_repo.get_git_ref(f"heads/{self.current_branch}") + branch_git_ref.edit(sha=commit.sha) def update_branch( self, From 3df05852dfc3577e6f81e6a149fba4bac0b70c20 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 15:23:14 +1000 Subject: [PATCH 14/24] working implementation --- src/repository.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/repository.py b/src/repository.py index 9ecaee33..38691cd1 100644 --- a/src/repository.py +++ b/src/repository.py @@ -363,10 +363,6 @@ def _github_client_push( """ branch = self._github_repo.get_branch(self.current_branch) current_tree = self._github_repo.get_git_tree(sha=branch.commit.sha) - commit_files = chain( - commit_files, - (commit_module.FileAdded(path=Path("test.text"), content="test content"),), - ) tree_elements = [_commit_file_to_tree_element(commit_file) for commit_file in commit_files] tree = self._github_repo.create_git_tree(tree_elements, current_tree) commit = self._github_repo.create_git_commit( @@ -417,7 +413,7 @@ def update_branch( # problem on failure try: logging.info( - "encountered error with push, try to use GutHub API to sign commits" + "encountered error with push, try to use GitHub API to sign commits" ) show_output = self._git_repo.git.show("--name-status") commit_files = commit_module.parse_git_show( From 4f2aa23d6099e26d93a900ad6272d8eab03d503e Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 15:45:31 +1000 Subject: [PATCH 15/24] add tests for using the GitHub client to create a commit --- tests/unit/test_repository.py | 93 ++++++++++------------------------- 1 file changed, 26 insertions(+), 67 deletions(-) diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index 7c541bdf..7301decf 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -838,11 +838,11 @@ def side_effect(*args): ) -def test__github_client_push_added(repository_client: Client, mock_github_repo): +def test__github_client_push_single(repository_client: Client, mock_github_repo): """ arrange: given Client with a mocked github client act: when _github_client_push is called with an added file - assert: then create_file is called on the github client with the added file. + assert: then the expected GitHub client interactions were executed. """ repository_client.switch(DEFAULT_BRANCH) path = Path("test.text") @@ -852,61 +852,28 @@ def test__github_client_push_added(repository_client: Client, mock_github_repo): repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) - mock_github_repo.create_file.assert_called_once_with( - path=str(path), - message=f"'{commit_msg} path {path}'", - content=content, - branch=DEFAULT_BRANCH, + mock_github_repo.get_branch.assert_called_once_with(DEFAULT_BRANCH) + mock_github_repo.get_git_tree.assert_called_once_with( + sha=mock_github_repo.get_branch.return_value.commit.sha ) - -def test__github_client_push_modified(repository_client: Client, mock_github_repo): - """ - arrange: given Client with a mocked github client - act: when _github_client_push is called with a modified file - assert: then create_file is called on the github client with the modified file. - """ - repository_client.switch(DEFAULT_BRANCH) - path = Path("test.text") - content = "content 1" - commit_file = commit.FileModified(path=path, content=content) - commit_msg = "commit-1" - mock_contents = mock.MagicMock() - mock_github_repo.get_contents.return_value = mock_contents - - repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) - - mock_github_repo.get_contents.assert_called_once_with(path=str(path), ref=DEFAULT_BRANCH) - mock_github_repo.update_file.assert_called_once_with( - path=mock_contents.path, - message=f"'{commit_msg} path {path}'", - content=content, - sha=mock_contents.sha, - branch=DEFAULT_BRANCH, + # The InputGitTreeElement does not expose its arguments, can only check that the correct number + # were passed + mock_github_repo.create_git_tree.assert_called_once() + create_git_tree_call_args = mock_github_repo.create_git_tree.call_args_list[0][0] + assert len(create_git_tree_call_args) == 2 + assert len(create_git_tree_call_args[0]) == 1 + assert create_git_tree_call_args[1] == mock_github_repo.get_git_tree.return_value + + mock_github_repo.create_git_commit.assert_called_once_with( + message=commit_msg, + tree=mock_github_repo.create_git_tree.return_value, + parents=[mock_github_repo.get_branch.return_value.commit.commit], ) - -def test__github_client_push_deleted(repository_client: Client, mock_github_repo): - """ - arrange: given Client with a mocked github client - act: when _github_client_push is called with a deleted file - assert: then create_file is called on the github client with the deleted file. - """ - repository_client.switch(DEFAULT_BRANCH) - path = Path("test.text") - commit_file = commit.FileDeleted(path=path) - commit_msg = "commit-1" - mock_contents = mock.MagicMock() - mock_github_repo.get_contents.return_value = mock_contents - - repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) - - mock_github_repo.get_contents.assert_called_once_with(path=str(path), ref=DEFAULT_BRANCH) - mock_github_repo.delete_file.assert_called_once_with( - path=mock_contents.path, - message=f"'{commit_msg} path {path}'", - sha=mock_contents.sha, - branch=DEFAULT_BRANCH, + mock_github_repo.get_git_ref.assert_called_once_with(f"heads/{DEFAULT_BRANCH}") + mock_github_repo.get_git_ref.return_value.edit.assert_called_once_with( + sha=mock_github_repo.create_git_commit.return_value.sha ) @@ -914,7 +881,7 @@ def test__github_client_push_multiple(repository_client: Client, mock_github_rep """ arrange: given Client with a mocked github client act: when _github_client_push is called with multiple added files - assert: then create_file is called on the github client with the added files. + assert: then the expected GitHub client interactions were executed. """ repository_client.switch(DEFAULT_BRANCH) path_1 = Path("test_1.text") @@ -929,19 +896,11 @@ def test__github_client_push_multiple(repository_client: Client, mock_github_rep commit_files=(commit_file_1, commit_file_2), commit_msg=commit_msg ) - assert mock_github_repo.create_file.call_count == 2 - mock_github_repo.create_file.assert_any_call( - path=str(path_1), - message=f"'{commit_msg} path {path_1}'", - content=content_1, - branch=DEFAULT_BRANCH, - ) - mock_github_repo.create_file.assert_any_call( - path=str(path_2), - message=f"'{commit_msg} path {path_2}'", - content=content_2, - branch=DEFAULT_BRANCH, - ) + # Only check the calls that are different to test__github_client_push_single + mock_github_repo.create_git_tree.assert_called_once() + create_git_tree_call_args = mock_github_repo.create_git_tree.call_args_list[0][0] + assert len(create_git_tree_call_args) == 2 + assert len(create_git_tree_call_args[0]) == 2 def test_update_branch_unknown_error(monkeypatch, repository_client: Client): From cc09e738cccb816b48956ddee53f12f0edef35a6 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 15:51:58 +1000 Subject: [PATCH 16/24] add tests for _commit_file_to_tree_element --- src/repository.py | 1 - tests/unit/test_repository.py | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/repository.py b/src/repository.py index 38691cd1..5027ed6d 100644 --- a/src/repository.py +++ b/src/repository.py @@ -17,7 +17,6 @@ from git.diff import Diff from git.repo import Repo from github import Github -from github.ContentFile import ContentFile from github.GithubException import GithubException, UnknownObjectException from github.InputGitTreeElement import InputGitTreeElement from github.Repository import Repository diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index 7301decf..c22832ab 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -18,6 +18,7 @@ from github import Github from github.ContentFile import ContentFile from github.GithubException import GithubException, UnknownObjectException +from github.InputGitTreeElement import InputGitTreeElement from github.PullRequest import PullRequest from github.Repository import Repository @@ -34,6 +35,27 @@ from .helpers import assert_substrings_in_string +@pytest.mark.parametrize( + "commit_file", + [ + pytest.param(commit.FileAdded(path=Path("test.text"), content="content 1"), id="added"), + pytest.param( + commit.FileModified(path=Path("test.text"), content="content 1"), id="modified" + ), + pytest.param(commit.FileDeleted(path=Path("test.text")), id="deleted"), + ], +) +def test__commit_file_to_tree_element(commit_file: commit.FileAdded): + """ + arrange: given commit file + act: when _commit_file_to_tree_element is called with the commit file + assert: then a InputGitTreeElement is returned. + """ + tree_element = repository._commit_file_to_tree_element(commit_file=commit_file) + + assert isinstance(tree_element, InputGitTreeElement) + + def test__init__(git_repo: Repo, mock_github_repo: Repository): """ arrange: given a local git repository client and mock github repository client From 5a99bcf86b253ba050b4832e8dc9b7e4ed7e9c07 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 15:58:55 +1000 Subject: [PATCH 17/24] resolve linting problems --- src/repository.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/repository.py b/src/repository.py index 5027ed6d..63963d00 100644 --- a/src/repository.py +++ b/src/repository.py @@ -26,7 +26,7 @@ from src.types_ import Metadata from . import commit as commit_module -from .constants import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_TAG +from .constants import DOCUMENTATION_FOLDER_NAME from .exceptions import ( InputError, RepositoryClientError, @@ -142,6 +142,9 @@ def _commit_file_to_tree_element(commit_file: commit_module.FileAction) -> Input Returns: The git tree element. + + Raises: + NotImplementedError: for unsupported commit file types. """ match type(commit_file): case commit_module.FileAdded: @@ -160,7 +163,7 @@ def _commit_file_to_tree_element(commit_file: commit_module.FileAction) -> Input path=str(commit_file.path), mode="100644", type="blob", sha=None ) # Here just in case, should not occur in production - case _: ## pragma: no cover + case _: # pragma: no cover raise NotImplementedError(f"unsupported file in commit, {commit_file}") @@ -419,9 +422,10 @@ def update_branch( output=show_output, repository_path=self.base_path ) self._github_client_push(commit_files=commit_files, commit_msg=commit_msg) - except (GitCommandError, GithubException): - # Raise original exception - raise exc + except (GitCommandError, GithubException) as nested_exc: + # Raise original exception, flake8-docstrings-complete confuses this with a + # specific exception rather than re-raising + raise nested_exc from exc # noqa: DCO053 except GitCommandError as exc: raise RepositoryClientError( f"Unexpected error updating branch {self.current_branch}. {exc=!r}" From 002546cdbc1c564aa30bb2158465ae16aa71a559 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 16:31:47 +1000 Subject: [PATCH 18/24] add tests for using GitHub API to push commits --- src/repository.py | 2 +- tests/unit/test_repository.py | 107 ++++++++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/src/repository.py b/src/repository.py index 63963d00..49647905 100644 --- a/src/repository.py +++ b/src/repository.py @@ -425,7 +425,7 @@ def update_branch( except (GitCommandError, GithubException) as nested_exc: # Raise original exception, flake8-docstrings-complete confuses this with a # specific exception rather than re-raising - raise nested_exc from exc # noqa: DCO053 + raise exc from nested_exc # noqa: DCO053 except GitCommandError as exc: raise RepositoryClientError( f"Unexpected error updating branch {self.current_branch}. {exc=!r}" diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index c22832ab..c9e4a9f6 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -928,27 +928,78 @@ def test__github_client_push_multiple(repository_client: Client, mock_github_rep def test_update_branch_unknown_error(monkeypatch, repository_client: Client): """ arrange: given Client with a mocked local git repository client that raises an - exception when pushing commits + exception when adding, committing and pushing commits act: when update branch is called assert: RepositoryClientError is raised from GitCommandError. """ repository_client.switch(DEFAULT_BRANCH) - def side_effect(*args): - """Mock function. + mock_git_repository = mock.MagicMock(spec=Git) + mock_git_repository.add = mock.Mock(return_value=None) + mock_git_repository.commit = mock.Mock(return_value=None) + mock_git_repository.push = mock.Mock(side_effect=GitCommandError("mocked error")) + monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) - Args: - args: positional arguments + with pytest.raises(RepositoryClientError) as exc: + repository_client.update_branch("my-message") - Raises: - GitCommandError: when providing pop - """ - raise GitCommandError("mocked error") + assert_substrings_in_string(("unexpected error updating branch"), str(exc.value).lower()) + + +def test_update_branch_github_api_git_error( + monkeypatch, repository_client: Client, repository_path: Path, mock_github_repo +): + """ + arrange: given Client with a mocked local git repository client that raises an + exception when pushing commits and show + act: when update branch is called + assert: RepositoryClientError is raised from GitCommandError. + """ + repository_client.switch(DEFAULT_BRANCH) + + (repository_path / DOCUMENTATION_FOLDER_NAME).mkdir() + (repository_path / (file_path := Path(f"{DOCUMENTATION_FOLDER_NAME}/test.text"))).write_text( + "content 1", encoding="utf-8" + ) + + mock_git_repository = mock.MagicMock(spec=Git) + mock_git_repository.add = mock.Mock(return_value=None) + mock_git_repository.commit = mock.Mock(return_value=None) + mock_git_repository.show = mock.Mock(return_value=f"A {file_path}") + mock_git_repository.push = mock.Mock(side_effect=[None, GitCommandError("mocked push error")]) + mock_git_repository.show = mock.Mock(side_effect=GitCommandError("mocked show error")) + monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + + with pytest.raises(RepositoryClientError) as exc: + repository_client.update_branch("my-message") + + assert_substrings_in_string(("unexpected error updating branch"), str(exc.value).lower()) + + +def test_update_branch_github_api_github_error( + monkeypatch, repository_client: Client, repository_path: Path, mock_github_repo +): + """ + arrange: given Client with a mocked local git repository client that raises an + exception when pushing commits and getting a branch + act: when update branch is called + assert: RepositoryClientError is raised from GitCommandError. + """ + repository_client.switch(DEFAULT_BRANCH) + + (repository_path / DOCUMENTATION_FOLDER_NAME).mkdir() + (repository_path / (file_path := Path(f"{DOCUMENTATION_FOLDER_NAME}/test.text"))).write_text( + "content 1", encoding="utf-8" + ) mock_git_repository = mock.MagicMock(spec=Git) mock_git_repository.add = mock.Mock(return_value=None) mock_git_repository.commit = mock.Mock(return_value=None) - mock_git_repository.push = mock.Mock(side_effect=side_effect) + mock_git_repository.show = mock.Mock(return_value=f"A {file_path}") + mock_git_repository.push = mock.Mock(side_effect=[None, GitCommandError("mocked error")]) + mock_github_repo.get_branch.side_effect = GithubException(0, "", None) monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) @@ -958,6 +1009,42 @@ def side_effect(*args): assert_substrings_in_string(("unexpected error updating branch"), str(exc.value).lower()) +def test_update_branch_github_api( + monkeypatch, + repository_client: Client, + repository_path: Path, + mock_github_repo, + caplog: pytest.LogCaptureFixture, +): + """ + arrange: given Client with a mocked local git repository client that raises an + exception when pushing commits + act: when update branch is called + assert: then PyGithub is used to push instead. + """ + repository_client.switch(DEFAULT_BRANCH) + + (repository_path / DOCUMENTATION_FOLDER_NAME).mkdir() + (repository_path / (file_path := Path(f"{DOCUMENTATION_FOLDER_NAME}/test.text"))).write_text( + "content 1", encoding="utf-8" + ) + + mock_git_repository = mock.MagicMock(spec=Git) + mock_git_repository.add = mock.Mock(return_value=None) + mock_git_repository.commit = mock.Mock(return_value=None) + mock_git_repository.show = mock.Mock(return_value=f"A {file_path}") + mock_git_repository.push = mock.Mock(side_effect=[None, GitCommandError("mocked error")]) + monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + + repository_client.update_branch("my-message") + + # Check that the branch.edit was called, more detailed checks are in + # test__github_client_push_single + mock_github_repo.get_git_ref.return_value.edit.assert_called_once() + assert_substrings_in_string(("error", "push", "github", "api"), caplog.text.lower()) + + def test_get_single_pull_request(monkeypatch, repository_client: Client, mock_pull_request): """ arrange: given Client with a mocked local github client that mock an existing pull request on From 5899659becd9e9175eaa38ef3fab7292811b2465 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 16:49:06 +1000 Subject: [PATCH 19/24] small fixes --- src/commit.py | 5 +++-- src/repository.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/commit.py b/src/commit.py index dfc60e2c..1ce723b9 100644 --- a/src/commit.py +++ b/src/commit.py @@ -54,7 +54,7 @@ class FileDeleted(NamedTuple): def parse_git_show(output: str, repository_path: Path) -> Iterator[FileAction]: - """Parse the output of a git show with --name-status intmanageable files. + """Parse the output of a git show with --name-status into manageable data. Args: output: The output of the git show command. @@ -64,7 +64,7 @@ def parse_git_show(output: str, repository_path: Path) -> Iterator[FileAction]: Information about each of the files that changed in the commit. """ # Processing in reverse up to empty line to detect end of file changes as an empty line. - # Example output: + # Example git show output: # git show --name-status # commit (HEAD -> ) # Author: @@ -77,6 +77,7 @@ def parse_git_show(output: str, repository_path: Path) -> Iterator[FileAction]: # D delete-file.txt # R100 renamed-file.text is-renamed-file.text # C100 to-be-copied-file.text copied-file.text + # The copied example is a guess, was not able to get the copied status during testing lines = takewhile(bool, reversed(output.splitlines())) for line in lines: if (added_match := _ADDED_PATTERN.match(line)) is not None: diff --git a/src/repository.py b/src/repository.py index 640a9423..524898ec 100644 --- a/src/repository.py +++ b/src/repository.py @@ -704,6 +704,6 @@ def create_repository_client(access_token: str | None, base_path: Path) -> Clien logging.info("executing in git repository in the directory: %s", local_repo.working_dir) github_client = Github(login_or_token=access_token) remote_url = local_repo.remote().url - repository_fullname = _get_repository_name_from_git_url(remote_url) + repository_fullname = _get_repository_name_from_git_url(remote_url=remote_url) remote_repo = github_client.get_repo(repository_fullname) return Client(repository=local_repo, github_repository=remote_repo) From 5f6542a6769934f2b9c88be6360c375f15c661b5 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 16:56:29 +1000 Subject: [PATCH 20/24] fix linting issues --- tests/unit/test_commit.py | 46 +++++++++++++++++------------------ tests/unit/test_repository.py | 3 ++- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/tests/unit/test_commit.py b/tests/unit/test_commit.py index 4144e4c6..61d5028f 100644 --- a/tests/unit/test_commit.py +++ b/tests/unit/test_commit.py @@ -1,7 +1,7 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -"""Unit tests for git.""" +"""Unit tests for commit module.""" # Need access to protected functions for testing # pylint: disable=protected-access @@ -13,10 +13,10 @@ from src.repository import Client -def test__parse_git_show_empty(): +def test_parse_git_show_empty(): """ arrange: given empty show output - act: when output is passed to _parse_git_show + act: when output is passed to parse_git_show assert: then no files are returned. """ show_output = "" @@ -26,10 +26,10 @@ def test__parse_git_show_empty(): assert len(commit_files) == 0 -def test__parse_git_show_unsupported(): +def test_parse_git_show_unsupported(): """ arrange: given show output that includes a line that is unknown - act: when output is passed to _parse_git_show + act: when output is passed to parse_git_show assert: then no files are returned. """ show_output = "X file.text" @@ -39,11 +39,11 @@ def test__parse_git_show_unsupported(): assert len(commit_files) == 0 -def test__parse_git_show_added(repository_client: Client): +def test_parse_git_show_added(repository_client: Client): """ arrange: given git repository - act: when a file is added and show is called and the output passed to _parse_git_show - assert: then _CommitFileAdded is returned. + act: when a file is added and show is called and the output passed to parse_git_show + assert: then FileAdded is returned. """ repository_path = repository_client.base_path (repository_path / (file := Path("file.text"))).write_text( @@ -64,12 +64,12 @@ def test__parse_git_show_added(repository_client: Client): assert commit_file.content == contents -def test__parse_git_show_modified(repository_client: Client): +def test_parse_git_show_modified(repository_client: Client): """ arrange: given git repository act: when a file is added and then modified and show is called and the output passed to - _parse_git_show - assert: then _CommitFileModified is returned. + parse_git_show + assert: then FileModified is returned. """ repository_path = repository_client.base_path (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") @@ -90,12 +90,12 @@ def test__parse_git_show_modified(repository_client: Client): assert commit_file.content == contents -def test__parse_git_show_deleted(repository_client: Client): +def test_parse_git_show_deleted(repository_client: Client): """ arrange: given git repository act: when a file is added and then deleted and show is called and the output passed to - _parse_git_show - assert: then _CommitFileDeleted is returned. + parse_git_show + assert: then FileDeleted is returned. """ repository_path = repository_client.base_path (repository_path / (file := Path("file.text"))).write_text("content 1", encoding="utf-8") @@ -115,12 +115,12 @@ def test__parse_git_show_deleted(repository_client: Client): assert isinstance(commit_file, commit.FileDeleted) -def test__parse_git_show_renamed(repository_client: Client): +def test_parse_git_show_renamed(repository_client: Client): """ arrange: given git repository act: when a file is added and then renamed and show is called and the output passed to - _parse_git_show - assert: then _CommitFileDeleted and _CommitFileAdded is returned. + parse_git_show + assert: then FileDeleted and FileAdded is returned. """ repository_path = repository_client.base_path (repository_path / (file := Path("file.text"))).write_text( @@ -146,12 +146,12 @@ def test__parse_git_show_renamed(repository_client: Client): assert commit_file_add.content == contents -def test__parse_git_show_copied(repository_client: Client): +def test_parse_git_show_copied(repository_client: Client): """ arrange: given git repository act: when a file is added and then renamed and show is called and the output modified to copied - and passed to _parse_git_show - assert: then _CommitFileAdded is returned. + and passed to parse_git_show + assert: then FileAdded is returned. """ repository_path = repository_client.base_path (repository_path / (file := Path("file.text"))).write_text( @@ -176,11 +176,11 @@ def test__parse_git_show_copied(repository_client: Client): assert commit_file.content == contents -def test__parse_git_show_multiple(repository_client: Client): +def test_parse_git_show_multiple(repository_client: Client): """ arrange: given git repository - act: when multiple files are added and show is called and the output passed to _parse_git_show - assert: then multiple _CommitFileAdded is returned. + act: when multiple files are added and show is called and the output passed to parse_git_show + assert: then multiple FileAdded is returned. """ repository_path = repository_client.base_path (repository_path / (file_1 := Path("file_1.text"))).write_text( diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index c5b46d2d..e1502480 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -53,6 +53,7 @@ def test__commit_file_to_tree_element(commit_file: commit.FileAdded): """ tree_element = repository._commit_file_to_tree_element(commit_file=commit_file) + # InputGitTreeElement don't expose any data, can only check that the correct type is returned assert isinstance(tree_element, InputGitTreeElement) @@ -948,7 +949,7 @@ def test_update_branch_unknown_error(monkeypatch, repository_client: Client): def test_update_branch_github_api_git_error( - monkeypatch, repository_client: Client, repository_path: Path, mock_github_repo + monkeypatch, repository_client: Client, repository_path: Path ): """ arrange: given Client with a mocked local git repository client that raises an From 9326ac34cfb7aecfc4a808f5157f77c8361b3944 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 17:03:00 +1000 Subject: [PATCH 21/24] resolve linting issue --- tests/unit/test_commit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_commit.py b/tests/unit/test_commit.py index 61d5028f..208d0cf0 100644 --- a/tests/unit/test_commit.py +++ b/tests/unit/test_commit.py @@ -23,7 +23,7 @@ def test_parse_git_show_empty(): commit_files = tuple(commit.parse_git_show(show_output, repository_path=Path())) - assert len(commit_files) == 0 + assert not commit_files def test_parse_git_show_unsupported(): @@ -36,7 +36,7 @@ def test_parse_git_show_unsupported(): commit_files = tuple(commit.parse_git_show(show_output, repository_path=Path())) - assert len(commit_files) == 0 + assert not commit_files def test_parse_git_show_added(repository_client: Client): From 7e59dd49d15314e40162f15d4892d8a146b25456 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Thu, 6 Jul 2023 17:18:21 +1000 Subject: [PATCH 22/24] fix test doc --- tests/unit/test_commit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_commit.py b/tests/unit/test_commit.py index 208d0cf0..50c971bd 100644 --- a/tests/unit/test_commit.py +++ b/tests/unit/test_commit.py @@ -28,7 +28,7 @@ def test_parse_git_show_empty(): def test_parse_git_show_unsupported(): """ - arrange: given show output that includes a line that is unknown + arrange: given show output that includes a line with a file with unknown status act: when output is passed to parse_git_show assert: then no files are returned. """ From f86fbfa38fc4b4a529ddccb14948d2e8ceffb879 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Fri, 7 Jul 2023 00:20:00 +1000 Subject: [PATCH 23/24] address comments --- src/commit.py | 42 +++++++++++++---------------------- src/repository.py | 9 ++------ tests/unit/test_commit.py | 12 +++++----- tests/unit/test_repository.py | 27 +++++++++------------- 4 files changed, 33 insertions(+), 57 deletions(-) diff --git a/src/commit.py b/src/commit.py index 1ce723b9..215fbeff 100644 --- a/src/commit.py +++ b/src/commit.py @@ -10,20 +10,8 @@ from typing import NamedTuple -class FileAdded(NamedTuple): - """File that was added or copied in a commit. - - Attributes: - path: The location of the file on disk. - content: The content of the file. - """ - - path: Path - content: str - - -class FileModified(NamedTuple): - """File that was modified in a commit. +class FileAddedOrModified(NamedTuple): + """File that was added, mofied or copied copied in a commit. Attributes: path: The location of the file on disk. @@ -45,7 +33,7 @@ class FileDeleted(NamedTuple): # Copied will be mapped to added and renamed will be mapped to be a delete and add -FileAction = FileAdded | FileModified | FileDeleted +FileAction = FileAddedOrModified | FileDeleted _ADDED_PATTERN = re.compile(r"A\s*(\S*)") _MODIFIED_PATTERN = re.compile(r"M\s*(\S*)") _DELETED_PATTERN = re.compile(r"D\s*(\S*)") @@ -80,19 +68,14 @@ def parse_git_show(output: str, repository_path: Path) -> Iterator[FileAction]: # The copied example is a guess, was not able to get the copied status during testing lines = takewhile(bool, reversed(output.splitlines())) for line in lines: - if (added_match := _ADDED_PATTERN.match(line)) is not None: - path = Path(added_match.group(1)) - yield FileAdded(path, (repository_path / path).read_text(encoding="utf-8")) - continue - - if (copied_match := _COPIED_PATTERN.match(line)) is not None: - path = Path(copied_match.group(2)) - yield FileAdded(path, (repository_path / path).read_text(encoding="utf-8")) - continue - if (modified_match := _MODIFIED_PATTERN.match(line)) is not None: path = Path(modified_match.group(1)) - yield FileModified(path, (repository_path / path).read_text(encoding="utf-8")) + yield FileAddedOrModified(path, (repository_path / path).read_text(encoding="utf-8")) + continue + + if (added_match := _ADDED_PATTERN.match(line)) is not None: + path = Path(added_match.group(1)) + yield FileAddedOrModified(path, (repository_path / path).read_text(encoding="utf-8")) continue if (delete_match := _DELETED_PATTERN.match(line)) is not None: @@ -104,5 +87,10 @@ def parse_git_show(output: str, repository_path: Path) -> Iterator[FileAction]: old_path = Path(renamed_match.group(1)) path = Path(renamed_match.group(2)) yield FileDeleted(old_path) - yield FileAdded(path, (repository_path / path).read_text(encoding="utf-8")) + yield FileAddedOrModified(path, (repository_path / path).read_text(encoding="utf-8")) + continue + + if (copied_match := _COPIED_PATTERN.match(line)) is not None: + path = Path(copied_match.group(2)) + yield FileAddedOrModified(path, (repository_path / path).read_text(encoding="utf-8")) continue diff --git a/src/repository.py b/src/repository.py index 524898ec..f04f48a0 100644 --- a/src/repository.py +++ b/src/repository.py @@ -148,13 +148,8 @@ def _commit_file_to_tree_element(commit_file: commit_module.FileAction) -> Input NotImplementedError: for unsupported commit file types. """ match type(commit_file): - case commit_module.FileAdded: - commit_file = cast(commit_module.FileAdded, commit_file) - return InputGitTreeElement( - path=str(commit_file.path), mode="100644", type="blob", content=commit_file.content - ) - case commit_module.FileModified: - commit_file = cast(commit_module.FileModified, commit_file) + case commit_module.FileAddedOrModified: + commit_file = cast(commit_module.FileAddedOrModified, commit_file) return InputGitTreeElement( path=str(commit_file.path), mode="100644", type="blob", content=commit_file.content ) diff --git a/tests/unit/test_commit.py b/tests/unit/test_commit.py index 50c971bd..b0b620bd 100644 --- a/tests/unit/test_commit.py +++ b/tests/unit/test_commit.py @@ -60,7 +60,7 @@ def test_parse_git_show_added(repository_client: Client): assert len(commit_files) == 1 commit_file = commit_files[0] assert commit_file.path == file - assert isinstance(commit_file, commit.FileAdded) + assert isinstance(commit_file, commit.FileAddedOrModified) assert commit_file.content == contents @@ -86,7 +86,7 @@ def test_parse_git_show_modified(repository_client: Client): assert len(commit_files) == 1 commit_file = commit_files[0] assert commit_file.path == file - assert isinstance(commit_file, commit.FileModified) + assert isinstance(commit_file, commit.FileAddedOrModified) assert commit_file.content == contents @@ -142,7 +142,7 @@ def test_parse_git_show_renamed(repository_client: Client): assert isinstance(commit_file_delete, commit.FileDeleted) commit_file_add = commit_files[1] assert commit_file_add.path == new_file - assert isinstance(commit_file_add, commit.FileAdded) + assert isinstance(commit_file_add, commit.FileAddedOrModified) assert commit_file_add.content == contents @@ -172,7 +172,7 @@ def test_parse_git_show_copied(repository_client: Client): assert len(commit_files) == 1 commit_file = commit_files[0] assert commit_file.path == new_file - assert isinstance(commit_file, commit.FileAdded) + assert isinstance(commit_file, commit.FileAddedOrModified) assert commit_file.content == contents @@ -200,9 +200,9 @@ def test_parse_git_show_multiple(repository_client: Client): assert len(commit_files) == 2 commit_file_1 = commit_files[1] assert commit_file_1.path == file_1 - assert isinstance(commit_file_1, commit.FileAdded) + assert isinstance(commit_file_1, commit.FileAddedOrModified) assert commit_file_1.content == contents_1 commit_file_2 = commit_files[0] assert commit_file_2.path == file_2 - assert isinstance(commit_file_2, commit.FileAdded) + assert isinstance(commit_file_2, commit.FileAddedOrModified) assert commit_file_2.content == contents_2 diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index e1502480..a04e1a10 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -38,14 +38,13 @@ @pytest.mark.parametrize( "commit_file", [ - pytest.param(commit.FileAdded(path=Path("test.text"), content="content 1"), id="added"), pytest.param( - commit.FileModified(path=Path("test.text"), content="content 1"), id="modified" + commit.FileAddedOrModified(path=Path("test.text"), content="content 1"), id="added" ), pytest.param(commit.FileDeleted(path=Path("test.text")), id="deleted"), ], ) -def test__commit_file_to_tree_element(commit_file: commit.FileAdded): +def test__commit_file_to_tree_element(commit_file: commit.FileAction): """ arrange: given commit file act: when _commit_file_to_tree_element is called with the commit file @@ -851,7 +850,7 @@ def side_effect(*args): mock_git_repository.add = mock.Mock(return_value=None) mock_git_repository.stash = mock.Mock(side_effect=side_effect) monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) - monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *_args, **_kwargs: True) with pytest.raises(RepositoryClientError) as exc: repository_client.switch("branchname-1") @@ -870,7 +869,7 @@ def test__github_client_push_single(repository_client: Client, mock_github_repo) repository_client.switch(DEFAULT_BRANCH) path = Path("test.text") content = "content 1" - commit_file = commit.FileAdded(path=path, content=content) + commit_file = commit.FileAddedOrModified(path=path, content=content) commit_msg = "commit-1" repository_client._github_client_push(commit_files=(commit_file,), commit_msg=commit_msg) @@ -909,10 +908,10 @@ def test__github_client_push_multiple(repository_client: Client, mock_github_rep repository_client.switch(DEFAULT_BRANCH) path_1 = Path("test_1.text") content_1 = "content 1" - commit_file_1 = commit.FileAdded(path=path_1, content=content_1) + commit_file_1 = commit.FileAddedOrModified(path=path_1, content=content_1) path_2 = Path("test_2.text") content_2 = "content 2" - commit_file_2 = commit.FileAdded(path=path_2, content=content_2) + commit_file_2 = commit.FileAddedOrModified(path=path_2, content=content_2) commit_msg = "commit-1" repository_client._github_client_push( @@ -940,7 +939,7 @@ def test_update_branch_unknown_error(monkeypatch, repository_client: Client): mock_git_repository.commit = mock.Mock(return_value=None) mock_git_repository.push = mock.Mock(side_effect=GitCommandError("mocked error")) monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) - monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *_args, **_kwargs: True) with pytest.raises(RepositoryClientError) as exc: repository_client.update_branch("my-message") @@ -959,19 +958,13 @@ def test_update_branch_github_api_git_error( """ repository_client.switch(DEFAULT_BRANCH) - (repository_path / DOCUMENTATION_FOLDER_NAME).mkdir() - (repository_path / (file_path := Path(f"{DOCUMENTATION_FOLDER_NAME}/test.text"))).write_text( - "content 1", encoding="utf-8" - ) - mock_git_repository = mock.MagicMock(spec=Git) mock_git_repository.add = mock.Mock(return_value=None) mock_git_repository.commit = mock.Mock(return_value=None) - mock_git_repository.show = mock.Mock(return_value=f"A {file_path}") mock_git_repository.push = mock.Mock(side_effect=[None, GitCommandError("mocked push error")]) mock_git_repository.show = mock.Mock(side_effect=GitCommandError("mocked show error")) monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) - monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *_args, **_kwargs: True) with pytest.raises(RepositoryClientError) as exc: repository_client.update_branch("my-message") @@ -1002,7 +995,7 @@ def test_update_branch_github_api_github_error( mock_git_repository.push = mock.Mock(side_effect=[None, GitCommandError("mocked error")]) mock_github_repo.get_branch.side_effect = GithubException(0, "", None) monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) - monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *_args, **_kwargs: True) with pytest.raises(RepositoryClientError) as exc: repository_client.update_branch("my-message") @@ -1036,7 +1029,7 @@ def test_update_branch_github_api( mock_git_repository.show = mock.Mock(return_value=f"A {file_path}") mock_git_repository.push = mock.Mock(side_effect=[None, GitCommandError("mocked error")]) monkeypatch.setattr(repository_client._git_repo, "git", mock_git_repository) - monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *args, **kwargs: True) + monkeypatch.setattr(repository_client._git_repo, "is_dirty", lambda *_args, **_kwargs: True) repository_client.update_branch("my-message") From cc256cf49daf1455b6b7e0eb4aaf75fae3cb1a44 Mon Sep 17 00:00:00 2001 From: jdkandersson Date: Fri, 7 Jul 2023 00:30:53 +1000 Subject: [PATCH 24/24] add more specific checks --- tests/unit/test_repository.py | 52 +++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/tests/unit/test_repository.py b/tests/unit/test_repository.py index a04e1a10..54293f91 100644 --- a/tests/unit/test_repository.py +++ b/tests/unit/test_repository.py @@ -18,7 +18,6 @@ from github import Github from github.ContentFile import ContentFile from github.GithubException import GithubException, UnknownObjectException -from github.InputGitTreeElement import InputGitTreeElement from github.PullRequest import PullRequest from github.Repository import Repository @@ -35,25 +34,36 @@ from .helpers import assert_substrings_in_string -@pytest.mark.parametrize( - "commit_file", - [ - pytest.param( - commit.FileAddedOrModified(path=Path("test.text"), content="content 1"), id="added" - ), - pytest.param(commit.FileDeleted(path=Path("test.text")), id="deleted"), - ], -) -def test__commit_file_to_tree_element(commit_file: commit.FileAction): +def test__commit_file_to_tree_element_added(): + """ + arrange: given an added commit file + act: when _commit_file_to_tree_element is called with the commit file + assert: then a InputGitTreeElement is returned with the expected identity. """ - arrange: given commit file + commit_file = commit.FileAddedOrModified(path=Path("test.text"), content="content 1") + + tree_element = repository._commit_file_to_tree_element(commit_file=commit_file) + + assert tree_element._identity["path"] == str(commit_file.path) + assert tree_element._identity["content"] == commit_file.content + assert tree_element._identity["mode"] == "100644" + assert tree_element._identity["type"] == "blob" + + +def test__commit_file_to_tree_element_deleted(): + """ + arrange: given an deleted commit file act: when _commit_file_to_tree_element is called with the commit file - assert: then a InputGitTreeElement is returned. + assert: then a InputGitTreeElement is returned with the expected identity. """ + commit_file = commit.FileDeleted(path=Path("test.text")) + tree_element = repository._commit_file_to_tree_element(commit_file=commit_file) - # InputGitTreeElement don't expose any data, can only check that the correct type is returned - assert isinstance(tree_element, InputGitTreeElement) + assert tree_element._identity["path"] == str(commit_file.path) + assert tree_element._identity["sha"] is None + assert tree_element._identity["mode"] == "100644" + assert tree_element._identity["type"] == "blob" def test__init__(git_repo: Repo, mock_github_repo: Repository): @@ -879,12 +889,12 @@ def test__github_client_push_single(repository_client: Client, mock_github_repo) sha=mock_github_repo.get_branch.return_value.commit.sha ) - # The InputGitTreeElement does not expose its arguments, can only check that the correct number - # were passed mock_github_repo.create_git_tree.assert_called_once() create_git_tree_call_args = mock_github_repo.create_git_tree.call_args_list[0][0] assert len(create_git_tree_call_args) == 2 assert len(create_git_tree_call_args[0]) == 1 + assert create_git_tree_call_args[0][0]._identity["path"] == str(path) + assert create_git_tree_call_args[0][0]._identity["content"] == content assert create_git_tree_call_args[1] == mock_github_repo.get_git_tree.return_value mock_github_repo.create_git_commit.assert_called_once_with( @@ -923,6 +933,10 @@ def test__github_client_push_multiple(repository_client: Client, mock_github_rep create_git_tree_call_args = mock_github_repo.create_git_tree.call_args_list[0][0] assert len(create_git_tree_call_args) == 2 assert len(create_git_tree_call_args[0]) == 2 + assert create_git_tree_call_args[0][0]._identity["path"] == str(path_1) + assert create_git_tree_call_args[0][0]._identity["content"] == content_1 + assert create_git_tree_call_args[0][1]._identity["path"] == str(path_2) + assert create_git_tree_call_args[0][1]._identity["content"] == content_2 def test_update_branch_unknown_error(monkeypatch, repository_client: Client): @@ -947,9 +961,7 @@ def test_update_branch_unknown_error(monkeypatch, repository_client: Client): assert_substrings_in_string(("unexpected error updating branch"), str(exc.value).lower()) -def test_update_branch_github_api_git_error( - monkeypatch, repository_client: Client, repository_path: Path -): +def test_update_branch_github_api_git_error(monkeypatch, repository_client: Client): """ arrange: given Client with a mocked local git repository client that raises an exception when pushing commits and show