Skip to content

Commit

Permalink
Fix prospector tests
Browse files Browse the repository at this point in the history
* As of Python3 each class inherit from object, removing it's explicit
  inheritance. Reported by newer pylint.
* Convert if/elif/else blocks with raise or returns with if/if/raise
  or return ones. Reported by newer pylint.
* Remove explicit dependency on older vulture, as the bug in prospector
  was fixed.

Change-Id: I827657adc51148da1587578dfa15cff55ecbd351
  • Loading branch information
volans- committed Jul 31, 2018
1 parent d1760d6 commit ece8461
Show file tree
Hide file tree
Showing 15 changed files with 44 additions and 41 deletions.
2 changes: 1 addition & 1 deletion cumin/backends/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class InvalidQueryError(CuminError):
"""Custom exception class for invalid queries."""


class BaseQuery(object, metaclass=ABCMeta):
class BaseQuery(metaclass=ABCMeta):
"""Query abstract class.
All backends query classes must inherit, directly or indirectly, from this one.
Expand Down
10 changes: 6 additions & 4 deletions cumin/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
# - nodes: is a ClusterShell.NodeSet.NodeSet instance
# - output: is a ClusterShell.MsgTree.MsgTreeElem instance
# h(): print this help message.
# help(object): Python default interactive help and documentation of the given object.
# help(object_name): Python default interactive help and documentation of the given object.
= Example usage:
for nodes, output in worker.get_results():
Expand Down Expand Up @@ -312,10 +312,12 @@ def get_hosts(args, config):
if args.dry_run:
stderr('DRY-RUN mode enabled, aborting')
return []
elif args.force:

if args.force:
stderr('FORCE mode enabled, continuing without confirmation')
return hosts
elif not sys.stdout.isatty(): # pylint: disable=no-member

if not sys.stdout.isatty(): # pylint: disable=no-member
message = 'Not in a TTY but neither DRY-RUN nor FORCE mode were specified.'
stderr(message)
raise cumin.CuminError(message)
Expand Down Expand Up @@ -393,7 +395,7 @@ def run(args, config):
if args.interactive:
# Define a help function h() that will be available in the interactive shell to print the help message.
# The name is to not shadow the Python built-in help() that might be usefult too to inspect objects.
def h(): # pylint: disable=unused-variable,invalid-name
def h(): # pylint: disable=possibly-unused-variable,invalid-name
"""Print the help message in interactive shell."""
tqdm.write(INTERACTIVE_BANNER)
code.interact(banner=INTERACTIVE_BANNER, local=locals())
Expand Down
6 changes: 3 additions & 3 deletions cumin/grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ def _import_backend(module, available_backends):
try:
backend = importlib.import_module(module)
except ImportError as e:
if module.startswith(INTERNAL_BACKEND_PREFIX):
return (None, None) # Internal backend not available, are all the dependencies installed?
else:
if not module.startswith(INTERNAL_BACKEND_PREFIX):
raise CuminError("Unable to import backend '{module}': {e}".format(module=module, e=e))

return (None, None) # Internal backend not available, are all the dependencies installed?

name = module.split('.')[-1]
message = "Unable to register backend '{name}' in module '{module}'".format(name=name, module=module)
try:
Expand Down
2 changes: 1 addition & 1 deletion cumin/tests/integration/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def get_ls_expected_lines(params):


@add_variants_methods(range(len(_VARIANTS_PARAMETERS)))
class TestCLI(object):
class TestCLI():
"""CLI module tests."""

def setup_method(self, _):
Expand Down
2 changes: 1 addition & 1 deletion cumin/tests/unit/backends/external/wrong_inheritance.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Test external backend module with wrong inheritance of the query class."""


class WrongInheritance(object):
class WrongInheritance():
"""Test query class with wrong inheritance."""


Expand Down
2 changes: 1 addition & 1 deletion cumin/tests/unit/backends/test_direct.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def test_direct_query_class():
assert isinstance(query, BaseQuery)


class TestDirectQuery(object):
class TestDirectQuery():
"""Direct backend query test class."""

