Bug 37588

Summary: Make new-run-webkit-tests work for the Qt port
Product: WebKit Reporter: Tor Arne Vestbø <vestbo>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on: 37765    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Tor Arne Vestbø 2010-04-14 11:21:01 PDT
Make new-run-webkit-tests work for the Qt port
Comment 1 Tor Arne Vestbø 2010-04-14 11:23:01 PDT
Created attachment 53346 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-04-14 11:33:53 PDT
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.
Comment 3 Eric Seidel (no email) 2010-04-14 11:34:29 PDT
Comment on attachment 53346 [details]
Patch

I don't want to stand in your way, so r+, but consider the above comments.
Comment 4 Tor Arne Vestbø 2010-04-14 12:34:39 PDT
(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.
Comment 5 Eric Seidel (no email) 2010-04-14 13:21:36 PDT
(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.
Comment 6 Eric Seidel (no email) 2010-04-14 13:22:06 PDT
Comment on attachment 53346 [details]
Patch

Thanks for the added explanation.
Comment 7 WebKit Commit Bot 2010-04-15 02:44:09 PDT
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)
Comment 8 Eric Seidel (no email) 2010-04-16 11:53:05 PDT
Sorry again Tor.  I'll fix this today.
Comment 9 Eric Seidel (no email) 2010-04-19 12:24:10 PDT
Sorry.  If bug 37765 isn't landed today, I'll land this change by hand.
Comment 10 Csaba Osztrogonác 2010-04-20 03:58:02 PDT
(In reply to comment #8)
> Sorry again Tor.  I'll fix this today.
It would be good, because webkit-patch hates me too. :)
Comment 11 Eric Seidel (no email) 2010-04-20 12:57:42 PDT
Comment on attachment 53346 [details]
Patch

Sorry again for the trouble.
Comment 12 Eric Seidel (no email) 2010-04-21 03:48:59 PDT
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.
Comment 13 Eric Seidel (no email) 2010-04-21 04:03:31 PDT
Comment on attachment 53346 [details]
Patch

Clearing flags on attachment: 53346

Committed r57965: <http://trac.webkit.org/changeset/57965>
Comment 14 Eric Seidel (no email) 2010-04-21 04:03:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Eric Seidel (no email) 2010-04-21 04:04:41 PDT
I just went ahead and landed it by hand (using my locally fixed copy of webkit-patch).  Again, apologies for the trouble.