Skip to content

Commit

Permalink
fix(cli): exit on config parse error, instead of crashing
Browse files Browse the repository at this point in the history
* Exit and hint user about possible errors
* test: adjust test cases to config missing error
  • Loading branch information
max-wittig authored and gpocentek committed Nov 4, 2018
1 parent 742243f commit 6ad9da0
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 6 deletions.
11 changes: 8 additions & 3 deletions gitlab/cli.py
Expand Up @@ -17,6 +17,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from __future__ import print_function

import argparse
import functools
import importlib
Expand Down Expand Up @@ -143,9 +144,13 @@ def main():
# load the propermodule (v3 or v4) accordingly. At that point we don't have
# any subparser setup
(options, args) = parser.parse_known_args(sys.argv)

config = gitlab.config.GitlabConfigParser(options.gitlab,
options.config_file)
try:
config = gitlab.config.GitlabConfigParser(
options.gitlab,
options.config_file
)
except gitlab.config.ConfigError as e:
sys.exit(e)
cli_module = importlib.import_module('gitlab.v%s.cli' % config.api_version)

# Now we build the entire set of subcommands and do the complete parsing
Expand Down
17 changes: 17 additions & 0 deletions gitlab/config.py
Expand Up @@ -37,10 +37,27 @@ class GitlabDataError(ConfigError):
pass


class GitlabConfigMissingError(ConfigError):
pass


class GitlabConfigParser(object):
def __init__(self, gitlab_id=None, config_files=None):
self.gitlab_id = gitlab_id
_files = config_files or _DEFAULT_FILES
file_exist = False
for file in _files:
if os.path.exists(file):
file_exist = True
if not file_exist:
raise GitlabConfigMissingError(
"Config file not found. \nPlease create one in "
"one of the following locations: {} \nor "
"specify a config file using the '-c' parameter.".format(
", ".join(_DEFAULT_FILES)
)
)

self._config = configparser.ConfigParser()
self._config.read(_files)

Expand Down
20 changes: 17 additions & 3 deletions gitlab/tests/test_config.py
Expand Up @@ -76,11 +76,20 @@


class TestConfigParser(unittest.TestCase):
@mock.patch('os.path.exists')
def test_missing_config(self, path_exists):
path_exists.return_value = False
with self.assertRaises(config.GitlabConfigMissingError):
config.GitlabConfigParser('test')

@mock.patch('os.path.exists')
@mock.patch('six.moves.builtins.open')
def test_invalid_id(self, m_open):
def test_invalid_id(self, m_open, path_exists):
fd = six.StringIO(no_default_config)
fd.close = mock.Mock(return_value=None)
m_open.return_value = fd
path_exists.return_value = True
config.GitlabConfigParser('there')
self.assertRaises(config.GitlabIDError, config.GitlabConfigParser)

fd = six.StringIO(valid_config)
Expand All @@ -90,12 +99,15 @@ def test_invalid_id(self, m_open):
config.GitlabConfigParser,
gitlab_id='not_there')

@mock.patch('os.path.exists')
@mock.patch('six.moves.builtins.open')
def test_invalid_data(self, m_open):
def test_invalid_data(self, m_open, path_exists):
fd = six.StringIO(missing_attr_config)
fd.close = mock.Mock(return_value=None,
side_effect=lambda: fd.seek(0))
m_open.return_value = fd
path_exists.return_value = True

config.GitlabConfigParser('one')
config.GitlabConfigParser('one')
self.assertRaises(config.GitlabDataError, config.GitlabConfigParser,
Expand All @@ -107,11 +119,13 @@ def test_invalid_data(self, m_open):
self.assertEqual('Unsupported per_page number: 200',
emgr.exception.args[0])

@mock.patch('os.path.exists')
@mock.patch('six.moves.builtins.open')
def test_valid_data(self, m_open):
def test_valid_data(self, m_open, path_exists):
fd = six.StringIO(valid_config)
fd.close = mock.Mock(return_value=None)
m_open.return_value = fd
path_exists.return_value = True

cp = config.GitlabConfigParser()
self.assertEqual("one", cp.gitlab_id)
Expand Down

0 comments on commit 6ad9da0

Please sign in to comment.