RESOLVED FIXED Bug 77248
run-webkit-tests calls out to webkit-build-directory twice 
https://bugs.webkit.org/show_bug.cgi?id=77248
Summary run-webkit-tests calls out to webkit-build-directory twice
Ojan Vafai
Reported 2012-01-27 14:53:26 PST
run-webkit-tests calls out to webkit-build-directory twice
Attachments
Patch (11.00 KB, patch)
2012-01-27 14:56 PST, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch (9.50 KB, patch)
2012-01-30 12:47 PST, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch (7.92 KB, patch)
2012-01-30 13:49 PST, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch (8.07 KB, patch)
2012-01-31 16:05 PST, Ojan Vafai 		dpranke: review+
dpranke: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2012-01-27 14:56:38 PST
Created attachment 124377 [details] Patch
Dirk Pranke
Comment 2 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.
Ojan Vafai
Comment 3 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.
Ojan Vafai
Comment 4 2012-01-30 12:47:49 PST
Created attachment 124584 [details] Patch
Ojan Vafai
Comment 5 2012-01-30 13:49:11 PST
Created attachment 124592 [details] Patch
Ojan Vafai
Comment 6 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.
Dirk Pranke
Comment 7 2012-01-30 14:06:38 PST
Comment on attachment 124592 [details] Patch very sneaky reusing the options argument like this. Good idea :).
WebKit Review Bot
Comment 8 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>
WebKit Review Bot
Comment 9 2012-01-30 17:05:05 PST
All reviewed patches have been landed. Closing bug.
Dirk Pranke
Comment 11 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?
Ojan Vafai
Comment 12 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.
Ojan Vafai
Comment 13 2012-01-31 16:05:02 PST
Created attachment 124839 [details] Patch
Ojan Vafai
Comment 14 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.
Dirk Pranke
Comment 15 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.
Ojan Vafai
Comment 16 2012-01-31 16:33:07 PST
Committed r106412: <http://trac.webkit.org/changeset/106412>
Note You need to log in before you can comment on or make changes to this bug.