URL: https://github.com/freeipa/freeipa/pull/1384
Author: tiran
 Title: #1384: [Backport][ipa-4-6] make fastcheck -- run linters and tests in 
30 seconds
Action: opened

PR body:
"""
This PR was opened automatically because PR #1380 was pushed to master and 
backport to ipa-4-6 is required.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1384/head:pr1384
git checkout pr1384
From c13f71bc82dc29cc1bae0a88d7aad61e9129390f Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Mon, 11 Dec 2017 07:48:13 +0100
Subject: [PATCH 1/2] Add marker needs_ipaapi and option to skip tests

The new marker needs_ipaapi is used to mark tests that needs an
initialized API (ipalib.api) or some sort of other API services (running
LDAP server) to work. Some packages use api.Command or api.Backend on
module level. They are not marked but rather skipped entirely.

A new option ``skip-ipaapi`` is added to skip all API based tests. With
the option, only simple unit tests are executed. As of now, freeIPA
contains more than 500 unit tests that can be executed in about 5
seconds.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 ipatests/conftest.py                            | 10 ++++++++++
 ipatests/test_cmdline/test_cli.py               |  2 ++
 ipatests/test_cmdline/test_help.py              |  3 +++
 ipatests/test_cmdline/test_ipagetkeytab.py      |  1 +
 ipatests/test_install/test_updates.py           |  1 +
 ipatests/test_integration/__init__.py           |  1 +
 ipatests/test_ipalib/test_rpc.py                |  2 ++
 ipatests/test_ipapython/test_session_storage.py |  1 +
 ipatests/test_ipaserver/test_ldap.py            |  2 ++
 ipatests/test_ipaserver/test_serverroles.py     |  2 ++
 ipatests/test_webui/__init__.py                 |  1 +
 ipatests/test_webui/ui_driver.py                |  1 +
 ipatests/test_xmlrpc/__init__.py                |  1 +
 ipatests/util.py                                | 12 ++++++++++++
 14 files changed, 40 insertions(+)

diff --git a/ipatests/conftest.py b/ipatests/conftest.py
index c6c846e79f..6cc89838bc 100644
--- a/ipatests/conftest.py
+++ b/ipatests/conftest.py
@@ -41,6 +41,7 @@
     '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',
+    'needs_ipaapi: Test needs IPA API',
 ]
 
 
@@ -95,6 +96,11 @@ def pytest_addoption(parser):
         help='Run ipaclient unit tests only (no RPC and ipaserver)',
         action='store_true'
     )
+    group.addoption(
+        '--skip-ipaapi',
+        help='Do not run tests that depends on IPA API',
+        action='store_true',
+    )
 
 
 def pytest_cmdline_main(config):
@@ -124,3 +130,7 @@ def pytest_runtest_setup(item):
             # pylint: disable=no-member
             if pytest.config.option.ipaclient_unittests:
                 pytest.skip("Skip in ipaclient unittest mode")
+        if item.get_marker('needs_ipaapi'):
+            # pylint: disable=no-member
+            if pytest.config.option.skip_ipaapi:
+                pytest.skip("Skip tests that needs an IPA API")
diff --git a/ipatests/test_cmdline/test_cli.py b/ipatests/test_cmdline/test_cli.py
index eec0be7393..999b84dd3a 100644
--- a/ipatests/test_cmdline/test_cli.py
+++ b/ipatests/test_cmdline/test_cli.py
@@ -20,7 +20,9 @@
 HERE = os.path.abspath(os.path.dirname(__file__))
 BASE_DIR = os.path.abspath(os.path.join(HERE, os.pardir, os.pardir))
 
+
 @pytest.mark.tier0
+@pytest.mark.needs_ipaapi
 class TestCLIParsing(object):
     """Tests that commandlines are correctly parsed to Command keyword args
     """
diff --git a/ipatests/test_cmdline/test_help.py b/ipatests/test_cmdline/test_help.py
index b28aa2303d..2656a8df4c 100644
--- a/ipatests/test_cmdline/test_help.py
+++ b/ipatests/test_cmdline/test_help.py
@@ -30,6 +30,9 @@
     unicode = str
 
 
+pytestmark = pytest.mark.needs_ipaapi
+
+
 @pytest.mark.tier0
 class CLITestContext(object):
     """Context manager that replaces stdout & stderr, and catches SystemExit
