Repository: VirtusLab/git-machete
Pull request #214 (2021-09-07 10:06:51)

Add regression tests for github methods

PawelLipski: Whoops must be rebased onto recent develop... also https://github.com/VirtusLab/git-machete/issues/220

Patch:
diff --git a/README.md b/README.md
index 0a220b6..32227df 100644
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@
 [![CircleCI](https://circleci.com/gh/VirtusLab/git-machete/tree/master.svg?style=shield)](https://app.circleci.com/pipelines/github/VirtusLab/git-machete?branch=master)
 [![Read the Docs](https://readthedocs.org/projects/git-machete/badge/?version=latest)](https://git-machete.readthedocs.io/en/latest)
 [![PyPI package](https://img.shields.io/pypi/v/git-machete.svg)](https://pypi.org/project/git-machete)
-[![PyPI package monthly downloads](https://img.shields.io/pypi/dm/git-machete.svg)](https://pypistats.org/packages/git-machete)
+<!-- [![PyPI package monthly downloads](https://img.shields.io/pypi/dm/git-machete.svg)](https://pypistats.org/packages/git-machete) -->
 [![Snap](https://snapcraft.io/git-machete/badge.svg)](https://snapcraft.io/git-machete)
 [![License: MIT](https://img.shields.io/github/license/VirtusLab/git-machete)](https://github.com/VirtusLab/git-machete/blob/master/LICENSE)
 
diff --git a/git_machete/cli.py b/git_machete/cli.py
index 22601fb..e0d554f 100644
--- a/git_machete/cli.py
+++ b/git_machete/cli.py
@@ -7,6 +7,7 @@ import sys
 import textwrap
 from typing import Dict, List, Optional, Tuple, TypeVar
 
+import git_machete.options
 from git_machete import __version__
 from git_machete import utils
 from git_machete.client import MacheteClient, allowed_directions
@@ -14,7 +15,6 @@ from git_machete.constants import EscapeCodes
 from git_machete.docs import short_docs, long_docs
 from git_machete.exceptions import MacheteException, StopTraversal
 from git_machete.git_operations import GitContext
-from git_machete.options import CommandLineOptions
 from git_machete.utils import fmt, underline, excluding, warn
 
 T = TypeVar('T')
@@ -90,7 +90,7 @@ def main() -> None:
 
 
 def launch(orig_args: List[str]) -> None:
-    cli_opts = CommandLineOptions()
+    cli_opts = git_machete.options.CommandLineOptions()
     git = GitContext(cli_opts)
 
     if sys.version_info.major == 2 or (sys.version_info.major == 3 and sys.version_info.minor < 6):
@@ -116,7 +116,7 @@ def launch(orig_args: List[str]) -> None:
             elif opt in ("-d", "--down-fork-point"):
                 cli_opts.opt_down_fork_point = arg
             elif opt == "--debug":
-                CommandLineOptions.opt_debug = True
+                git_machete.options.CommandLineOptions.opt_debug = True
             elif opt == "--draft":
                 cli_opts.opt_draft = True
             elif opt in ("-F", "--fetch"):
@@ -175,7 +175,7 @@ def launch(orig_args: List[str]) -> None:
             elif opt == "--unset-override":
                 cli_opts.opt_unset_override = True
             elif opt in ("-v", "--verbose"):
-                CommandLineOptions.opt_verbose = True
+                git_machete.options.CommandLineOptions.opt_verbose = True
             elif opt == "--version":
                 version()
                 sys.exit()
diff --git a/git_machete/client.py b/git_machete/client.py
index b07f363..132471e 100644
--- a/git_machete/client.py
+++ b/git_machete/client.py
@@ -6,6 +6,7 @@ import shutil
 import sys
 from typing import List, Dict, Optional, Tuple, Generator, Callable
 
+import git_machete.options
 from git_machete import utils
 from git_machete.constants import (
     DISCOVER_DEFAULT_FRESH_BRANCH_COUNT, PICK_FIRST_ROOT, UNTRACKED,
@@ -18,7 +19,6 @@ from git_machete.github import (
     get_parsed_github_remote_url, is_github_remote_url, create_pull_request,
     derive_current_user_login, set_milestone_of_pull_request,
     add_assignees_to_pull_request, add_reviewers_to_pull_request)
-from git_machete.options import CommandLineOptions
 from git_machete.utils import (
     get_pretty_choices, flat_map, excluding, fmt, tupled, warn, debug, bold,
     colored, underline, dim, get_second)
@@ -36,8 +36,8 @@ def allowed_directions(allow_current: bool) -> str:
 
 class MacheteClient:
 
-    def __init__(self, cli_opts: CommandLineOptions, git: GitContext) -> None:
-        self.__cli_opts: CommandLineOptions = cli_opts
+    def __init__(self, cli_opts: git_machete.options.CommandLineOptions, git: GitContext) -> None:
+        self.__cli_opts: git_machete.options.CommandLineOptions = cli_opts
         self.__git: GitContext = git
         self._definition_file_path: str = self.__git.get_git_subpath("machete")
         self._managed_branches: List[str] = []
diff --git a/git_machete/github.py b/git_machete/github.py
index 6ba66e1..2943a61 100644
--- a/git_machete/github.py
+++ b/git_machete/github.py
@@ -9,7 +9,7 @@ import shutil
 import subprocess
 from pathlib import Path
 from typing import Dict, List, Optional, Any, Tuple
-from urllib.request import Request, urlopen
+import urllib.request
 from urllib.error import HTTPError
 
 from git_machete.utils import debug, fmt
@@ -115,12 +115,12 @@ def __fire_github_api_request(method: str, path: str, token: Optional[str], requ
     host = 'https://api.' + GITHUB_DOMAIN
     url = host + path
     json_body: Optional[str] = json.dumps(request_body) if request_body else None
-    http_request = Request(url, headers=headers, data=json_body.encode() if json_body else None, method=method.upper())
+    http_request = urllib.request.Request(url, headers=headers, data=json_body.encode() if json_body else None, method=method.upper())
     debug(f'__fire_github_api_request({method}, {path}, ..., ...)',
           f'firing a {method} request to {url} with {"a" if token else "no"} bearer token and request body {json_body or "<none>"}')
 
     try:
-        with urlopen(http_request) as response:
+        with urllib.request.urlopen(http_request) as response:
             parsed_response_body: Any = json.loads(response.read().decode())
             return parsed_response_body
     except HTTPError as err:


Test patch:
diff --git a/git_machete/tests/functional/test_machete.py b/git_machete/tests/functional/test_machete.py
index a654af5..84a31d9 100644
--- a/git_machete/tests/functional/test_machete.py
+++ b/git_machete/tests/functional/test_machete.py
@@ -1,4 +1,5 @@
 import io
+import json
 import os
 import random
 import re
@@ -8,7 +9,10 @@ import time
 import unittest
 import subprocess
 from contextlib import redirect_stdout
-from typing import Iterable
+from http import HTTPStatus
+from typing import Any, Dict, Iterable, List, Optional, Union
+from unittest import mock
+from urllib.parse import urlparse, ParseResult, parse_qs
 
 from git_machete import cli
 from git_machete.client import MacheteClient
@@ -26,6 +30,182 @@ def get_head_commit_hash() -> str:
     return os.popen("git rev-parse HEAD").read().strip()
 
 
+class FakeCommandLineOptions(CommandLineOptions):
+    def __init__(self) -> None:
+        super().__init__()
+        self.opt_no_interactive_rebase: bool = True
+        self.opt_yes: bool = True
+
+
+class MockGithubAPIState:
+    def __init__(self, pulls: List[Dict[str, Any]]) -> None:
+        self.pulls: List[Dict[str, Any]] = pulls
+        self.user: Dict[str, str] = {'login': 'github_user', 'type': 'User', 'company': 'VirtusLab'}
+        self.issues: List[Dict[str, Any]] = []
+
+    def new_request(self) -> "MockGithubAPIRequest":
+        return MockGithubAPIRequest(self)
+
+    def get_issue(self, issue_no: str) -> Optional[Dict[str, Any]]:
+        for issue in self.issues:
+            if issue['number'] == issue_no:
+                return issue
+        return None
+
+    def get_pull(self, pull_no: str) -> Optional[Dict[str, Any]]:
+        for pull in self.pulls:
+            if pull['number'] == pull_no:
+                return pull
+        return None
+
+
+class MockGithubAPIResponse:
+    def __init__(self, status_code: int, response_data: Union[List[Dict[str, Any]], Dict[str, Any]]) -> None:
+        self.response_data: Union[List[Dict[str, Any]], Dict[str, Any]] = response_data
+        self.status_code: int = status_code
+
+    def read(self) -> bytes:
+        return json.dumps(self.response_data).encode()
+
+
+class MockGithubAPIRequest:
+    def __init__(self, github_api_state: MockGithubAPIState) -> None:
+        self.github_api_state: MockGithubAPIState = github_api_state
+
+    def __call__(self, url: str, headers: Dict[str, str] = None, data: Union[str, bytes, None] = None, method: str = '') -> "MockGithubAPIResponse":
+        self.parsed_url: ParseResult = urlparse(url, allow_fragments=True)
+        self.parsed_query: Dict[str, List[str]] = parse_qs(self.parsed_url.query)
+        self.json_data: Union[str, bytes] = data
+        self.return_data: Optional[Union[List[Dict[str, Any]], Dict[str, Any]]] = None
+        self.headers: Dict[str, str] = headers
+        return self.handle_method(method)
+
+    def handle_method(self, method: str) -> "MockGithubAPIResponse":
+        if method == "GET":
+            return self.handle_get()
+        elif method == "PATCH":
+            return self.handle_patch()
+        elif method == "POST":
+            return self.handle_post()
+        else:
+            return self.make_response_object(HTTPStatus.METHOD_NOT_ALLOWED, [])
+
+    def handle_get(self) -> "MockGithubAPIResponse":
+        if self.parsed_url.path.endswith('pulls'):
+            full_head_name: Optional[List[str]] = self.parsed_query.get('head')
+            if full_head_name:
+                head: str = full_head_name[0].split(':')[1]
+                for pr in self.github_api_state.pulls:
+                    if pr['head']['ref'] == head:
+                        return self.make_response_object(HTTPStatus.OK, [pr])
+                return self.make_response_object(HTTPStatus.NOT_FOUND, [])
+            else:
+                return self.make_response_object(HTTPStatus.OK, self.github_api_state.pulls)
+        elif self.parsed_url.path.endswith('user'):
+            return self.make_response_object(HTTPStatus.OK, [self.github_api_state.user])
+        else:
+            return self.make_response_object(HTTPStatus.NOT_FOUND, [])
+
+    def handle_patch(self) -> "MockGithubAPIResponse":
+        if 'issues' in self.parsed_url.path:
+            return self.update_issue()
+        elif 'pulls' in self.parsed_url.path:
+            return self.update_pull_request()
+        else:
+            return self.make_response_object(HTTPStatus.NOT_FOUND, [])
+
+    def handle_post(self) -> "MockGithubAPIResponse":
+        assert not self.parsed_query
+        if self.parsed_url.path.endswith('issues'):
+            return self.update_issue()
+        elif self.parsed_url.path.endswith('pulls'):
+            return self.update_pull_request()
+        else:
+            return self.make_response_object(HTTPStatus.NOT_FOUND, [])
+
+    def update_pull_request(self) -> "MockGithubAPIResponse":
+        pull_no: str = self.find_number(self.parsed_url.path)
+        if not pull_no:
+            return self.create_pull_request()
+        pull: Dict[str, Any] = self.github_api_state.get_pull(pull_no)
+        return self.fill_pull_request_data(json.loads(self.json_data), pull)
+
+    def create_pull_request(self) -> "MockGithubAPIResponse":
+        pull = {'number': self.get_next_free_number(self.github_api_state.pulls),
+                'user': {'login': 'github_user'},
+                'html_url': 'www.github.com'}
+        return self.fill_pull_request_data(json.loads(self.json_data), pull)
+
+    def fill_pull_request_data(self, data: Dict[str, Any], pull: Dict[str, Any]) -> "MockGithubAPIResponse":
+        index = self.get_index_or_None(pull, self.github_api_state.issues)
+        for key in data.keys():
+            if key in ('base', 'head'):
+                pull[key] = {'ref': ""}
+                pull[key]['ref'] = json.loads(self.json_data)[key]
+            else:
+                pull[key] = json.loads(self.json_data)[key]
+        if index:
+            self.github_api_state.pulls[index] = pull
+        else:
+            self.github_api_state.pulls.append(pull)
+        return self.make_response_object(HTTPStatus.CREATED, pull)
+
+    def update_issue(self) -> "MockGithubAPIResponse":
+        issue_no: str = self.find_number(self.parsed_url.path)
+        if not issue_no:
+            return self.create_issue()
+        issue: Dict[str, Any] = self.github_api_state.get_issue(issue_no)
+        return self.fill_issue_data(json.loads(self.json_data), issue)
+
+    def create_issue(self) -> "MockGithubAPIResponse":
+        issue = {'number': self.get_next_free_number(self.github_api_state.issues)}
+        return self.fill_issue_data(json.loads(self.json_data), issue)
+
+    def fill_issue_data(self, data: Dict[str, Any], issue: Dict[str, Any]) -> "MockGithubAPIResponse":
+        index = self.get_index_or_None(issue, self.github_api_state.issues)
+        for key in data.keys():
+            issue[key] = data[key]
+        if index:
+            self.github_api_state.issues[index] = issue
+        else:
+            self.github_api_state.issues.append(issue)
+        return self.make_response_object(HTTPStatus.CREATED, issue)
+
+    @staticmethod
+    def get_index_or_None(entity: Dict[str, Any], base: List[Dict[str, Any]]) -> Optional[int]:
+        try:
+            return base.index(entity)
+        except ValueError:
+            return None
+
+    @staticmethod
+    def make_response_object(status_code: int, response_data: Union[List[Dict[str, Any]], Dict[str, Any]]) -> "MockGithubAPIResponse":
+        return MockGithubAPIResponse(status_code, response_data)
+
+    @staticmethod
+    def find_number(url: str) -> Optional[str]:
+        m = re.search(r'\d+', url)
+        if m:
+            return m.group()
+        return None
+
+    @staticmethod
+    def get_next_free_number(entities: List[Dict[str, Any]]) -> str:
+        numbers = [int(item['number']) for item in entities]
+        return str(max(numbers) + 1)
+
+
+class MockContextManager:
+    def __init__(self, obj: MockGithubAPIRequest) -> None:
+        self.obj = obj
+
+    def __enter__(self) -> MockGithubAPIRequest:
+        return self.obj
+
+    def __exit__(self, *args: Any) -> None:
+        pass
+
+
 class GitRepositorySandbox:
     def __init__(self) -> None:
         self.remote_path = os.popen("mktemp -d").read().strip()
@@ -81,6 +261,10 @@ class GitRepositorySandbox:
         self.execute(f'git branch -d "{branch}"')
         return self
 
+    def add_remote(self, remote: str, url: str) -> "GitRepositorySandbox":
+        self.execute(f'git remote add {remote} {url}')
+        return self
+
 
 class MacheteTester(unittest.TestCase):
     @staticmethod
@@ -101,8 +285,8 @@ class MacheteTester(unittest.TestCase):
         with open(os.path.join(os.getcwd(), definition_file_path), 'w') as def_file:
             def_file.writelines(new_body)
 
-    def assert_command(self, cmds: Iterable[str], expected_result: str) -> None:
-        self.assertEqual(self.launch_command(*cmds), self.adapt(expected_result))
+    def assert_command(self, cmds: Iterable[str], expected_result: str, strip_indentation: bool = True) -> None:
+        self.assertEqual(self.launch_command(*cmds), self.adapt(expected_result) if strip_indentation else expected_result)
 
     def setUp(self) -> None:
         # Status diffs can be quite large, default to ~256 lines of diff context
@@ -1382,3 +1566,245 @@ class MacheteTester(unittest.TestCase):
                 "<commit_hash>' drops the commits until (included) fork point "
                 "specified by the option '-f' from the current branch."
         )
+
+    git_api_state_for_test_retarget_pr = MockGithubAPIState(
+        [{'head': {'ref': 'feature'}, 'user': {'login': 'github_user'}, 'base': {'ref': 'root'}, 'number': '15',
+          'html_url': 'www.github.com'}])
+
+    @mock.patch('urllib.request.Request', git_api_state_for_test_retarget_pr.new_request())
+    @mock.patch('urllib.request.urlopen', MockContextManager)
+    def test_retarget_pr(self) -> None:
+        branchs_first_commit_msg = "First commit on branch."
+        branchs_second_commit_msg = "Second commit on branch."
+        (
+            self.repo_sandbox.new_branch("root")
+                .commit("First commit on root.")
+                .new_branch("branch-1")
+                .commit(branchs_first_commit_msg)
+                .commit(branchs_second_commit_msg)
+                .push()
+                .new_branch('feature')
+                .commit('introduce feature')
+                .push()
+                .check_out('feature')
+                .add_remote('new_origin', 'https://github.com/user/repo.git')
+        )
+
+        self.launch_command("discover", "-y")
+        self.assert_command(['github', 'retarget-pr'], 'The base branch of PR #15 has been switched to `branch-1`\n', strip_indentation=False)
+        self.assert_command(['github', 'retarget-pr'], 'The base branch of PR #15 is already `branch-1`\n', strip_indentation=False)
+
+    git_api_state_for_test_anno_prs = MockGithubAPIState([
+        {'head': {'ref': 'ignore-trailing'}, 'user': {'login': 'github_user'}, 'base': {'ref': 'hotfix/add-trigger'}, 'number': '3', 'html_url': 'www.github.com'},
+        {'head': {'ref': 'allow-ownership-link'}, 'user': {'login': 'github_user'}, 'base': {'ref': 'develop'}, 'number': '7', 'html_url': 'www.github.com'},
+        {'head': {'ref': 'call-ws'}, 'user': {'login': 'github_user'}, 'base': {'ref': 'develop'}, 'number': '31', 'html_url': 'www.github.com'}
+    ])
+
+    @mock.patch('urllib.request.urlopen', MockContextManager)
+    @mock.patch('urllib.request.Request', git_api_state_for_test_anno_prs.new_request())
+    def test_anno_prs(self) -> None:
+
+        (
+            self.repo_sandbox.new_branch("root")
+                .commit("root")
+                .new_branch("develop")
+                .commit("develop commit")
+                .new_branch("allow-ownership-link")
+                .commit("Allow ownership links")
+                .push()
+                .new_branch("build-chain")
+                .commit("Build arbitrarily long chains")
+                .check_out("allow-ownership-link")
+                .commit("1st round of fixes")
+                .check_out("develop")
+                .commit("Other develop commit")
+                .push()
+                .new_branch("call-ws")
+                .commit("Call web service")
+                .commit("1st round of fixes")
+                .push()
+                .new_branch("drop-constraint")
+                .commit("Drop unneeded SQL constraints")
+                .check_out("call-ws")
+                .commit("2nd round of fixes")
+                .check_out("root")
+                .new_branch("master")
+                .commit("Master commit")
+                .push()
+                .new_branch("hotfix/add-trigger")
+                .commit("HOTFIX Add the trigger")
+                .push()
+                .commit_amend("HOTFIX Add the trigger (amended)")
+                .new_branch("ignore-trailing")
+                .commit("Ignore trailing data")
+                .sleep(1)
+                .commit_amend("Ignore trailing data (amended)")
+                .push()
+                .reset_to("ignore-trailing@{1}")
+                .delete_branch("root")
+                .add_remote('new_origin', 'https://github.com/user/repo.git')
+        )
+        self.launch_command("discover", "-y")
+        self.launch_command('github', 'anno-prs')
+        self.assert_command(
+            ["status"],
+            """
+            master
+            |
+            o-hotfix/add-trigger (diverged from origin)
+              |
+              o-ignore-trailing *  PR #3 (github_user) (diverged from & older than origin)
+
+            develop
+            |
+            x-allow-ownership-link  PR #7 (github_user) (ahead of origin)
+            | |
+            | x-build-chain (untracked)
+            |
+            o-call-ws  PR #31 (github_user) (ahead of origin)
+              |
+              x-drop-constraint (untracked)
+            """,
+        )
+
+    git_api_state_for_test_create_pr = MockGithubAPIState([{'head': {'ref': 'ignore-trailing'}, 'user': {'login': 'github_user'}, 'base': {'ref': 'hotfix/add-trigger'}, 'number': '3', 'html_url': 'www.github.com'}])
+
+    @mock.patch('git_machete.options.CommandLineOptions', FakeCommandLineOptions)
+    @mock.patch('urllib.request.urlopen', MockContextManager)
+    @mock.patch('urllib.request.Request', git_api_state_for_test_create_pr.new_request())
+    def test_github_create_pr(self) -> None:
+
+        (
+            self.repo_sandbox.new_branch("root")
+                .commit("initial commit")
+                .new_branch("develop")
+                .commit("first commit")
+                .new_branch("allow-ownership-link")
+                .commit("Enable ownership links")
+                .push()
+                .new_branch("build-chain")
+                .commit("Build arbitrarily long chains of PRs")
+                .check_out("allow-ownership-link")
+                .commit("fixes")
+                .check_out("develop")
+                .commit("Other develop commit")
+                .push()
+                .new_branch("call-ws")
+                .commit("Call web service")
+                .commit("1st round of fixes")
+                .push()
+                .new_branch("drop-constraint")
+                .commit("Drop unneeded SQL constraints")
+                .check_out("call-ws")
+                .commit("2nd round of fixes")
+                .check_out("root")
+                .new_branch("master")
+                .commit("Master commit")
+                .push()
+                .new_branch("hotfix/add-trigger")
+                .commit("HOTFIX Add the trigger")
+                .push()
+                .commit_amend("HOTFIX Add the trigger (amended)")
+                .new_branch("ignore-trailing")
+                .commit("Ignore trailing data")
+                .sleep(1)
+                .commit_amend("Ignore trailing data (amended)")
+                .push()
+                .reset_to("ignore-trailing@{1}")
+                .delete_branch("root")
+                .new_branch('chore/fields')
+                .commit("remove outdated fields")
+                .add_remote('new_origin', 'https://github.com/user/repo.git')
+                .check_out("call-ws")
+        )
+
+        self.launch_command("discover")
+        self.launch_command("github", "create-pr")
+        # ahead of origin state, push is advised and accepted
+        self.assert_command(
+            ['status'],
+            """
+            master
+            |
+            o-hotfix/add-trigger (diverged from origin)
+              |
+              o-ignore-trailing (diverged from & older than origin)
+                |
+                o-chore/fields (untracked)
+
+            develop
+            |
+            x-allow-ownership-link (ahead of origin)
+            | |
+            | x-build-chain (untracked)
+            |
+            o-call-ws *  PR #4
+              |
+              x-drop-constraint (untracked)
+            """,
+        )
+        self.repo_sandbox.check_out('chore/fields')
+        #  untracked state (can only create pr when branch is pushed)
+        self.launch_command("github", "create-pr", "--draft")
+        self.assert_command(
+            ['status'],
+            """
+            master
+            |
+            o-hotfix/add-trigger (diverged from origin)
+              |
+              o-ignore-trailing (diverged from & older than origin)
+                |
+                o-chore/fields *  PR #5
+
+            develop
+            |
+            x-allow-ownership-link (ahead of origin)
+            | |
+            | x-build-chain (untracked)
+            |
+            o-call-ws  PR #4
+              |
+              x-drop-constraint (untracked)
+            """,
+        )
+
+        (
+            self.repo_sandbox.check_out('hotfix/add-trigger')
+            .commit('trigger released')
+            .commit('minor changes applied')
+        )
+
+        # diverged from and newer than origin
+        self.launch_command("github", "create-pr")
+        self.assert_command(
+            ['status'],
+            """
+            master
+            |
+            o-hotfix/add-trigger *  PR #6
+              |
+              x-ignore-trailing (diverged from & older than origin)
+                |
+                o-chore/fields  PR #5
+
+            develop
+            |
+            x-allow-ownership-link (ahead of origin)
+            | |
+            | x-build-chain (untracked)
+            |
+            o-call-ws  PR #4
+              |
+              x-drop-constraint (untracked)
+            """,
+        )
+        # check against attempt to create already existing pull request
+        machete_client = MacheteClient(cli_opts, git)
+        expected_error_message = "Pull request for branch hotfix/add-trigger is already created under link www.github.com!\nPR details: PR #6 by github_user: hotfix/add-trigger -> master"
+        machete_client.read_definition_file()
+        with self.assertRaises(MacheteException) as e:
+            machete_client.create_github_pr('hotfix/add-trigger', draft=False)
+        if e:
+            self.assertEqual(e.exception.parameter, expected_error_message,
+                             'Verify that expected error message has appeared when given pull request to create is already created.')