[autotest] Add autotest for browser hang detection [chromiumos/third_party/autotest : master]

3 views
Skip to first unread message

Chris Masone (Code Review)

unread,
Nov 19, 2012, 8:06:50 PM11/19/12
to Chris Sosa
Hello Chris Sosa,

I'd like you to do a code review. Please visit

https://gerrit.chromium.org/gerrit/38347

to review the following change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................

[autotest] Add autotest for browser hang detection

Add desktopui_HangDetector to verify that session_manager's hang detection
for the browser process works.
It will start in the 'regression' suite as an experimental test.

BUG=chromium-os:35729
TEST=run it with run_remote_tests.
CQ-DEPEND=I3e60e1cbda652a3e52126cd69b2ff4cd02cc6a74
CQ-DEPEND=Ied5c9e897485aaf75fdefd211445219712972bdf

Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
---
M client/bin/site_utils.py
M client/cros/constants.py
A client/site_tests/desktopui_HangDetector/control
A client/site_tests/desktopui_HangDetector/desktopui_HangDetector.py
4 files changed, 145 insertions(+), 2 deletions(-)


git pull ssh://gerrit.chromium.org:29418/chromiumos/third_party/autotest refs/changes/47/38347/1
--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>

Chris Masone (Code Review)

unread,
Nov 19, 2012, 8:07:32 PM11/19/12
to Gerrit, Chris Sosa
Chris Masone has posted comments on this change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................


Patch Set 1: Verified

--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>

Chris Sosa (Code Review)

unread,
Nov 19, 2012, 8:30:44 PM11/19/12
to Chris Masone, Gerrit
Chris Sosa has posted comments on this change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File client/bin/site_utils.py
Line 74: str_pid = utils.system_output('pgrep -o ^%s$' % name).rstrip()
I do not think this will ever return None as pgrep fails if it doesn't match anything. I'd prob go with ignore_status=True and remove that raises error.CmdError bit in your docstring

....................................................
File client/site_tests/desktopui_HangDetector/control
Line 9: TIME = "FAST"
SHORT

....................................................
File client/site_tests/desktopui_HangDetector/desktopui_HangDetector.py
Line 81: if browser_pid == self._get_oldest_pid_by_name(constants.BROWSER):
how unlikely is this? how often do pid's get reused? Seems like if utils.pid_is_alive succeded this check wouldn't be necessary unless the above had a bug.

--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>

Chris Masone (Code Review)

unread,
Nov 20, 2012, 1:37:50 PM11/20/12
to Gerrit, Chris Sosa
Chris Masone has posted comments on this change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File client/bin/site_utils.py
Line 74: str_pid = utils.system_output('pgrep -o ^%s$' % name).rstrip()
Done

....................................................
File client/site_tests/desktopui_HangDetector/control
Line 9: TIME = "FAST"
In the best-practices doc, we included FAST < 1m as an acceptable value here.

....................................................
File client/site_tests/desktopui_HangDetector/desktopui_HangDetector.py
Line 81: if browser_pid == self._get_oldest_pid_by_name(constants.BROWSER):
Done

--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>

Chris Masone (Code Review)

unread,
Nov 20, 2012, 1:38:13 PM11/20/12
to Gerrit, Chris Sosa
Chris Masone has posted comments on this change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................


Patch Set 2: Verified

--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>

Chris Sosa (Code Review)

unread,
Nov 20, 2012, 3:50:24 PM11/20/12
to Chris Masone, Gerrit
Chris Sosa has posted comments on this change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................


Patch Set 2: Looks good to me, approved

--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>

Chris Masone (Code Review)

unread,
Nov 20, 2012, 4:02:28 PM11/20/12
to Gerrit, Chris Sosa
Chris Masone has posted comments on this change.

Change subject: [autotest] Add autotest for browser hang detection
......................................................................


Patch Set 2: Ready

--
To view, visit https://gerrit.chromium.org/gerrit/38347
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ba7a4f5db97fb1b92fd5366a18c984a7f5494bc
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Masone <cma...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Gerrit <chrom...@google.com>
Reply all
Reply to author
Forward
0 new messages