diff --git a/ipatests/test_cmdline/test_ipagetkeytab.py b/ipatests/test_cmdline/test_ipagetkeytab.py
index 2f74ae92df..f04f0b9304 100644
--- a/ipatests/test_cmdline/test_ipagetkeytab.py
+++ b/ipatests/test_cmdline/test_ipagetkeytab.py
@@ -67,6 +67,7 @@ def test_service(request, test_host):
     return service_tracker.make_fixture(request)
 
 
+@pytest.mark.needs_ipaapi
 class KeytabRetrievalTest(cmdline_test):
     """
     Base class for keytab retrieval tests
diff --git a/ipatests/test_install/test_updates.py b/ipatests/test_install/test_updates.py
index 2b1705ffd8..6e904f3b10 100644
--- a/ipatests/test_install/test_updates.py
+++ b/ipatests/test_install/test_updates.py
@@ -48,6 +48,7 @@
 
 
 @pytest.mark.tier0
+@pytest.mark.needs_ipaapi
 class test_update(unittest.TestCase):
     """
     Test the LDAP updater.
diff --git a/ipatests/test_integration/__init__.py b/ipatests/test_integration/__init__.py
index 2b4d5350f2..045a1e8fde 100644
--- a/ipatests/test_integration/__init__.py
+++ b/ipatests/test_integration/__init__.py
@@ -20,3 +20,4 @@
 
 
 ipatests.util.check_ipaclient_unittests()
+ipatests.util.check_no_ipaapi()
diff --git a/ipatests/test_ipalib/test_rpc.py b/ipatests/test_ipalib/test_rpc.py
index f9ce0b8372..20afa8b121 100644
--- a/ipatests/test_ipalib/test_rpc.py
+++ b/ipatests/test_ipalib/test_rpc.py
@@ -260,6 +260,7 @@ class user_add(Command):
 
 
 @pytest.mark.skip_ipaclient_unittest
+@pytest.mark.needs_ipaapi
 class test_xml_introspection(object):
     @classmethod
     def setup_class(cls):
@@ -346,6 +347,7 @@ def test_help_many_params(self):
 
 
 @pytest.mark.skip_ipaclient_unittest
+@pytest.mark.needs_ipaapi
 class test_rpcclient_context(PluginTester):
     """
     Test the context in `ipalib.rpc.rpcclient` plugin.
diff --git a/ipatests/test_ipapython/test_session_storage.py b/ipatests/test_ipapython/test_session_storage.py
index 4f9091db36..dde0fe9274 100644
--- a/ipatests/test_ipapython/test_session_storage.py
+++ b/ipatests/test_ipapython/test_session_storage.py
@@ -11,6 +11,7 @@
 
 
 @pytest.mark.skip_ipaclient_unittest
+@pytest.mark.needs_ipaapi
 class test_session_storage(object):
     """
     Test the session storage interface
diff --git a/ipatests/test_ipaserver/test_ldap.py b/ipatests/test_ipaserver/test_ldap.py
index bcf497debb..fb4927f892 100644
--- a/ipatests/test_ipaserver/test_ldap.py
+++ b/ipatests/test_ipaserver/test_ldap.py
@@ -43,6 +43,7 @@
 
 
 @pytest.mark.tier0
+@pytest.mark.needs_ipaapi
 class test_ldap(object):
     """
     Test various LDAP client bind methods.
@@ -135,6 +136,7 @@ def test_autobind(self):
 
 
 @pytest.mark.tier0
+@pytest.mark.needs_ipaapi
 class test_LDAPEntry(object):
     """
     Test the LDAPEntry class
