[ews-build] loadConfig should ensure that the triggers are valid
authoraakash_jain@apple.com <aakash_jain@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jul 2018 23:22:22 +0000 (23:22 +0000)
committeraakash_jain@apple.com <aakash_jain@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jul 2018 23:22:22 +0000 (23:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188134

Reviewed by Lucas Forschler.

* BuildSlaveSupport/ews-build/loadConfig.py:
(checkValidBuilder): Added a check to ensure that the builder doesn't refernce non-existing scheduler.
(checkValidSchedulers): Ensures that the scheduler is not un-used.
* BuildSlaveSupport/ews-build/loadConfig_unittest.py: Updated unit-tests.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234395 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/BuildSlaveSupport/ews-build/loadConfig.py
Tools/BuildSlaveSupport/ews-build/loadConfig_unittest.py
Tools/ChangeLog

index 3d80d79..4fcc483 100644 (file)
@@ -40,7 +40,8 @@ STEP_NAME_LENGTH_LIMIT = 50
 def loadBuilderConfig(c, use_localhost_worker=False, master_prefix_path='./'):
     config = json.load(open(os.path.join(master_prefix_path, 'config.json')))
     passwords = json.load(open(os.path.join(master_prefix_path, 'passwords.json')))
-    checkWorkersAndBuildersForConsistency(config['workers'], config['builders'])
+    checkWorkersAndBuildersForConsistency(config, config['workers'], config['builders'])
+    checkValidSchedulers(config, config['schedulers'])
 
     c['workers'] = [Worker(worker['name'], passwords.get(worker['name'], 'password')) for worker in config['workers']]
     if use_localhost_worker:
@@ -93,7 +94,7 @@ def checkValidWorker(worker):
         raise Exception('Worker {} does not have platform defined.'.format(worker['name']))
 
 
-def checkValidBuilder(builder):
+def checkValidBuilder(config, builder):
     if not builder:
         raise Exception('Builder is None or Empty.')
 
@@ -115,8 +116,33 @@ def checkValidBuilder(builder):
     if not builder.get('platform'):
         raise Exception('Builder {} does not have platform defined.'.format(builder['name']))
 
+    for trigger in builder.get('triggers') or []:
+        if not doesTriggerExist(config, trigger):
+            raise Exception('Trigger: {} in builder {} does not exist in list of Trigerrable schedulers.'.format(trigger, builder['name']))
 
-def checkWorkersAndBuildersForConsistency(workers, builders):
+
+def checkValidSchedulers(config, schedulers):
+    for scheduler in config.get('schedulers') or []:
+        if scheduler.get('type') == 'Triggerable':
+            if not isTriggerUsedByAnyBuilder(config, scheduler['name']):
+                raise Exception('Trigger: {} is not used by any builder in config.json'.format(scheduler['name']))
+
+
+def doesTriggerExist(config, trigger):
+    for scheduler in config.get('schedulers') or []:
+        if scheduler['name'] == trigger:
+            return True
+    return False
+
+
+def isTriggerUsedByAnyBuilder(config, trigger):
+    for builder in config.get('builders'):
+        if trigger in (builder.get('triggers') or []):
+            return True
+    return False
+
+
+def checkWorkersAndBuildersForConsistency(config, workers, builders):
     def _find_worker_with_name(workers, worker_name):
         for worker in workers:
             if worker['name'] == worker_name:
@@ -127,7 +153,7 @@ def checkWorkersAndBuildersForConsistency(workers, builders):
         checkValidWorker(worker)
 
     for builder in builders:
-        checkValidBuilder(builder)
+        checkValidBuilder(config, builder)
         for worker_name in builder['workernames']:
             worker = _find_worker_with_name(workers, worker_name)
             if worker is None:
index 2757bc1..57511cd 100644 (file)
@@ -88,42 +88,47 @@ class TestcheckValidWorker(unittest.TestCase):
 class TestcheckValidBuilder(unittest.TestCase):
     def test_invalid_builder(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({})
+            loadConfig.checkValidBuilder({}, {})
         self.assertEqual(context.exception.args, ('Builder is None or Empty.',))
 
     def test_builder_with_missing_name(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({'platform': 'mac-sierra'})
+            loadConfig.checkValidBuilder({}, {'platform': 'mac-sierra'})
         self.assertEqual(context.exception.args, ('Builder "{\'platform\': \'mac-sierra\'}" does not have name defined.',))
 
     def test_builder_with_invalid_identifier(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({'name': 'mac-wk2(test)'})
+            loadConfig.checkValidBuilder({}, {'name': 'mac-wk2(test)'})
         self.assertEqual(context.exception.args, ('Builder name mac-wk2(test) is not a valid buildbot identifier.',))
 
     def test_builder_with_extra_long_name(self):
         longName = 'a' * 71
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({'name': longName})
+            loadConfig.checkValidBuilder({}, {'name': longName})
         self.assertEqual(context.exception.args, ('Builder name {} is longer than maximum allowed by Buildbot (70 characters).'.format(longName),))
 
     def test_builder_with_invalid_configuration(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({'name': 'mac-wk2', 'configuration': 'asan'})
+            loadConfig.checkValidBuilder({}, {'name': 'mac-wk2', 'configuration': 'asan'})
         self.assertEqual(context.exception.args, ('Invalid configuration: asan for builder: mac-wk2',))
 
     def test_builder_with_missing_factory(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({'name': 'mac-wk2', 'configuration': 'release'})
+            loadConfig.checkValidBuilder({}, {'name': 'mac-wk2', 'configuration': 'release'})
         self.assertEqual(context.exception.args, ('Builder mac-wk2 does not have factory defined.',))
 
+    def test_builder_with_missing_scheduler(self):
+        with self.assertRaises(Exception) as context:
+            loadConfig.checkValidBuilder({}, {'name': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory', 'platform': 'mac-sierra', 'triggers': ['api-tests-mac-ews']})
+        self.assertEqual(context.exception.args, ('Trigger: api-tests-mac-ews in builder mac-wk2 does not exist in list of Trigerrable schedulers.',))
+
     def test_builder_with_missing_platform(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkValidBuilder({'name': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory'})
+            loadConfig.checkValidBuilder({}, {'name': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory'})
         self.assertEqual(context.exception.args, ('Builder mac-wk2 does not have platform defined.',))
 
     def test_valid_builder(self):
-        loadConfig.checkValidBuilder({'name': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory', 'platform': 'mac-sierra'})
+        loadConfig.checkValidBuilder({}, {'name': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory', 'platform': 'mac-sierra'})
 
 
 class TestcheckWorkersAndBuildersForConsistency(unittest.TestCase):
@@ -135,16 +140,16 @@ class TestcheckWorkersAndBuildersForConsistency(unittest.TestCase):
 
     def test_checkWorkersAndBuildersForConsistency(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkWorkersAndBuildersForConsistency([], [self.WK2Builder])
+            loadConfig.checkWorkersAndBuildersForConsistency({}, [], [self.WK2Builder])
         self.assertEqual(context.exception.args, ('Builder mac-wk2 has worker ews101, which is not defined in workers list!',))
 
     def test_checkWorkersAndBuildersForConsistency1(self):
         with self.assertRaises(Exception) as context:
-            loadConfig.checkWorkersAndBuildersForConsistency([self.ews101, self.ews102], [self.WK2Builder])
+            loadConfig.checkWorkersAndBuildersForConsistency({}, [self.ews101, self.ews102], [self.WK2Builder])
         self.assertEqual(context.exception.args, ('Builder mac-wk2 is for platform mac-sierra, but has worker ews102 for platform ios-11!',))
 
     def test_success(self):
-        loadConfig.checkWorkersAndBuildersForConsistency([self.ews101, {'name': 'ews102', 'platform': 'mac-sierra'}], [self.WK2Builder])
+        loadConfig.checkWorkersAndBuildersForConsistency({}, [self.ews101, {'name': 'ews102', 'platform': 'mac-sierra'}], [self.WK2Builder])
 
 
 if __name__ == '__main__':
index 86d4490..62aca12 100644 (file)
@@ -1,3 +1,15 @@
+2018-07-30  Aakash Jain  <aakash_jain@apple.com>
+
+        [ews-build] loadConfig should ensure that the triggers are valid
+        https://bugs.webkit.org/show_bug.cgi?id=188134
+
+        Reviewed by Lucas Forschler.
+
+        * BuildSlaveSupport/ews-build/loadConfig.py:
+        (checkValidBuilder): Added a check to ensure that the builder doesn't refernce non-existing scheduler.
+        (checkValidSchedulers): Ensures that the scheduler is not un-used.
+        * BuildSlaveSupport/ews-build/loadConfig_unittest.py: Updated unit-tests.
+
 2018-07-27  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         Unreviewed, fix typo in test expectations and mark another test as timing out