Bug 77248 - run-webkit-tests calls out to webkit-build-directory twice
Summary: run-webkit-tests calls out to webkit-build-directory twice
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on: 77472
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-27 14:53 PST by Ojan Vafai
Modified: 2012-01-31 16:33 PST (History)
6 users (show)

See Also:


Attachments
Patch (11.00 KB, patch)
2012-01-27 14:56 PST, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch (9.50 KB, patch)
2012-01-30 12:47 PST, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch (7.92 KB, patch)
2012-01-30 13:49 PST, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch (8.07 KB, patch)
2012-01-31 16:05 PST, Ojan Vafai
dpranke: review+
dpranke: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2012-01-27 14:53:26 PST
run-webkit-tests calls out to webkit-build-directory twice
Comment 1 Ojan Vafai 2012-01-27 14:56:38 PST
Created attachment 124377 [details]
Patch
Comment 2 Dirk Pranke 2012-01-27 15:01:37 PST
Comment on attachment 124377 [details]
Patch

I'm gonna need to start at this one a bit, so I won't be able to get to it until later this afternoon. We need to be really careful about what data propagates across process boundaries due to the way processes are started on Windows. I think the config object is probably safe, but I need to double check. I'm also a bit surprised at all the changes you had to make to config_unittest.py, but I haven't looked closely yet.
Comment 3 Ojan Vafai 2012-01-27 16:25:31 PST
Comment on attachment 124377 [details]
Patch

As discussed on #webkit, we should rely on Config being pickleable, we should instead pass the webkit_base_dir and configurations to the subprocess and use those when creating the port to avoid having to call out to webkit-build-directory.
Comment 4 Ojan Vafai 2012-01-30 12:47:49 PST
Created attachment 124584 [details]
Patch
Comment 5 Ojan Vafai 2012-01-30 13:49:11 PST
Created attachment 124592 [details]
Patch
Comment 6 Ojan Vafai 2012-01-30 13:50:01 PST
Tweaked a bunch of the code in this last patch to make it more unittest-friendly so that it didn't require each test that created a Port instance to have a mock executive.
Comment 7 Dirk Pranke 2012-01-30 14:06:38 PST
Comment on attachment 124592 [details]
Patch

very sneaky reusing the options argument like this. Good idea :).
Comment 8 WebKit Review Bot 2012-01-30 17:05:00 PST
Comment on attachment 124592 [details]
Patch

Clearing flags on attachment: 124592

Committed r106302: <http://trac.webkit.org/changeset/106302>
Comment 9 WebKit Review Bot 2012-01-30 17:05:05 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Dirk Pranke 2012-01-31 13:29:16 PST
(In reply to comment #10)
> It appears that this patch broke Mac bots:
> http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/36811/steps/layout-test/logs/stdio

Odd. That path is right, isn't it?
Comment 12 Ojan Vafai 2012-01-31 15:31:09 PST
This was rolled out in http://trac.webkit.org/changeset/106389. There's a bug with setting --root causing us not to build DRT.
Comment 13 Ojan Vafai 2012-01-31 16:05:02 PST
Created attachment 124839 [details]
Patch
Comment 14 Ojan Vafai 2012-01-31 16:05:59 PST
The bug was that --root apparently implies --no-build, which meant we wouldn't build DRT. Instead, store the build directory in the existing --build-directory option.
Comment 15 Dirk Pranke 2012-01-31 16:20:15 PST
Comment on attachment 124839 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:383
> +            # Set --root here Since this modifies the options object used by the worker subprocesses,

Nit: fix the comment. Looks fine otherwise.
Comment 16 Ojan Vafai 2012-01-31 16:33:07 PST
Committed r106412: <http://trac.webkit.org/changeset/106412>