diff --git a/jupyterlab_pullrequests/handlers.py b/jupyterlab_pullrequests/handlers.py index fb0da05..2a21c06 100644 --- a/jupyterlab_pullrequests/handlers.py +++ b/jupyterlab_pullrequests/handlers.py @@ -4,6 +4,7 @@ import json import logging import traceback +from typing import Optional from http import HTTPStatus import tornado @@ -198,26 +199,33 @@ def get_body_value(handler): ] -def setup_handlers(web_app: "NotebookWebApplication", config: PRConfig): +def setup_handlers(web_app: tornado.web.Application, config: PRConfig, log: Optional[logging.Logger]=None): host_pattern = ".*$" base_url = url_path_join(web_app.settings["base_url"], NAMESPACE) - logger = get_logger() + log = log or logging.getLogger(__name__) manager_class = MANAGERS.get(config.provider) if manager_class is None: - logger.error(f"No manager defined for provider '{config.provider}'.") + log.error(f"PR Manager: No manager defined for provider '{config.provider}'.") raise NotImplementedError() - manager = manager_class(config.api_base_url, config.access_token) - - web_app.add_handlers( - host_pattern, - [ - ( - url_path_join(base_url, pat), - handler, - {"logger": logger, "manager": manager}, - ) - for pat, handler in default_handlers - ], - ) + log.info(f"PR Manager Class {manager_class}") + try: + manager = manager_class(config) + except Exception as err: + import traceback + logging.error("PR Manager Exception", exc_info=1) + raise err + + handlers = [ + ( + url_path_join(base_url, pat), + handler, + {"logger": log, "manager": manager}, + ) + for pat, handler in default_handlers + ] + + log.debug(f"PR Handlers: {handlers}") + + web_app.add_handlers(host_pattern, handlers) diff --git a/jupyterlab_pullrequests/managers/github.py b/jupyterlab_pullrequests/managers/github.py index 48cedad..44384f8 100644 --- a/jupyterlab_pullrequests/managers/github.py +++ b/jupyterlab_pullrequests/managers/github.py @@ -6,23 +6,20 @@ from tornado.httputil import url_concat from tornado.web import HTTPError -from ..base import CommentReply, NewComment +from ..base import CommentReply, NewComment, PRConfig from .manager import PullRequestsManager class GitHubManager(PullRequestsManager): """Pull request manager for GitHub.""" - def __init__( - self, base_api_url: str = "https://api.github.com", access_token: str = "" - ) -> None: - """ - Args: - base_api_url: Base REST API url for the versioning service - access_token: Versioning service access token - """ - super().__init__(base_api_url=base_api_url, access_token=access_token) - self._pull_requests_cache = {} # Dict[str, Dict] + def __init__(self, config: PRConfig) -> None: + super().__init__(config) + self._pull_requests_cache = {} + + @property + def base_api_url(self): + return self._config.api_base_url or "https://api.github.com" @property def per_page_argument(self) -> Optional[Tuple[str, int]]: @@ -40,7 +37,7 @@ async def get_current_user(self) -> Dict[str, str]: Returns: JSON description of the user matching the access token """ - git_url = url_path_join(self._base_api_url, "user") + git_url = url_path_join(self.base_api_url, "user") data = await self._call_github(git_url, has_pagination=False) return {"username": data["login"]} @@ -186,7 +183,7 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]: # Use search API to find matching pull requests and return git_url = url_path_join( - self._base_api_url, "/search/issues?q=+state:open+type:pr" + search_filter + self.base_api_url, "/search/issues?q=+state:open+type:pr" + search_filter ) results = await self._call_github(git_url) @@ -273,7 +270,7 @@ async def _call_github( """ headers = { "Accept": media_type, - "Authorization": f"token {self._access_token}", + "Authorization": f"token {self._config.access_token}", } return await super()._call_provider( diff --git a/jupyterlab_pullrequests/managers/gitlab.py b/jupyterlab_pullrequests/managers/gitlab.py index ff6373a..d99a62e 100644 --- a/jupyterlab_pullrequests/managers/gitlab.py +++ b/jupyterlab_pullrequests/managers/gitlab.py @@ -13,7 +13,7 @@ from tornado.httputil import url_concat from tornado.web import HTTPError -from ..base import CommentReply, NewComment +from ..base import CommentReply, NewComment, PRConfig from ..log import get_logger from .manager import PullRequestsManager @@ -25,15 +25,9 @@ class GitLabManager(PullRequestsManager): MINIMAL_VERSION = "13.1" # Due to pagination https://docs.gitlab.com/ee/api/README.html#pagination - def __init__( - self, base_api_url: str = "https://gitlab.com/api/v4/", access_token: str = "" - ) -> None: - """ - Args: - base_api_url: Base REST API url for the versioning service - access_token: Versioning service access token - """ - super().__init__(base_api_url=base_api_url, access_token=access_token) + def __init__(self, config: PRConfig) -> None: + super().__init__(config) + # Creating new file discussion required some commit sha's so we will cache them self._merge_requests_cache = {} # Dict[str, Dict] # Creating discussion on unmodified line requires to figure out the line number @@ -41,6 +35,10 @@ def __init__( # we cache the diff to speed up the process. self._file_diff_cache = {} # Dict[Tuple[str, str], List[difflib.Match]] + @property + def base_api_url(self): + return self._config.api_base_url or "https://gitlab.com/api/v4/" + @property def per_page_argument(self) -> Optional[Tuple[str, int]]: """Returns query argument to set number of items per page. @@ -57,7 +55,7 @@ async def check_server_version(self) -> bool: Returns: Whether the server version is higher than the minimal supported version. """ - url = url_path_join(self._base_api_url, "version") + url = url_path_join(self.base_api_url, "version") data = await self._call_gitlab(url, has_pagination=False) server_version = data.get("version", "") is_valid = True @@ -79,7 +77,7 @@ async def get_current_user(self) -> Dict[str, str]: # Check server compatibility await self.check_server_version() - git_url = url_path_join(self._base_api_url, "user") + git_url = url_path_join(self.base_api_url, "user") data = await self._call_gitlab(git_url, has_pagination=False) return {"username": data["username"]} @@ -227,7 +225,7 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]: # Use search API to find matching pull requests and return git_url = url_path_join( - self._base_api_url, "/merge_requests?state=opened&" + search_filter + self.base_api_url, "/merge_requests?state=opened&" + search_filter ) results = await self._call_gitlab(git_url) @@ -235,7 +233,7 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]: data = [] for result in results: url = url_path_join( - self._base_api_url, + self.base_api_url, "projects", str(result["project_id"]), "merge_requests", @@ -374,7 +372,7 @@ async def _call_gitlab( """ headers = { - "Authorization": f"Bearer {self._access_token}", + "Authorization": f"Bearer {self._config.access_token}", "Accept": "application/json", } return await super()._call_provider( @@ -481,7 +479,7 @@ def _response_to_comment(result: Dict[str, str]) -> Dict[str, str]: async def __get_content(self, project_id: int, filename: str, sha: str) -> str: url = url_concat( url_path_join( - self._base_api_url, + self.base_api_url, "projects", str(project_id), "repository/files", diff --git a/jupyterlab_pullrequests/managers/manager.py b/jupyterlab_pullrequests/managers/manager.py index 24e8894..f7e3e57 100644 --- a/jupyterlab_pullrequests/managers/manager.py +++ b/jupyterlab_pullrequests/managers/manager.py @@ -10,25 +10,19 @@ from .._version import __version__ from ..log import get_logger - +from ..base import PRConfig class PullRequestsManager(abc.ABC): """Abstract base class for pull requests manager.""" - def __init__(self, base_api_url: str = "", access_token: str = "") -> None: - """ - Args: - base_api_url: Base REST API url for the versioning service - access_token: Versioning service access token - """ + def __init__(self, config: PRConfig) -> None: + self._config = config self._client = tornado.httpclient.AsyncHTTPClient() - self._base_api_url = base_api_url - self._access_token = access_token @property def base_api_url(self) -> str: """The provider base REST API URL""" - return self._base_api_url + return self._config.api_base_url @property def log(self) -> logging.Logger: @@ -142,7 +136,7 @@ async def _call_provider( List or Dict: Create from JSON response body if load_json is True str: Raw response body if load_json is False """ - if not self._access_token: + if not self._config.access_token: raise tornado.web.HTTPError( status_code=http.HTTPStatus.BAD_REQUEST, reason="No access token specified. Please set PRConfig.access_token in your user jupyter_server_config file.", @@ -154,8 +148,8 @@ async def _call_provider( headers["Content-Type"] = "application/json" body = tornado.escape.json_encode(body) - if not url.startswith(self._base_api_url): - url = url_path_join(self._base_api_url, url) + if not url.startswith(self.base_api_url): + url = url_path_join(self.base_api_url, url) with_pagination = False if ( diff --git a/jupyterlab_pullrequests/tests/conftest.py b/jupyterlab_pullrequests/tests/conftest.py index c661b53..052602a 100644 --- a/jupyterlab_pullrequests/tests/conftest.py +++ b/jupyterlab_pullrequests/tests/conftest.py @@ -1 +1,40 @@ +import pytest + +from ..base import PRConfig + +# the preferred method for loading jupyter_server (because entry_points) pytest_plugins = ["jupyter_server.pytest_plugin"] + + +@pytest.fixture +def pr_base_config(): + return PRConfig() + + +@pytest.fixture +def pr_github_config(pr_base_config): + return pr_base_config() + + +@pytest.fixture +def pr_github_manager(pr_base_config): + from ..managers.github import GitHubManager + return GitHubManager(pr_base_config) + + +@pytest.fixture +def pr_valid_github_manager(pr_github_manager): + pr_github_manager._config.access_token = "valid" + return pr_github_manager + + +@pytest.fixture +def pr_gitlab_manger(pr_base_config): + from ..managers.gitlab import GitLabManager + return GitLabManager(pr_base_config) + + +@pytest.fixture +def pr_valid_gitlab_manager(pr_gitlab_manger): + pr_gitlab_manger._config.access_token = "valid" + return pr_gitlab_manger diff --git a/jupyterlab_pullrequests/tests/test_github.py b/jupyterlab_pullrequests/tests/test_github.py index bb25a94..85e52c0 100644 --- a/jupyterlab_pullrequests/tests/test_github.py +++ b/jupyterlab_pullrequests/tests/test_github.py @@ -20,21 +20,19 @@ def read_sample_response(filename): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_user_pat_empty(mock_call_provider): - manager = GitHubManager(access_token="") +async def test_GitHubManager_user_pat_empty(mock_call_provider, pr_github_manager): with (pytest.raises(HTTPError)) as e: - await manager.get_current_user() + await pr_github_manager.get_current_user() assert e.value.status_code == HTTPStatus.BAD_REQUEST assert "No access token specified" in e.value.reason @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_user_pat_valid(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_user_pat_valid(mock_call_provider, pr_valid_github_manager): mock_call_provider.return_value = read_sample_response("github_current_user.json") - result = await manager.get_current_user() + result = await pr_valid_github_manager.get_current_user() assert result == {"username": "timnlupo"} @@ -44,10 +42,8 @@ async def test_GitHubManager_user_pat_valid(mock_call_provider): "jupyterlab_pullrequests.managers.github.GitHubManager._call_github", new_callable=AsyncMock, ) -async def test_GitHubManager_list_prs_created(mock_call_provider): - manager = GitHubManager(access_token="valid") - - await manager.list_prs("octocat", "created") +async def test_GitHubManager_list_prs_created(mock_call_provider, pr_valid_github_manager): + await pr_valid_github_manager.list_prs("octocat", "created") mock_call_provider.assert_called_once_with( "https://api.github.com/search/issues?q=+state:open+type:pr+author:octocat", @@ -59,10 +55,8 @@ async def test_GitHubManager_list_prs_created(mock_call_provider): "jupyterlab_pullrequests.managers.github.GitHubManager._call_github", new_callable=AsyncMock, ) -async def test_GitHubManager_list_prs_assigned(mock_call_provider): - manager = GitHubManager(access_token="valid") - - await manager.list_prs("notoctocat", "assigned") +async def test_GitHubManager_list_prs_assigned(mock_call_provider, pr_valid_github_manager): + await pr_valid_github_manager.list_prs("notoctocat", "assigned") mock_call_provider.assert_called_once_with( "https://api.github.com/search/issues?q=+state:open+type:pr+assignee:notoctocat", @@ -71,11 +65,10 @@ async def test_GitHubManager_list_prs_assigned(mock_call_provider): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_list_prs_result(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_list_prs_result(mock_call_provider, pr_valid_github_manager): mock_call_provider.return_value = read_sample_response("github_list_prs.json") - result = await manager.list_prs("octocat", "assigned") + result = await pr_valid_github_manager.list_prs("octocat", "assigned") assert result == [ { @@ -90,11 +83,10 @@ async def test_GitHubManager_list_prs_result(mock_call_provider): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_list_files_call(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_list_files_call(mock_call_provider, pr_valid_github_manager): mock_call_provider.return_value = read_sample_response("github_list_files.json") - result = await manager.list_files( + result = await pr_valid_github_manager.list_files( "https://api.github.com/repos/octocat/repo/pulls/1" ) @@ -108,14 +100,13 @@ async def test_GitHubManager_list_files_call(mock_call_provider): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_get_file_diff(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_get_file_diff(mock_call_provider, pr_valid_github_manager): mock_call_provider.side_effect = [ read_sample_response("github_pr_links.json"), MagicMock(body=b"test code content"), MagicMock(body=b"test new code content"), ] - result = await manager.get_file_diff("valid-prid", "valid-filename") + result = await pr_valid_github_manager.get_file_diff("valid-prid", "valid-filename") assert mock_call_provider.call_count == 3 assert result == { "base": { @@ -133,11 +124,10 @@ async def test_GitHubManager_get_file_diff(mock_call_provider): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_get_threads(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_get_threads(mock_call_provider, pr_valid_github_manager): mock_call_provider.return_value = read_sample_response("github_comments_get.json") - result = await manager.get_threads( + result = await pr_valid_github_manager.get_threads( "https://api.github.com/repos/octocat/repo/pulls/1", "test.ipynb" ) @@ -170,12 +160,11 @@ async def test_GitHubManager_get_threads(mock_call_provider): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_post_comment_valid_reply(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_post_comment_valid_reply(mock_call_provider, pr_valid_github_manager): mock_call_provider.return_value = read_sample_response("github_comments_post.json") body = CommentReply("test text", "test.ipynb", 123) - result = await manager.post_comment( + result = await pr_valid_github_manager.post_comment( "https://api.github.com/repos/octocat/repo/pulls/1", body ) @@ -203,8 +192,7 @@ async def test_GitHubManager_post_comment_valid_reply(mock_call_provider): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_post_comment_valid_new(mock_call_provider): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_post_comment_valid_new(mock_call_provider, pr_valid_github_manager): mock_call_provider.side_effect = [ read_sample_response("github_pr_links.json"), read_sample_response("github_comments_post.json"), @@ -213,7 +201,7 @@ async def test_GitHubManager_post_comment_valid_new(mock_call_provider): text="test text", filename="test.ipynb", line=3, originalLine=None ) - result = await manager.post_comment( + result = await pr_valid_github_manager.post_comment( "https://api.github.com/repos/octocat/repo/pulls/1", body ) diff --git a/jupyterlab_pullrequests/tests/test_gitlab.py b/jupyterlab_pullrequests/tests/test_gitlab.py index 4e89d39..c36b90f 100644 --- a/jupyterlab_pullrequests/tests/test_gitlab.py +++ b/jupyterlab_pullrequests/tests/test_gitlab.py @@ -30,16 +30,17 @@ def read_sample_response(filename): ), ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitLabManager_get_current_user(mock_call_provider, token, data, code): +async def test_GitLabManager_get_current_user(mock_call_provider, token, data, code, pr_gitlab_manger): mock_call_provider.return_value = read_sample_response("get_user.json") - manager = GitLabManager(access_token=token) + pr_gitlab_manger._config.access_token = token + if code is None: - result = await manager.get_current_user() + result = await pr_gitlab_manger.get_current_user() assert result == data else: with pytest.raises(HTTPError) as e: - await manager.get_current_user() + await pr_gitlab_manger.get_current_user() assert e.value.status_code == code assert data in e.value.reason @@ -53,16 +54,15 @@ async def test_GitLabManager_get_current_user(mock_call_provider, token, data, c ), ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitLabManager_list_prs(mock_call_provider, user, filter, expected): +async def test_GitLabManager_list_prs(mock_call_provider, user, filter, expected, pr_valid_gitlab_manager): mock_call_provider.return_value = read_sample_response("get_prs.json") - manager = GitLabManager(access_token="valid") - result = await manager.list_prs(user, filter) + result = await pr_valid_gitlab_manager.list_prs(user, filter) mock_call_provider.assert_called_once() assert ( mock_call_provider.call_args[0][0].url - == f"{manager.base_api_url}merge_requests?state=opened&{expected}&per_page=100" + == f"{pr_valid_gitlab_manager.base_api_url}/merge_requests?state=opened&{expected}&per_page=100" ) assert isinstance(result, list) @@ -85,13 +85,12 @@ async def test_GitLabManager_list_prs(mock_call_provider, user, filter, expected ), ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitLabManager_list_files(mock_call_provider, user, filter, expected): +async def test_GitLabManager_list_files(mock_call_provider, user, filter, expected, pr_valid_gitlab_manager): mock_call_provider.return_value = read_sample_response("get_pr_changes.json") - manager = GitLabManager(access_token="valid") - url = f"{manager.base_api_url}merge_requests/1" + url = f"{pr_valid_gitlab_manager.base_api_url}/merge_requests/1" - result = await manager.list_files(url) + result = await pr_valid_gitlab_manager.list_files(url) mock_call_provider.assert_called_once() assert mock_call_provider.call_args[0][0].url == url_path_join( url, "changes?per_page=100" @@ -167,13 +166,12 @@ async def test_GitLabManager_list_files(mock_call_provider, user, filter, expect "jupyterlab_pullrequests.managers.gitlab.GitLabManager._call_gitlab", new_callable=AsyncMock, ) -async def test_GitLabManager_list_files_status(mock_call_provider, change, status): +async def test_GitLabManager_list_files_status(mock_call_provider, change, status, pr_valid_gitlab_manager): mock_call_provider.return_value = [{"changes": [change]}] - manager = GitLabManager(access_token="valid") - url = f"{manager.base_api_url}merge_requests/1" + url = f"{pr_valid_gitlab_manager.base_api_url}/merge_requests/1" - result = await manager.list_files(url) + result = await pr_valid_gitlab_manager.list_files(url) assert result[0]["status"] == status @@ -191,11 +189,10 @@ async def test_GitLabManager_list_files_status(mock_call_provider, change, statu ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) async def test_GitLabManager_get_file_diff( - mock_call_provider, old_content, new_content + mock_call_provider, old_content, new_content, pr_valid_gitlab_manager ): mock_call_provider.return_value = read_sample_response("get_pr_changes.json") - manager = GitLabManager(access_token="valid") mock_call_provider.side_effect = [ read_sample_response("get_pr.json"), old_content @@ -205,7 +202,7 @@ async def test_GitLabManager_get_file_diff( if isinstance(new_content, Exception) else MagicMock(body=bytes(new_content, encoding="utf-8")), ] - result = await manager.get_file_diff("valid-prid", "valid-filename") + result = await pr_valid_gitlab_manager.get_file_diff("valid-prid", "valid-filename") assert mock_call_provider.call_count == 3 assert result == { "base": { @@ -300,16 +297,15 @@ async def test_GitLabManager_get_file_diff( ), ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitLabManager_get_threads(mock_call_provider, filename, expected): +async def test_GitLabManager_get_threads(mock_call_provider, filename, expected, pr_valid_gitlab_manager): mock_call_provider.return_value = read_sample_response("get_pr_comments.json") - manager = GitLabManager(access_token="valid") - result = await manager.get_threads("mergerequests-id", filename) + result = await pr_valid_gitlab_manager.get_threads("mergerequests-id", filename) mock_call_provider.assert_called_once() assert ( mock_call_provider.call_args[0][0].url - == f"{manager.base_api_url}mergerequests-id/discussions?per_page=100" + == f"{pr_valid_gitlab_manager.base_api_url}/mergerequests-id/discussions?per_page=100" ) assert result == expected @@ -388,9 +384,8 @@ async def test_GitLabManager_get_threads(mock_call_provider, filename, expected) ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) async def test_GitLabManager_post_comment( - mock_call_provider, filename, body, position, response, expected + mock_call_provider, filename, body, position, response, expected, pr_valid_gitlab_manager ): - manager = GitLabManager(access_token="valid") side_effect = [ read_sample_response(response), ] @@ -401,17 +396,17 @@ async def test_GitLabManager_post_comment( ) mock_call_provider.side_effect = side_effect - result = await manager.post_comment("mergerequest-id", body) + result = await pr_valid_gitlab_manager.post_comment("mergerequest-id", body) assert mock_call_provider.call_count == len(side_effect) # 1 = last call; 0 = args of call; 0 = first argument request = mock_call_provider.call_args_list[-1][0][0] if isinstance(body, NewComment): - assert request.url == f"{manager.base_api_url}mergerequest-id/discussions" + assert request.url == f"{pr_valid_gitlab_manager.base_api_url}/mergerequest-id/discussions" else: assert ( request.url - == f"{manager.base_api_url}mergerequest-id/discussions/discussion-id/notes" + == f"{pr_valid_gitlab_manager.base_api_url}/mergerequest-id/discussions/discussion-id/notes" ) assert json.loads(request.body.decode("utf-8")) == {"body": body.text, **position} @@ -422,15 +417,14 @@ async def test_GitLabManager_post_comment( @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) async def test_GitLabManager_post_comment_invalid_line_code( - mock_call_provider, monkeypatch + mock_call_provider, monkeypatch, pr_valid_gitlab_manager ): - manager = GitLabManager(access_token="valid") async def fake_file_diff(*args): # Fake that the 20 first lines are identical return [difflib.Match(a=0, b=0, size=20)] - monkeypatch.setattr(manager, "_get_file_diff", fake_file_diff) + monkeypatch.setattr(pr_valid_gitlab_manager, "_get_file_diff", fake_file_diff) body = NewComment("New discussion on plain text", "README.md", 17, None) @@ -449,13 +443,13 @@ async def fake_file_diff(*args): read_sample_response("posted_new_file_comment.json"), ] - result = await manager.post_comment("mergerequest-id", body) + result = await pr_valid_gitlab_manager.post_comment("mergerequest-id", body) assert mock_call_provider.call_count == 3 # 1 = last call; 0 = args of call; 0 = first argument request = mock_call_provider.call_args_list[-1][0][0] - assert request.url == f"{manager.base_api_url}mergerequest-id/discussions" + assert request.url == f"{pr_valid_gitlab_manager.base_api_url}/mergerequest-id/discussions" assert json.loads(request.body.decode("utf-8")) == { "body": body.text, "position": { diff --git a/jupyterlab_pullrequests/tests/test_handlers_integration.py b/jupyterlab_pullrequests/tests/test_handlers_integration.py index b3d4eee..9d70aad 100644 --- a/jupyterlab_pullrequests/tests/test_handlers_integration.py +++ b/jupyterlab_pullrequests/tests/test_handlers_integration.py @@ -20,7 +20,8 @@ def get_app(self): setup_handlers( app, PRConfig( - api_base_url=self.test_api_base_url, access_token=self.test_access_token + api_base_url=self.test_api_base_url, + access_token=self.test_access_token ), ) return app @@ -32,7 +33,7 @@ class TestListPullRequestsGithubUserHandlerEmptyPAT(TestPullRequest): # Test empty PAT def test_pat_empty(self): response = self.fetch("/pullrequests/prs/user?filter=created") - self.assertEqual(response.code, 400) + self.assertEqual(response.code, 400, f"{response.body}") self.assertIn("No access token specified", response.reason) @@ -90,7 +91,7 @@ class TestListPullRequestsGithubFilesHandlerBadID(TestPullRequest): # Test invalid id def test_id_invalid(self): response = self.fetch("/pullrequests/prs/files?id=https://google.com") - self.assertEqual(response.code, 404) + assert response.code >= 400, f"{response.body}" self.assertIn("Invalid response", response.reason) @@ -134,7 +135,7 @@ def test_id_invalid(self): response = self.fetch( f"/pullrequests/files/content?filename={valid_prfilename}&id=https://google.com" ) - self.assertEqual(response.code, 400) + assert response.code >=400, f"{response.body}" self.assertIn("Invalid response", response.reason) @@ -146,7 +147,7 @@ def test_id_missing(self): response = self.fetch( f"/pullrequests/files/comments?filename={valid_prfilename}" ) - self.assertEqual(response.code, 400) + assert response.code >=400, f"{response.body}" self.assertIn("Missing argument 'id'", response.reason) # Test no id @@ -178,7 +179,7 @@ def test_id_invalid(self): response = self.fetch( f"/pullrequests/files/comments?filename={valid_prfilename}&id=https://google.com" ) - self.assertEqual(response.code, 404) + assert response.code >=400, f"{response.body}" self.assertIn("Invalid response", response.reason) @@ -246,5 +247,5 @@ def test_id_invalid(self): method="POST", body='{"in_reply_to": 123, "text": "test"}', ) - self.assertEqual(response.code, 400) + assert response.code >=400, f"{response.body}" self.assertIn("Invalid response", response.reason) diff --git a/jupyterlab_pullrequests/tests/test_manager.py b/jupyterlab_pullrequests/tests/test_manager.py index 5fb8d5c..f5a0816 100644 --- a/jupyterlab_pullrequests/tests/test_manager.py +++ b/jupyterlab_pullrequests/tests/test_manager.py @@ -20,12 +20,11 @@ def read_sample_response(filename): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_call_provider_bad_gitresponse(mock_fetch): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_call_provider_bad_gitresponse(mock_fetch, pr_valid_github_manager): mock_fetch.side_effect = HTTPClientError(code=404) with pytest.raises(HTTPError) as e: - await manager._call_provider("invalid-link") + await pr_valid_github_manager._call_provider("invalid-link") assert e.value.status_code == 404 assert "Invalid response in" in e.value.reason @@ -34,12 +33,11 @@ async def test_GitHubManager_call_provider_bad_gitresponse(mock_fetch): @pytest.mark.asyncio @patch("json.loads", MagicMock(side_effect=json.JSONDecodeError("", "", 0))) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_call_provider_bad_parse(mock_fetch): +async def test_GitHubManager_call_provider_bad_parse(mock_fetch, pr_valid_github_manager): mock_fetch.return_value = MagicMock(headers={}) - manager = GitHubManager(access_token="valid") with (pytest.raises(HTTPError)) as e: - await manager._call_provider("invalid-link") + await pr_valid_github_manager._call_provider("invalid-link") assert e.value.status_code == HTTPStatus.BAD_REQUEST assert "Invalid response in" in e.value.reason @@ -47,12 +45,11 @@ async def test_GitHubManager_call_provider_bad_parse(mock_fetch): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_call_provider_bad_unknown(mock_fetch): - manager = GitHubManager(access_token="valid") +async def test_GitHubManager_call_provider_bad_unknown(mock_fetch, pr_valid_github_manager): mock_fetch.side_effect = Exception() with (pytest.raises(HTTPError)) as e: - await manager._call_provider("invalid-link") + await pr_valid_github_manager._call_provider("invalid-link") assert e.value.status_code == HTTPStatus.INTERNAL_SERVER_ERROR assert "Unknown error in" in e.value.reason @@ -60,10 +57,9 @@ async def test_GitHubManager_call_provider_bad_unknown(mock_fetch): @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_call_provider_valid(mock_fetch): +async def test_GitHubManager_call_provider_valid(mock_fetch, pr_valid_github_manager): mock_fetch.return_value = MagicMock(body=b'{"test1": "test2"}') - manager = GitHubManager(access_token="valid") - result = await manager._call_provider("valid-link") + result = await pr_valid_github_manager._call_provider("valid-link") assert result[0]["test1"] == "test2" @@ -78,37 +74,35 @@ async def test_GitHubManager_call_provider_valid(mock_fetch): ), ) @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_call_provider_pagination(mock_fetch, link, expected): +async def test_GitHubManager_call_provider_pagination(mock_fetch, link, expected, pr_valid_github_manager): expected_data = [{"name": "first"}, {"name": "second"}, {"name": "third"}] - manager = GitHubManager(access_token="valid") mock_fetch.side_effect = [ MagicMock(body=b'[{"name":"first"},{"name":"second"}]', headers=link), MagicMock(body=b'[{"name":"third"}]', headers={}), ] - result = await manager._call_provider("valid-link") + result = await pr_valid_github_manager._call_provider("valid-link") assert mock_fetch.await_count == expected assert mock_fetch.await_args_list[0][0][0].url.endswith("?per_page=100") if expected == 2: assert ( mock_fetch.await_args_list[1][0][0].url - == f"{manager.base_api_url}/next-url" + == f"{pr_valid_github_manager.base_api_url}/next-url" ) assert result == expected_data[: (expected + 1)] @pytest.mark.asyncio @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock) -async def test_GitHubManager_call_provider_pagination_dict(mock_fetch): +async def test_GitHubManager_call_provider_pagination_dict(mock_fetch, pr_valid_github_manager): """Check that paginated endpoint returning dictionary got aggregated in a list""" expected_data = [{"name": "first"}, {"name": "second"}] - manager = GitHubManager(access_token="valid") mock_fetch.side_effect = [ MagicMock(body=b'{"name":"first"}', headers={"Link": '; rel="next"'}), MagicMock(body=b'{"name":"second"}', headers={}), ] - result = await manager._call_provider("valid-link") + result = await pr_valid_github_manager._call_provider("valid-link") assert result == expected_data