Bug 133157 – [GTK] Allow to run the tests on the native X display (instead of running them inside of Xvfb)

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

This is useful for the performance bot #132862. Running the tests inside of Xvfb don't seems a good idea to get real-world performance metrics.

EFL port implemented a feature like this on #131036

[Carlos Alberto Lopez Perez]

Created attachment 231957 [details]
Patch

[Benjamin Poulain]

Comment on attachment 231957 [details]
Patch

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

Quick questions

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131
> +            optparse.make_option("--nocheck-sys-deps", action="store_true", default=False,
> +            help="Don't check the system dependencies (themes, fonts, ...)"),

Is this really necessary?

> Tools/Scripts/webkitpy/port/driver.py:493
> +    def check_driver(port):

Where is it used?

[Carlos Alberto Lopez Perez]

(In reply to comment #2)
> (From update of attachment 231957 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231957&action=review
> 
> Quick questions
> 
> > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131
> > +            optparse.make_option("--nocheck-sys-deps", action="store_true", default=False,
> > +            help="Don't check the system dependencies (themes, fonts, ...)"),
> 
> Is this really necessary?
> 
Not strictly necessary, but is consistent with the script for running the layout tests run-webkit-tests:

$ Tools/Scripts/run-webkit-tests --help|grep sys
    --nocheck-sys-deps  Don't check the system dependencies (themes)

I can drop it, if you think is better.

> > Tools/Scripts/webkitpy/port/driver.py:493
> > +    def check_driver(port):
> 
> Where is it used?

Is used on the drivers Xvfb, Weston and this new one (xorgdriver):

Tools/Scripts/webkitpy/port/xvfbdriver.py
Tools/Scripts/webkitpy/port/westondriver.py

[Benjamin Poulain]

Comment on attachment 231957 [details]
Patch

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

>>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131
>>> +            help="Don't check the system dependencies (themes, fonts, ...)"),
>> 
>> Is this really necessary?
> 
> Not strictly necessary, but is consistent with the script for running the layout tests run-webkit-tests:
> 
> $ Tools/Scripts/run-webkit-tests --help|grep sys
>     --nocheck-sys-deps  Don't check the system dependencies (themes)
> 
> I can drop it, if you think is better.

Unless it is necessary, let's drop this.
It is trivial to add later if needed.

[Carlos Alberto Lopez Perez]

Created attachment 232074 [details]
Patch

[Carlos Alberto Lopez Perez]

(In reply to comment #4)
> (From update of attachment 231957 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231957&action=review
> 
> >>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:131
> >>> +            help="Don't check the system dependencies (themes, fonts, ...)"),
> >> 
> >> Is this really necessary?
> > 
> > Not strictly necessary, but is consistent with the script for running the layout tests run-webkit-tests:
> > 
> > $ Tools/Scripts/run-webkit-tests --help|grep sys
> >     --nocheck-sys-deps  Don't check the system dependencies (themes)
> > 
> > I can drop it, if you think is better.
> 
> Unless it is necessary, let's drop this.
> It is trivial to add later if needed.

Thanks for your review :)

I have re-uploaded the patch without the "--nocheck-sys-deps" option

[Carlos Alberto Lopez Perez]

Created attachment 232077 [details]
Patch

[Carlos Alberto Lopez Perez]

(In reply to comment #7)
> Created an attachment (id=232077) [details]
> Patch

I have re-uploaded it again with the "Reviewed by" bit pre-filled on the Changelog.

Can someone cq+ it for me please?

Thanks!

[WebKit Commit Bot]

Comment on attachment 232077 [details]
Patch

Clearing flags on attachment: 232077

Committed r169344: <http://trac.webkit.org/changeset/169344>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.