[webkit-reviews] review granted: [Bug 62027] nrwt: handle missing httpd cleanly : [Attachment 97952] proposed fix

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has granted Kristóf Kosztyó
<Kosztyo.Kristof at stud.u-szeged.hu>'s request for review:
Bug 62027: nrwt: handle missing httpd cleanly
https://bugs.webkit.org/show_bug.cgi?id=62027

Attachment 97952: proposed fix
https://bugs.webkit.org/attachment.cgi?id=97952&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=97952&action=review

Change basically looks fine but I'm cq-'ing so you can clean up the nits.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:192
> +	       if not self.check_httpd():

Minor nit:

if needs_http:
  return self.check_httpd()
return True

You don't need the print (which should be _log.error() anyway) since
check_httpd() will log messages.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:193
> +		   print 'No httpd found. Cannot run http tests.'

Nit. Change this to _log.error()

> Tools/Scripts/webkitpy/layout_tests/port/base.py:244
> +	       if logging:

I don't know if you were copying code from check_image_diff, but logging isn't
passed in to this method, so you're testing against the imported module, which
will always be true. Just remove the if.