new-run-webkit-tests: read stderr from chromium DRT separately
Created attachment 89875 [details] Patch
easily the least portable python I've ever written, but it seems to work ...
on unix, it might be better to just mark the socket as non-blocking and do a select(), but the patch is posted for the record :).
Created attachment 89880 [details] use non-blocking sockets on unix instead
Comment on attachment 89880 [details] use non-blocking sockets on unix instead View in context: https://bugs.webkit.org/attachment.cgi?id=89880&action=review Looks fine to me. Can't say I really understand what _read_stderr is doing. Maybe tony or someone with my python experience can comment. I don't think we need to block getting this on on that though. The only issue, I'm pretty sure some tests currently have stderr output in their expected results, so you'll probably need to rebaseline them with this patch. > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:578 > + def _read_stderr(self): > + if sys.platform in ('win32', 'cygwin'): > + import msvcrt > + import win32pipe > + > + fd = self._proc.stderr.fileno() > + osf = msvcrt.get_osfhandle(fd) > + _, avail, _ = win32pipe.PeekNamedPipe(osf, 0) > + if avail: > + return self._proc.stderr.read(avail) > + return '' > + else: > + if not self._stderr_is_nonblocking: > + import fcntl > + import os > + fd = self._proc.stderr.fileno() > + fl = fcntl.fcntl(fd, fcntl.F_GETFL) > + fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK) > + self._stderr_is_nonblocking = True > + return self._proc.stderr.read() this doesn't seem chromium specific. don't other ports need this code as well?
(In reply to comment #5) > (From update of attachment 89880 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=89880&action=review > > Looks fine to me. Can't say I really understand what _read_stderr is doing. Maybe tony or someone with my python experience can comment. I don't think we need to block getting this on on that though. > > The only issue, I'm pretty sure some tests currently have stderr output in their expected results, so you'll probably need to rebaseline them with this patch. > Yeah, that wouldn't surprise me. > > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:578 > > + def _read_stderr(self): > > + if sys.platform in ('win32', 'cygwin'): > > + import msvcrt > > + import win32pipe > > + > > + fd = self._proc.stderr.fileno() > > + osf = msvcrt.get_osfhandle(fd) > > + _, avail, _ = win32pipe.PeekNamedPipe(osf, 0) > > + if avail: > > + return self._proc.stderr.read(avail) > > + return '' > > + else: > > + if not self._stderr_is_nonblocking: > > + import fcntl > > + import os > > + fd = self._proc.stderr.fileno() > > + fl = fcntl.fcntl(fd, fcntl.F_GETFL) > > + fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK) > > + self._stderr_is_nonblocking = True > > + return self._proc.stderr.read() > > this doesn't seem chromium specific. don't other ports need this code as well? All of the other ports use the server_process.py code (via port/webkit.py) which uses select and non-blocking reads. As a result we don't have any other ports that will actually work correctly on windows (see bug 38756). As a part of that, we'll likely need to re-use this code there as well in some form.
This will probably allow us to figure out whether or not to fix bug 37426, as well.
Committed r84111: <http://trac.webkit.org/changeset/84111>
Comment on attachment 89880 [details] use non-blocking sockets on unix instead View in context: https://bugs.webkit.org/attachment.cgi?id=89880&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:562 > + import msvcrt > + import win32pipe Do both of these exist in cygwin python? I thought cygwin python could import fcntl so maybe it can go through the other code path?
This got reverted in http://trac.webkit.org/changeset/84128 but the bug wasn't reopened. I think this is (at least part of) the problem with https://bugs.webkit.org/show_bug.cgi?id=66806
Reopening, patch was reverted
closing, this is fixed w/ r115490 and r115903.