rpms
/
leapp
7
0
Fork 0
Code Issues Pull Requests Releases Activity
leapp/SOURCES/0005-Necessary-fixes-to-mak...

381 lines
17 KiB
Diff
Raw Blame History

From 0cde014f5cc42f2dfd2e3f6a8b5bb3ab6311bb42 Mon Sep 17 00:00:00 2001
From: Inessa Vasilevskaya <ivasilev@redhat.com>
Date: Fri, 25 Mar 2022 16:08:17 +0100
Subject: [PATCH 5/9] Necessary fixes to make linter happy again
Because unit tests in upstream were not enabled for some
time the following technical debt accumulated.
This patch takes care of make lint command only, not the
unit tests.
It also enables linter checks in container tests.
Related: OAMG-6723
---
.pylintrc | 11 ++++++++++-
Makefile | 15 +++++++++++++--
leapp/actors/__init__.py | 2 +-
leapp/dialogs/dialog.py | 2 +-
leapp/dialogs/renderer.py | 2 +-
leapp/libraries/stdlib/eventloop.py | 2 +-
leapp/models/__init__.py | 4 +++-
leapp/reporting/__init__.py | 2 +-
leapp/repository/__init__.py | 2 +-
leapp/snactor/commands/new_actor.py | 2 +-
leapp/snactor/fixture.py | 2 +-
leapp/snactor/utils.py | 2 ++
leapp/utils/__init__.py | 4 +++-
leapp/utils/actorapi.py | 2 +-
leapp/utils/audit/contextclone.py | 2 +-
leapp/utils/report.py | 2 +-
res/container-tests/Containerfile.ubi7 | 3 +--
res/container-tests/Containerfile.ubi8 | 3 +--
res/container-tests/Containerfile.ubi9 | 3 +--
tests/scripts/test_reporting.py | 24 ++++++++++++------------
tests/scripts/test_rerun.py | 2 +-
21 files changed, 58 insertions(+), 35 deletions(-)
diff --git a/.pylintrc b/.pylintrc
index d64c53e..f67aa74 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -31,18 +31,27 @@ disable=
too-many-locals,
too-many-public-methods,
too-many-statements,
+ print-statement,
# new for python3 version of pylint
useless-object-inheritance,
super-with-arguments, # required in python 2
raise-missing-from, # no 'raise from' in python 2
+ consider-using-f-string, # nope, we still have py2 to support
+ redundant-u-string-prefix, # nope, we still have py2 to support
+ exception-message-attribute, # nope, still have py2 to support
+ no-absolute-import, # XXX FIXME nice to have one day
+ unspecified-encoding, # XXX FIXME May be a good thing to have one day though
[FORMAT]
# Maximum number of characters on a single line.
max-line-length=120
[DESIGN]
-max-args=11 # 2x + 1 from default
+max-args=12 # 2x + 2 from default
max-attributes=21 # 4x + 1 from default
[TYPECHECK]
disable=bad-option-value # unavoidable as the project requires both py2 and py3
+
+[IMPORTS]
+known-third-party=leapp
diff --git a/Makefile b/Makefile
index 611d255..7a8f030 100644
--- a/Makefile
+++ b/Makefile
@@ -139,8 +139,19 @@ test: lint
lint:
@ $(ENTER_VENV) \
- pytest --cache-clear --pylint -m pylint leapp tests/scripts/*.py; \
- pytest --cache-clear --flake8 -m flake8 leapp tests/scripts/*.py
+ LINTABLES="$$(find . -name '*.py' | grep -E -e '^\./leapp\/' -e '^\./tests/scripts/' | sort -u )"; \
+ echo '==================================================' && \
+ echo '==================================================' && \
+ echo '=============== Running pylint ===============' && \
+ echo '==================================================' && \
+ echo '==================================================' && \
+ [[ "$(_PYTHON_VENV)" == "python2.7" ]] && echo "$$LINTABLES" | xargs pylint --py3k || echo "$$LINTABLES" | xargs pylint && echo '===> pylint PASSED' && \
+ echo '==================================================' && \
+ echo '==================================================' && \
+ echo '=============== Running flake8 ===============' && \
+ echo '==================================================' && \
+ echo '==================================================' && \
+ echo "$$LINTABLES" | xargs flake8 && echo '===> flake8 PASSED';
fast_lint:
@ $(ENTER_VENV) \
diff --git a/leapp/actors/__init__.py b/leapp/actors/__init__.py
index c472b84..7ae18ea 100644
--- a/leapp/actors/__init__.py
+++ b/leapp/actors/__init__.py
@@ -368,7 +368,7 @@ def _is_tuple_of(value_type):
if not value:
raise WrongAttributeTypeError(
'Actor {} attribute {} should contain at least one item of the type {}'.format(actor, name, value_type))
- if not all([isinstance(item, value_type) for item in value]):
+ if not all(isinstance(item, value_type) for item in value):
raise WrongAttributeTypeError(
'Actor {} attribute {} should contain only values of the type {}'.format(actor, name, value_type))
return value
diff --git a/leapp/dialogs/dialog.py b/leapp/dialogs/dialog.py
index d382ce9..a1d65c7 100644
--- a/leapp/dialogs/dialog.py
+++ b/leapp/dialogs/dialog.py
@@ -110,7 +110,7 @@ class Dialog(object):
:return: Dictionary with answers once retrieved
"""
store.translate(self)
- if any([component.value is None for component in self.components]):
+ if any(component.value is None for component in self.components):
self._store = store
renderer.render(self)
self._store = None
diff --git a/leapp/dialogs/renderer.py b/leapp/dialogs/renderer.py
index 991be7c..0dd2793 100644
--- a/leapp/dialogs/renderer.py
+++ b/leapp/dialogs/renderer.py
@@ -253,7 +253,7 @@ class CommandlineRenderer(DialogRendererBase):
default = ''
if component.default is not None:
if component.multi:
- default = ' [{}]'.format(tuple([component.choices.index(x) for x in component.default]))
+ default = ' [{}]'.format(tuple(component.choices.index(x) for x in component.default))
else:
default = ' [{}]'.format(component.choices.index(component.default))
if component.multi:
diff --git a/leapp/libraries/stdlib/eventloop.py b/leapp/libraries/stdlib/eventloop.py
index 44bb199..bad3348 100644
--- a/leapp/libraries/stdlib/eventloop.py
+++ b/leapp/libraries/stdlib/eventloop.py
@@ -47,7 +47,7 @@ class EventLoopKQUEUE(object):
results[fd] |= POLL_IN
elif e.filter == select.KQ_FILTER_WRITE:
results[fd] |= POLL_OUT
- return results.items()
+ return list(results.items())
def register(self, fd, mode):
self._fds[fd] = mode
diff --git a/leapp/models/__init__.py b/leapp/models/__init__.py
index 499f3b2..ff5db4b 100644
--- a/leapp/models/__init__.py
+++ b/leapp/models/__init__.py
@@ -77,6 +77,8 @@ class Model(with_metaclass(ModelMeta)):
Models are defining the data structure of the payload of messages and the
metadata required, such as a name and topic.
"""
+ # NOTE(ivasilev) Until someone comes up with a good hash implementation for Model, let's disable the warning
+ # pylint: disable=eq-without-hash
def __init__(self, init_method='from_initialization', **kwargs):
super(Model, self).__init__()
defined_fields = type(self).fields or {}
@@ -84,7 +86,7 @@ class Model(with_metaclass(ModelMeta)):
if key not in defined_fields:
raise ModelMisuseError(
'Trying to initialize model {} with value for undefined field {}'.format(type(self).__name__, key))
- for field in defined_fields:
+ for field in defined_fields: # noqa; pylint: disable=consider-using-dict-items
getattr(defined_fields[field], init_method)(kwargs, field, self)
topic = None
diff --git a/leapp/reporting/__init__.py b/leapp/reporting/__init__.py
index 61fac32..4c9df38 100644
--- a/leapp/reporting/__init__.py
+++ b/leapp/reporting/__init__.py
@@ -179,7 +179,7 @@ class Tags(BasePrimitive):
def __init__(self, value=None):
if not isinstance(value, list):
raise TypeError('Value of "Tags" must be a list')
- if not all([isinstance(v, Tags._Value) for v in value]):
+ if not all(isinstance(v, Tags._Value) for v in value):
raise TypeError('Unsupported tag value passed for Report Tags.')
# after the objects validation we need the actual values in the list
self._value = [v.value for v in value]
diff --git a/leapp/repository/__init__.py b/leapp/repository/__init__.py
index a43d691..b47af3e 100644
--- a/leapp/repository/__init__.py
+++ b/leapp/repository/__init__.py
@@ -209,7 +209,7 @@ class Repository(object):
"""
:return: Tuple of repository relative paths
"""
- return tuple([os.path.relpath(x, self._repo_dir) for x in paths])
+ return tuple(os.path.relpath(x, self._repo_dir) for x in paths)
@property
def actors(self):
diff --git a/leapp/snactor/commands/new_actor.py b/leapp/snactor/commands/new_actor.py
index 5151904..bcc5bf1 100644
--- a/leapp/snactor/commands/new_actor.py
+++ b/leapp/snactor/commands/new_actor.py
@@ -48,7 +48,7 @@ def cli(args):
tag_imports = ''
model_imports = ''
if args.tag:
- tag_imports = '\nfrom leapp.tags import {}'.format(', '.join(tuple([x.split('.')[0] for x in args.tag])))
+ tag_imports = '\nfrom leapp.tags import {}'.format(', '.join(tuple(x.split('.')[0] for x in args.tag)))
if args.consumes or args.produces:
models = set((args.produces or []) + (args.consumes or []))
model_imports = '\nfrom leapp.models import {}'.format(', '.join(models))
diff --git a/leapp/snactor/fixture.py b/leapp/snactor/fixture.py
index a6ad613..1623a27 100644
--- a/leapp/snactor/fixture.py
+++ b/leapp/snactor/fixture.py
@@ -323,7 +323,7 @@ if hasattr(pytest, 'hookimpl'):
:py:func:`current_actor_context` fixture. If it doesn't use the :py:func:`current_actor_context` fixture, it
will default to the default `pytest_pyfunc_call` implementation.
"""
- if not any([arg in pyfuncitem.funcargs for arg in ('current_actor_context', 'leapp_forked')]):
+ if not any(arg in pyfuncitem.funcargs for arg in ('current_actor_context', 'leapp_forked')):
return None
q = Queue()
p = Process(target=_execute_test, args=(q, pyfuncitem))
diff --git a/leapp/snactor/utils.py b/leapp/snactor/utils.py
index 28fe112..7e13bd5 100644
--- a/leapp/snactor/utils.py
+++ b/leapp/snactor/utils.py
@@ -104,6 +104,8 @@ def safe_discover(pivot):
# class SecondOrderModel(FirstOrderModel):
# pass
#
+ # NOTE(ivasilev) L126, disabling here not to get too long a line that will need another disable check comment
+ # pylint: disable=filter-builtin-not-iterating,map-builtin-not-iterating
collected_types = {
'models': set(['Model']),
'actors': set(['Actor']),
diff --git a/leapp/utils/__init__.py b/leapp/utils/__init__.py
index 174ec8a..68a5f38 100644
--- a/leapp/utils/__init__.py
+++ b/leapp/utils/__init__.py
@@ -2,7 +2,9 @@ import subprocess
def reboot_system():
- subprocess.Popen(['/sbin/shutdown', '-r', 'now'])
+ # NOTE(ivasilev) Maybe we could use run? We still can't rely on any code to be finished after workflow calls this
+ # method.
+ subprocess.Popen(['/sbin/shutdown', '-r', 'now']) # noqa; pylint: disable=consider-using-with
def get_api_models(actor, what):
diff --git a/leapp/utils/actorapi.py b/leapp/utils/actorapi.py
index 8e4fa6d..2ce1b0e 100644
--- a/leapp/utils/actorapi.py
+++ b/leapp/utils/actorapi.py
@@ -6,7 +6,7 @@ import requests.adapters
import requests.exceptions
try:
- import requests.packages.urllib3 as urllib3
+ import requests.packages.urllib3 as urllib3 # noqa # pylint: disable=consider-using-from-import
except ImportError:
import urllib3
diff --git a/leapp/utils/audit/contextclone.py b/leapp/utils/audit/contextclone.py
index 8719b55..2e28b70 100644
--- a/leapp/utils/audit/contextclone.py
+++ b/leapp/utils/audit/contextclone.py
@@ -15,7 +15,7 @@ def _fetch_table_for_context(db, table, context):
def _row_tuple(row, *fields):
- return tuple([row[name] for name in fields or row.keys()])
+ return tuple(row[name] for name in fields or row.keys())
def _dup_host(db, newcontext, oldcontext):
diff --git a/leapp/utils/report.py b/leapp/utils/report.py
index 5a8f68c..c3ec7d4 100644
--- a/leapp/utils/report.py
+++ b/leapp/utils/report.py
@@ -91,7 +91,7 @@ def importance(message):
def generate_report_file(messages_to_report, context, path, report_schema='1.1.0'):
# NOTE(ivasilev) Int conversion should not break as only specific formats of report_schema versions are allowed
- report_schema_tuple = tuple([int(x) for x in report_schema.split('.')])
+ report_schema_tuple = tuple(int(x) for x in report_schema.split('.'))
if path.endswith(".txt"):
with open(path, 'w') as f:
for message in sorted(messages_to_report, key=importance):
diff --git a/res/container-tests/Containerfile.ubi7 b/res/container-tests/Containerfile.ubi7
index bc434b9..9e36280 100644
--- a/res/container-tests/Containerfile.ubi7
+++ b/res/container-tests/Containerfile.ubi7
@@ -19,8 +19,7 @@ ENTRYPOINT virtualenv testenv && \
echo '=============== Running pylint ===============' && \
echo '==================================================' && \
echo '==================================================' && \
- # echo $LINTABLES | xargs pylint --py3k && echo '===> pylint PASSED' && \
- echo 'TEMPORARILY DISABLED' && \
+ echo $LINTABLES | xargs pylint --py3k && echo '===> pylint PASSED' && \
echo '==================================================' && \
echo '==================================================' && \
echo '=============== Running flake8 ===============' && \
diff --git a/res/container-tests/Containerfile.ubi8 b/res/container-tests/Containerfile.ubi8
index 2b8db2f..03d499d 100644
--- a/res/container-tests/Containerfile.ubi8
+++ b/res/container-tests/Containerfile.ubi8
@@ -22,8 +22,7 @@ ENTRYPOINT virtualenv testenv -p "/usr/bin/$PYTHON_VENV" && \
echo '=============== Running pylint ===============' && \
echo '==================================================' && \
echo '==================================================' && \
- # echo $LINTABLES | xargs pylint && echo '===> pylint PASSED' && \
- echo 'TEMPORARILY DISABLED' && \
+ echo $LINTABLES | xargs pylint && echo '===> pylint PASSED' && \
echo '==================================================' && \
echo '==================================================' && \
echo '=============== Running flake8 ===============' && \
diff --git a/res/container-tests/Containerfile.ubi9 b/res/container-tests/Containerfile.ubi9
index 4f31335..359d46b 100644
--- a/res/container-tests/Containerfile.ubi9
+++ b/res/container-tests/Containerfile.ubi9
@@ -20,8 +20,7 @@ ENTRYPOINT virtualenv testenv -p "/usr/bin/$PYTHON_VENV" && \
echo '=============== Running pylint ===============' && \
echo '==================================================' && \
echo '==================================================' && \
- # echo $LINTABLES | xargs pylint && echo '===> pylint PASSED' && \
- echo 'TEMPORARILY DISABLED' && \
+ echo $LINTABLES | xargs pylint && echo '===> pylint PASSED' && \
echo '==================================================' && \
echo '==================================================' && \
echo '=============== Running flake8 ===============' && \
diff --git a/tests/scripts/test_reporting.py b/tests/scripts/test_reporting.py
index 3f75af6..e2eabf7 100644
--- a/tests/scripts/test_reporting.py
+++ b/tests/scripts/test_reporting.py
@@ -238,17 +238,17 @@ def test_report_backwards_compatibility():
report = [msg1]
# make sure normal mode works
for report_format in ['.json', '.txt']:
- reportfile = tempfile.NamedTemporaryFile(suffix=report_format)
- generate_report_file(report, 'leapp-run-id', reportfile.name, '1.1.0')
+ with tempfile.NamedTemporaryFile(suffix=report_format) as reportfile:
+ generate_report_file(report, 'leapp-run-id', reportfile.name, '1.1.0')
# make sure report output conversion to specific version works as well
for report_format in ['.json', '.txt']:
- reportfile = tempfile.NamedTemporaryFile(suffix=report_format)
- generate_report_file(report, 'leapp-run-id', reportfile.name, '1.0.0')
- with open(reportfile.name) as f:
- data = f.read()
- if report_format == '.json':
- a_report = json.loads(data)
- msg = a_report['entries'][0]
- assert 'key' not in msg
- else:
- assert 'Key' not in data
+ with tempfile.NamedTemporaryFile(suffix=report_format) as reportfile:
+ generate_report_file(report, 'leapp-run-id', reportfile.name, '1.0.0')
+ with open(reportfile.name) as f:
+ data = f.read()
+ if report_format == '.json':
+ a_report = json.loads(data)
+ msg = a_report['entries'][0]
+ assert 'key' not in msg
+ else:
+ assert 'Key' not in data
diff --git a/tests/scripts/test_rerun.py b/tests/scripts/test_rerun.py
index 95d4ee6..addcbbb 100644
--- a/tests/scripts/test_rerun.py
+++ b/tests/scripts/test_rerun.py
@@ -10,7 +10,7 @@ def _evaluate_results(results, *expected):
if PY3:
results = results.decode('utf-8')
expected = set(expected)
- lines = set([line.split()[-1] for line in results.split('\n') if line.startswith('<<<TEST>>>: ')])
+ lines = set(line.split()[-1] for line in results.split('\n') if line.startswith('<<<TEST>>>: '))
assert lines == expected
--
2.35.3
Reference in New Issue View Git Blame Copy Permalink