Bug 77248 – run-webkit-tests calls out to webkit-build-directory twice

• Component: New Bugs
• Status: RESOLVED FIXED

[Ojan Vafai]

run-webkit-tests calls out to webkit-build-directory twice

[Ojan Vafai]

Created attachment 124377 [details]
Patch

[Dirk Pranke]

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 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]

Created attachment 124584 [details]
Patch

[Ojan Vafai]

Created attachment 124592 [details]
Patch

[Ojan Vafai]

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 on attachment 124592 [details]
Patch

very sneaky reusing the options argument like this. Good idea :).

[WebKit Review Bot]

Comment on attachment 124592 [details]
Patch

Clearing flags on attachment: 124592

Committed r106302: <http://trac.webkit.org/changeset/106302>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

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

[Dirk Pranke]

(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]

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]

Created attachment 124839 [details]
Patch

[Ojan Vafai]

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 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]

Committed r106412: <http://trac.webkit.org/changeset/106412>