Bug 141928 - Merge run-launcher into run-minibrowser
Summary: Merge run-launcher into run-minibrowser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-23 15:08 PST by Carlos Alberto Lopez Perez
Modified: 2015-04-27 11:20 PDT (History)
9 users (show)

See Also:


Attachments
Patch (10.68 KB, patch)
2015-04-27 08:01 PDT, Csaba Osztrogonác
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2015-02-23 15:08:44 PST
The script Tools/Scripts/run-minibrowser don't works for the EFL or GTK ports.

I think this script should gain both a --gtk and --efl switches to execute the minibrowser from this two ports.
Comment 1 Csaba Osztrogonác 2015-02-23 23:52:24 PST
Yes, it seems it is Mac only now. But run-launcher 
already works, you can copy/paste the logic from it.
Comment 2 Carlos Garcia Campos 2015-02-24 01:15:24 PST
What is it used for? will it replace run-launcher eventually?
Comment 3 Carlos Alberto Lopez Perez 2015-02-24 03:26:29 PST
I didn't knew about run-launcher. Until now I'm executing the minibrowser manually like:

Tools/jhbuild/jhbuild-wrapper --gtk run WebKitBuild/Release/bin/MiniBrowser


What is the difference between run-launcher and run-minibrowser ? Should we merge both ?
Comment 4 Csaba Osztrogonác 2015-02-24 04:39:05 PST
It seems run-launcher is used only EFL and GTK nowadays,
run-webkit-tests calls it after tests (efl.py, gtk.py)
And run-minibrowser seems to be Mac only script now.

I'd prefer merging run-launcher to run-minibrowser.
Comment 5 Michael Catanzaro 2015-02-24 06:59:24 PST
Be sure to update the build-webkit script as well; it instructs users to use run-launcher without any arguments when the build is complete, but that just prints a message saying your OS is unsupported, so this would be a good time to fix that.
Comment 6 Csaba Osztrogonác 2015-04-27 08:01:03 PDT
Created attachment 251741 [details]
Patch

additionally removing obsolete FIXMEs
Comment 7 Csaba Osztrogonác 2015-04-27 08:20:04 PDT
Comment on attachment 251741 [details]
Patch

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

> Tools/Scripts/webkitpy/port/efl.py:-132
> -        # FIXME: We should find a way to share this implmentation with Gtk,
> -        # or teach run-launcher how to call run-safari and move this down to WebKitPort.

It is only a oneliner, there is no need to try to share it with GTK.

> Tools/Scripts/webkitpy/port/efl.py:-135
> -        # FIXME: old-run-webkit-tests also added ["-graphicssystem", "raster", "-style", "windows"]
> -        # FIXME: old-run-webkit-tests converted results_filename path for cygwin.

Just some comment, why can we remove them:
- -graphicssystem raster -style windows are Qt crufts
- cygwin: I can't remember if we ever supported Windows.
Comment 8 Darin Adler 2015-04-27 09:02:21 PDT
Comment on attachment 251741 [details]
Patch

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

> Tools/Scripts/webkitpy/port/efl.py:131
> +        self._run_script("run-minibrowser", ["file://%s" % results_filename])

This is not the right way to construct a URL from a local file name. Isn’t there some python library function that can turn a filename into a URL that can handle things like "%" characters?

> Tools/Scripts/webkitpy/port/gtk.py:202
> +        self._run_script("run-minibrowser", ["file://%s" % results_filename])

Ditto.
Comment 9 Csaba Osztrogonác 2015-04-27 11:15:31 PDT
webkitpy.common.system.path.abspath_to_uri does it,
win.py already use it in its show_results_html_file.

The results is same, but I'll change it before landing.
Comment 10 Csaba Osztrogonác 2015-04-27 11:20:54 PDT
Committed r183400: <http://trac.webkit.org/changeset/183400>