reformat run-webkit-tests --driver-logging output to show ctrl chars.

reformat run-webkit-tests --driver-logging output to show ctrl chars.

Turns out that if you don't make the nonprintable characters visible,
you'll miss valuable information like whether there are LFs or CRLFs
in your output. This should also make dumping PNG data less likely
to completely mess up your terminal.

Also this patch adds a test for --driver-logging, but disables it
as the mock-ports seem to be somewhat broken at the moment. I'll fix
that issue in a separate bug.

R=ojan@chromium.org
BUG=276728
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156519

[Dirk Pranke, 2013-08-21 22:31:43]

New sample output (excerpted):

e:\src\c\src\out\Release\content_shell.exe --dump-render-tree -
 IN:
e:\\src\\c\\src\\third_party\\WebKit\\LayoutTests\\fast/html/article-element.html'--pixel-test\n
ERR:
[8108:10896:0821/153021:87904332:ERROR:desktop_root_window_host_win.cc(703)] NOT
IMPLEMENTED\r\n
OUT: Content-Type: text/plain\r\n
OUT: Various tests for the article element.\r\n
OUT: \r\n
OUT: On success, you will see a series of "PASS" messages, followed by "TEST
COMPLETE".\r\n
OUT: \r\n
OUT: \r\n
OUT: <article> closes <p>:\r\n
OUT: PASS article1.parentNode.nodeName == "p" is false\r\n
OUT: <p> does not close <article>:\r\n
OUT: PASS p1.parentNode.nodeName is "ARTICLE"\r\n
OUT: <article> can be nested inside <article>:\r\n
OUT: PASS article3.parentNode.id is "article2"\r\n
OUT: Residual style:\r\n
OUT: PASS getWeight("article4") is "bold"\r\n
OUT: PASS getWeight("span1") is "bold"\r\n
OUT: FormatBlock:\r\n
OUT: PASS document.getElementById("span2").parentNode.nodeName is "ARTICLE"\r\n
OUT: PASS successfullyParsed is true\r\n
OUT: \r\n
OUT: TEST COMPLETE\r\n
OUT: \r\n
OUT: #EOF\r\n
OUT: #EOF\r\n
ERR: #EOF\r\n
 IN: ^D
The test ran as expected.

[ojan, 2013-08-21 22:42:07]

lgtm


On Wed, Aug 21, 2013 at 3:31 PM, <dpranke@chromium.org> wrote:

