Bug 208860 – [GTK] Switch EWS api-gtk bot from buildAndTest to testOnly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Diego Pino]

[GTK] Switch EWS api-gtk bot from buildAndTest to testOnly

[Diego Pino]

Created attachment 393146 [details]
Patch

[Carlos Alberto Lopez Perez]

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()

[Diego Pino]

Created attachment 393157 [details]
Patch

[Carlos Alberto Lopez Perez]

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

[Diego Pino]

Created attachment 393167 [details]
Patch

[Diego Pino]

I updated the patch. Instead of completely removing GTKBuildFactory and leave it as an empty class, like iOSBuildFactory.

[Carlos Alberto Lopez Perez]

(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

[Diego Pino]

Created attachment 393226 [details]
Patch

[Carlos Alberto Lopez Perez]

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

[Aakash Jain]

(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

[Diego Pino]

Created attachment 393242 [details]
Patch

[Carlos Alberto Lopez Perez]

Comment on attachment 393242 [details]
Patch

r=me
thanks!

[Aakash Jain]

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'

[Diego Pino]

Created attachment 393251 [details]
Patch

[EWS]

Committed r258271: <https://trac.webkit.org/changeset/258271>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 393251 [details].

[Radar WebKit Bug Importer]

<rdar://problem/60332911>