Add test for turnserver software release

software/turnserver/test/README.md

+
+
+Tests for TurnServer Software Release
\ No newline at end of file

software/turnserver/test/setup.py

+##############################################################################
+#
+# Copyright (c) 2018 Nexedi SA and Contributors. All Rights Reserved.
+#
+# WARNING: This program as such is intended to be used by professional
+# programmers who take the whole responsibility of assessing all potential
+# consequences resulting from its eventual inadequacies and bugs
+# End users who are looking for a ready-to-use solution with commercial
+# guarantees and support are strongly adviced to contract a Free Software
+# Service Company
+#
+# This program is Free Software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 3
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+#
+##############################################################################
+from setuptools import setup, find_packages
+import glob
+import os
+
+version = '0.0.1.dev0'
+name = 'slapos.test.turnserver'
+long_description = open("README.md").read()
+
+setup(name=name,
+      version=version,
+      description="Test for TurnServer Software Release",
+      long_description=long_description,
+      long_description_content_type='text/markdown',
+      maintainer="Nexedi",
+      maintainer_email="info@nexedi.com",
+      url="https://lab.nexedi.com/nexedi/slapos",
+      packages=find_packages(),
+      install_requires=[
+          'slapos.core',
+          'slapos.libnetworkcache',
+          'erp5.util',
+          'supervisor',
+          'psutil',
+      ],
+      zip_safe=True,
+      test_suite='test',
+      )
\ No newline at end of file

software/turnserver/test/test.py

