[webkit-reviews] review denied: [Bug 62251] webkitpy: add integration tests for new-run-webkit-httpd, stop calling shut_down_http_server : [Attachment 96353] ready for review, I think

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 62251: webkitpy: add integration tests for new-run-webkit-httpd, stop
calling shut_down_http_server
https://bugs.webkit.org/show_bug.cgi?id=62251

Attachment 96353: ready for review, I think
https://bugs.webkit.org/attachment.cgi?id=96353&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96353&action=review

>>> Tools/Scripts/new-run-webkit-httpd:76
>>> +		 port_obj._executive.kill_all('httpd')
>> 
>> What's the reason for using kill_all rather than the stop method here?
> 
> In this context, start() has not been called, and so stop has no pid that it
can use to kill. The older version of the code would call kill_all ; I'm just
making this explicit since this was the only place that *was* calling kill_all,
and it was forcing us to have a stupid hook into the port object.
> 
> A better fix would be for the script to be able to fetch the started pid from
a pidfile and use that; that will come in a later patch once all of this stuff
is cleaned up and consistent.

It looks like on Linux, the binary is called apache2.  Do we need to add that
here?  Also, it looks like on Windows you need the .exe or it doesn't kill the
process.  This new code doesn't seem like it'll work on all platforms.