Attachment 361548 Details for Bug 194461 – Proposed patch

[patch] Proposed patch

ews-build-shortname-unit-test.patch (text/plain), 7.98 KB, created by Aakash Jain on 2019-02-08 15:53:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Aakash Jain

Created: 2019-02-08 15:53:08 PST

Size: 7.98 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 241215)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-02-08  Aakash Jain  <aakash_jain@apple.com>
>+
>+        [ews-build] Ensure that every builder in config.json has short name
>+        https://bugs.webkit.org/show_bug.cgi?id=194461
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * BuildSlaveSupport/ews-build/loadConfig.py:
>+        * BuildSlaveSupport/ews-build/loadConfig_unittest.py: Updated unit-tests.
>+
> 2019-02-08  Jonathan Bedard  <jbedard@apple.com>
> 
>         webkitpy: CrashLogs.find_all_logs may return a string or a dictionary
>Index: Tools/BuildSlaveSupport/ews-build/loadConfig.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/loadConfig.py	(revision 241214)
>+++ Tools/BuildSlaveSupport/ews-build/loadConfig.py	(working copy)
>@@ -101,6 +101,9 @@ def checkValidBuilder(config, builder):
>     if not builder.get('name'):
>         raise Exception('Builder "{}" does not have name defined.'.format(builder))
> 
>+    if not builder.get('shortname'):
>+        raise Exception('Builder "{}" does not have short name defined. This name is needed for EWS status-bubbles.'.format(builder.get('name')))
>+
>     if not buildbot_identifiers.ident_re.match(builder['name']):
>         raise Exception('Builder name {} is not a valid buildbot identifier.'.format(builder['name']))
> 
>Index: Tools/BuildSlaveSupport/ews-build/loadConfig_unittest.py
>===================================================================
>--- Tools/BuildSlaveSupport/ews-build/loadConfig_unittest.py	(revision 241214)
>+++ Tools/BuildSlaveSupport/ews-build/loadConfig_unittest.py	(working copy)
>@@ -39,7 +39,7 @@ class ConfigDotJSONTest(unittest.TestCas
>         cwd = os.path.dirname(os.path.abspath(__file__))
>         config = json.load(open(os.path.join(cwd, 'config.json')))
>         valid_builder_keys = ['additionalArguments', 'architectures', 'builddir', 'configuration', 'description',
>-                              'defaultProperties', 'env', 'factory', 'locks', 'name', 'platform', 'properties', 'tags',
>+                              'defaultProperties', 'env', 'factory', 'locks', 'name', 'platform', 'properties', 'shortname', 'tags',
>                               'triggers', 'workernames', 'workerbuilddir']
>         for builder in config.get('builders', []):
>             for key in builder:
>@@ -107,44 +107,49 @@ class TestcheckValidBuilder(unittest.Tes
>             loadConfig.checkValidBuilder({}, {'platform': 'mac-sierra'})
>         self.assertEqual(context.exception.args, ('Builder "{\'platform\': \'mac-sierra\'}" does not have name defined.',))
> 
>+    def test_builder_with_missing_shortname(self):
>+        with self.assertRaises(Exception) as context:
>+            loadConfig.checkValidBuilder({}, {'platform': 'mac-sierra', 'name': 'mac-wk2(test)'})
>+        self.assertEqual(context.exception.args, ('Builder "mac-wk2(test)" does not have short name defined. This name is needed for EWS status-bubbles.',))
>+
>     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)', '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})
>+            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', 'configuration': 'asan'})
>+            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', 'configuration': 'release'})
>+            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', 'configuration': 'release', 'factory': 'WK2Factory', 'platform': 'mac-sierra', 'triggers': ['api-tests-mac-ews']})
>+            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', 'configuration': 'release', 'factory': 'WK2Factory'})
>+            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': 'mac-wk2', 'configuration': 'release', 'factory': 'WK2Factory', 'platform': 'mac-sierra'})
>+        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': 'mac-wk2', 'factory': 'WK2Factory', 'platform': 'mac-sierra', 'workernames': ['ews101', 'ews102']}
>+        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)
>@@ -152,12 +157,12 @@ class TestcheckWorkersAndBuildersForCons
>     def test_checkWorkersAndBuildersForConsistency(self):
>         with self.assertRaises(Exception) as context:
>             loadConfig.checkWorkersAndBuildersForConsistency({}, [], [self.WK2Builder])
>-        self.assertEqual(context.exception.args, ('Builder mac-wk2 has worker ews101, which is not defined in workers list!',))
>+        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 mac-wk2 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_success(self):
>         loadConfig.checkWorkersAndBuildersForConsistency({}, [self.ews101, {'name': 'ews102', 'platform': 'mac-sierra'}], [self.WK2Builder])

Flags:

lforschler: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194461: 361548