diff --git a/.binder/environment.yml b/.binder/environment.yml new file mode 100644 index 0000000..aadd60d --- /dev/null +++ b/.binder/environment.yml @@ -0,0 +1,27 @@ +name: pull-requests + +channels: + - conda-forge + - nodefaults + +dependencies: + - git + - nodejs >=14,<15 + - jupyterlab >=3,<4 + - python >=3.7,<3.9 + # gitlab + - diff-match-patch + # test + - codecov + - mock >=4.0.0 + - pytest + - pytest-asyncio + - pytest-cov + - pytest-tornasync + # not-yet-on-conda-forge + - pip + # binder demo stuff + - nbgitpuller + - jupyterlab-tour + - pip: + - jupyterlab-git >=0.30.0b3,<0.40.0a0 diff --git a/.binder/jupyter_config.example.json b/.binder/jupyter_config.example.json new file mode 100644 index 0000000..dbf8166 --- /dev/null +++ b/.binder/jupyter_config.example.json @@ -0,0 +1,15 @@ +{ + "LabApp": { + "tornado_settings": { + "page_config_data": { + "buildCheck": false, + "buildAvailable": false + } + } + }, + "PRConfig": { + "provider": "github-anonymous", + "owner": "jupyterlab", + "repo": "jupyterlab" + } +} diff --git a/.binder/postBuild b/.binder/postBuild new file mode 100644 index 0000000..d789cb1 --- /dev/null +++ b/.binder/postBuild @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +set -eux + +jlpm --prefer-optional --ignore-offline +jlpm build:lib +jlpm build:labextension:dev + +python -m pip install -e . --ignore-installed --no-deps + +jupyter labextension develop . --overwrite +jupyter server extension enable --py jupyterlab_pullrequests --sys-prefix +jupyter serverextension enable --py jupyterlab_pullrequests --sys-prefix + +jupyter server extension list +jupyter serverextension list +jupyter labextension list + +# copy custom jupyter config out of binder +cp .binder/jupyter_config.example.json ./jupyter_config.json +cp .binder/jupyter_config.example.json ./jupyter_notebook_config.json +cp .binder/jupyter_config.example.json ./jupyter_server_config.json diff --git a/.gitignore b/.gitignore index 66cfe01..2b93884 100644 --- a/.gitignore +++ b/.gitignore @@ -117,3 +117,5 @@ dmypy.json .DS_Store coverage/ + +jupyter_config.json diff --git a/README.md b/README.md index 21603a0..a30b688 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,8 @@ [![NPM Version](https://img.shields.io/npm/v/@jupyterlab/pullrequests.svg)](https://www.npmjs.com/package/@jupyterlab/pullrequests) [![Pypi Version](https://img.shields.io/pypi/v/jupyterlab-pullrequests.svg)](https://pypi.org/project/jupyterlab-pullrequests/) [![Conda Version](https://img.shields.io/conda/vn/conda-forge/jupyterlab-pullrequests.svg)](https://anaconda.org/conda-forge/jupyterlab-pullrequests) +[![Binder](https://mybinder.org/badge_logo.svg)](https://mybinder.org/v2/gh/jupyterlab/pull-requests/HEAD?urlpath=lab) + A JupyterLab extension for reviewing pull requests. @@ -14,7 +16,7 @@ For now, it supports GitHub and GitLab providers. ## Prerequisites - JupyterLab 3.x - - for JupyterLab 2.x, see the [`2.x` branch](https://github.com/jupyterlab/pull-requests/tree/2.x) + - for JupyterLab 2.x, see the [`2.x` branch](https://github.com/jupyterlab/pull-requests/tree/2.x) - [jupyterlab-git](https://github.com/jupyterlab/jupyterlab-git) >=0.30.0 > For GitLab, you will need also `diff-match-patch` @@ -51,6 +53,35 @@ Or with conda: conda install -c conda-forge diff-match-patch ``` +### 1.5. Anonymous GitHub + +The `github-anonymous` is good for quick demos, as it doesn't require an _access token_. +Combined with some binder configuration, this can be tailored for low-barrier, read-only +access to PR discussions on e.g. Binder. + +The limitations: +- at best, rate-limited to 60 requests on e.g. Binder +- it cannot use the search APIs, and + +Because of these, it requires an `owner` and `repo` to list (the first) page of +a particular repo's PRs: + +```python +c.PRConfig.provider = 'github-anonymous' +c.PRConfig.owner = 'jupyterlab' +c.PRConfig.repo = 'pull-requests +``` + +> TODO: move to a deeper reference section +```http +GET https://api.github.com/repos/jupyterlab/pull-requests/pulls?state=all + + x-ratelimit-limit: 60 + x-ratelimit-remaining: 52 + x-ratelimit-reset: 1617905825 + x-ratelimit-used: 8 +``` + ### 2. Getting your access token For GitHub, the documentation is [there](https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token). The token scope must be **repo**. diff --git a/jupyterlab_pullrequests/__init__.py b/jupyterlab_pullrequests/__init__.py index f53bed2..eb1b978 100644 --- a/jupyterlab_pullrequests/__init__.py +++ b/jupyterlab_pullrequests/__init__.py @@ -19,6 +19,14 @@ def _load_jupyter_server_extension(server_app): from .base import PRConfig from .handlers import setup_handlers + log = server_app.log config = PRConfig(config=server_app.config) - setup_handlers(server_app.web_app, config) - server_app.log.info("Registered jupyterlab_pullrequests extension") + + setup_handlers(server_app.web_app, config, log=log) + + log.info("Registered jupyterlab_pullrequests extension") + + +# for legacy launching with notebok (e.g. Binder) +_jupyter_server_extension_paths = _jupyter_server_extension_points +load_jupyter_server_extension = _load_jupyter_server_extension diff --git a/jupyterlab_pullrequests/base.py b/jupyterlab_pullrequests/base.py index 7fbc8e6..20e29e0 100644 --- a/jupyterlab_pullrequests/base.py +++ b/jupyterlab_pullrequests/base.py @@ -1,6 +1,6 @@ from typing import List, NamedTuple, Optional -from traitlets import Enum, Unicode, default +from traitlets import Enum, Unicode, default, Bool from traitlets.config import Configurable @@ -50,6 +50,24 @@ class PRConfig(Configurable): help="Base URL of the versioning service REST API.", ) + anonymous = Bool( + config=True, + default=False, + help="Whether API request should be made without authorization headers", + ) + + repo = Unicode( + config=True, + allow_none=True, + help="An optional repo name to seed PR listing, used by github-anonymous" + ) + + owner = Unicode( + config=True, + allow_none=True, + help="An option user/organization to seed PR listing, used by github-anonymous" + ) + @default("api_base_url") def set_default_api_base_url(self): if self.provider == "gitlab": @@ -58,7 +76,7 @@ def set_default_api_base_url(self): return "https://api.github.com" provider = Enum( - ["github", "gitlab"], + ["github", "github-anonymous", "gitlab"], default_value="github", config=True, help="The source control provider.", 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/__init__.py b/jupyterlab_pullrequests/managers/__init__.py index 536d5f2..87b5c68 100644 --- a/jupyterlab_pullrequests/managers/__init__.py +++ b/jupyterlab_pullrequests/managers/__init__.py @@ -1,5 +1,10 @@ +from .github_anonymous import AnonymousGithubManager from .github import GitHubManager from .gitlab import GitLabManager # Supported third-party services -MANAGERS = {"github": GitHubManager, "gitlab": GitLabManager} +MANAGERS = { + "github-anonymous": AnonymousGithubManager, + "github": GitHubManager, + "gitlab": GitLabManager, +} diff --git a/jupyterlab_pullrequests/managers/github.py b/jupyterlab_pullrequests/managers/github.py index 48cedad..a5327ee 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) @@ -204,7 +201,7 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]: ) # Reset cache - self._pull_requests_cache = {} + self._pull_requests_cache = None return data @@ -251,6 +248,7 @@ async def _call_github( params: Optional[Dict[str, str]] = None, media_type: str = "application/vnd.github.v3+json", has_pagination: bool = True, + **headers ) -> Union[dict, str]: """Call GitHub @@ -271,10 +269,11 @@ async def _call_github( List or Dict: Create from JSON response body if load_json is True str: Raw response body if load_json is False """ - headers = { - "Accept": media_type, - "Authorization": f"token {self._access_token}", - } + headers = headers or {} + headers.update({"Accept": media_type}) + + if not self.anonymous: + headers.update({"Authorization": f"token {self._config.access_token}"}) return await super()._call_provider( url, diff --git a/jupyterlab_pullrequests/managers/github_anonymous.py b/jupyterlab_pullrequests/managers/github_anonymous.py new file mode 100644 index 0000000..d14a7f6 --- /dev/null +++ b/jupyterlab_pullrequests/managers/github_anonymous.py @@ -0,0 +1,67 @@ +from .github import GitHubManager + +from typing import List, Dict + +from notebook.utils import url_path_join + + +class AnonymousGithubManager(GitHubManager): + """work-in-progress best-effort anonymous github manager + + Without guidance, this manager will rapidly exhaust the free API limit, + so may best be configured for per-PR views + """ + + @property + def anonymous(self) -> bool: + """Whether the provider should use Authorization headers""" + return True + + + async def get_current_user(self) -> Dict[str, str]: + """Get the current user information. + + Returns: + JSON description of the user matching the access token + """ + return {"username": "anonymous"} + + async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]: + """Returns the list of pull requests for the given user. + + TODO: figure out more sane handling + state: all|(open? closed? green?) + + Args: + username: User ID for the versioning service + pr_filter: Filter to add to the pull requests requests + Returns: + The list of pull requests + """ + # use the Repos API to fetch some PRs + git_url = url_path_join( + self.base_api_url, + "repos", + self._config.owner, + self._config.repo, + "pulls" + ) + + results = await self._call_github( + git_url, + params=dict(state="all", per_page=100), + has_pagination=False + ) + + self._pull_requests_cache = {result["id"]: result for result in results} + + return [ + { + "id": result["url"], + "title": result["title"], + "body": result["body"], + "internalId": result["id"], + "link": result["html_url"], + } + for result in results + ] diff --git a/jupyterlab_pullrequests/managers/gitlab.py b/jupyterlab_pullrequests/managers/gitlab.py index ff6373a..85c430f 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", @@ -252,7 +250,7 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]: ) # Reset cache - self._merge_requests_cache = {} + self._merge_requests_cache = None return data @@ -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..7b1501c 100644 --- a/jupyterlab_pullrequests/managers/manager.py +++ b/jupyterlab_pullrequests/managers/manager.py @@ -10,25 +10,24 @@ 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 anonymous(self) -> str: + """Whether the provider should use Authorization headers""" + return self._config.anonymous @property def log(self) -> logging.Logger: @@ -142,10 +141,15 @@ 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.anonymous and 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.", + reason=( + """No access token specified. """ + """Please set PRConfig.access_token (or an anonymous provider) """ + """in your user jupyter_server_config file.""" + ) ) if body is not None: @@ -154,8 +158,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 ( @@ -203,7 +207,7 @@ async def _call_provider( break new_ = json.loads(result) - if next_url is not None: + if has_pagination and next_url is not None: next_ = await self._call_provider( next_url, load_json=load_json, 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 diff --git a/yarn.lock b/yarn.lock index 36421f3..13bf843 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6348,7 +6348,7 @@ natural-compare@^1.4.0: resolved "https://registry.yarnpkg.com/natural-compare/-/natural-compare-1.4.0.tgz#4abebfeed7541f2c27acfb29bdbbd15c8d5ba4f7" integrity sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc= -nbdime-jupyterlab@2.1.0-beta.1: +nbdime-jupyterlab@^2.1.0-beta.1: version "2.1.0-beta.1" resolved "https://registry.yarnpkg.com/nbdime-jupyterlab/-/nbdime-jupyterlab-2.1.0-beta.1.tgz#9b650bd044e94bec24e8ac80b1f25616fd669851" integrity sha512-4QTXnQbSJMbFVnNM35lcrDFLmYF8UCVLa2kUFcEKzsp5gHgQ+zRtxOq0aS4hLI26p394wzAuME24k8bnk2PvaA==