Don't log xvfb errors under normal circumstances in run-webkit-tests

Don't log xvfb errors under normal circumstances in run-webkit-tests.

This CL hides a bunch of the debug logging we added for crbug.com/708681
so that we only get the info if something goes awry (and not all of the time).

R=tansell@chromium.org
BUG=708681
Review-Url: https://codereview.chromium.org/2846593004
Cr-Commit-Position: refs/heads/master@{#471184}
Committed: https://chromium.googlesource.com/chromium/src/+/b629361e28a37d2d72454e4efcec3d60cb312f2b

[Dirk Pranke, 2017-04-27 02:09:49]

Description was changed from

==========
Don't log xvfb errors under normal circumstances

BUG=
==========

to

==========
Don't log xvfb errors under normal circumstances in run-webkit-tests.

This CL hides a bunch of the debug logging we added for crbug.com/708681
so that we only get the info if something goes awry (and not all of the time).

R=tansell@chromium.org
BUG=708681
==========

[Dirk Pranke, 2017-04-27 02:09:49]

dpranke@chromium.org changed reviewers:
+ tansell@chromium.org

[Dirk Pranke, 2017-04-27 02:09:54]

The CQ bit was checked by dpranke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 02:10:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 03:33:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-27 03:34:00]

Dry run: This issue passed the CQ dry run.

[mithro, 2017-04-27 03:34:51]

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py
(right):

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:154: def
_start_xvfb(self):
I think one of the _log statements which describes which display was set and
xvfb was started should be an .info level message.

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:180: while
self.host.time() - start_time < self.XVFB_START_TIMEOUT:
BTW - We should probably be checking self._xvfb_process.poll() == None in this
loop.

Shall I send out a CL for that?

[Dirk Pranke, 2017-04-27 17:18:31]

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py
(right):

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:154: def
_start_xvfb(self):
On 2017/04/27 03:34:51, mithro wrote:
> I think one of the _log statements which describes which display was set and
> xvfb was started should be an .info level message.

I've tried pretty hard to keep run-webkit-tests as quiet as possible. I think
this is more of a debug-level message, since in normal operation the user
shouldn't need to know or care which port is being used.

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:180: while
self.host.time() - start_time < self.XVFB_START_TIMEOUT:
On 2017/04/27 03:34:51, mithro wrote:
> BTW - We should probably be checking self._xvfb_process.poll() == None in this
> loop.
> 
> Shall I send out a CL for that?

Sure.

[mithro, 2017-04-28 05:46:31]

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py
(right):

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:154: def
_start_xvfb(self):
On 2017/04/27 17:18:31, Dirk Pranke wrote:
> On 2017/04/27 03:34:51, mithro wrote:
> > I think one of the _log statements which describes which display was set and
> > xvfb was started should be an .info level message.
> 
> I've tried pretty hard to keep run-webkit-tests as quiet as possible. I think
> this is more of a debug-level message, since in normal operation the user
> shouldn't need to know or care which port is being used.

I agree having the output for a human being readable and short is a good plan. 

However, when running on the bots it is really hard to know what is going wrong.
Maybe we could have a default output log which isn't stdout for this type of
output?

[Dirk Pranke, 2017-04-28 15:45:09]

On 2017/04/28 05:46:31, mithro wrote:
> However, when running on the bots it is really hard to know what is going
wrong.
> Maybe we could have a default output log which isn't stdout for this type of
> output?

The bots run (or should be running) with --debug-rwt-logging, so you'll get
these messages on them already.

[Dirk Pranke, 2017-05-09 00:55:58]

ping ... any objection to landing this now?

[mithro, 2017-05-09 02:00:09]

LGTM. Feel free to land.

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py (left):

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py:360:
_log.warn('DISPLAY = %s', self.host.environ.get('DISPLAY', ''))
Could we make this a _log.debug?

[Dirk Pranke, 2017-05-09 03:42:20]

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py (left):

https://codereview.chromium.org/2846593004/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py:360:
_log.warn('DISPLAY = %s', self.host.environ.get('DISPLAY', ''))
On 2017/05/09 02:00:09, mithro wrote:
> Could we make this a _log.debug?

Sure.

[qyearsley, 2017-05-09 20:45:56]

qyearsley@chromium.org changed reviewers:
+ qyearsley@chromium.org

[qyearsley, 2017-05-09 20:45:57]

lgtm

[Dirk Pranke, 2017-05-12 00:32:46]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2017-05-12 00:32:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
qyearsley@chromium.org, tansell@chromium.org
Link to the patchset: https://codereview.chromium.org/2846593004/#ps40001
(title: "merge, address review feedback")

[commit-bot: I haz the power, 2017-05-12 00:34:49]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 02:07:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494549166504720,
"parent_rev": "410c20c087bed3fa610e8442d7b90b8b5729a155", "commit_rev":
"b629361e28a37d2d72454e4efcec3d60cb312f2b"}

[commit-bot: I haz the power, 2017-05-12 02:07:50]

Description was changed from

==========
Don't log xvfb errors under normal circumstances in run-webkit-tests.

This CL hides a bunch of the debug logging we added for crbug.com/708681
so that we only get the info if something goes awry (and not all of the time).

R=tansell@chromium.org
BUG=708681
==========

to

==========
Don't log xvfb errors under normal circumstances in run-webkit-tests.

This CL hides a bunch of the debug logging we added for crbug.com/708681
so that we only get the info if something goes awry (and not all of the time).

R=tansell@chromium.org
BUG=708681

Review-Url: https://codereview.chromium.org/2846593004
Cr-Commit-Position: refs/heads/master@{#471184}
Committed:
https://chromium.googlesource.com/chromium/src/+/b629361e28a37d2d72454e4efcec...
==========

[commit-bot: I haz the power, 2017-05-12 02:07:51]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b629361e28a37d2d72454e4efcec...