diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index be1da76..d6db543 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -29,22 +29,6 @@ about: Report a reproducible bug in the current release of Phabfive 3. -### Schema - - -``` - -``` - - -### Data - - -``` - -``` - - ### Expected Behavior diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 7d70428..e20ae4b 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -13,8 +13,11 @@ about: Propose a new Phabfive feature or enhancement --> ### Environment -* Phabfive version: -* Python version: + +* Phabfive version: + + +* Python version: ### Use Case + * None + ### Other? -None + * None diff --git a/Makefile b/Makefile index 670de11..322d039 100644 --- a/Makefile +++ b/Makefile @@ -37,3 +37,6 @@ bdist: ## build a wheel distribution install: ## install package python setup.py install + +mkdocs-dev: + mkdocs serve diff --git a/README.md b/README.md index caccec8..8c948aa 100644 --- a/README.md +++ b/README.md @@ -1,32 +1,18 @@ # phabfive -A command line tool to interact with Phabricator. - -The complete documentation for Phabfive can be found at [Read the Docs](https://phabfive.readthedocs.io/en/latest/) - +[![Travis CI](https://travis-ci.com/dynamist/phabfive.svg?branch=master)](https://travis-ci.com/dynamist/phabfive) +[![Current release](https://img.shields.io/github/v/release/dynamist/phabfive.svg)](https://github.com/dynamist/phabfive/releases) +[![GitHub issues](https://img.shields.io/github/issues/dynamist/phabfive.svg?maxAge=360)](https://github.com/dynamist/phabfive/issues) +[![License](https://img.shields.io/github/license/dynamist/phabfive?color=%23fe0000)](https://github.com/dynamist/phabfive/blob/master/LICENSE) +[![Python2](https://img.shields.io/badge/python2-2.7-blue.svg)](https://github.com/dynamist/phabfive/) +[![Python3](https://img.shields.io/badge/python3-3.6,3.7,3.8-blue.svg)](https://github.com/dynamist/phabfive/) -## Features - -A summary of the currently supported actions, as well as planned features: +A command line tool to interact with Phabricator. -- Passphrase - - [X] Get specified secret -- Diffusion - - [X] List repositories names - - [X] Get branches for specified repository - - [X] Get clone URI:s for specified repository - - [X] Add repository - - [X] Edit URI - - [X] Observe repositories: create uri -- Paste - - [X] List pastes - - [X] Get specified paste - - [X] Add paste -- User - - [X] Who am I: information about the logged-in user +The complete documentation and detailed documentation of all implemented commands for Phabfive can be found at [Read the Docs](https://phabfive.readthedocs.io/en/latest/) -## Example usage +## Basic example Grab a Phabricator token at https:///settings/panel/apitokens/ @@ -40,9 +26,11 @@ Usage: phabfive passphrase K123 +More detailed examples can be found on the [Read the Docs](https://phabfive.readthedocs.io/en/latest/) website + ## LICENSE -Copyright (c) 2017-2019 Dynamist AB +Copyright (c) 2017-2020 Dynamist AB See the LICENSE file provided with the source distribution for full details. diff --git a/docs/modules/diffusion.md b/docs/modules/diffusion.md new file mode 100644 index 0000000..970f0b2 --- /dev/null +++ b/docs/modules/diffusion.md @@ -0,0 +1,11 @@ +# Module Diffusion + + +## Impelmented commands + +- List repositories names +- Get branches for specified repository +- Get clone URI:s for specified repository +- Add repository +- Edit URI +- Observe repositories: create uri diff --git a/docs/modules/passphrase.md b/docs/modules/passphrase.md new file mode 100644 index 0000000..e1daab7 --- /dev/null +++ b/docs/modules/passphrase.md @@ -0,0 +1,6 @@ +# Module Passphrase + + +## Impelmented commands + +- Get specified secret \ No newline at end of file diff --git a/docs/modules/paste.md b/docs/modules/paste.md new file mode 100644 index 0000000..23aa1e6 --- /dev/null +++ b/docs/modules/paste.md @@ -0,0 +1,8 @@ +# Module Paste + + +## Impelmented commands + +- List pastes +- Get specified paste +- Add paste diff --git a/docs/modules/user.md b/docs/modules/user.md new file mode 100644 index 0000000..2e0c06e --- /dev/null +++ b/docs/modules/user.md @@ -0,0 +1,6 @@ +# Module User + + +## Impelmented commands + +- Who am I: information about the logged-in user diff --git a/mkdocs.yml b/mkdocs.yml index 9c6de2f..d62ecb0 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -5,6 +5,11 @@ repo_url: https://github.com/dynamist/phabfive nav: - Introduction: 'index.md' - Development: 'development.md' + - Modules: + - Diffusion: 'modules/diffusion.md' + - Passphrase: 'modules/passphrase.md' + - Paste: 'modules/paste.md' + - User: 'modules/user.md' markdown_extensions: - admonition: diff --git a/phabfive/cli.py b/phabfive/cli.py index 45a897f..ac9af13 100644 --- a/phabfive/cli.py +++ b/phabfive/cli.py @@ -8,7 +8,7 @@ # phabfive imports from phabfive import passphrase, diffusion, paste, user -from phabfive.constants import MONOGRAMS, REPO_STATUS_CHOICES +from phabfive.constants import * from phabfive.exceptions import ( PhabfiveConfigException, PhabfiveDataException, diff --git a/phabfive/core.py b/phabfive/core.py index e0589b6..52a0871 100644 --- a/phabfive/core.py +++ b/phabfive/core.py @@ -39,24 +39,23 @@ class Phabfive(object): def __init__(self): - # Get super-early debugging by `export PHABFIVE_DEBUG=1` if "PHABFIVE_DEBUG" in os.environ: log.setLevel(logging.DEBUG) log.info( - "Loglevel is: {}".format(logging.getLevelName(log.getEffectiveLevel())) + "Loglevel is: {0}".format(logging.getLevelName(log.getEffectiveLevel())) ) self.conf = self.load_config() maxlen = 8 + len(max(dict(self.conf).keys(), key=len)) for k, v in dict(self.conf).items(): - log.debug("{} {} {}".format(k, "." * (maxlen - len(k)), v)) + log.debug("{0} {1} {2}".format(k, "." * (maxlen - len(k)), v)) # check for required configurables for k, v in dict(self.conf).items(): if k in REQUIRED and not v: - error = "{} is not configured".format(k) + error = "{0} is not configured".format(k) example = CONFIG_EXAMPLES.get(k) if example: error += ", " + example @@ -65,11 +64,12 @@ def __init__(self): # check validity of configurables for k in VALIDATORS.keys(): if not re.match(VALIDATORS[k], self.conf[k]): - error = "{} is malformed".format(k) + error = "{0} is malformed".format(k) example = VALID_EXAMPLES.get(k) if example: error += ", " + example raise PhabfiveConfigException(error) + self.phab = Phabricator( host=self.conf.get("PHAB_URL"), token=self.conf.get("PHAB_TOKEN") ) @@ -105,7 +105,7 @@ def load_config(self): os.environ["XDG_CONFIG_DIRS"] = "/etc" site_conf_file = os.path.join(appdirs.site_config_dir("phabfive") + ".yaml") - log.debug("Loading configuration file: {}".format(site_conf_file)) + log.debug("Loading configuration file: {0}".format(site_conf_file)) anyconfig.merge( conf, { @@ -120,7 +120,7 @@ def load_config(self): site_conf_dir = os.path.join( appdirs.site_config_dir("phabfive") + ".d", "*.yaml" ) - log.debug("Loading configuration files: {}".format(site_conf_dir)) + log.debug("Loading configuration files: {0}".format(site_conf_dir)) anyconfig.merge( conf, { @@ -131,7 +131,7 @@ def load_config(self): ) user_conf_file = os.path.join(appdirs.user_config_dir("phabfive")) + ".yaml" - log.debug("Loading configuration file: {}".format(user_conf_file)) + log.debug("Loading configuration file: {0}".format(user_conf_file)) anyconfig.merge( conf, { @@ -146,7 +146,7 @@ def load_config(self): user_conf_dir = os.path.join( appdirs.user_config_dir("phabfive") + ".d", "*.yaml" ) - log.debug("Loading configuration files: {}".format(user_conf_dir)) + log.debug("Loading configuration files: {0}".format(user_conf_dir)) anyconfig.merge( conf, { diff --git a/phabfive/diffusion.py b/phabfive/diffusion.py index 9ded69f..0903754 100644 --- a/phabfive/diffusion.py +++ b/phabfive/diffusion.py @@ -4,12 +4,7 @@ import re # phabfive imports -from phabfive.constants import ( - DISPLAY_CHOICES, - IO_NEW_URI_CHOICES, - MONOGRAMS, - REPO_STATUS_CHOICES, -) +from phabfive.constants import * from phabfive.core import Phabfive from phabfive.exceptions import PhabfiveDataException, PhabfiveConfigException from phabfive import passphrase diff --git a/phabfive/passphrase.py b/phabfive/passphrase.py index 3894368..ec2a47b 100644 --- a/phabfive/passphrase.py +++ b/phabfive/passphrase.py @@ -7,7 +7,7 @@ # phabfive imports from phabfive.core import Phabfive -from phabfive.constants import MONOGRAMS +from phabfive.constants import * from phabfive.exceptions import PhabfiveDataException, PhabfiveRemoteException # 3rd party imports diff --git a/phabfive/paste.py b/phabfive/paste.py index fde2679..75b2993 100644 --- a/phabfive/paste.py +++ b/phabfive/paste.py @@ -6,7 +6,7 @@ # phabfive imports from phabfive.core import Phabfive from phabfive.exceptions import PhabfiveDataException -from phabfive.constants import MONOGRAMS +from phabfive.constants import * # 3rd party imports from phabricator import APIError @@ -21,6 +21,9 @@ def _validate_identifier(self, id_): def _convert_ids(self, ids): """Method used by print function.""" + if not isinstance(ids, list): + raise PhabfiveDataException("variable ids must be of list type") + ids_list_int = [] for id_ in ids: @@ -48,12 +51,14 @@ def create_paste( :rtype: dict """ - text = None tags = tags if tags else [] subscribers = subscribers if subscribers else [] - with open(file, "r") as f: - text = f.read() + if file: + with open(file, "r") as f: + text = f.read() + else: + text = None transactions = [] transactions_values = [ diff --git a/setup.py b/setup.py index 63abaef..a3ec111 100644 --- a/setup.py +++ b/setup.py @@ -19,9 +19,9 @@ CHANGELOG = f.read() install_requires = ["anyconfig", "appdirs", "phabricator", "pyyaml", "docopt"] -tests_require = ["coverage", "flake8", "pytest", "tox"] -docs_require = ["docs"] -download_url = "{}/tarball/v{}".format( +tests_require = ["coverage", "flake8", "pytest", "tox", "mock"] +docs_require = ["mkdocs"] +download_url = "{0}/tarball/v{1}".format( "https://github.com/dynamist/phabfive", phabfive.__version__ ) @@ -41,7 +41,7 @@ extras_require={"test": tests_require, "docs": docs_require}, packages=["phabfive"], entry_points={"console_scripts": ["phabfive = phabfive.cli:cli_entrypoint"]}, - python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*", + python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*", classifiers=[ # 'Development Status :: 1 - Planning', # "Development Status :: 2 - Pre-Alpha", @@ -56,9 +56,9 @@ "Environment :: Console", "Programming Language :: Python", "Programming Language :: Python :: 2.7", - "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", "License :: OSI Approved :: Apache Software License", "Natural Language :: English", ], diff --git a/tests/test_core.py b/tests/test_core.py new file mode 100644 index 0000000..e6e84f0 --- /dev/null +++ b/tests/test_core.py @@ -0,0 +1,86 @@ +import os + +from phabricator import Phabricator, APIError +from phabfive.core import Phabfive +from phabfive.exceptions import PhabfiveConfigException, PhabfiveRemoteException + +# 3rd party imports +import mock +import pytest +from mock import patch, Mock + + +def test_class_init(): + """ + Should be possible to create the main core class w/o any errors when nothing is configred + """ + p = Phabfive() + + # This should always default to False + assert "PHABFIVE_DEBUG" in p.conf + assert p.conf["PHABFIVE_DEBUG"] == False + + +@mock.patch.dict(os.environ, {"PHABFIVE_DEBUG": "1"}) +def test_phabfive_debug_envrion(): + """ + """ + p = Phabfive() + # This should always default to False + assert "PHABFIVE_DEBUG" in p.conf + assert p.conf["PHABFIVE_DEBUG"] == "1" + + +@mock.patch.dict(os.environ, {"PHAB_URL": "", "PHAB_TOKEN": ""}) +def test_empty_url_or_token(): + """ + If the config can't figure out a valid url or token it should raise a exception in the code + """ + with pytest.raises(PhabfiveConfigException) as ex: + p = Phabfive() + + +@mock.patch.dict(os.environ, {"PHAB_URL": "foobar", "PHAB_TOKEN": "barfoo"}) +def test_validator_phab_url(): + """ + When providing some data to PHAB_URL it should raise a config validation error + """ + with pytest.raises(PhabfiveConfigException): + p = Phabfive() + + +@mock.patch.dict( + os.environ, {"PHAB_URL": "http://127.0.0.1/api", "PHAB_TOKEN": '1'}, +) +def test_validator_phab_token(): + """ + When providing some data to PHAB_URL it should raise a config validation error + """ + with pytest.raises(PhabfiveConfigException): + p = Phabfive() + + +@mock.patch.dict( + os.environ, + { + "PHAB_URL": "http://127.0.0.1/api/", + "PHAB_TOKEN": "api-bjmudcq4mjtxprkk7w3s4fkdqz6z", + }, +) +def test_whoami_api_error(): + """ + When creating the object, it tries to run user.whoami() method on Phabricator class. + + Validate that when whoami() call returns a internal APIError we should reraise it as a PhabfiveRemoteException exception. + """ + with patch.object( + Phabricator, "__getattr__", autospec=True + ) as dynamic_phabricator_getattr: + + def side_effect(self, *args, **kwargs): + raise APIError(1, "foobar") + + dynamic_phabricator_getattr.side_effect = side_effect + + with pytest.raises(PhabfiveRemoteException): + p = Phabfive() diff --git a/tests/test_diffusion.py b/tests/test_diffusion.py new file mode 100644 index 0000000..152d22d --- /dev/null +++ b/tests/test_diffusion.py @@ -0,0 +1,57 @@ +# python std lib +import os + +# phabfive imports +from phabfive.core import Phabfive +from phabfive.diffusion import Diffusion +from phabfive.exceptions import PhabfiveConfigException + +# 3rd party imports +import mock +import pytest +from mock import patch, Mock + + +@mock.patch.dict( + os.environ, + { + "PHAB_URL": "http://127.0.0.1/api/", + "PHAB_TOKEN": "api-bjmudcq4mjtxprkk7w3s4fkdqz6z", + }, +) +def test_diffusion_class(): + """ + Basic class tests for issues caused when creating the object and that it inherits from + the correct parent class that we expect + """ + d = Diffusion() + + # Validate that Diffusion class inherits from Phabfive parent class + assert Phabfive in d.__class__.__bases__ + + +@mock.patch.dict(os.environ, {"PHAB_URL": "", "PHAB_TOKEN": ""}) +def test_empty_url_or_token(): + """ + Validate that Diffusion works the same way as Phabricator parent class + """ + with pytest.raises(PhabfiveConfigException) as ex: + d = Diffusion() + + +@mock.patch.dict( + os.environ, + { + "PHAB_URL": "http://127.0.0.1/api/", + "PHAB_TOKEN": "api-bjmudcq4mjtxprkk7w3s4fkdqz6z", + }, +) +def test_validator(): + """ + We assume that passphrase identifier validator is + R[0-9]+ + """ + d = Diffusion() + + assert d._validate_identifier('R1') is not None + assert d._validate_identifier('foobar') is None diff --git a/tests/test_passphrase.py b/tests/test_passphrase.py new file mode 100644 index 0000000..aabf1d0 --- /dev/null +++ b/tests/test_passphrase.py @@ -0,0 +1,43 @@ +# python std lib +import os + +# phabfive imports +from phabfive.core import Phabfive +from phabfive.passphrase import Passphrase +from phabfive.exceptions import PhabfiveConfigException + +# 3rd party imports +import mock +import pytest +from mock import patch, Mock + + +def test_passphrase_class(): + """ + Basic class tests for issues caused when creating the object and that it inherits from + the correct parent class that we expect. + """ + p = Passphrase() + + # Validate that Passphrase class inherits from Phabfive parent class + assert Phabfive in p.__class__.__bases__ + + +@mock.patch.dict(os.environ, {"PHAB_URL": "", "PHAB_TOKEN": ""}) +def test_empty_url_or_token(): + """ + Validate that Diffusion works the same way as Phabricator parent class + """ + with pytest.raises(PhabfiveConfigException) as ex: + p = Passphrase() + + +def test_validator(): + """ + We assume that passphrase identifier validator is + K[0-9]+ + """ + p = Passphrase() + + assert p._validate_identifier('K1') is not None + assert p._validate_identifier('foobar') is None diff --git a/tests/test_paste.py b/tests/test_paste.py new file mode 100644 index 0000000..93b504f --- /dev/null +++ b/tests/test_paste.py @@ -0,0 +1,157 @@ +# python std lib +import os + +# phabfive imports +from phabricator import APIError, Phabricator +from phabfive.core import Phabfive +from phabfive.exceptions import PhabfiveConfigException, PhabfiveDataException +from phabfive.paste import Paste + +# 3rd party imports +import mock +import pytest +from mock import patch, Mock + + +class MockEditResource(): + def __init__(self, wanted_response): + self.wanted_response = wanted_response + + def edit(self, *args, **kwargs): + self.edit_args = args + self.edit_kwargs = kwargs + + return self.wanted_response + + +class MockPhabricator(): + + def __init__(self, wanted_response, *args, **kwargs): + self.wanted_response = wanted_response + self.paste = MockEditResource(self.wanted_response) + + +def test_paste_class(): + """ + Basic class tests for issues caused when creating the object and that it inherits from + the correct parent class that we expect + """ + p = Paste() + + # Validate that Paste class inherits from Phabfive parent class + assert Phabfive in p.__class__.__bases__ + + +@mock.patch.dict(os.environ, {"PHAB_URL": "", "PHAB_TOKEN": ""}) +def test_empty_url_or_token(): + """ + Validate that Diffusion works the same way as Phabricator parent class + """ + with pytest.raises(PhabfiveConfigException) as ex: + p = Paste() + + +def test_validator(): + """ + We assume that passphrase identifier validator is + P[0-9]+ + """ + p = Paste() + + assert p._validate_identifier('P1') is not None + assert p._validate_identifier('foobar') is None + + +def test_convert_ids(): + """ + """ + p = Paste() + + # No ID:s to convert so return empty list + with pytest.raises(PhabfiveDataException): + p._convert_ids(None) + + assert p._convert_ids([]) == [] + + # If we pass in a ID that will not validate against the MONOGRAMS + with pytest.raises(PhabfiveDataException): + p._convert_ids(['foobar']) + + assert p._convert_ids(['P1', 'P11']) == [1, 11] + + +def test_create_paste_api_error_on_edit(): + """ + When inputting valid arguments but the backend sends up APIError we should get a wrapped + PhabfiveDataException raised back up to us. + """ + with patch.object(Phabricator, "__call__", autospec=True) as dynamic_phabricator_call: + def side_effect(self, *args, **kwargs): + print("inside", args, kwargs) + raise APIError(1, "foobar") + + dynamic_phabricator_call.side_effect = side_effect + + with pytest.raises(PhabfiveDataException): + p = Paste() + p.create_paste( + title="title_foo", + file=None, + language="lang_foo", + subscribers="subs_foo", + ) + + +def test_create_paste_invalid_file_error(): + """ + If we input a file path that do not compute to a file on disk we should check + for the normal FileNotFoundError python exception. + """ + with pytest.raises(FileNotFoundError): + p = Paste() + p.create_paste( + title='title_foo', + file='random_foobar_file.txt', + language='lang_foo', + subscribers='subs_foo', + ) + + +def test_create_paste_transaction_values(): + """ + When inputting all valid data we need to check that transactions values is as expected and + built up propelry to all data that we need. + """ + p = Paste() + p.phab = MockPhabricator({'object': 'foobar'}) + + result = p.create_paste( + title="title_foo", + file=None, + language="lang_foo", + subscribers="subs_foo", + ) + assert result == 'foobar' + print(p.phab.paste.edit_args) + print(p.phab.paste.edit_kwargs) + + expected_transactions = { + "transactions": [ + { + "type": "title", + "value": "title_foo" + }, + { + "type": "language", + "value": "lang_foo" + }, + { + "type": "projects.add", + "value": [] + }, + { + "type": "subscribers.add", + "value": "subs_foo" + } + ] + } diff --git a/tests/test_user.py b/tests/test_user.py new file mode 100644 index 0000000..0062280 --- /dev/null +++ b/tests/test_user.py @@ -0,0 +1,36 @@ +# python std lib +import os + +# phabfive imports +from phabfive.core import Phabfive +from phabfive.exceptions import PhabfiveConfigException +from phabfive.user import User + +# 3rd party imports +import mock +import pytest +from mock import patch, Mock + + +def test_user_class(): + """ + Basic class tests for issues caused when creating the object and that it inherits from + the correct parent class that we expect + """ + u = User() + + # Validate that User class inherits from Phabfive parent class + assert Phabfive in u.__class__.__bases__ + + +@mock.patch.dict(os.environ, {"PHAB_URL": "", "PHAB_TOKEN": ""}) +def test_empty_url_or_token(): + """ + Validate that Diffusion works the same way as Phabricator parent class + """ + with pytest.raises(PhabfiveConfigException) as ex: + u = User() + + +def test_whoami(): + pass