def setup_method(self, _):
Expand Down
2 changes: 1 addition & 1 deletion cumin/tests/unit/backends/test_knownhosts.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def test_knownhosts_query_class():
assert isinstance(query, BaseQuery)


class TestKnownhostsQuery(object):
class TestKnownhostsQuery():
"""Knownhosts backend query test class."""

def setup_method(self, _):
Expand Down
2 changes: 1 addition & 1 deletion cumin/tests/unit/backends/test_openstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_key_value_tokens():
@mock.patch('cumin.backends.openstack.keystone_client.Client')
@mock.patch('cumin.backends.openstack.keystone_session.Session')
@mock.patch('cumin.backends.openstack.keystone_identity.Password')
class TestOpenStackQuery(object):
class TestOpenStackQuery():
"""OpenStack backend query test class."""

def setup_method(self, _):
Expand Down
8 changes: 4 additions & 4 deletions cumin/tests/unit/backends/test_puppetdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_hosts_selection():
assert parsed[0].asDict() == hosts


class TestPuppetDBQueryV3(object):
class TestPuppetDBQueryV3():
"""PuppetDB backend query test class for API version 3."""

def setup_method(self, _):
Expand All @@ -51,7 +51,7 @@ def test_instantiation(self):
assert self.query.url == 'https://localhost:443/v3/'


class TestPuppetDBQueryV4(object):
class TestPuppetDBQueryV4():
"""PuppetDB backend query test class for API version 4."""

def setup_method(self, _):
Expand Down Expand Up @@ -102,7 +102,7 @@ def test_puppetdb_query_init_invalid():


@mock.patch.object(puppetdb.PuppetDBQuery, '_api_call')
class TestPuppetDBQueryBuildV3(object):
class TestPuppetDBQueryBuildV3():
"""PuppetDB backend API v3 query build test class."""

def setup_method(self, _):
Expand Down Expand Up @@ -155,7 +155,7 @@ def test_and_and(self, mocked_api_call):


@mock.patch.object(puppetdb.PuppetDBQuery, '_api_call')
class TestPuppetDBQueryBuildV4(object):
class TestPuppetDBQueryBuildV4():
"""PuppetDB backend API v4 query build test class."""

def setup_method(self, _):
Expand Down
6 changes: 3 additions & 3 deletions cumin/tests/unit/transports/test_clustershell.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def test_worker_class(task_self):
task_self.assert_called_once_with()


class TestClusterShellWorker(object):
class TestClusterShellWorker():
"""ClusterShell backend worker test class."""

@mock.patch('cumin.transports.clustershell.Task.task_self')
Expand Down Expand Up @@ -144,7 +144,7 @@ def test_handler_getter(self):

def test_handler_setter_invalid(self):
"""Raise WorkerError if trying to set it to an invalid class or value"""
class InvalidClass(object):
class InvalidClass():
"""Invalid class."""

pass
Expand Down Expand Up @@ -177,7 +177,7 @@ def iter_buffers():
yield 'output {}'.format(i), ['node{}0'.format(i), 'node{}1'.format(i), 'node{}2'.format(i)]


class TestBaseEventHandler(object):
class TestBaseEventHandler():
"""BaseEventHandler test class."""

def setup_method(self, *args): # pylint: disable=arguments-differ
Expand Down
16 changes: 8 additions & 8 deletions cumin/tests/unit/transports/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def handler(self, value):
self._handler = value


class Commands(object):
class Commands():
"""Helper class to define a list of commands to test."""

command_with_options = r'command --with "options" -a -n -d params with\ spaces'
Expand Down Expand Up @@ -63,7 +63,7 @@ def __init__(self):


@pytest.mark.parametrize('command', Commands().commands)
class TestCommandParametrized(object):
class TestCommandParametrized():
"""Command class tests executed for each parametrized command."""

def test_instantiation(self, command):
Expand Down Expand Up @@ -134,7 +134,7 @@ def test_ok_codes_getter(self, command):
assert command['obj'].ok_codes == command.get('ok_codes', [0])


class TestCommand(object):
class TestCommand():
"""Command class non parametrized tests."""

