| Summary: | run-webkit-tests calls out to webkit-build-directory twice | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||||||
| Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | abarth, dpranke, eric, rniwa, tony, webkit.review.bot | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | 77472 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Ojan Vafai
2012-01-27 14:53:26 PST
Created attachment 124377 [details]
Patch
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 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.
Created attachment 124584 [details]
Patch
Created attachment 124592 [details]
Patch
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 on attachment 124592 [details]
Patch
very sneaky reusing the options argument like this. Good idea :).
Comment on attachment 124592 [details] Patch Clearing flags on attachment: 124592 Committed r106302: <http://trac.webkit.org/changeset/106302> All reviewed patches have been landed. Closing bug. 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 (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? This was rolled out in http://trac.webkit.org/changeset/106389. There's a bug with setting --root causing us not to build DRT. Created attachment 124839 [details]
Patch
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 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. Committed r106412: <http://trac.webkit.org/changeset/106412> |