Bug 188533 – [ews-build] Allow * as platform for builders and workers

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

When a worker (e.g.: webkit-misc) or builder (e.g.: Bindings-tests) do not need a very specific platform, we use * as the platform. However, loadConfig has a check to ensure that the platform matches for worker and platform. This prevents bot with * as platform to connect to multiple queues, or a builder with * as platform to connect to a worker with a specific platform.

We should allow the case when either worker or builder has * as platform, but the other has a specific platform. This would also help in testing (by allowing one bot to be used for testing multiple queues).

[Aakash Jain]

Created attachment 347042 [details]
Proposed patch

[Lucas Forschler]

Comment on attachment 347042 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347042&action=review

> Tools/BuildSlaveSupport/ews-build/loadConfig.py:162
> +            if worker['platform'] != builder['platform'] and worker['platform'] != '*' and builder['platform'] != '*':

I'm not sure what our supported configurations might be, so my initial thought is to break this logic...

Is it possible we would have a configuration where either worker['platform'] was * OR builder['platform'] = * but not both?
In that case, would we prefer to do something like
and (worker['platform'] != '*' or builder['platform'] != '*')

[EWS Watchlist]

Comment on attachment 347042 [details]
Proposed patch

Attachment 347042 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8882931

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
http/tests/security/local-video-source-from-remote.html

[EWS Watchlist]

Created attachment 347291 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Aakash Jain]

> > Tools/BuildSlaveSupport/ews-build/loadConfig.py:162
> > +            if worker['platform'] != builder['platform'] and worker['platform'] != '*' and builder['platform'] != '*':
> 
> I'm not sure what our supported configurations might be, so my initial thought is to break this logic...
> 
> Is it possible we would have a configuration where either worker['platform'] was * OR builder['platform'] = * but not both?

Yes, Recently I have been using such a configuration on https://ews-build.webkit-uat.org, especially because I have limited bots for testing and I want a bot to process builds from multiple queues (both mac and ios). In this case worker['platform'] is * since worker can connect to multiple queues. builder['platform'] remains ios/mac accordingly. e.g.: https://bugs.webkit.org/show_bug.cgi?id=194304


> In that case, would we prefer to do something like
> and (worker['platform'] != '*' or builder['platform'] != '*')
This 'if' is to detect an error case, so it should be 'and' of all three conditions. The one in the patch has been working fine in my testing for a while.

[WebKit Commit Bot]

Comment on attachment 347042 [details]
Proposed patch

Clearing flags on attachment: 347042

Committed r240999: <https://trac.webkit.org/changeset/240999>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47834681>