mgr/dashboard: clean-up python unit tests (ceph#28696)

mgr/dashboard: clean-up python unit tests

Reviewed-by: Alfonso Martínez <[email protected]>
Reviewed-by: Kefu Chai <[email protected]>
Reviewed-by: Stephan Müller <[email protected]>

cmake/modules/AddCephTest.cmake

@@ -69,7 +69,7 @@ function(add_tox_test name)
     list(APPEND tox_envs py3)
   endif()
   if(DEFINED TOXTEST_TOX_ENVS)
-    set(tox_envs ${TOXTEST_TOX_ENVS})
+    list(APPEND tox_envs ${TOXTEST_TOX_ENVS})
   endif()
   string(REPLACE ";" "," tox_envs "${tox_envs}")
   add_custom_command(

install-deps.sh

@@ -439,9 +439,11 @@ function populate_wheelhouse() {
 
     # although pip comes with virtualenv, having a recent version
     # of pip matters when it comes to using wheel packages
-    pip --timeout 300 $install 'setuptools >= 0.8' 'pip >= 7.0' 'wheel >= 0.24' || return 1
+    PIP_OPTS="--timeout 300 --exists-action i"
+    pip $PIP_OPTS $install \
+      'setuptools >= 0.8' 'pip >= 7.0' 'wheel >= 0.24' 'tox >= 2.9.1' || return 1
     if test $# != 0 ; then
-        pip --timeout 300 $install $@ || return 1
+        pip $PIP_OPTS $install $@ || return 1
     fi
 }
 
@@ -485,21 +487,24 @@ wip_wheelhouse=wheelhouse-wip
 find . -name tox.ini | while read ini ; do
     (
         cd $(dirname $ini)
-        require=$(ls *requirements.txt 2>/dev/null | sed -e 's/^/-r /')
+        require_files=$(ls *requirements*.txt 2>/dev/null) || true
+        constraint_files=$(ls *constraints*.txt 2>/dev/null) || true
+        require=$(echo -n "$require_files" | sed -e 's/^/-r /')
+        constraint=$(echo -n "$constraint_files" | sed -e 's/^/-c /')
         md5=wheelhouse/md5
         if test "$require"; then
-            if ! test -f $md5 || ! md5sum -c $md5 ; then
+            if ! test -f $md5 || ! md5sum -c $md5 > /dev/null; then
                 rm -rf wheelhouse
             fi
         fi
         if test "$require" && ! test -d wheelhouse ; then
             for interpreter in python2.7 python3 ; do
                 type $interpreter > /dev/null 2>&1 || continue
                 activate_virtualenv $top_srcdir $interpreter || exit 1
-                populate_wheelhouse "wheel -w $wip_wheelhouse" $require || exit 1
+                populate_wheelhouse "download -d $wip_wheelhouse" $require $constraint || exit 1
             done
             mv $wip_wheelhouse wheelhouse
-            md5sum *requirements.txt > $md5
+            md5sum $require_files $constraint_files > $md5
         fi
     )
 done

src/pybind/mgr/dashboard/.pylintrc

@@ -393,7 +393,7 @@ ignored-classes=optparse.Values,thread._local,_thread._local
 # (useful for modules/projects where namespaces are manipulated during runtime
 # and thus existing member attributes cannot be deduced by static analysis. It
 # supports qualified module names, as well as Unix pattern matching.
-ignored-modules=cherrypy,distutils
+ignored-modules=cherrypy,distutils,rados,rbd,cephfs
 
 # Show a hint with possible names when a member name was not found. The aspect
 # of finding the hint is based on edit distance.
@@ -432,7 +432,7 @@ ignore-comments=yes
 ignore-docstrings=yes
 
 # Ignore imports when computing similarities.
-ignore-imports=no
+ignore-imports=yes
 
 # Minimum lines number of a similarity.
 min-similarity-lines=4

src/pybind/mgr/dashboard/CMakeLists.txt

@@ -98,12 +98,5 @@ endif(WITH_MGR_DASHBOARD_FRONTEND AND NOT CMAKE_SYSTEM_PROCESSOR MATCHES "aarch6
 
 if(WITH_TESTS)
   include(AddCephTest)
-  if(WITH_PYTHON2)
-    list(APPEND dashboard_tox_envs py27-cov py27-lint py27-check)
-  endif()
-  if(WITH_PYTHON3)
-    list(APPEND dashboard_tox_envs py3-cov py3-lint py3-check)
-  endif()
-  add_tox_test(mgr-dashboard
-    TOX_ENVS ${dashboard_tox_envs})
+  add_tox_test(mgr-dashboard TOX_ENVS lint check)
 endif()

src/pybind/mgr/dashboard/HACKING.rst

@@ -129,7 +129,7 @@ Run ``npm run test`` to execute the unit tests via `Jest
 <https://facebook.github.io/jest/>`_.
 
 If you get errors on all tests, it could be because `Jest
-<https://facebook.github.io/jest/>`_ or something else was updated.
+<https://facebook.github.io/jest/>`__ or something else was updated.
 There are a few ways how you can try to resolve this:
 
 - Remove all modules with ``rm -rf dist node_modules`` and run ``npm install``
@@ -314,7 +314,7 @@ This components are declared on the components module:
 `src/pybind/mgr/dashboard/frontend/src/app/shared/components`.
 
 Helper
-......
+~~~~~~
 
 This component should be used to provide additional information to the user.
 
@@ -453,9 +453,9 @@ To do that, check the settings in the i18n config file
 ``src/pybind/mgr/dashboard/frontend/i18n.config.json``:: and make sure that the
 organization is *ceph*, the project is *ceph-dashboard* and the resource is
 the one you want to pull from and push to e.g. *Master:master*. To find a list
-of avaiable resources visit ``https://www.transifex.com/ceph/ceph-dashboard/content/``::
+of avaiable resources visit `<https://www.transifex.com/ceph/ceph-dashboard/content/>`_.
 
-After you checked the config go to the directory ``src/pybind/mgr/dashboard/frontend``:: and run
+After you checked the config go to the directory ``src/pybind/mgr/dashboard/frontend`` and run::
 
   $ npm run i18n
 
@@ -467,7 +467,7 @@ The tool will ask you for an api token, unless you added it by running:
 
   $ npm run i18n:token
 
-To create a transifex api token visit ``https://www.transifex.com/user/settings/api/``::
+To create a transifex api token visit `<https://www.transifex.com/user/settings/api/>`_.
 
 After the command ran successfully, build the UI and check if everything is
 working as expected. You also might want to run the frontend tests.
@@ -477,7 +477,7 @@ Suggestions
 
 Strings need to start and end in the same line as the element:
 
-.. code-block:: xml
+.. code-block:: html
 
   <!-- avoid -->
   <span i18n>
@@ -500,7 +500,7 @@ Strings need to start and end in the same line as the element:
 
 Isolated interpolations should not be translated:
 
-.. code-block:: xml
+.. code-block:: html
 
   <!-- avoid -->
   <span i18n>{{ foo }}</span>
@@ -510,14 +510,14 @@ Isolated interpolations should not be translated:
 
 Interpolations used in a sentence should be kept in the translation:
 
-.. code-block:: xml
+.. code-block:: html
 
   <!-- recommended -->
   <span i18n>There are {{ x }} OSDs.</span>
 
 Remove elements that are outside the context of the translation:
 
-.. code-block:: xml
+.. code-block:: html
 
   <!-- avoid -->
   <label i18n>
@@ -533,7 +533,7 @@ Remove elements that are outside the context of the translation:
 
 Keep elements that affect the sentence:
 
-.. code-block:: xml
+.. code-block:: html
 
   <!-- recommended -->
   <span i18n>Profile <b>foo</b> will be removed.</span>
@@ -575,25 +575,25 @@ Alternatively, you can use Python's native package installation method::
   $ pip install tox
   $ pip install coverage
 
-To run the tests, run ``run-tox.sh`` in the dashboard directory (where
+To run the tests, run ``run_tox.sh`` in the dashboard directory (where
 ``tox.ini`` is located)::
 
   ## Run Python 2+3 tests+lint commands:
-  $ ./run-tox.sh
+  $ ../../../script/run_tox.sh --tox-env py27,py3,lint,check
 
   ## Run Python 3 tests+lint commands:
-  $ WITH_PYTHON2=OFF ./run-tox.sh
+  $ ../../../script/run_tox.sh --tox-env py3,lint,check
 
   ## Run Python 3 arbitrary command (e.g. 1 single test):
-  $ WITH_PYTHON2=OFF ./run-tox.sh pytest tests/test_rgw_client.py::RgwClientTest::test_ssl_verify
+  $ WITH_PYTHON2=OFF ../../../script/run_tox.sh --tox-env py3 "" tests/test_rgw_client.py::RgwClientTest::test_ssl_verify
 
-You can also run tox instead of ``run-tox.sh``::
+You can also run tox instead of ``run_tox.sh``::
 
   ## Run Python 3 tests command:
-  $ CEPH_BUILD_DIR=.tox tox -e py3-cov
+  $ tox -e py3
 
   ## Run Python 3 arbitrary command (e.g. 1 single test):
-  $ CEPH_BUILD_DIR=.tox tox -e py3-run pytest tests/test_rgw_client.py::RgwClientTest::test_ssl_verify
+  $ tox -e py3 tests/test_rgw_client.py::RgwClientTest::test_ssl_verify
 
 We also collect coverage information from the backend code when you run tests. You can check the
 coverage information provided by the tox output, or by running the following
@@ -779,17 +779,17 @@ endpoint:
     # URL: /ping/{key}?opt1=...&opt2=...
     @Endpoint(path="/", query_params=['opt1'])
     def index(self, key, opt1, opt2=None):
-      # ...
+      """..."""
 
     # URL: /ping/{key}?opt1=...&opt2=...
     @Endpoint(query_params=['opt1'])
     def __call__(self, key, opt1, opt2=None):
-      # ...
+      """..."""
 
     # URL: /ping/post/{key1}/{key2}
     @Endpoint('POST', path_params=['key1', 'key2'])
     def post(self, key1, key2, data1, data2=None):
-      # ...
+      """..."""
 
 
 In the above example we see how the ``path`` option can be used to override the
@@ -826,7 +826,7 @@ Consider the following example:
     # URL: /ping/{node}/stats/{date}/latency?unit=...
     @Endpoint(path="/{date}/latency")
     def latency(self, node, date, unit="ms"):
-      # ...
+      """ ..."""
 
 In this example we explicitly declare a path parameter ``{node}`` in the
 controller URL path, and a path parameter ``{date}`` in the ``latency``
@@ -864,9 +864,10 @@ Example:
 
     @Proxy()
     def proxy(self, path, **params):
-      # if requested URL is "/foo/proxy/access/service?opt=1"
-      # then path is "access/service" and params is {'opt': '1'}
-      # ...
+      """
+      if requested URL is "/foo/proxy/access/service?opt=1"
+      then path is "access/service" and params is {'opt': '1'}
+      """
 
 
 How does the RESTController work?
@@ -1587,6 +1588,7 @@ type and not as a string. Allowed values are ``str``, ``int``, ``bool``, ``float
 .. code-block:: python
 
  @EndpointDoc(parameters={'my_string': (str, 'Description of my_string')})
+ def method(my_string): pass
 
 For body parameters, more complex cases are possible. If the parameter is a
 dictionary, the type should be replaced with a ``dict`` containing its nested
@@ -1605,13 +1607,15 @@ for nested parameters).
       'item2': (str, 'Description of item2', True),  # item2 is optional
       'item3': (str, 'Description of item3', True, 'foo'),  # item3 is optional with 'foo' as default value
   }, 'Description of my_dictionary')})