+##############################################################################
+#
+# Copyright (c) 2018 Nexedi SA and Contributors. All Rights Reserved.
+#
+# WARNING: This program as such is intended to be used by professional
+# programmers who take the whole responsibility of assessing all potential
+# consequences resulting from its eventual inadequacies and bugs
+# End users who are looking for a ready-to-use solution with commercial
+# guarantees and support are strongly adviced to contract a Free Software
+# Service Company
+#
+# This program is Free Software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 3
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+#
+##############################################################################
+
+import os
+import subprocess
+import json
+import glob
+import ConfigParser
+
+import utils
+
+# for development: debugging logs and install Ctrl+C handler
+if os.environ.get('SLAPOS_TEST_DEBUG'):
+  import logging
+  logging.basicConfig(level=logging.DEBUG)
+  import unittest
+  unittest.installHandler()
+
+def subprocess_status_output(*args, **kwargs):
+  prc = subprocess.Popen(
+    stdout=subprocess.PIPE,
+    stderr=subprocess.STDOUT,
+    *args,
+    **kwargs)
+  out, err = prc.communicate()
+  return prc.returncode, out
+
+class InstanceTestCase(utils.SlapOSInstanceTestCase):
+  @classmethod
+  def getSoftwareURLList(cls):
+    return (os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'software.cfg')), )
+
+
+
+class ServicesTestCase(InstanceTestCase):
+
+  @staticmethod
+  def generateHash(file_list):
+    import hashlib
+    hasher = hashlib.md5()
+    for path in file_list:
+      with open(path, 'r') as afile:
+        buf = afile.read()
+      hasher.update("%s\n" % len(buf))
+      hasher.update(buf)
+    hash = hasher.hexdigest()
+    return hash
+
+  def test_process_list(self):
+    hash_list = [
+      'software_release/buildout.cfg',
+    ]
+    expected_process_names = [
+      'bootstrap-monitor',
+      'turnserver-{hash}-on-watch',
+      'certificate_authority-{hash}-on-watch',
+      'crond-{hash}-on-watch',
+      'monitor-httpd-{hash}-on-watch',
+      'monitor-httpd-graceful',
+    ]
+
+    supervisor = self.getSupervisorRPCServer().supervisor
+    process_name_list = [process['name']
+                     for process in supervisor.getAllProcessInfo()]
+
+    hash_file_list = [os.path.join(self.computer_partition_root_path, path)
+                      for path in hash_list]
+
+    for name in expected_process_names:
+      h = ServicesTestCase.generateHash(hash_file_list)
+      expected_process_name = name.format(hash=h)
+
+      self.assertIn(expected_process_name, process_name_list)
+
+  def test_default_deployment(self):
+    partition_path_list = glob.glob(os.path.join(self.instance_path, '*'))
+    instance_folder = None
+    for partition_path in partition_path_list:
+      if os.path.exists(os.path.join(partition_path, 'etc/turnserver.conf')):
+        instance_folder = partition_path
+        break
+
+    secret_file = os.path.join(instance_folder, 'etc/.turnsecret')
+    self.assertTrue(os.path.exists(instance_folder))
+    self.assertTrue(os.path.exists(secret_file))
+    config = ConfigParser.ConfigParser()
+    config.readfp(open(secret_file))
+    secret = config.get('turnserver', 'secret')
+
+    expected_config = """listening-port=3478
+tls-listening-port=5349
+fingerprint
+lt-cred-mech
+use-auth-secret
+static-auth-secret=%(secret)s
+listening-ip=%(ipv4)s
+server-name=turn.example.com
+realm=turn.example.com
+total-quota=100
+bps-capacity=0
+stale-nonce=600
+cert=%(instance_path)s/etc/ssl/cert.pem
+pkey=%(instance_path)s/etc/ssl/key.pem
+dh-file=%(instance_path)s/etc/ssl/dhparam.pem
+cipher-list="ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:DH+3DES:RSA+AES:RSA+3DES:!ADH:!AECDH:!MD5"
+no-loopback-peers
+no-multicast-peers
+mobility
+no-tlsv1
+no-tlsv1_1
+no-stdout-log
+log-file=%(instance_path)s/var/log/turnserver.log
+userdb=%(instance_path)s/srv/turndb
+pidfile=%(instance_path)s/var/run/turnserver.pid
+verbose""" % {'instance_path': instance_folder, 'secret': secret, 'ipv4': self.config['ipv4_address']}
+
+    with open(os.path.join(instance_folder, 'etc/turnserver.conf')) as f:
+      current_config = f.read().strip()
+
+    self.assertEqual(current_config, expected_config)
+
+  def test_turnserver_promises(self):
+    partition_path_list = glob.glob(os.path.join(self.instance_path, '*'))
+    instance_folder = None
+    for partition_path in partition_path_list:
+      if os.path.exists(os.path.join(partition_path, 'etc/turnserver.conf')):
+        instance_folder = partition_path
+        break
+    self.assertTrue(os.path.exists(instance_folder))
+
+    promise_path_list = glob.glob(os.path.join(instance_folder, 'etc/plugin/*.py'))
+    promise_name_list = [x for x in
+                         os.listdir(os.path.join(instance_folder, 'etc/plugin'))
+                         if not x.endswith('.pyc')]
+    partition_name = os.path.basename(instance_folder.rstrip('/'))
+    self.assertEqual(sorted(promise_name_list),
+                    sorted([
+                      "__init__.py",
+                      "check-free-disk-space.py",
+                      "monitor-http-frontend.py",
+                      "buildout-%s-status.py" % partition_name,
+                      "monitor-bootstrap-status.py",
+                      "monitor-httpd-listening-on-tcp.py",
+                      "turnserver-port-listening.py",
+                      "turnserver-tls-port-listening.py",
+                    ]))
+
+    ignored_plugin_list = [
+      '__init__.py',
+      'monitor-http-frontend.py',
+    ]
+    runpromise_bin = os.path.join(
+      self.software_path, 'bin', 'monitor.runpromise')
+    monitor_conf = os.path.join(instance_folder, 'etc', 'monitor.conf')
+    msg = []
+    status = 0
+    for plugin_path in promise_path_list:
+      plugin_name = os.path.basename(plugin_path)
+      if plugin_name in ignored_plugin_list:
+        continue
+      plugin_status, plugin_result = subprocess_status_output([
+        runpromise_bin,
+        '-c', monitor_conf,
+        '--run-only', plugin_name,
+        '--force',
+        '--check-anomaly'
+      ])
+      status += plugin_status
+      if plugin_status == 1:
+        msg.append(plugin_result)
+      # sanity check
+      if 'Checking promise %s' % plugin_name not in plugin_result:
+        plugin_status = 1
+        msg.append(plugin_result)
+    msg = ''.join(msg).strip()
+    self.assertEqual(status, 0, msg)
+
+class ParametersTestCase(InstanceTestCase):
+  @classmethod
+  def getInstanceParameterDict(cls):
+    return {
+      'server-name': "turn.site.com",
+      'port': 3488,
+      'tls-port': 5369,
+      'external-ip': '127.0.0.1',
+      'listening-ip': '127.0.0.1'
+    }
+
+  def test_turnserver_with_parameters(self):
+    partition_path_list = glob.glob(os.path.join(self.instance_path, '*'))
+    instance_folder = None
+    for partition_path in partition_path_list:
+      if os.path.exists(os.path.join(partition_path, 'etc/turnserver.conf')):
+        instance_folder = partition_path
+        break
+
+    secret_file = os.path.join(instance_folder, 'etc/.turnsecret')
+    self.assertTrue(os.path.exists(instance_folder))
+    self.assertTrue(os.path.exists(secret_file))
+    config = ConfigParser.ConfigParser()
+    config.readfp(open(secret_file))
+    secret = config.get('turnserver', 'secret')
+
+    expected_config = """listening-port=%(port)s
+tls-listening-port=%(tls_port)s
+fingerprint
+lt-cred-mech
+use-auth-secret
+static-auth-secret=%(secret)s
+listening-ip=%(ipv4)s
+external-ip=%(external_ip)s
+server-name=%(name)s
+realm=%(name)s
+total-quota=100
+bps-capacity=0
+stale-nonce=600
+cert=%(instance_path)s/etc/ssl/cert.pem
+pkey=%(instance_path)s/etc/ssl/key.pem
+dh-file=%(instance_path)s/etc/ssl/dhparam.pem
+cipher-list="ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:DH+3DES:RSA+AES:RSA+3DES:!ADH:!AECDH:!MD5"
+no-loopback-peers
+no-multicast-peers
+mobility
+no-tlsv1
+no-tlsv1_1
+no-stdout-log
+log-file=%(instance_path)s/var/log/turnserver.log
+userdb=%(instance_path)s/srv/turndb
+pidfile=%(instance_path)s/var/run/turnserver.pid
+verbose""" % {'instance_path': instance_folder,
+              'secret': secret,
+              'ipv4': '127.0.0.1',
+              'name': 'turn.site.com',
+              'external_ip': '127.0.0.1',
+              'port': 3488,
+              'tls_port': 5369,}
+
+    with open(os.path.join(instance_folder, 'etc/turnserver.conf')) as f:
+      current_config = f.read().strip()
+
+    self.assertEqual(current_config, expected_config)
+
+

