URL: https://github.com/freeipa/freeipa/pull/593 Author: tiran Title: #593: WIP: Add make patchcheck for developers Action: opened
PR body: """ Ticket 6604 makes pylint and jsl optional dependencies. The change is controversal, because some developers prefer that pylint and jsl should be required unless explicitly disabled. `make patchcheck` is my answer to address the concerns. It's a superior solution to `make lint` as pre-commit check. It combines several additional checks under a single, easy rememberable and convenient make target: * build all * acilint, apiclient, jslint, polint * make check * pylint under Python 2 and 3 * subset of unit test suite https://fedorahosted.org/freeipa/ticket/6604 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/593/head:pr593 git checkout pr593
From c61e7365abd7c07f2ae5b5167d184dba65ea3174 Mon Sep 17 00:00:00 2001 From: Christian Heimes <chei...@redhat.com> Date: Fri, 17 Feb 2017 08:39:54 +0100 Subject: [PATCH 1/2] Add options to run only ipaclient unittests A new option for ipa-run-tests makes the test runner ignore subdirectories or skips tests that depend on the ipaserver package or on a running framework for RPC integration tests. The new option enables testing of client-only builds. $ ipatests/ipa-run-tests --ipaclient-unittests ... platform linux2 -- Python 2.7.13, pytest-2.9.2, py-1.4.32, pluggy-0.3.1 rootdir: /home/heimes/redhat, inifile: tox.ini plugins: sourceorder-0.5, cov-2.3.0, betamax-0.7.1, multihost-1.1 collected 451 items test_util.py ........ util.py .. test_ipaclient/test_csrgen.py ..............ssss... test_ipalib/test_aci.py ................... test_ipalib/test_backend.py ........ test_ipalib/test_base.py ............... test_ipalib/test_capabilities.py . test_ipalib/test_cli.py ... test_ipalib/test_config.py ............... test_ipalib/test_crud.py ............... test_ipalib/test_errors.py ....... test_ipalib/test_frontend.py ........................................ test_ipalib/test_messages.py .... test_ipalib/test_output.py ... test_ipalib/test_parameters.py ............................................................. test_ipalib/test_plugable.py ........ test_ipalib/test_rpc.py ......ssssssss test_ipalib/test_text.py ............................. test_ipalib/test_x509.py ... test_ipapython/test_cookie.py ............ test_ipapython/test_dn.py ........................... test_ipapython/test_ipautil.py .................................................................. test_ipapython/test_ipavalidate.py .......... test_ipapython/test_kerberos.py .............. test_ipapython/test_keyring.py .......... test_ipapython/test_ssh.py ............................... test_pkcs10/test_pkcs10.py ..... https://fedorahosted.org/freeipa/ticket/6517 Signed-off-by: Christian Heimes <chei...@redhat.com> --- ipatests/conftest.py | 34 +++++++++++++++++++++- ipatests/pytest_plugins/integration.py | 8 +++-- ipatests/setup.py | 1 - ipatests/test_cmdline/__init__.py | 6 ++++ ipatests/test_install/__init__.py | 5 ++++ ipatests/test_integration/__init__.py | 5 ++++ ipatests/test_ipalib/test_rpc.py | 2 ++ ipatests/test_ipaserver/__init__.py | 6 ++++ ipatests/test_ipaserver/test_install/__init__.py | 0 .../test_install/test_adtrustinstance.py | 8 ++--- ipatests/test_webui/__init__.py | 5 ++++ ipatests/test_xmlrpc/__init__.py | 5 ++++ ipatests/util.py | 17 +++++++++-- pylint_plugins.py | 16 +++++++++- 14 files changed, 103 insertions(+), 15 deletions(-) create mode 100644 ipatests/test_ipaserver/test_install/__init__.py diff --git a/ipatests/conftest.py b/ipatests/conftest.py index 511d7b7..61e889d 100644 --- a/ipatests/conftest.py +++ b/ipatests/conftest.py @@ -7,13 +7,21 @@ import pprint import sys +import pytest + from ipalib import api from ipalib.cli import cli_plugins + +try: + import ipaplatform # pylint: disable=unused-import +except ImportError: + ipaplatform = None try: import ipaserver except ImportError: ipaserver = None +HERE = os.path.dirname(os.path.abspath(__file__)) pytest_plugins = [ 'ipatests.pytest_plugins.additional_config', @@ -31,6 +39,7 @@ 'tier1: functional API tests', 'cs_acceptance: Acceptance test suite for Dogtag Certificate Server', 'ds_acceptance: Acceptance test suite for 389 Directory Server', + 'skip_ipaclient_unittest: Skip in ipaclient unittest mode', ] @@ -46,6 +55,7 @@ 'install/share' ] + INIVALUES = { 'python_classes': ['test_', 'Test'], 'python_files': ['test_*.py'], @@ -75,13 +85,27 @@ def pytest_configure(config): config.option.doctestmodules = True +def pytest_addoption(parser): + group = parser.getgroup("IPA integration tests") + group.addoption( + '--ipaclient-unittests', + help='Run ipaclient unit tests only (no RPC and ipaserver)', + action='store_true' + ) + + def pytest_cmdline_main(config): api.bootstrap( context=u'cli', in_server=False, in_tree=True, fallback=False ) for klass in cli_plugins: api.add_plugin(klass) - api.finalize() + + # XXX workaround until https://fedorahosted.org/freeipa/ticket/6408 has + # been resolved. + if ipaserver is not None: + api.finalize() + if config.option.verbose: print('api.env: ') pprint.pprint({k: api.env[k] for k in api.env}) @@ -89,3 +113,11 @@ def pytest_cmdline_main(config): print("euid: {}, egid: {}".format(os.geteuid(), os.getegid())) print("working dir: {}".format(os.path.abspath(os.getcwd()))) print('sys.version: {}'.format(sys.version)) + + +def pytest_runtest_setup(item): + if isinstance(item, item.Function): + if item.get_marker('skip_ipaclient_unittest'): + # pylint: disable=no-member + if pytest.config.option.ipaclient_unittests: + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/pytest_plugins/integration.py b/ipatests/pytest_plugins/integration.py index de37bf3..ae94dbc 100644 --- a/ipatests/pytest_plugins/integration.py +++ b/ipatests/pytest_plugins/integration.py @@ -30,9 +30,6 @@ from ipapython import ipautil from ipapython.ipa_log_manager import log_mgr -from ipatests.test_integration import tasks -from ipatests.test_integration.config import Config -from ipatests.test_integration.env_config import get_global_config log = log_mgr.get_logger(__name__) @@ -155,6 +152,11 @@ def integration_logs(class_integration_logs, request): def mh(request, class_integration_logs): """IPA's multihost fixture object """ + # TODO: cleanup modules + from ipatests.test_integration import tasks + from ipatests.test_integration.config import Config + from ipatests.test_integration.env_config import get_global_config + cls = request.cls domain_description = { diff --git a/ipatests/setup.py b/ipatests/setup.py index 46d51ff..337b7d6 100644 --- a/ipatests/setup.py +++ b/ipatests/setup.py @@ -64,7 +64,6 @@ "gssapi", "ipaclient", "ipalib", - "ipaplatform", "ipapython", "nose", "polib", diff --git a/ipatests/test_cmdline/__init__.py b/ipatests/test_cmdline/__init__.py index 9da42e7..af8867e 100644 --- a/ipatests/test_cmdline/__init__.py +++ b/ipatests/test_cmdline/__init__.py @@ -1,3 +1,9 @@ # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # + +import pytest + + +if pytest.config.getoption('ipaclient_unittests', False): + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/test_install/__init__.py b/ipatests/test_install/__init__.py index 0ca31f1..54ef9eb 100644 --- a/ipatests/test_install/__init__.py +++ b/ipatests/test_install/__init__.py @@ -20,3 +20,8 @@ """ Package containing LDAP updates unit tests. """ +import pytest + + +if pytest.config.getoption('ipaclient_unittests', False): + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/test_integration/__init__.py b/ipatests/test_integration/__init__.py index b63d650..8779f2f 100644 --- a/ipatests/test_integration/__init__.py +++ b/ipatests/test_integration/__init__.py @@ -16,3 +16,8 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. +import pytest + + +if pytest.config.getoption('ipaclient_unittests', False): + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/test_ipalib/test_rpc.py b/ipatests/test_ipalib/test_rpc.py index 80cf2e7..7fe0580 100644 --- a/ipatests/test_ipalib/test_rpc.py +++ b/ipatests/test_ipalib/test_rpc.py @@ -23,6 +23,7 @@ from __future__ import print_function import nose +import pytest import six # pylint: disable=import-error from six.moves.xmlrpc_client import Binary, Fault, dumps, loads @@ -256,6 +257,7 @@ class user_add(Command): assert context.xmlclient.conn._calledall() is True +@pytest.mark.skip_ipaclient_unittest class test_xml_introspection(object): @classmethod def setup_class(cls): diff --git a/ipatests/test_ipaserver/__init__.py b/ipatests/test_ipaserver/__init__.py index 2192cc2..76942c7 100644 --- a/ipatests/test_ipaserver/__init__.py +++ b/ipatests/test_ipaserver/__init__.py @@ -20,3 +20,9 @@ """ Sub-package containing unit tests for `ipaserver` package. """ + +import pytest + + +if pytest.config.getoption('ipaclient_unittests', False): + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/test_ipaserver/test_install/__init__.py b/ipatests/test_ipaserver/test_install/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/ipatests/test_ipaserver/test_install/test_adtrustinstance.py b/ipatests/test_ipaserver/test_install/test_adtrustinstance.py index 6a050c8..86596c5 100755 --- a/ipatests/test_ipaserver/test_install/test_adtrustinstance.py +++ b/ipatests/test_ipaserver/test_install/test_adtrustinstance.py @@ -19,12 +19,8 @@ """ Test `adtrustinstance` """ - -import os - -import six -import nose import pytest +import six from ipaserver.install import adtrustinstance @@ -33,7 +29,7 @@ @pytest.mark.tier0 -class test_adtrustinstance: +class test_adtrustinstance(object): """ Test `adtrustinstance`. """ diff --git a/ipatests/test_webui/__init__.py b/ipatests/test_webui/__init__.py index 56d10d9..cb2f361 100644 --- a/ipatests/test_webui/__init__.py +++ b/ipatests/test_webui/__init__.py @@ -20,3 +20,8 @@ """ Sub-package containing Web UI integration tests """ +import pytest + + +if pytest.config.getoption('ipaclient_unittests', False): + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/test_xmlrpc/__init__.py b/ipatests/test_xmlrpc/__init__.py index 1a8ecf1..720c61b 100644 --- a/ipatests/test_xmlrpc/__init__.py +++ b/ipatests/test_xmlrpc/__init__.py @@ -20,3 +20,8 @@ """ Sub-package containing unit tests for `xmlrpc` package. """ +import pytest + + +if pytest.config.getoption('ipaclient_unittests', False): + pytest.skip("Skip in ipaclient unittest mode") diff --git a/ipatests/util.py b/ipatests/util.py index f55ef73..4379c30 100644 --- a/ipatests/util.py +++ b/ipatests/util.py @@ -39,12 +39,23 @@ import ipalib from ipalib import api -from ipalib.install.kinit import kinit_keytab, kinit_password from ipalib.plugable import Plugin from ipalib.request import context from ipapython.dn import DN from ipapython.ipautil import run -from ipaplatform.paths import paths + +try: + # not available with client-only wheel packages + from ipalib.install.kinit import kinit_keytab, kinit_password +except ImportError: + kinit_keytab = kinit_password = None + +try: + # not available with client-only wheel packages + from ipaplatform.paths import paths +except ImportError: + paths = None + if six.PY3: unicode = str @@ -336,7 +347,7 @@ def assert_deepequal(expected, got, doc='', stack=tuple()): their elements does not matter. """ try: - pretty_print = pytest.config.getoption("pretty_print") # pylint: disable=no-member + pretty_print = pytest.config.getoption("pretty_print") except (AttributeError, ValueError): pretty_print = False diff --git a/pylint_plugins.py b/pylint_plugins.py index db80efe..472328c 100644 --- a/pylint_plugins.py +++ b/pylint_plugins.py @@ -7,12 +7,14 @@ import copy import os.path import sys +import textwrap -from astroid import MANAGER +from astroid import MANAGER, register_module_extender from astroid import scoped_nodes from pylint.checkers import BaseChecker from pylint.checkers.utils import check_messages from pylint.interfaces import IAstroidChecker +from astroid.builder import AstroidBuilder def register(linter): @@ -258,6 +260,18 @@ def fix_ipa_classes(cls): MANAGER.register_transform(scoped_nodes.Class, fix_ipa_classes) +def pytest_config_transform(): + """pylint.config attribute + """ + return AstroidBuilder(MANAGER).string_build(textwrap.dedent(''' + from _pytest.config import get_config + config = get_config() + ''')) + + +register_module_extender(MANAGER, 'pytest', pytest_config_transform) + + class IPAChecker(BaseChecker): __implements__ = IAstroidChecker From c2366cb4cce3bcda544fd5b87aac6d8bc0487708 Mon Sep 17 00:00:00 2001 From: Christian Heimes <chei...@redhat.com> Date: Wed, 15 Mar 2017 08:31:38 +0100 Subject: [PATCH 2/2] WIP: Add make patchcheck for developers Ticket 6604 makes pylint and jsl optional dependencies. The change is controversal, because some developers prefer that pylint and jsl should be required unless explicitly disabled. `make patchcheck` is my answer to address the concerns. It's a superior solution to `make lint` as pre-commit check. It combines several additional checks under a single, easy rememberable and convenient make target: * build all * acilint, apiclient, jslint, polint * make check * pylint under Python 2 and 3 * subset of unit test suite https://fedorahosted.org/freeipa/ticket/6604 Signed-off-by: Christian Heimes <chei...@redhat.com> --- Makefile.am | 34 +++++++++++++++++++++++++++++++++- configure.ac | 12 ++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 5d41e4a..379c1d3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -146,6 +146,38 @@ JSLINT_TARGET = jslint endif WITH_JSLINT lint: acilint apilint $(POLINT_TARGET) $(PYLINT_TARGET) $(JSLINT_TARGET) +.PHONY: patchcheck +patchcheck: all +if ! WITH_POLINT + @echo "ERROR: polint not available"; exit 1 +endif +if ! WITH_PYLINT + @echo "ERROR: pylint not available"; exit 1 +endif +if ! WITH_JSLINT + @echo "ERROR: jslint not available"; exit 1 +endif +if ! WITH_PYTHON2 + @echo "ERROR: python2 not available"; exit 1 +endif + @ # run all linters, tests, and check with Python 2 + PYTHONPATH=$(top_srcdir) $(PYTHON2) ipatests/ipa-run-tests \ + --ipaclient-unittests + $(MAKE) $(AM_MAKEFLAGS) acilint apilint polint jslint check + $(MAKE) $(AM_MAKEFLAGS) PYTHON=$(PYTHON2) pylint +if WITH_PYTHON3 + @ # just tests and pylint on Python 3 + PYTHONPATH=$(top_srcdir) $(PYTHON3) ipatests/ipa-run-tests \ + --ipaclient-unittests + $(MAKE) $(AM_MAKEFLAGS) PYTHON=$(PYTHON3) pylint +else + @echo "WARNING: python3 not available" +endif + +.PHONY: $(top_builddir)/ipapython/version.py +$(top_builddir)/ipapython/version.py: + (cd $(top_builddir)/ipapython && make version.py) + .PHONY: acilint acilint: $(top_builddir)/ipapython/version.py cd $(srcdir); ./makeaci --validate @@ -178,7 +210,7 @@ pylint: $(top_builddir)/ipapython/version.py ipasetup.py -name '*~' -o \ -name '*.py' -print -o \ -type f -exec grep -qsm1 '^#!.*\bpython' '{}' \; -print`; \ - echo "Pylint is running, please wait ..."; \ + echo "Pylint on $(PYTHON) is running, please wait ..."; \ PYTHONPATH=$(top_srcdir) $(PYTHON) -m pylint \ --rcfile=$(top_srcdir)/pylintrc \ --load-plugins pylint_plugins \ diff --git a/configure.ac b/configure.ac index 4a3ba15..0ea5bcf 100644 --- a/configure.ac +++ b/configure.ac @@ -111,6 +111,18 @@ if test "x$PYTHON" = "x" ; then fi dnl --------------------------------------------------------------------------- +dnl - Check for Python 2/3 for patchcheck +dnl --------------------------------------------------------------------------- + +AC_PATH_PROG(PYTHON2, python2) +AC_SUBST([PYTHON2]) +AM_CONDITIONAL([WITH_PYTHON2], [test "x${PYTHON2}" != "x"]) + +AC_PATH_PROG(PYTHON3, python3) +AC_SUBST([PYTHON3]) +AM_CONDITIONAL([WITH_PYTHON3], [test "x${PYTHON3}" != "x"]) + +dnl --------------------------------------------------------------------------- dnl - Check for cmocka unit test framework http://cmocka.cryptomilk.org/ dnl --------------------------------------------------------------------------- PKG_CHECK_EXISTS(cmocka,
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
