Diff
Modified: trunk/Tools/CISupport/build-webkit-org/loadConfig.py (273741 => 273742)
--- trunk/Tools/CISupport/build-webkit-org/loadConfig.py 2021-03-02 19:16:10 UTC (rev 273741)
+++ trunk/Tools/CISupport/build-webkit-org/loadConfig.py 2021-03-02 19:18:10 UTC (rev 273742)
@@ -20,24 +20,23 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import json
+import operator
import os
+import re
-from buildbot.worker import Worker
from buildbot.scheduler import AnyBranchScheduler, Triggerable, Nightly
from buildbot.schedulers.forcesched import FixedParameter, ForceScheduler, StringParameter, BooleanParameter
from buildbot.schedulers.filter import ChangeFilter
from buildbot.process import buildstep, factory, properties
+from buildbot.util import identifiers as buildbot_identifiers
+from buildbot.worker import Worker
from factories import *
-
-import re
-import json
-import operator
-
import wkbuild
trunk_filter = ChangeFilter(branch=["trunk", None])
-buildbot_identifiers_re = re.compile('^[a-zA-Z_-][a-zA-Z0-9_-]*$')
BUILDER_NAME_LENGTH_LIMIT = 70
STEP_NAME_LENGTH_LIMIT = 50
@@ -47,17 +46,20 @@
return max(requests, key=operator.attrgetter("submittedAt"))
-def loadBuilderConfig(c, is_test_mode_enabled=False):
+def loadBuilderConfig(c, is_test_mode_enabled=False, master_prefix_path='./'):
# FIXME: These file handles are leaked.
+ config = json.load(open(os.path.join(master_prefix_path, 'config.json')))
if is_test_mode_enabled:
passwords = {}
else:
- passwords = json.load(open('passwords.json'))
+ passwords = json.load(open(os.path.join(master_prefix_path, 'passwords.json')))
results_server_api_key = passwords.get('results-server-api-key')
if results_server_api_key:
os.environ['RESULTS_SERVER_API_KEY'] = results_server_api_key
- config = json.load(open('config.json'))
+ checkWorkersAndBuildersForConsistency(config, config['workers'], config['builders'])
+ checkValidSchedulers(config, config['schedulers'])
+
c['workers'] = [Worker(worker['name'], passwords.get(worker['name'], 'password'), max_builds=1) for worker in config['workers']]
c['schedulers'] = []
@@ -77,37 +79,24 @@
c['builders'] = []
for builder in config['builders']:
- for workerName in builder['workernames']:
- for worker in config['workers']:
- if worker['name'] != workerName or worker['platform'] == '*':
- continue
-
- if worker['platform'] != builder['platform']:
- raise Exception('Builder {} is for platform {} but has worker {} for platform {}!'.format(builder['name'], builder['platform'], worker['name'], worker['platform']))
- break
-
+ builder['tags'] = getTagsForBuilder(builder)
platform = builder['platform']
-
factoryName = builder.pop('factory')
factory = globals()[factoryName]
factorykwargs = {}
- for key in "platform", "configuration", "architectures", "triggers", "additionalArguments", "device_model":
+ for key in ['platform', 'configuration', 'architectures', 'triggers', 'additionalArguments', 'device_model']:
value = builder.pop(key, None)
if value:
factorykwargs[key] = value
- builder["factory"] = factory(**factorykwargs)
+ builder['factory'] = factory(**factorykwargs)
builder_name = builder['name']
- if len(builder_name) > BUILDER_NAME_LENGTH_LIMIT:
- raise Exception('Builder name "{}" is longer than maximum allowed by Buildbot ({} characters).'.format(builder_name, BUILDER_NAME_LENGTH_LIMIT))
- if not buildbot_identifiers_re.match(builder_name):
- raise Exception('Builder name "{}" is not a valid buildbot identifier.'.format(builder_name))
for step in builder["factory"].steps:
step_name = step.buildStep().name
if len(step_name) > STEP_NAME_LENGTH_LIMIT:
raise Exception('step name "{}" is longer than maximum allowed by Buildbot ({} characters).'.format(step_name, STEP_NAME_LENGTH_LIMIT))
- if not buildbot_identifiers_re.match(step_name):
+ if not buildbot_identifiers.ident_re.match(step_name):
raise Exception('step name "{}" is not a valid buildbot identifier.'.format(step_name))
if platform.startswith('mac'):
@@ -130,7 +119,6 @@
if (category in ('AppleMac', 'AppleWin', 'iOS')) and factoryName != 'BuildFactory':
builder['nextBuild'] = pickLatestBuild
- builder['tags'] = getTagsForBuilder(builder)
c['builders'].append(builder)
@@ -144,6 +132,91 @@
return wkbuild.should_build(self.platform, change.files)
+def checkValidWorker(worker):
+ if not worker:
+ raise Exception('Worker is None or Empty.')
+
+ if not worker.get('name'):
+ raise Exception('Worker "{}" does not have name defined.'.format(worker))
+
+ if not worker.get('platform'):
+ raise Exception('Worker {} does not have platform defined.'.format(worker['name']))
+
+
+def checkValidBuilder(config, builder):
+ if not builder:
+ raise Exception('Builder is None or Empty.')
+
+ if not builder.get('name'):
+ raise Exception('Builder "{}" does not have name defined.'.format(builder))
+
+ if not buildbot_identifiers.ident_re.match(builder['name']):
+ raise Exception('Builder name {} is not a valid buildbot identifier.'.format(builder['name']))
+
+ if len(builder['name']) > BUILDER_NAME_LENGTH_LIMIT:
+ raise Exception('Builder name {} is longer than maximum allowed by Buildbot ({} characters).'.format(builder['name'], BUILDER_NAME_LENGTH_LIMIT))
+
+ if 'configuration' in builder and builder['configuration'] not in ['debug', 'production', 'release']:
+ raise Exception('Invalid configuration: {} for builder: {}'.format(builder.get('configuration'), builder.get('name')))
+
+ if not builder.get('factory'):
+ raise Exception('Builder {} does not have factory defined.'.format(builder['name']))
+
+ 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 checkValidSchedulers(config, schedulers):
+ for scheduler in config.get('schedulers') or []:
+ if scheduler.get('type') == 'Triggerable':
+ if not isTriggerUsedByAnyBuilder(config, scheduler['name']) and 'build' not in scheduler['name'].lower():
+ 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.get('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):
+ result = None
+ for worker in workers:
+ if worker['name'] == worker_name:
+ if not result:
+ result = worker
+ else:
+ raise Exception('Duplicate worker entry found for {}.'.format(worker['name']))
+ return result
+
+ for worker in workers:
+ checkValidWorker(worker)
+
+ for builder in builders:
+ checkValidBuilder(config, builder)
+ for worker_name in builder['workernames']:
+ worker = _find_worker_with_name(workers, worker_name)
+ if worker is None:
+ raise Exception('Builder {} has worker {}, which is not defined in workers list!'.format(builder['name'], worker_name))
+
+ if worker['platform'] != builder['platform'] and worker['platform'] != '*' and builder['platform'] != '*':
+ raise Exception('Builder "{0}" is for platform "{1}", but has worker "{2}" for platform "{3}"!'.format(
+ builder['name'], builder['platform'], worker['name'], worker['platform']))
+
+
def getInvalidTags():
"""
We maintain a list of words which we do not want to display as tag in buildbot.
Modified: trunk/Tools/CISupport/build-webkit-org/loadConfig_unittest.py (273741 => 273742)
--- trunk/Tools/CISupport/build-webkit-org/loadConfig_unittest.py 2021-03-02 19:16:10 UTC (rev 273741)
+++ trunk/Tools/CISupport/build-webkit-org/loadConfig_unittest.py 2021-03-02 19:18:10 UTC (rev 273742)
@@ -1,6 +1,6 @@
#!/usr/bin/env python
#
-# Copyright (C) 2020 Apple Inc. All rights reserved.
+# Copyright (C) 2020-2021 Apple Inc. All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
@@ -29,11 +29,16 @@
import loadConfig
+
class ConfigDotJSONTest(unittest.TestCase):
def get_config(self):
cwd = os.path.dirname(os.path.abspath(__file__))
return json.load(open(os.path.join(cwd, 'config.json')))
+ def test_configuration(self):
+ cwd = os.path.dirname(os.path.abspath(__file__))
+ loadConfig.loadBuilderConfig({}, is_test_mode_enabled=True, master_prefix_path=cwd)
+
def test_builder_keys(self):
config = self.get_config()
valid_builder_keys = ['additionalArguments', 'architectures', 'builddir', 'configuration', 'description',
@@ -97,6 +102,98 @@
self.assertEqual(invalidTags, expectedTags)
+class TestcheckValidWorker(unittest.TestCase):
+ def test_invalid_worker(self):
+ with self.assertRaises(Exception) as context:
+ loadConfig.checkValidWorker({})
+ self.assertEqual(context.exception.args, ('Worker is None or Empty.',))
+
+ def test_worker_with_missing_name(self):
+ with self.assertRaises(Exception) as context:
+ loadConfig.checkValidWorker({'platform': 'mac-sierra'})
+ self.assertEqual(context.exception.args, ('Worker "{\'platform\': \'mac-sierra\'}" does not have name defined.',))
+
+ def test_worker_with_missing_platName(self):
+ with self.assertRaises(Exception) as context:
+ loadConfig.checkValidWorker({'name': 'ews101'})
+ self.assertEqual(context.exception.args, ('Worker ews101 does not have platform defined.',))
+
+ def test_valid_worker(self):
+ loadConfig.checkValidWorker({'name': 'ews101', 'platform': 'mac-sierra'})
+
+
+class TestcheckValidBuilder(unittest.TestCase):
+ def test_invalid_builder(self):
+ with self.assertRaises(Exception) as context:
+ 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'})
+ 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)', 'shortname': 'mac-wk2'})
+ 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, 'shortname': 'a'})
+ 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', 'shortname': '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', 'shortname': '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', 'shortname': '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', 'shortname': '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': 'macOS-High-Sierra-WK2-EWS', 'shortname': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory', 'platform': 'mac-sierra'})
+
+
+class TestcheckWorkersAndBuildersForConsistency(unittest.TestCase):
+ def __init__(self, *args, **kwargs):
+ self.WK2Builder = {'name': 'macOS-High-Sierra-WK2-EWS', 'shortname': 'mac-wk2', 'factory': 'WK2Factory', 'platform': 'mac-sierra', 'workernames': ['ews101', 'ews102']}
+ self.ews101 = {'name': 'ews101', 'platform': 'mac-sierra'}
+ self.ews102 = {'name': 'ews102', 'platform': 'ios-11'}
+ super(TestcheckWorkersAndBuildersForConsistency, self).__init__(*args, **kwargs)
+
+ def test_checkWorkersAndBuildersForConsistency(self):
+ with self.assertRaises(Exception) as context:
+ loadConfig.checkWorkersAndBuildersForConsistency({}, [], [self.WK2Builder])
+ self.assertEqual(context.exception.args, ('Builder macOS-High-Sierra-WK2-EWS 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])
+ self.assertEqual(context.exception.args, ('Builder "macOS-High-Sierra-WK2-EWS" is for platform "mac-sierra", but has worker "ews102" for platform "ios-11"!',))
+
+ def test_duplicate_worker(self):
+ with self.assertRaises(Exception) as context:
+ loadConfig.checkWorkersAndBuildersForConsistency({}, [self.ews101, self.ews101], [self.WK2Builder])
+ self.assertEqual(context.exception.args, ('Duplicate worker entry found for ews101.',))
+
+ def test_success(self):
+ loadConfig.checkWorkersAndBuildersForConsistency({}, [self.ews101, {'name': 'ews102', 'platform': 'mac-sierra'}], [self.WK2Builder])
+
+
if __name__ == '__main__':
from steps_unittest_old import BuildBotConfigLoader
BuildBotConfigLoader()._add_dependent_modules_to_sys_modules()
Modified: trunk/Tools/CISupport/ews-build/loadConfig.py (273741 => 273742)
--- trunk/Tools/CISupport/ews-build/loadConfig.py 2021-03-02 19:16:10 UTC (rev 273741)
+++ trunk/Tools/CISupport/ews-build/loadConfig.py 2021-03-02 19:18:10 UTC (rev 273742)
@@ -175,7 +175,7 @@
raise Exception('Builder {} has worker {}, which is not defined in workers list!'.format(builder['name'], worker_name))
if worker['platform'] != builder['platform'] and worker['platform'] != '*' and builder['platform'] != '*':
- raise Exception('Builder {0} is for platform {1}, but has worker {2} for platform {3}!'.format(
+ raise Exception('Builder "{0}" is for platform "{1}", but has worker "{2}" for platform "{3}"!'.format(
builder['name'], builder['platform'], worker['name'], worker['platform']))
Modified: trunk/Tools/CISupport/ews-build/loadConfig_unittest.py (273741 => 273742)
--- trunk/Tools/CISupport/ews-build/loadConfig_unittest.py 2021-03-02 19:16:10 UTC (rev 273741)
+++ trunk/Tools/CISupport/ews-build/loadConfig_unittest.py 2021-03-02 19:18:10 UTC (rev 273742)
@@ -224,7 +224,7 @@
def test_checkWorkersAndBuildersForConsistency1(self):
with self.assertRaises(Exception) as context:
loadConfig.checkWorkersAndBuildersForConsistency({}, [self.ews101, self.ews102], [self.WK2Builder])
- self.assertEqual(context.exception.args, ('Builder macOS-High-Sierra-WK2-EWS is for platform mac-sierra, but has worker ews102 for platform ios-11!',))
+ self.assertEqual(context.exception.args, ('Builder "macOS-High-Sierra-WK2-EWS" is for platform "mac-sierra", but has worker "ews102" for platform "ios-11"!',))
def test_duplicate_worker(self):
with self.assertRaises(Exception) as context:
Modified: trunk/Tools/ChangeLog (273741 => 273742)
--- trunk/Tools/ChangeLog 2021-03-02 19:16:10 UTC (rev 273741)
+++ trunk/Tools/ChangeLog 2021-03-02 19:18:10 UTC (rev 273742)
@@ -1,3 +1,45 @@
+2021-03-02 Aakash Jain <aakash_j...@apple.com>
+
+ Make build.webkit.org loadconfig similar to EWS
+ https://bugs.webkit.org/show_bug.cgi?id=222597
+
+ Reviewed by Jonathan Bedard.
+
+ * CISupport/build-webkit-org/loadConfig.py:
+ (loadBuilderConfig):
+ (checkValidWorker): Method to validate worker.
+ (checkValidBuilder): Method to validate builder.
+ (checkValidSchedulers): Method to validate schedulers.
+ (doesTriggerExist):
+ (isTriggerUsedByAnyBuilder):
+ (checkWorkersAndBuildersForConsistency):
+ * CISupport/build-webkit-org/loadConfig_unittest.py: Added unit-tests.
+ (ConfigDotJSONTest.test_configuration):
+ (TestcheckValidWorker):
+ (TestcheckValidWorker.test_invalid_worker):
+ (TestcheckValidWorker.test_worker_with_missing_name):
+ (TestcheckValidWorker.test_worker_with_missing_platName):
+ (TestcheckValidWorker.test_valid_worker):
+ (TestcheckValidBuilder):
+ (TestcheckValidBuilder.test_invalid_builder):
+ (TestcheckValidBuilder.test_builder_with_missing_name):
+ (TestcheckValidBuilder.test_builder_with_invalid_identifier):
+ (TestcheckValidBuilder.test_builder_with_extra_long_name):
+ (TestcheckValidBuilder.test_builder_with_invalid_configuration):
+ (TestcheckValidBuilder.test_builder_with_missing_factory):
+ (TestcheckValidBuilder.test_builder_with_missing_scheduler):
+ (TestcheckValidBuilder.test_builder_with_missing_platform):
+ (TestcheckValidBuilder.test_valid_builder):
+ (TestcheckWorkersAndBuildersForConsistency):
+ (TestcheckWorkersAndBuildersForConsistency.test_checkWorkersAndBuildersForConsistency):
+ (TestcheckWorkersAndBuildersForConsistency.test_checkWorkersAndBuildersForConsistency1):
+ (TestcheckWorkersAndBuildersForConsistency.test_duplicate_worker):
+ (TestcheckWorkersAndBuildersForConsistency.test_success):
+ * CISupport/ews-build/loadConfig.py:
+ (checkWorkersAndBuildersForConsistency): Added double-quotes in error message for better readability.
+ * CISupport/ews-build/loadConfig_unittest.py:
+ (TestcheckWorkersAndBuildersForConsistency.test_checkWorkersAndBuildersForConsistency1):
+
2021-03-02 Carlos Garcia Campos <cgar...@igalia.com>
REGRESSION(r263094): [GTK][WPE] API test /webkit/WebKitWebContext/languages is failing