software/turnserver/test/utils.py

+##############################################################################
+#
+# Copyright (c) 2018 Nexedi SA and Contributors. All Rights Reserved.
+#
+# WARNING: This program as such is intended to be used by professional
+# programmers who take the whole responsibility of assessing all potential
+# consequences resulting from its eventual inadequacies and bugs
+# End users who are looking for a ready-to-use solution with commercial
+# guarantees and support are strongly adviced to contract a Free Software
+# Service Company
+#
+# This program is Free Software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 3
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+#
+##############################################################################
+
+import unittest
+import os
+import socket
+from contextlib import closing
+import logging
+import StringIO
+import xmlrpclib
+
+import supervisor.xmlrpc
+from erp5.util.testnode.SlapOSControler import SlapOSControler
+from erp5.util.testnode.ProcessManager import ProcessManager
+
+
+# Utility functions
+def findFreeTCPPort(ip=''):
+  """Find a free TCP port to listen to.
+  """
+  family = socket.AF_INET6 if ':' in ip else socket.AF_INET
+  with closing(socket.socket(family, socket.SOCK_STREAM)) as s:
+    s.bind((ip, 0))
+    return s.getsockname()[1]
+
+
+# TODO:
+#  - allow requesting multiple instances ?
+
+class SlapOSInstanceTestCase(unittest.TestCase):
+  """Install one slapos instance.
+
+  This test case install software(s) and request one instance during `setUpClass`
+  and destroy the instance during `tearDownClass`.
+
+  Software Release URL, Instance Software Type and Instance Parameters can be defined
+  on the class.
+
+  All tests from the test class will run with the same instance.
+
+  The following class attributes are available:
+
+    * `computer_partition`:  the computer partition instance, implementing
+    `slapos.slap.interface.slap.IComputerPartition`.
+
+    * `computer_partition_root_path`: the path of the instance root directory.
+
+  """
+
+  # Methods to be defined by subclasses.
+  @classmethod
+  def getSoftwareURLList(cls):
+    """Return URL of software releases to install.
+
+    To be defined by subclasses.
+    """
+    raise NotImplementedError()
+
+  @classmethod
+  def getInstanceParameterDict(cls):
+    """Return instance parameters
+
+    To be defined by subclasses if they need to request instance with specific
+    parameters.
+    """
+    return {}
+
+  @classmethod
+  def getInstanceSoftwareType(cls):
+    """Return software type for instance, default "default"
+
+    To be defined by subclasses if they need to request instance with specific
+    software type.
+    """
+    return "default"
+
+  # Utility methods.
+  def getSupervisorRPCServer(self):
+    """Returns a XML-RPC connection to the supervisor used by slapos node
+
+    Refer to http://supervisord.org/api.html for details of available methods.
+    """
+    # xmlrpc over unix socket https://stackoverflow.com/a/11746051/7294664
+    return xmlrpclib.ServerProxy(
+       'http://slapos-supervisor',
+       transport=supervisor.xmlrpc.SupervisorTransport(
+           None,
+           None,
+           # XXX hardcoded socket path
+           serverurl="unix://{working_directory}/inst/supervisord.socket".format(
+             **self.config)))
+
+  # Unittest methods
+  @classmethod
+  def setUpClass(cls):
+    """Setup the class, build software and request an instance.
+
+    If you have to override this method, do not forget to call this method on
+    parent class.
+    """
+    try:
+      cls.setUpWorkingDirectory()
+      cls.setUpConfig()
+      cls.setUpSlapOSController()
+
+      cls.runSoftwareRelease()
+      # XXX instead of "runSoftwareRelease", it would be better to be closer to slapos usage:
+      # cls.supplySoftwares()
+      # cls.installSoftwares()
+
+      cls.runComputerPartition()
+      # XXX instead of "runComputerPartition", it would be better to be closer to slapos usage:
+      # cls.requestInstances()
+      # cls.createInstances()
+      # cls.requestInstances()
+
+    except Exception:
+      cls.stopSlapOSProcesses()
+      raise
+    cls.instance_path = os.path.join(
+      cls.config['working_directory'],
+      'inst')
+    cls.software_path = os.path.realpath(os.path.join(
+      cls.computer_partition_root_path, 'software_release'))
+
+  @classmethod
+  def tearDownClass(cls):
+    """Tear down class, stop the processes and destroy instance.
+    """
+    cls.stopSlapOSProcesses()
+
+  # Implementation
+  @classmethod
+  def stopSlapOSProcesses(cls):
+    if hasattr(cls, '_process_manager'):
+      cls._process_manager.killPreviousRun()
+
+  @classmethod
+  def setUpWorkingDirectory(cls):
+    """Initialise the directories"""
+    cls.working_directory = os.environ.get(
+        'SLAPOS_TEST_WORKING_DIR',
+        os.path.join(os.path.dirname(__file__), '.slapos'))
+    # To prevent error: Cannot open an HTTP server: socket.error reported
+    # AF_UNIX path too long This `working_directory` should not be too deep.
+    # Socket path is 108 char max on linux
+    # https://github.com/torvalds/linux/blob/3848ec5/net/unix/af_unix.c#L234-L238
+    # Supervisord socket name contains the pid number, which is why we add
+    # .xxxxxxx in this check.
+    if len(cls.working_directory + '/inst/supervisord.socket.xxxxxxx') > 108:
+      raise RuntimeError('working directory ( {} ) is too deep, try setting '
+              'SLAPOS_TEST_WORKING_DIR'.format(cls.working_directory))
+
+    if not os.path.exists(cls.working_directory):
+      os.mkdir(cls.working_directory)
+
+  @classmethod
+  def setUpConfig(cls):
+    """Create slapos configuration"""
+    cls.config = {
+      "working_directory": cls.working_directory,
+      "slapos_directory": cls.working_directory,
+      "log_directory": cls.working_directory,
+      "computer_id": 'slapos.test',  # XXX
+      'proxy_database': os.path.join(cls.working_directory, 'proxy.db'),
+      'partition_reference': cls.__name__,
+      # "proper" slapos command must be in $PATH
+      'slapos_binary': 'slapos',
+      'node_quantity': '3',
+    }
+    # Some tests are expecting that local IP is not set to 127.0.0.1
+    ipv4_address = os.environ.get('SLAPOS_TEST_IPV4', '127.0.1.1')
+    ipv6_address = os.environ['SLAPOS_TEST_IPV6']
+
+    cls.config['proxy_host'] = cls.config['ipv4_address'] = ipv4_address
+    cls.config['ipv6_address'] = ipv6_address
+    cls.config['proxy_port'] = findFreeTCPPort(ipv4_address)
+    cls.config['master_url'] = 'http://{proxy_host}:{proxy_port}'.format(
+      **cls.config)
+
+  @classmethod
+  def setUpSlapOSController(cls):
+    """Create the a "slapos controller" and supply softwares from `getSoftwareURLList`.
+
+    This is equivalent to:
+
+    slapos proxy start
+    for sr in getSoftwareURLList; do
+      slapos supply $SR $COMP
+    done
+    """
+    cls._process_manager = ProcessManager()
+
+    # XXX this code is copied from testnode code
+    cls.slapos_controler = SlapOSControler(
+        cls.working_directory,
+        cls.config
+    )
+
+    slapproxy_log = os.path.join(cls.config['log_directory'], 'slapproxy.log')
+    logger = logging.getLogger(__name__)
+    logger.debug('Configured slapproxy log to %r', slapproxy_log)
+
+    cls.software_url_list = cls.getSoftwareURLList()
+    cls.slapos_controler.initializeSlapOSControler(
+        slapproxy_log=slapproxy_log,
+        process_manager=cls._process_manager,
+        reset_software=False,
+        software_path_list=cls.software_url_list)
+
+    # XXX we should check *earlier* if that pidfile exist and if supervisord
+    # process still running, because if developer started supervisord (or bugs?)
+    # then another supervisord will start and starting services a second time
+    # will fail.
+    cls._process_manager.supervisord_pid_file = os.path.join(
+      cls.slapos_controler.instance_root, 'var', 'run', 'supervisord.pid')
+
+  @classmethod
+  def runSoftwareRelease(cls):
+    """Run all the software releases that were supplied before.
+
+    This is the equivalent of `slapos node software`.
+
+    The tests will be marked file if software building fail.
+    """
+    logger = logging.getLogger()
+    logger.level = logging.DEBUG
+    stream = StringIO.StringIO()
+    stream_handler = logging.StreamHandler(stream)
+    logger.addHandler(stream_handler)
+
+    try:
+      cls.software_status_dict = cls.slapos_controler.runSoftwareRelease(
+        cls.config, environment=os.environ)
+      stream.seek(0)
+      stream.flush()
+      message = ''.join(stream.readlines()[-100:])
+      assert cls.software_status_dict['status_code'] == 0, message
+    finally:
+      logger.removeHandler(stream_handler)
+      del stream
+
+
+  @classmethod
+  def runComputerPartition(cls):
+    """Instanciate the software.
+
+    This is the equivalent of doing:
+
+    slapos request --type=getInstanceSoftwareType --parameters=getInstanceParameterDict
+    slapos node instance
+
+    and return the slapos request instance parameters.
+
+    This can be called by tests to simulate re-request with different parameters.
+    """
+    logger = logging.getLogger()
+    logger.level = logging.DEBUG
+    stream = StringIO.StringIO()
+    stream_handler = logging.StreamHandler(stream)
+    logger.addHandler(stream_handler)
+
+    if cls.getInstanceSoftwareType() != 'default':
+      raise NotImplementedError
+
+    instance_parameter_dict = cls.getInstanceParameterDict()
+    try:
+      cls.instance_status_dict = cls.slapos_controler.runComputerPartition(
+        cls.config,
+        cluster_configuration=instance_parameter_dict,
+        environment=os.environ)
+      stream.seek(0)
+      stream.flush()
+      message = ''.join(stream.readlines()[-100:])
+      assert cls.instance_status_dict['status_code'] == 0, message
+    finally:
+      logger.removeHandler(stream_handler)
+      del stream
+
+    # FIXME: similar to test node, only one (root) partition is really
+    #        supported for now.
+    computer_partition_list = []
+    for i in range(len(cls.software_url_list)):
+      computer_partition_list.append(
+          cls.slapos_controler.slap.registerOpenOrder().request(
+            cls.software_url_list[i],
+            # This is how testnode's SlapOSControler name created partitions
+            partition_reference='testing partition {i}'.format(
+              i=i, **cls.config),
+            partition_parameter_kw=instance_parameter_dict))
+
+    # expose some class attributes so that tests can use them:
+    # the ComputerPartition instances, to getInstanceParameterDict
+    cls.computer_partition = computer_partition_list[0]
+
+    # the path of the instance on the filesystem, for low level inspection
+    cls.computer_partition_root_path = os.path.join(
+        cls.config['working_directory'],
+        'inst',
+        cls.computer_partition.getId())
+
+