def test_eq_equivalent(self):
Expand Down Expand Up @@ -197,7 +197,7 @@ def test_ok_codes_setter(self):
command.ok_codes = codes


class TestState(object):
class TestState():
"""State class tests."""

def test_instantiation_no_init(self):
Expand Down Expand Up @@ -308,7 +308,7 @@ def test_update_ok(self):
assert state.current == transports.State.pending


class TestTarget(object):
class TestTarget():
"""Target class tests."""

def setup_method(self, _):
Expand Down Expand Up @@ -403,7 +403,7 @@ def test_first_batch(self):
assert isinstance(target.first_batch, NodeSet)


class TestBaseWorker(object):
class TestBaseWorker():
"""Concrete BaseWorker class for tests."""

def test_instantiation(self):
Expand All @@ -427,7 +427,7 @@ def test_init(self):
assert worker.config == config


class TestConcreteBaseWorker(object):
class TestConcreteBaseWorker():
"""BaseWorker test class."""

def setup_method(self, _):
Expand Down Expand Up @@ -501,7 +501,7 @@ def test_success_threshold_setter(self):
assert self.worker._success_threshold == pytest.approx(0.3)


class TestModuleFunctions(object):
class TestModuleFunctions():
"""Transports module functions test class."""

def test_validate_list(self):
Expand Down
2 changes: 1 addition & 1 deletion cumin/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from cumin import CuminError


class Transport(object):
class Transport():
"""Transport factory class.
The transport layer is the one used to convey the commands to be executed into the selected hosts. The transport
Expand Down
22 changes: 12 additions & 10 deletions cumin/transports/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class InvalidStateError(CuminError):
"""Exception raised when an invalid transition for a node's State was attempted."""


class Command(object):
class Command():
"""Class to represent a command."""

def __init__(self, command, timeout=None, ok_codes=None):
Expand Down Expand Up @@ -179,7 +179,7 @@ def ok_codes(self, value):
self._ok_codes = value


class State(object):
class State():
"""State machine for the state of a host.
.. attribute:: current
Expand Down Expand Up @@ -251,10 +251,11 @@ def __getattr__(self, name):
"""
if name == 'current':
return self._state
elif name.startswith('is_') and name[3:] in self.states_representation:

if name.startswith('is_') and name[3:] in self.states_representation:
return getattr(self, name[3:]) == self._state
else:
raise AttributeError("'State' object has no attribute '{name}'".format(name=name))

raise AttributeError("'State' object has no attribute '{name}'".format(name=name))

def __repr__(self):
"""Return the representation of the :py:class:`State`.
Expand Down Expand Up @@ -387,13 +388,14 @@ def _cmp(self, other):
"""
if isinstance(other, int):
return self._state - other
elif isinstance(other, State):

if isinstance(other, State):
return self._state - other._state # pylint: disable=protected-access
else:
raise ValueError("Unable to compare instance of '{other}' with State instance".format(other=type(other)))

raise ValueError("Unable to compare instance of '{other}' with State instance".format(other=type(other)))


class Target(object):
class Target():
"""Targets management class."""

def __init__(self, hosts, batch_size=None, batch_size_ratio=None, batch_sleep=None):
Expand Down Expand Up @@ -495,7 +497,7 @@ def _compute_batch_sleep(batch_sleep):
return batch_sleep or 0.0


class BaseWorker(object, metaclass=ABCMeta):
class BaseWorker(metaclass=ABCMeta):
"""Worker interface to be extended by concrete workers."""

def __init__(self, config, target):
Expand Down
2 changes: 1 addition & 1 deletion cumin/transports/clustershell.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def handler(self, value):
value)


class Node(object):
class Node():
"""Node class to represent each target node."""

def __init__(self, name, commands):
Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
'sphinx_rtd_theme>=0.1.6',
'sphinx-argparse>=0.1.15',
'Sphinx>=1.4.9',
'vulture>=0.6,<0.25', # Required for https://github.com/landscapeio/prospector/issues/230
],
}

Expand Down

0 comments on commit ece8461

Please sign in to comment.