+  def method(my_dictionary): pass
 
 If the parameter is a ``list`` of primitive types, the type should be
 surrounded with square brackets.
 
 .. code-block:: python
 
   @EndpointDoc(parameters={'my_list': ([int], 'Description of my_list')})
+  def method(my_list): pass
 
 If the parameter is a ``list`` with nested parameters, the nested parameters
 should be placed in a dictionary and surrounded with square brackets.
@@ -1623,6 +1627,7 @@ should be placed in a dictionary and surrounded with square brackets.
       'list_item': (str, 'Description of list_item'),
       'list_item2': (str, 'Description of list_item2')
   }], 'Description of my_list')})
+  def method(my_list): pass
 
 
 ``responses``: A dict used for describing responses. Rules for describing
@@ -1633,7 +1638,8 @@ example below:
 .. code-block:: python
 
   @EndpointDoc(responses={
-    '400':{'my_response': (str, 'Description of my_response')}
+    '400':{'my_response': (str, 'Description of my_response')}})
+  def method(): pass
 
 
 Error Handling in Python
@@ -1743,7 +1749,7 @@ The available interfaces are the following:
 - ``CanLog``: provides the plug-in with access to the Ceph Dashboard logger under ``self.log``.
 - ``Setupable``: requires overriding ``setup()`` hook. This method is run in the Ceph Dashboard ``serve()`` method, right after CherryPy has been configured, but before it is started. It's a placeholder for the plug-in initialization logic.
 - ``HasOptions``: requires overriding ``get_options()`` hook by returning a list of ``Options()``. The options returned here are added to the ``MODULE_OPTIONS``.