> Reviewers: ojan,
>
> Message:
> New sample output (excerpted):
>
> e:\src\c\src\out\Release\**content_shell.exe --dump-render-tree -
>  IN:
> e:\\src\\c\\src\\third_party\\**WebKit\\LayoutTests\\fast/**
> html/article-element.html'--**pixel-test\n
> ERR:
>
[8108:10896:0821/153021:**87904332:ERROR:desktop_root_**window_host_win.cc(703)]
> NOT
> IMPLEMENTED\r\n
> OUT: Content-Type: text/plain\r\n
> OUT: Various tests for the article element.\r\n
> OUT: \r\n
> OUT: On success, you will see a series of "PASS" messages, followed by
> "TEST
> COMPLETE".\r\n
> OUT: \r\n
> OUT: \r\n
> OUT: <article> closes <p>:\r\n
> OUT: PASS article1.parentNode.nodeName == "p" is false\r\n
> OUT: <p> does not close <article>:\r\n
> OUT: PASS p1.parentNode.nodeName is "ARTICLE"\r\n
> OUT: <article> can be nested inside <article>:\r\n
> OUT: PASS article3.parentNode.id is "article2"\r\n
> OUT: Residual style:\r\n
> OUT: PASS getWeight("article4") is "bold"\r\n
> OUT: PASS getWeight("span1") is "bold"\r\n
> OUT: FormatBlock:\r\n
> OUT: PASS document.getElementById("**span2").parentNode.nodeName is
> "ARTICLE"\r\n
> OUT: PASS successfullyParsed is true\r\n
> OUT: \r\n
> OUT: TEST COMPLETE\r\n
> OUT: \r\n
> OUT: #EOF\r\n
> OUT: #EOF\r\n
> ERR: #EOF\r\n
>  IN: ^D
> The test ran as expected.
>
>
> Description:
> reformat run-webkit-tests --driver-logging output to show ctrl chars.
>
> Turns out that if you don't make the nonprintable characters visible,
> you'll miss valuable information like whether there are LFs or CRLFs
> in your output. This should also make dumping PNG data less likely
> to completely mess up your terminal.
>
> Also this patch adds a test for --driver-logging, but disables it
> as the mock-ports seem to be somewhat broken at the moment. I'll fix
> that issue in a separate bug.
>
> R=ojan@chromium.org
> BUG=276728
>
> Please review this at
https://codereview.chromium.**org/23093014/<https://codereview.chromium.org/2...
>
> SVN Base: https://chromium.googlesource.**com/chromium/blink.git@eof_**
> crlf <https://chromium.googlesource.com/chromium/blink.git@eof_crlf>
>
> Affected files:
>   M Tools/Scripts/webkitpy/layout_**tests/port/server_process.py
>   M Tools/Scripts/webkitpy/layout_**tests/port/server_process_**
> unittest.py
>   M Tools/Scripts/webkitpy/layout_**tests/run_webkit_tests_**
> integrationtest.py
>
>
> Index: Tools/Scripts/webkitpy/layout_**tests/port/server_process.py
> diff --git a/Tools/Scripts/webkitpy/**layout_tests/port/server_**process.py
> b/Tools/Scripts/webkitpy/**layout_tests/port/server_**process.py
> index 70d0754599c7c713dab43abb0dc6e7**39166c03ca..**
> 58ccb5c7ad317118f375c55ba8fb9e**1c88cc6f19 100644
> --- a/Tools/Scripts/webkitpy/**layout_tests/port/server_**process.py
> +++ b/Tools/Scripts/webkitpy/**layout_tests/port/server_**process.py
> @@ -30,6 +30,7 @@
>
>  import errno
>  import logging
> +import re
>  import signal
>  import sys
>  import time
> @@ -58,6 +59,19 @@ from webkitpy.common.system.**executive import
> ScriptError
>  _log = logging.getLogger(__name__)
>
>
> +_trailing_spaces_re = re.compile('(.*[^ ])?( +)$')
> +
> +
> +def quote_data(data):
> +    txt = repr(data).replace('\\n', '\\n\n')[1:-1]
> +    lines = []
> +    for l in txt.splitlines():
> +        m = _trailing_spaces_re.match(l)
> +        if m:
> +            l = m.group(1) + m.group(2).replace(' ', '\x20')
> +        lines.append(l)
> +    return lines
> +
>  class ServerProcess(object):
>      """This class provides a wrapper around a subprocess that
>      implements a simple request/response usage model. The primary benefit
> @@ -121,7 +135,7 @@ class ServerProcess(object):
>              env_str = ''
>              if self._env:
>                  env_str += '\n'.join("%s=%s" % (k, v) for k, v in
> self._env.items()) + '\n'
> -            _log.info('CMD: """\n%s%s\n"""', env_str,
> _quote_cmd(self._cmd))
> +            _log.info('CMD: \n%s%s\n', env_str, _quote_cmd(self._cmd))
>          self._proc = self._host.executive.popen(**self._cmd,
> stdin=self._host.executive.**PIPE,
>              stdout=self._host.executive.**PIPE,
>              stderr=self._host.executive.**PIPE,
> @@ -162,8 +176,7 @@ class ServerProcess(object):
>          if not self._proc:
>              self._start()
>          try:
> -            if self._logging:
> -                _log.info(' IN: """%s"""', bytes)
> +            self._log_data(' IN', bytes)
>              self._proc.stdin.write(bytes)
>          except IOError, e:
>              self.stop(0.0)
> @@ -224,6 +237,11 @@ class ServerProcess(object):
>          _log.info('')
>          _log.info(message)
>
> +    def _log_data(self, prefix, data):
> +        if self._logging and data and len(data):
> +            for line in quote_data(data):
> +                _log.info('%s: %s', prefix, line)
> +
>      def _handle_timeout(self):
>          self.timed_out = True
>          self._port.sample_process(**self._name, self._proc.pid)
> @@ -267,16 +285,14 @@ class ServerProcess(object):
>                  data = self._proc.stdout.read()
>                  if not data and not stopping and
> (self._treat_no_data_as_crash or self._proc.poll()):
>                      self._crashed = True
> -                if self._logging and len(data):
> -                    _log.info('OUT: """%s"""', data)
> +                self._log_data('OUT', data)
>                  self._output += data
>
>              if err_fd in read_fds:
>                  data = self._proc.stderr.read()
>                  if not data and not stopping and
> (self._treat_no_data_as_crash or self._proc.poll()):
>                      self._crashed = True
> -                if self._logging and len(data):
> -                    _log.info('ERR: """%s"""', data)
> +                self._log_data('ERR', data)
>                  self._error += data
>          except IOError, e:
>              # We can ignore the IOErrors because we will detect if the
> subporcess crashed
> @@ -292,7 +308,9 @@ class ServerProcess(object):
>          err_fh = msvcrt.get_osfhandle(self._**proc.stderr.fileno())
>          while (self._proc.poll() is None) and (now < deadline):
>              output = self._non_blocking_read_win32(**out_fh)
> +            self._log_data('OUT', output)
>              error = self._non_blocking_read_win32(**err_fh)
> +            self._log_data('ERR', error)
>              if output or error:
>                  if output:
>                      self._output += output
> @@ -352,7 +370,7 @@ class ServerProcess(object):
>          now = time.time()
>          if self._proc.stdin:
>              if self._logging:
> -                _log.info(' IN: """^D"""')
> +                _log.info(' IN: ^D')
>              self._proc.stdin.close()
>              self._proc.stdin = None
>          killed = False
> Index: Tools/Scripts/webkitpy/layout_**tests/port/server_process_**
> unittest.py
> diff --git
a/Tools/Scripts/webkitpy/**layout_tests/port/server_**process_unittest.py
> b/Tools/Scripts/webkitpy/**layout_tests/port/server_**process_unittest.py
> index e17d7a9e762e730dbbd526004f7d21**33ab5221c4..**
> 0b1db7391737993258a6a42aed67d1**cfb5f53356 100644
> --- a/Tools/Scripts/webkitpy/**layout_tests/port/server_**
> process_unittest.py
> +++ b/Tools/Scripts/webkitpy/**layout_tests/port/server_**
> process_unittest.py
> @@ -37,7 +37,6 @@ from webkitpy.common.system.**systemhost_mock import
> MockSystemHost
>  from webkitpy.common.system.**outputcapture import OutputCapture
>
>
> -
>  class TrivialMockPort(object):
>      def __init__(self):
>          self.host = MockSystemHost()
> @@ -147,3 +146,24 @@ class TestServerProcess(unittest.**TestCase):
>          self.assertTrue(server_**process.has_crashed())
>          self.assertIsNone(server_**process._proc)
>          self.assertEqual(server_**process.broken_pipes,
> [server_process.stdin])
> +
> +
> +class TestQuoteData(unittest.**TestCase):
> +    def test_plain(self):
> +        qd = server_process.quote_data
> +        self.assertEqual(qd("foo"), ["foo"])
> +
> +    def test_trailing_spaces(self):
> +        qd = server_process.quote_data
> +        self.assertEqual(qd("foo  "),
> +                         ["foo\x20\x20"])
> +
> +    def test_newlines(self):
> +        qd = server_process.quote_data
> +        self.assertEqual(qd("foo \nbar\n"),
> +                         ["foo\x20\\n", "bar\\n"])
> +
> +    def test_binary_data(self):
> +        qd = server_process.quote_data
> +        self.assertEqual(qd("\x00\**x01ab"),
> +                         ["\\x00\\x01ab"])
> Index: Tools/Scripts/webkitpy/layout_**tests/run_webkit_tests_**
> integrationtest.py
> diff --git
a/Tools/Scripts/webkitpy/**layout_tests/run_webkit_tests_**integrationtest.py
> b/Tools/Scripts/webkitpy/**layout_tests/run_webkit_tests_**
> integrationtest.py
> index 4b9a8663c3ef7d320cecfd7752f2c3**57e214a419..**
> cfb3e6f614570a17d86ac397b95141**18e8e548a8 100644
> --- a/Tools/Scripts/webkitpy/**layout_tests/run_webkit_tests_**
> integrationtest.py
> +++ b/Tools/Scripts/webkitpy/**layout_tests/run_webkit_tests_**
> integrationtest.py
> @@ -794,6 +794,13 @@ class RunTest(unittest.TestCase, StreamTestingMixin):
>          self.assertTrue('text.html passed' in logging_stream.getvalue())
>          self.assertTrue('image.html passed' in logging_stream.getvalue())
>
> +    def disabled_test_driver_logging(**self):
> +        # FIXME: Figure out how to either use a mock-test port to
> +        # get output or mack mock ports work again.
> +        host = Host()
> +        _, err, _ = logging_run(['--platform', 'mock-win',
> '--driver-logging', 'fast/harness/results.html'],
> +                                tests_included=True, host=host)
> +        self.assertTrue('OUT:' in err.getvalue())
>
>  class EndToEndTest(unittest.**TestCase):
>      def test_reftest_with_two_notrefs(**self):
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-08-21 22:47:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/23093014/1

[Dirk Pranke, 2013-08-21 22:51:14]

Note also (for the record), that we had to fix the underlying issue w/
content_shell and setfmode regardless, since otherwise trying to dump PNGs over
stdout wouldn't work (the PNG data would be "translated"/corrupted).

[commit-bot: I haz the power, 2013-08-21 22:58:59]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-08-21 22:59:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/23093014/1

[commit-bot: I haz the power, 2013-08-22 00:23:36]

Change committed as 156519