[webkit-reviews] review denied: [Bug 62615] nrwt: should clean up stale server processes from a previous run : [Attachment 97058] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 62615: nrwt: should clean up stale server processes from a previous run
https://bugs.webkit.org/show_bug.cgi?id=62615

Attachment 97058: Patch
https://bugs.webkit.org/attachment.cgi?id=97058&action=review

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

>>> Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py:181

>> 
>> How do we verify that this worked?
> 
> "working" here is defined as not raising an exception. If the first start()
worked, there's a server running. If the second start() gets called, it'll
check that it can bind to port 8000, and raise an error if it can't (see lines
170, 171, above for that test). In this case, it thinks the previous server is
now stale, and kills it.
> 
> If I wanted to be more thorough, I could do any of the following things:
verify that the first server was in fact killed, verify that a second process
was running, or verify that we can connect to the port. None of those seemed
strictly necessary given the other tests which ensure that things basically
work, but I can add them if you'd prefer.

Verifying that we can connect to the port seems sufficient and not a lot of
work.