Bug 208860 - [GTK] Switch EWS api-gtk bot from buildAndTest to testOnly
Summary: [GTK] Switch EWS api-gtk bot from buildAndTest to testOnly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Diego Pino
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-10 07:38 PDT by Diego Pino
Modified: 2020-03-11 11:35 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.00 KB, patch)
2020-03-10 07:39 PDT, Diego Pino
no flags Details | Formatted Diff | Diff
Patch (5.12 KB, patch)
2020-03-10 09:22 PDT, Diego Pino
no flags Details | Formatted Diff | Diff
Patch (5.97 KB, patch)
2020-03-10 11:14 PDT, Diego Pino
no flags Details | Formatted Diff | Diff
Patch (5.98 KB, patch)
2020-03-11 05:23 PDT, Diego Pino
no flags Details | Formatted Diff | Diff
Patch (7.05 KB, patch)
2020-03-11 07:57 PDT, Diego Pino
no flags Details | Formatted Diff | Diff
Patch (7.79 KB, patch)
2020-03-11 09:29 PDT, Diego Pino
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Pino 2020-03-10 07:38:00 PDT
[GTK] Switch EWS api-gtk bot from buildAndTest to testOnly
Comment 1 Diego Pino 2020-03-10 07:39:14 PDT
Created attachment 393146 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2020-03-10 08:04:12 PDT
Comment on attachment 393146 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/factories.py:191
> +class GTKTestFactory(Factory):
>      LayoutTestClass = None
>      APITestClass = None
>  
> +    def getProduct(self):
> +        self.addStep(InstallGtkDependencies())
> +        self.addStep(DownloadBuiltProduct())
> +        self.addStep(ExtractBuiltProduct())
> +
>      def __init__(self, platform, configuration=None, architectures=None, additionalArguments=None, **kwargs):
> -        GTKBuildFactory.__init__(self, platform=platform, configuration=configuration, architectures=architectures, buildOnly=True, additionalArguments=additionalArguments)
> +        Factory.__init__(self, platform=platform, configuration=configuration, architectures=architectures, buildOnly=False, additionalArguments=additionalArguments)
> +        self.getProduct()

I think we can now get rid of the class GTKTestFactory() and simply use the class TestFactory()
I think the only thing different we need to do in TestFactory() for GTK its to simply call InstallGtkDependencies() when platform == 'gtk' in the class init method before calling self.getProduct()
Comment 3 Diego Pino 2020-03-10 09:22:36 PDT
Created attachment 393157 [details]
Patch
Comment 4 Carlos Alberto Lopez Perez 2020-03-10 09:40:20 PDT
Comment on attachment 393157 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/config.json:522
> +      "factory": "GTKAPITestsFactory",

And this can be now simply APITestsFactory and we can get also rid of the class GTKAPITestsFactory

> Tools/BuildSlaveSupport/ews-build/loadConfig.py:33
>  from factories import (APITestsFactory, BindingsFactory, BuildFactory, CommitQueueFactory, Factory, GTKBuildFactory,

And maybe we can even get also rid of the class GTKBuildFactory and simply use BuildFactory by adding conditionally the step InstallGtkDependencies() to it after KillOldProcesses() when platform is gtk
Comment 5 Diego Pino 2020-03-10 11:14:09 PDT
Created attachment 393167 [details]
Patch
Comment 6 Diego Pino 2020-03-10 11:15:30 PDT
I updated the patch. Instead of completely removing GTKBuildFactory and leave it as an empty class, like iOSBuildFactory.
Comment 7 Carlos Alberto Lopez Perez 2020-03-11 04:17:06 PDT
(In reply to Diego Pino from comment #6)
> I updated the patch. Instead of completely removing GTKBuildFactory and
> leave it as an empty class, like iOSBuildFactory.

Interesting. Looks fine.

Please check the EWS red bubbles and fix them
Comment 8 Diego Pino 2020-03-11 05:23:51 PDT
Created attachment 393226 [details]
Patch
Comment 9 Carlos Alberto Lopez Perez 2020-03-11 06:32:22 PDT
Comment on attachment 393226 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/config.json:609
> +    },
> +    {
> +      "type": "Triggerable",
> +      "name": "api-tests-gtk-ews",
> +      "builderNames": [
> +        "API-Tests-GTK-EWS"
> +      ]

Now that you are adding here the queue "API-Tests-GTK-EWS" as Triggerable type to the schedulers list, you have to remove it from the Try_Userpass type above.
The queue should be only once on the "schedulers" list.
I miss a test for this on the unit tests
Comment 10 Aakash Jain 2020-03-11 07:25:51 PDT
(In reply to Carlos Alberto Lopez Perez from comment #9)
> I miss a test for this on the unit tests
Adding in https://bugs.webkit.org/show_bug.cgi?id=208917
Comment 11 Diego Pino 2020-03-11 07:57:40 PDT
Created attachment 393242 [details]
Patch
Comment 12 Carlos Alberto Lopez Perez 2020-03-11 08:02:40 PDT
Comment on attachment 393242 [details]
Patch

r=me
thanks!
Comment 13 Aakash Jain 2020-03-11 08:25:36 PDT
LGTM.
Can you also add following to QUEUE_TRIGGERS in https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py#L47

'api-gtk': 'gtk'
Comment 14 Diego Pino 2020-03-11 09:29:08 PDT
Created attachment 393251 [details]
Patch
Comment 15 EWS 2020-03-11 11:34:24 PDT
Committed r258271: <https://trac.webkit.org/changeset/258271>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 393251 [details].
Comment 16 Radar WebKit Bug Importer 2020-03-11 11:35:13 PDT
<rdar://problem/60332911>