-- ``HasCommands``: requires overriding ``register_commands()`` hook by defining the commands the plug-in can handle and decorating them with ``@CLICommand`. The commands can be optionally returned, so that they can be invoked externally (which makes unit testing easier).
+- ``HasCommands``: requires overriding ``register_commands()`` hook by defining the commands the plug-in can handle and decorating them with ``@CLICommand``. The commands can be optionally returned, so that they can be invoked externally (which makes unit testing easier).
 - ``HasControllers``: requires overriding ``get_controllers()`` hook by defining and returning the controllers as usual.
 - ``FilterRequest.BeforeHandler``: requires overriding ``filter_request_before_handler()`` hook. This method receives a ``cherrypy.request`` object for processing. A usual implementation of this method will allow some requests to pass or will raise a ``cherrypy.HTTPError` based on the ``request`` metadata and other conditions.
 

src/pybind/mgr/dashboard/__init__.py

@@ -35,7 +35,8 @@ def __getattr__(self, item):
     mgr = _ModuleProxy()
     logger = _LoggerProxy()
 
-    from .module import Module, StandbyModule
+    # DO NOT REMOVE: required for ceph-mgr to load a module
+    from .module import Module, StandbyModule  # noqa: F401
 else:
     import logging
     logging.basicConfig(level=logging.DEBUG)