[Julien Muchembled]

stop copy&paste

fix quickly or I revert everything

[Julien Muchembled]

Well, this is not the only place it's copy-pasted and by "fix" I mean take the opportunity to clean up the whole repository, using slapos.recipe.wrapper as reference.

I also react this way because this is a perfect example of how copy-paste kills us. There's actually a bug in the above code that is hard to spot. IOW? you spent time at writing more code and then different people in separate project will lose time by having bugs, understand them, and fix.

/cc @nexedi

[Rafael Monnerat]

There is an initiative from @jerome to move part of the test code (duplicated along the other places that test it) into a some util (or similar). It is probably possible to @alain.takoudjou take the initative and work to finish what jerome started or at least make a lib.

[Alain Takoudjou]

Yes, I will make a MR for it.

[Jérome Perrin]

you mean fixing the bug ? or finishing slapos.core!64 (merged) ? I was discussing this MR with @luke and I will probably be able to have a first mergeable state next week.

[Alain Takoudjou]

I mean fix bug and remove duplicated generateHash everywhere.

[Julien Muchembled]

finishing slapos.core!64 (merged) ? I was discussing this MR with @luke and I will probably be able to have a first mergeable state next week.

This is more ambitious than trivial refactoring within slapos.git

I mean fix bug and remove duplicated generateHash everywhere.

