Bug 37588 – Make new-run-webkit-tests work for the Qt port

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

[Tor Arne Vestbø]

Make new-run-webkit-tests work for the Qt port

[Tor Arne Vestbø]

Created attachment 53346 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 53346 [details]
Patch

Technically we use " instead of ' (or try to in our python).

Eventually we need to set up a self._built_bin_path for the linux ports since both DumpRenderTree and ImageDiff will have this trouble.  For Qt it's "bin", for Gtk its "Programs", migth as well share the code.  But we can come back and do that later.

How does this detect the right configuration?  I still owe Adam Barth a patch to have WebKit.default_configuration read from $BUILD_DIR/Configuration instead of the current path hack that it does.

[Eric Seidel (no email)]

Comment on attachment 53346 [details]
Patch

I don't want to stand in your way, so r+, but consider the above comments.

[Tor Arne Vestbø]

(In reply to comment #2)
> (From update of attachment 53346 [details])
> Technically we use " instead of ' (or try to in our python).

Noted.

> Eventually we need to set up a self._built_bin_path for the linux ports since
> both DumpRenderTree and ImageDiff will have this trouble.  For Qt it's "bin",
> for Gtk its "Programs", migth as well share the code.  But we can come back and
> do that later.

You mean something that returns the suffix ("bin", "Programs"), or the full path, so that both the Qt and Gtk port implementations would concat _build_path() and the suffix?

Happy to do either as part of this patch just to make it cleaner.

> How does this detect the right configuration?  I still owe Adam Barth a patch
> to have WebKit.default_configuration read from $BUILD_DIR/Configuration instead
> of the current path hack that it does.

It's not picking up custom configurations, but at least on Linux webkit-build-directory actually works for Debug vs Release, so hard coding to "Release" seems wrong in hindsight.

[Eric Seidel (no email)]

(In reply to comment #4)
> > Eventually we need to set up a self._built_bin_path for the linux ports since
> > both DumpRenderTree and ImageDiff will have this trouble.  For Qt it's "bin",
> > for Gtk its "Programs", migth as well share the code.  But we can come back and
> > do that later.
> 
> You mean something that returns the suffix ("bin", "Programs"), or the full
> path, so that both the Qt and Gtk port implementations would concat
> _build_path() and the suffix?

I had figured a _build_path replacement for the places that have more specific needs.

like _built_bin_path("DumpRenderTree") would be the call for all ports, and on linux ports that would make the right _build_path("bin", *components) call, but on normal ports it would just call _build_path(*components).  Don't know.  Hadn't written it yet.  It's not essential for this change and is somethign we can clean up later when we have more data about our uses.

> It's not picking up custom configurations, but at least on Linux
> webkit-build-directory actually works for Debug vs Release, so hard coding to
> "Release" seems wrong in hindsight.

Sounds good to me.

[Eric Seidel (no email)]

Comment on attachment 53346 [details]
Patch

Thanks for the added explanation.

[WebKit Commit Bot]

Comment on attachment 53346 [details]
Patch

Rejecting patch 53346 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '53346', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
', 'commit', '--all', '-F', '-'], input=message)
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/system/executive.py", line 85, in run_command
    return Executive().run_command(*args, **kwargs)
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/system/executive.py", line 186, in run_command
    string_to_communicate = str(input)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in position 26: ordinal not in range(128)

[Eric Seidel (no email)]

Sorry again Tor.  I'll fix this today.

[Eric Seidel (no email)]

Sorry.  If bug 37765 isn't landed today, I'll land this change by hand.

[Csaba Osztrogonác]

(In reply to comment #8)
> Sorry again Tor.  I'll fix this today.
It would be good, because webkit-patch hates me too. :)

[Eric Seidel (no email)]

Comment on attachment 53346 [details]
Patch

Sorry again for the trouble.

[Eric Seidel (no email)]

Comment on attachment 53346 [details]
Patch

The fix for bug 37765 was rolled out.  So this will fail to land.  Hopefully I'll reland the fix tomorrow at which point I'll cq+ this again.

[Eric Seidel (no email)]

Comment on attachment 53346 [details]
Patch

Clearing flags on attachment: 53346

Committed r57965: <http://trac.webkit.org/changeset/57965>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

I just went ahead and landed it by hand (using my locally fixed copy of webkit-patch).  Again, apologies for the trouble.