@@ -47,7 +48,11 @@ def __getattr__(self, item):
     # Mock ceph module otherwise every module that is involved in a testcase and imports it will
     # raise an ImportError
     import sys
-    import mock
+    try:
+        import mock
+    except ImportError:
+        import unittest.mock as mock
+
     sys.modules['ceph_module'] = mock.Mock()
 
     mgr = mock.Mock()

src/pybind/mgr/dashboard/conftest.py

@@ -0,0 +1,26 @@
+import sys
+
+try:
+    from mock import Mock
+except ImportError:
+    from unittest.mock import Mock
+
+
+class MockRadosError(Exception):
+    def __init__(self, message, errno=None):
+        super(MockRadosError, self).__init__(message)
+        self.errno = errno
+
+    def __str__(self):
+        msg = super(MockRadosError, self).__str__()
+        if self.errno is None:
+            return msg
+        return '[errno {0}] {1}'.format(self.errno, msg)
+
+
+def pytest_configure(config):
+    sys.modules.update({
+        'rados': Mock(Error=MockRadosError, OSError=MockRadosError),
+        'rbd': Mock(),
+        'cephfs': Mock(),
+    })

src/pybind/mgr/dashboard/constraints.txt

@@ -0,0 +1,10 @@
+CherryPy==13.1.0
+enum34==1.1.6
+more-itertools==4.1.0
+PyJWT==1.6.4
+pyopenssl==17.5.0
+bcrypt==3.1.4
+python3-saml==1.4.1
+requests==2.20.0
+Routes==2.4.1
+six==1.11.0

src/pybind/mgr/dashboard/controllers/__init__.py

@@ -10,10 +10,8 @@
 import pkgutil
 import sys
 
-if sys.version_info >= (3, 0):
-    from urllib.parse import unquote  # pylint: disable=no-name-in-module,import-error
-else:
-    from urllib import unquote  # pylint: disable=no-name-in-module
+import six
+from six.moves.urllib.parse import unquote
 
 # pylint: disable=wrong-import-position
 import cherrypy
@@ -30,13 +28,13 @@ def EndpointDoc(description="", group="", parameters=None, responses=None):
     if not isinstance(description, str):
         raise Exception("%s has been called with a description that is not a string: %s"
                         % (EndpointDoc.__name__, description))
-    elif not isinstance(group, str):
+    if not isinstance(group, str):
         raise Exception("%s has been called with a groupname that is not a string: %s"
                         % (EndpointDoc.__name__, group))
-    elif parameters and not isinstance(parameters, dict):
+    if parameters and not isinstance(parameters, dict):
         raise Exception("%s has been called with parameters that is not a dict: %s"
                         % (EndpointDoc.__name__, parameters))
-    elif responses and not isinstance(responses, dict):
+    if responses and not isinstance(responses, dict):
         raise Exception("%s has been called with responses that is not a dict: %s"
                         % (EndpointDoc.__name__, responses))
 
@@ -637,9 +635,7 @@ def _request_wrapper(func, method, json_response, xml):  # pylint: disable=unuse
         @wraps(func)
         def inner(*args, **kwargs):
             for key, value in kwargs.items():
-                # pylint: disable=undefined-variable
-                if (sys.version_info < (3, 0) and isinstance(value, unicode)) \
-                        or isinstance(value, str):
+                if isinstance(value, six.text_type):
                     kwargs[key] = unquote(value)
 
             # Process method arguments.