From 47658cc583ca598db7c649cb23ae42c255f78b3a Mon Sep 17 00:00:00 2001
From: Marco Clemencic <marco.clemencic@cern.ch>
Date: Fri, 24 Jan 2020 11:40:55 +0100
Subject: [PATCH] Add semantic checks to slot configuration and improve
 platform regex

---
 python/LbNightlyTools/Configuration.py        | 54 +++++++++++++-
 .../tests/test_configuration_check.py         | 73 +++++++++++++++++++
 2 files changed, 124 insertions(+), 3 deletions(-)
 create mode 100644 python/LbNightlyTools/tests/test_configuration_check.py

diff --git a/python/LbNightlyTools/Configuration.py b/python/LbNightlyTools/Configuration.py
index d0bd5dda..1ec0babb 100644
--- a/python/LbNightlyTools/Configuration.py
+++ b/python/LbNightlyTools/Configuration.py
@@ -59,6 +59,7 @@ def slot_factory(f):
 # constants
 GP_EXP = re.compile(r'gaudi_project\(([^)]+)\)')
 HT_EXP = re.compile(r'set\(\s*heptools_version\s+([^)]+)\)')
+VALID_PLATFORM_RE = r'^[0-9a-z_+.]+-[0-9a-z_]+-[0-9a-z_+]+-[0-9a-z_+]+$'
 
 # all configured slots (Slot instances)
 slots = {}
@@ -1547,9 +1548,7 @@ class Slot(with_metaclass(_SlotMeta, object)):
         if isinstance(self.platforms, basestring):
             self.platforms = [self.platforms]
         for p in self.platforms:
-            assert re.match(
-                r'^[0-9a-z_+]+-[0-9a-z_]+-[0-9a-z_+]+-[0-9a-z_+]+$',
-                p), 'invalid platform: %r' % p
+            assert re.match(VALID_PLATFORM_RE, p), 'invalid platform: %r' % p
 
         self.error_exceptions = kwargs.get('error_exceptions', [])
         self.warning_exceptions = kwargs.get('warning_exceptions', [])
@@ -2281,6 +2280,51 @@ def pushDataToFrontEnd(config_module):
     response.read()
 
 
+def check_slot(slot):
+    '''
+    Check that a slot configuration is valid.
+    '''
+    good = True
+    log = logging.getLogger(slot.name)
+
+    def check_type(field_name, types, value=None):
+        if value is None:
+            value = getattr(slot, field_name)
+        if not isinstance(value, types):
+            log.error(
+                'invalid %s type: found %s, expected one of [%s]', field_name,
+                type(value).__name__, ', '.join(t.__name__ for t in types))
+            return False
+        return True
+
+    def check_list_of_strings(field_name, name, regex):
+        good = check_type(field_name, (list, tuple))
+        if good:
+            for x in getattr(slot, field_name):
+                if check_type(name, (str, ), x):
+                    if not re.match(regex, x):
+                        log.error('invalid %s value: %r', name, x)
+                        good = False
+                else:
+                    good = False
+        return good
+
+    for field_name in ('warning_exceptions', 'error_exceptions'):
+        good &= check_type(field_name, (list, tuple))
+        for x in getattr(slot, field_name):
+            try:
+                re.compile(x)
+            except Exception as err:
+                good = False
+                log.error('%s: invalid value %r: %s', field_name, x, err)
+
+    good &= check_list_of_strings('platforms', 'platform', VALID_PLATFORM_RE)
+    good &= check_list_of_strings('env', 'env setting',
+                                  r'^[a-zA-Z_][a-z0-9A-Z_]*=')
+
+    return good
+
+
 def check_config():
     from LbNightlyTools.Configuration import loadConfig
     import LbNightlyTools.Configuration as LBNC
@@ -2338,6 +2382,10 @@ def check_config():
                    headers=('slot', 'enabled', 'source'),
                    tablefmt='grid'))
 
+    logging.debug('running semantics checks')
+    if not all(check_slot(slot) for slot in slots.values()):
+        return 1
+
     logging.debug('converting slots to JSON')
     json_str = json.dumps([slots[name].toDict() for name in sorted(slots)],
                           indent=2)
diff --git a/python/LbNightlyTools/tests/test_configuration_check.py b/python/LbNightlyTools/tests/test_configuration_check.py
new file mode 100644
index 00000000..89903b1e
--- /dev/null
+++ b/python/LbNightlyTools/tests/test_configuration_check.py
@@ -0,0 +1,73 @@
+###############################################################################
+# (c) Copyright 2013 CERN                                                     #
+#                                                                             #
+# This software is distributed under the terms of the GNU General Public      #
+# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING".   #
+#                                                                             #
+# In applying this licence, CERN does not waive the privileges and immunities #
+# granted to it by virtue of its status as an Intergovernmental Organization  #
+# or submit itself to any jurisdiction.                                       #
+###############################################################################
+# Uncomment to disable the tests.
+#__test__ = False
+
+from LbNightlyTools.Configuration import Slot, check_slot
+
+
+def test_ok():
+    assert check_slot(Slot('slot'))
+
+
+def test_warning_exceptions():
+    assert not check_slot(Slot('slot',
+                               warning_exceptions="")), 'bad type not detected'
+    assert not check_slot(Slot(
+        'slot', warning_exceptions=[123])), 'bad inner type not detected'
+
+
+def test_error_exceptions():
+    assert not check_slot(Slot('slot',
+                               error_exceptions="")), 'bad type not detected'
+    assert not check_slot(Slot(
+        'slot', error_exceptions=[123])), 'bad inner type not detected'
+
+
+def test_platforms():
+    # note: we use Slot.fromDict to bypass the check on platform string in the constructor
+    assert not check_slot(Slot.fromDict(dict(
+        slot='slot', platforms=""))), 'bad type not detected'
+    assert not check_slot(Slot.fromDict(dict(
+        slot='slot', platforms=[123]))), 'bad inner type not detected'
+    assert not check_slot(
+        Slot.fromDict(
+            dict(slot='slot', platforms=["x86_64-centos7-gcc9-opt, x"
+                                         ]))), 'bad inner value not detected'
+    assert not check_slot(
+        Slot.fromDict(dict(slot='slot', platforms=[
+            "x86_64-centos7-gcc9"
+        ]))), 'bad inner value not detected'
+
+    good_platforms = [
+        "x86_64-centos7-gcc9-do0",
+        'x86_64+avx2+fma-centos7-gcc8-opt+g',
+        'skylake_avx512-centos8-clang9-opt',
+        'x86_64_v3-centos8-clang9+cuda123-opt',
+        'armv8.1_a-centos7-gcc11-opt',
+        'armv8.1_a-el9-gcc13-opt',
+    ]
+    assert check_slot(Slot('slot', platforms=good_platforms)), 'false positive'
+    assert check_slot(
+        Slot.fromDict(dict(slot='slot',
+                           platforms=good_platforms))), 'false positive'
+
+
+def test_env():
+    assert not check_slot(Slot('slot', env="")), 'bad type not detected'
+    assert not check_slot(Slot('slot', env=[123
+                                            ])), 'bad inner type not detected'
+    assert not check_slot(Slot('slot', env=["dummy"
+                                            ])), 'bad inner value not detected'
+    assert not check_slot(Slot('slot', env=["123-"
+                                            ])), 'bad inner value not detected'
+    assert check_slot(Slot('slot', env=('a=1',
+                                        'BINARY_TAG=stuff'))), 'false positive'
-- 
GitLab