First, refactoring does fix the bug by itself so no need to fix anything before.

And the whole point is to develop more efficiently. If Jerome thinks slapos.core!64 (merged) can be finished quickly, then let's not do useless refactoring.

Note that the bug is about Python 3: the urgency is not the bug itself but the code quality.

[Alain Takoudjou]

I did this !617 (merged) yesterday, I don't think it conflict with what @jerome is doing here slapos.core!64 (merged) (he can confirm). The test result is here: https://nexedi.erp5.net/test_result_module/20190902-1D76E436/view?ignore_layout:int=1

[Jérome Perrin]

I don't think it would conflict, don't worry. Thanks !

[Julien Muchembled]

generateHashFromFiles is only the tip of iceberg. I commented only about this function because I almost don't know software/*/test. It does not mean that's the only code that's duplicated the way and as a developer of this part, you should know. For example:

$ diff -s software/{powerdns,re6stnet}/test/utils.py
Files software/powerdns/test/utils.py and software/re6stnet/test/utils.py are identical
$ wc software/{powerdns,re6stnet}/test/utils.py
  322  1072 11045 software/powerdns/test/utils.py
  322  1072 11045 software/re6stnet/test/utils.py
  644  2144 22090 total

IOW, that's even worse than generateHashFromFiles.

I haven't read slapos.core!64 (merged) so I don't know how much code removal it implies here. Anything that would remain should be refactored as well.

[Jérome Perrin]

For these tests, we started with accepting duplication of software/*/test/util.py in all SR tests, with the plan to replace it soon by a testing API, that is being develop in slapos.core!64 (merged).

In parrallel of slapos.core!64 (merged), there is also a branch of slapos which removes all the software/*/test/util.py and replace the by some imports from that new testing API.

I did not expect it would take me so much time ...

[ edit: takes me so much time]

[Łukasz Nowak]

I did not expect it would take me so much time ...

[ edit: takes me so much time]

Please note, that as Jerome already pointed out, we discussed about how to reduce amount of work for Jerome.

He will push the new API as soon as possible, and will only try to migrate software releases to the new API. Possibly, the most hard one would be caddy-frontend, so then I will be the guy to do the migration (which could end up with rewriting some tests I have).

So now hopefully the biggest blockers is removed, and I think Jerome can even migrate only minimal amount of SR tests so that others can follow-up his work and migrate their owns. I can help here, as I am silent helper here.

So in phases we will drop the utils.py duplication (and anyway more and more assertions are similar between SRs, so they shall be shared). I will handle this, eg. in !614 (merged), for which I think I'll wait for Jerome to finish slapos.core!64 (merged), then do required work to minimise duplication.