[ews] Add build bots for JSC MIPS and ARMv7 archirectures
Created attachment 399624 [details] Patch
Created attachment 399718 [details] Patch
Created attachment 399723 [details] Patch
Created attachment 399724 [details] Patch
As I commented on slack, a few words on why we want these additional bots. For mips+arm EWS bots, the bulk of the time is spent in running tests on (slow) target devices, while cross-compilation only takes a few minutes. Having a build-only EWS would allow developers to see more quickly build issues (which I think are a good part of the issues that we see detected by EWS), and to get quick feedback after they rework a patch to address the issue. The idea is that these build-only bots will only take a couple minutes to process a patch, and their queue would almost always be empty, so that new patches would be processed quickly on these. Note also that this is in addition to our efforts to add more mips and arm devices to our EWS testbots so that these get faster too.
Comment on attachment 399724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399724&action=review Looks good in terms of functionality. My biggest concern is UI. This would add two more status-bubbles. Do we really need to add two more status-bubbles for this, and are we ok with that? > Tools/BuildSlaveSupport/ews-build/config.json:472 > + "shortname": "jsc-build-mips", This name will show up in status-bubble on bugzilla. Can we come up with a shorter name for all these status-bubbles? maybe: jsc-mips and jsc-mips-tests Also would need to update the shortnames in https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py#L46 so that they can start displaying in status-bubbles. > Tools/BuildSlaveSupport/ews-build/config.json:483 > + "shortname": "jsc-tests-mips", Can we come up with a shorter name for all these status-bubbles? > Tools/BuildSlaveSupport/ews-build/config.json:494 > + "shortname": "jsc-armv7-build", Can we come up with a shorter name for all these status-bubbles? > Tools/BuildSlaveSupport/ews-build/config.json:505 > + "shortname": "jsc-armv7-tests", Can we come up with a shorter name for all these status-bubbles? > Tools/BuildSlaveSupport/ews-build/config.json:601 > + "JSC-Build-ARMv7-32bits-EWS", "JSC-i386-32bits-EWS", "JSC-Build-MIPSEL-32bits-EWS", "JSC-Tests-EWS", "macOS-Mojave-Debug-Build-EWS", Nit: let's keep it in alphabetical order. > Tools/BuildSlaveSupport/ews-build/factories.py:140 > +class JSCTestOnlyFactory(Factory): Let's rename this to JSCTestsFactory. Can rename it later in separate patch as well. > Tools/BuildSlaveSupport/ews-build/factories.py:149 > class JSCTestsFactory(Factory): Let's rename this to JSCBuildAndTestsFactory. Can rename it later in separate patch as well.
Also it might be better to land patch in Bug 212803 first, make sure the new bots are working fine (no configuration issues), and then land this.
Created attachment 402221 [details] Patch
Comment on attachment 402221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402221&action=review r- due to services ews being red. > Tools/BuildSlaveSupport/ews-build/config.json:17 > + "name": "igalia-jsc32-mipsel-ews-03", Let's add it in a separate patch and land it first (to ensure that the bot is working fine without any configuration isssues). > Tools/BuildSlaveSupport/ews-build/config.json:491 > + "shortname": "jsc-mips-t", This name might be confusing for people. probably "jsc-mips-tests" is fine. > Tools/BuildSlaveSupport/ews-build/config.json:513 > + "shortname": "jsc-armv7-t", Ditto. This name might be confusing for people. probably "jsc-armv7-tests" is fine.
Created attachment 402267 [details] Patch
Created attachment 402558 [details] Patch
Comment on attachment 402558 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402558&action=review > Tools/BuildSlaveSupport/ews-build/config.json:472 > + "factory": "JSCTestsFactory", I believe this is unintentional. > Tools/BuildSlaveSupport/ews-build/factories.py:150 > + def __init__(self, platform, configuration='release', architectures=None, remotes=None, additionalArguments=None, runTests='true', **kwargs): runTests isn't required, please remove.
Created attachment 402623 [details] Patch
Comment on attachment 402623 [details] Patch In JSCBuildFactory.__init__ please add 'remotes' parameter and pass it to Factory.__init__ , otherwise triggers aren't working (as tested on uat instance https://ews-build.webkit-uat.org/#/builders/37/builds/47)
> In JSCBuildFactory.__init__ please add 'remotes' parameter Typo, 'remotes' -> 'triggers'
Created attachment 402823 [details] Patch
Created attachment 406851 [details] Patch
Created attachment 406852 [details] Patch
Comment on attachment 406852 [details] Patch look good to me. Also https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py would need updation as well in ALL_QUEUES and QUEUE_TRIGGERS. That can be done either in this or a separate patch.
Committed r265958: <https://trac.webkit.org/changeset/265958> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406852 [details].
<rdar://problem/67500231>
Restarted EWS to pick up this change.