Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2836403003:
Add Xvfb debugging code to run-webkit-tests. (Closed)
|
Created:
3 years, 8 months ago by Dirk Pranke Modified:
3 years, 7 months ago CC:
blink-reviews, chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd Xvfb debugging code to run-webkit-tests.
This CL adds some debugging code to try and figure out
why Xvfb is dying on the Site Isolation Linux bot.
R=tansell@chromium.org
BUG=708681
Review-Url: https://codereview.chromium.org/2836403003
Cr-Commit-Position: refs/heads/master@{#467288}
Committed: https://chromium.googlesource.com/chromium/src/+/07fefc1ec0552e64747b0688b2598649375016bc
Patch Set 1 #
Total comments: 6
Patch Set 2 : address review feedback #Patch Set 3 : tweak output #
Messages
Total messages: 24 (12 generated)
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
I'm actually tracking down a problem with xvfb is failing on swarming. My theory is that xvfb is sometimes taking longer to start and clients are trying to connect to it too soon. See https://codereview.chromium.org/2837773002/
LGTM, just two small comments. https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py (right): https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:173: if self._xvfb_process.poll() is not None: Can you just call _stop_xvfb here? Or maybe share the code between the two functions? https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (left): https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:599: # Temporary process dump for debugging windows timeout issues, see crbug.com/522396. This ideally would be a separate CL.
https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py (right): https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py:360: _log.warn('DISPLAY = %s', self.host.environ['DISPLAY']) This needs to be self.host.environ.get('DISPLAY', None) or similar (see your failing bot).
Your theory about Xvfb being slow seems plausible. Let's see if your CL magically fixes things, and if not, we can land this and get more debug info. https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py (right): https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py:360: _log.warn('DISPLAY = %s', self.host.environ['DISPLAY']) On 2017/04/26 01:58:33, mithro wrote: > This needs to be self.host.environ.get('DISPLAY', None) or similar (see your > failing bot). ah, yeah :). https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py (right): https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py:173: if self._xvfb_process.poll() is not None: On 2017/04/26 01:57:17, mithro wrote: > Can you just call _stop_xvfb here? Or maybe share the code between the two > functions? Yeah, I should be able to refactor it. https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (left): https://codereview.chromium.org/2836403003/diff/1/third_party/WebKit/Tools/Sc... third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:599: # Temporary process dump for debugging windows timeout issues, see crbug.com/522396. On 2017/04/26 01:57:17, mithro wrote: > This ideally would be a separate CL. Yeah, ideally :).
Description was changed from ========== Add Xvfb debugging code to run-webkit-tests. This CL adds some debugging code to try and figure out why Xvfb is dying on the Site Isolation Linux bot. This patch also deletes the dump of the running processes at the end of the run; we no longer need that. TBR=tansell@chromium.org BUG=708681 ========== to ========== Add Xvfb debugging code to run-webkit-tests. This CL adds some debugging code to try and figure out why Xvfb is dying on the Site Isolation Linux bot. R=tansell@chromium.org BUG=708681 ==========
Updated ... please take another look.
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tansell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tansell@chromium.org Link to the patchset: https://codereview.chromium.org/2836403003/#ps40001 (title: "tweak output")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493202424816760,
"parent_rev": "79eaaf7eefb10a3006da5d1a56007e8c6f72ffe1", "commit_rev":
"07fefc1ec0552e64747b0688b2598649375016bc"}
Message was sent while issue was closed.
Description was changed from ========== Add Xvfb debugging code to run-webkit-tests. This CL adds some debugging code to try and figure out why Xvfb is dying on the Site Isolation Linux bot. R=tansell@chromium.org BUG=708681 ========== to ========== Add Xvfb debugging code to run-webkit-tests. This CL adds some debugging code to try and figure out why Xvfb is dying on the Site Isolation Linux bot. R=tansell@chromium.org BUG=708681 Review-Url: https://codereview.chromium.org/2836403003 Cr-Commit-Position: refs/heads/master@{#467288} Committed: https://chromium.googlesource.com/chromium/src/+/07fefc1ec0552e64747b0688b259... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/07fefc1ec0552e64747b0688b259...
Message was sent while issue was closed.
qyearsley@chromium.org changed reviewers: + qyearsley@chromium.org
Message was sent while issue was closed.
Now that the issue on the "Site Isolation Linux" builder is fixed, might this be reverted now? In general, Xvfb stderr is a bit noisy, and logging Xvfb's output will probably be unnecessary now, I think.
Message was sent while issue was closed.
On 2017/05/09 17:42:33, qyearsley wrote: > Now that the issue on the "Site Isolation Linux" builder is fixed, might this be > reverted now? > > In general, Xvfb stderr is a bit noisy, and logging Xvfb's output will probably > be unnecessary now, I think. https://codereview.chromium.org/2846593004/ will make this only show up when we fail to start it. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