diff --git a/ipatests/test_ipaserver/test_serverroles.py b/ipatests/test_ipaserver/test_serverroles.py
index e8967517d0..a770c5ff4a 100644
--- a/ipatests/test_ipaserver/test_serverroles.py
+++ b/ipatests/test_ipaserver/test_serverroles.py
@@ -15,6 +15,8 @@
 from ipalib import api, create_api, errors
 from ipapython.dn import DN
 
+pytestmark = pytest.mark.needs_ipaapi
+
 
 def _make_service_entry(ldap_backend, dn, enabled=True, other_config=None):
     mods = {
diff --git a/ipatests/test_webui/__init__.py b/ipatests/test_webui/__init__.py
index 3f1b63aa8d..003034a2a6 100644
--- a/ipatests/test_webui/__init__.py
+++ b/ipatests/test_webui/__init__.py
@@ -24,3 +24,4 @@
 
 
 ipatests.util.check_ipaclient_unittests()
+ipatests.util.check_no_ipaapi()  # also ignore in make fasttest
diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py
index 23d8127a90..2cd2ba86ac 100644
--- a/ipatests/test_webui/ui_driver.py
+++ b/ipatests/test_webui/ui_driver.py
@@ -59,6 +59,7 @@
     NO_YAML = True
 from ipaplatform.paths import paths
 
+
 ENV_MAP = {
     'MASTER': 'ipa_server',
     'ADMINID': 'ipa_admin',
diff --git a/ipatests/test_xmlrpc/__init__.py b/ipatests/test_xmlrpc/__init__.py
index 0ee42fb4f2..ae47d98931 100644
--- a/ipatests/test_xmlrpc/__init__.py
+++ b/ipatests/test_xmlrpc/__init__.py
@@ -24,3 +24,4 @@
 
 
 ipatests.util.check_ipaclient_unittests()
+ipatests.util.check_no_ipaapi()  # also ignore in make fasttest
diff --git a/ipatests/util.py b/ipatests/util.py
index 98369ed8d9..1dc2a81e38 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -78,6 +78,18 @@ def check_ipaclient_unittests(reason="Skip in ipaclient unittest mode"):
             raise pytest.skip(reason)
 
 
+def check_no_ipaapi(reason="Skip tests that needs an IPA API"):
+    """Call this in a package to skip the package in no-ipaapi mode
+    """
+    if pytest.config.getoption('skip_ipaapi', False):
+        if PYTEST_VERSION[0] >= 3:
+            # pylint: disable=unexpected-keyword-arg
+            raise pytest.skip.Exception(reason, allow_module_level=True)
+            # pylint: enable=unexpected-keyword-arg
+        else:
+            raise pytest.skip(reason)
+
+
 class TempDir(object):
     def __init__(self):
         self.__path = tempfile.mkdtemp(prefix='ipa.tests.')

From 89d5a4d9c160f2424bbe7d91c16e1aeb62a9bb65 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Mon, 11 Dec 2017 07:57:40 +0100
Subject: [PATCH 2/2] Add make targets for fast linting and testing

Fast linting only needs modified files with pylint and diff with
pycodestyle. It's good enough to detect most code errors very fast. It
typically takes less than 10 seconds. A complete full pylint run uses
all CPU cores for several minutes. PEP 8 violations are typically
reported after 30 minutes to several hours on Travis CI.

Fast lintings uses git diff and git merge-base to find all modified
files in a branch or working tree. There is no easy way to find the
branch source. On Travis the information is provided by Travis. For
local development it's a new variable IPA_GIT_BRANCH in VERSION.m4.

Fast testing execute all unit tests that do not depend on ipalib.api.

In total it takes about 30-40 seconds (!) to execute linting, PEP 8 checks
and unittests for both Python 2 and 3.

Signed-off-by: Christian Heimes <chei...@redhat.com>
---
 .gitignore      |  1 +
 BUILD.txt       | 12 ++++++++++--
 Makefile.am     | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 VERSION.m4      | 11 +++++++++++
 configure.ac    |  1 +
 freeipa.spec.in |  5 ++++-
 6 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/.gitignore b/.gitignore
index cb1d98a563..872d716ebc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -112,3 +112,4 @@ freeipa2-dev-doc
 /ipapython/version.py
 /ipapython/.DEFAULT_PLUGINS
 
+/ipatests/.cache/
diff --git a/BUILD.txt b/BUILD.txt
index 20548bdd33..e3898aa68d 100644
--- a/BUILD.txt
+++ b/BUILD.txt
@@ -66,9 +66,9 @@ changes are required.
 Testing
 -------
 
-For more information, see http://www.freeipa.org/page/Testing
+For more information, see https://www.freeipa.org/page/Testing
 
-We use python nosetests to test for regressions in the management framework
+We use python pytest to test for regressions in the management framework
 and plugins. All test dependencies are required by the freeipa-tests package.
 
 To run all of the tests you will need 2 sessions, one to run the lite-server
@@ -82,6 +82,14 @@ Some tests may be skipped. For example, all the XML-RPC tests will be skipped
 if you haven't started the lite-server. The DNS tests will be skipped if
 the underlying IPA installation doesn't configure DNS, etc.
 
+To just execute fast unittest and code linters, use the fastcheck target.
+Fast tests only execute a subset of the test suite that does not depend on
+an initialized API and server instance. Fast linting just verifies modified
+files / lines.
+
+% make fastcheck
+
+
 API.txt
 -------
 The purpose of the file API.txt is to prevent accidental API changes. The
diff --git a/Makefile.am b/Makefile.am
index e16a5379fb..85c020506f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -169,13 +169,13 @@ 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 \
+	PYTHONPATH=$(abspath $(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 \
+	PYTHONPATH=$(abspath $(top_srcdir)) $(PYTHON3) ipatests/ipa-run-tests \
 	    --ipaclient-unittests
 	$(MAKE) $(AM_MAKEFLAGS) PYTHON=$(PYTHON3) pylint
 else
@@ -183,6 +183,49 @@ else
 endif
 	@echo "All tests passed."
 
+.PHONY: fastcheck fasttest fastlint
+fastcheck:
+if WITH_PYTHON2
+	@$(MAKE) -j1 $(AM_MAKEFLAGS) PYTHON=$(PYTHON2) fastlint fasttest
+endif
+if WITH_PYTHON3
+	@$(MAKE) -j1 $(AM_MAKEFLAGS) PYTHON=$(PYTHON3) fastlint fasttest
+endif
+
+fasttest: $(GENERATED_PYTHON_FILES) ipasetup.py
+	@ # --ignore doubles speed of total test run compared to pytest.skip()
+	@ # on module.
+	PYTHONPATH=$(abspath $(top_srcdir)) $(PYTHON3) ipatests/ipa-run-tests \
+	    --skip-ipaapi \
+	    --ignore $(abspath $(top_srcdir))/ipatests/test_integration \
+	    --ignore $(abspath $(top_srcdir))/ipatests/test_xmlrpc
+
+fastlint: $(GENERATED_PYTHON_FILES) ipasetup.py
+if ! WITH_PYLINT
+	@echo "ERROR: pylint not available"; exit 1
+endif
+	@echo "Fast linting with $(PYTHON) from branch '$(GIT_BRANCH)'"
+
+	@MERGEBASE=$$(git merge-base --fork-point $(GIT_BRANCH)); \
+	FILES=$$(git diff --name-only $${MERGEBASE} \
+	    | grep -E '\.py$$'); \
+	if [ -n "$${FILES}" ]; then \
+		echo "Fast linting files: $${FILES}"; \
+		echo "pylint"; \
+		echo "------"; \
+	    PYTHONPATH=$(abspath $(top_srcdir)) $(PYTHON) -m pylint \
+	        --rcfile=$(top_srcdir)/pylintrc \
+	        --load-plugins pylint_plugins \
+	        $${FILES} || exit $?; \
+	    echo "pycodestyle"; \
+	    echo "-----------"; \
+	    git diff $${MERGEBASE} | \
+	        $(PYTHON) -m pycodestyle --diff || exit $?; \
+	else \
+	    echo "No modified Python files found"; \
+	fi
+
+
 .PHONY: $(top_builddir)/ipaplatform/override.py
 $(top_builddir)/ipaplatform/override.py:
 	(cd $(top_builddir)/ipaplatform && make override.py)
diff --git a/VERSION.m4 b/VERSION.m4
index 48d24f277a..77802121de 100644
--- a/VERSION.m4
+++ b/VERSION.m4
@@ -48,6 +48,16 @@ define(IPA_VERSION_PRE_RELEASE, )
 ########################################################
 define(IPA_VERSION_IS_GIT_SNAPSHOT, yes)
 
+########################################################
+# git development branch:                              #
+#                                                      #
+# - master: define(IPA_GIT_BRANCH, master)             #
+# - ipa-X-X: define(IPA_GIT_BRANCH,                    #
+#       ipa-IPA_VERSION_MAJOR-IPA_VERSION_MINOR)       #
+########################################################
+define(IPA_GIT_BRANCH, master)
+dnl define(IPA_GIT_BRANCH, ipa-IPA_VERSION_MAJOR-IPA_VERSION_MINOR)
+
 ########################################################
 # The version of IPA data. This is used to identify    #
 # incompatibilities in data that could cause issues    #
@@ -128,6 +138,7 @@ NEWLINE)) dnl IPA_VERSION end
 dnl DEBUG: uncomment following lines and run command m4 VERSION.m4
 dnl `IPA_VERSION: ''IPA_VERSION'
 dnl `IPA_GIT_VERSION: ''IPA_GIT_VERSION'
+dnf `IPA_GIT_BRANCH: ''IPA_GIT_BRANCH'
 dnl `IPA_API_VERSION: ''IPA_API_VERSION'
 dnl `IPA_DATA_VERSION: ''IPA_DATA_VERSION'
 dnl `IPA_NUM_VERSION: ''IPA_NUM_VERSION'
diff --git a/configure.ac b/configure.ac
index a89d69c461..abef733e37 100644
--- a/configure.ac
+++ b/configure.ac
@@ -364,6 +364,7 @@ AC_SUBST([NUM_VERSION], [IPA_NUM_VERSION])
 AC_SUBST(VENDOR_SUFFIX)
 AC_SUBST([VERSION], [IPA_VERSION])
 AC_SUBST([GIT_VERSION], [IPA_GIT_VERSION])
+AC_SUBST([GIT_BRANCH], [IPA_GIT_BRANCH])
 # used by Makefile.am for files depending on templates
 AC_SUBST([CONFIG_STATUS])
 
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 62b4a832e1..c68b166d02 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -174,18 +174,20 @@ BuildRequires:  python3-wheel
 %endif # with_wheels
 
 #
-# Build dependencies for lint
+# Build dependencies for lint and fastcheck
 #
 %if 0%{?with_lint}
 BuildRequires:  python2-samba
 # 1.6: x509.Name.rdns (https://github.com/pyca/cryptography/issues/3199)
 BuildRequires:  python2-cryptography >= 1.6
 BuildRequires:  python2-gssapi >= 1.2.0-5
+BuildRequires:  softhsm
 %if 0%{?fedora} >= 26
 BuildRequires:  python2-pylint
 %else
 BuildRequires:  pylint >= 1.7
 %endif
+BuildRequires:  python2-pycodestyle
 # workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1096506
 BuildRequires:  python2-polib
 BuildRequires:  python2-libipa_hbac
@@ -223,6 +225,7 @@ BuildRequires:  python3-samba
 BuildRequires:  python3-cryptography >= 1.6
 BuildRequires:  python3-gssapi >= 1.2.0
 BuildRequires:  python3-pylint >= 1.7
+BuildRequires:  python3-pycodestyle
 # workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1096506
 BuildRequires:  python3-polib
 BuildRequires:  python3-libipa_hbac
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to