Pull in and install the pc_dep for PageCycler [chromiumos/third_party/autotest : master]

0 views
Skip to first unread message

Sonny Rao (Code Review)

unread,
May 16, 2012, 4:05:34 AM5/16/12
to
Sonny Rao has uploaded a new change for review.

Change subject: Pull in and install the pc_dep for PageCycler
......................................................................

Pull in and install the pc_dep for PageCycler

BUG=chromium-os:23990
TEST=run desktopui_PyAutoPerfTests with perf.PageCyclerTest

Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
---
M client/site_tests/desktopui_PyAutoPerfTests/desktopui_PyAutoPerfTests.py
1 file changed, 11 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Nirnimesh (Code Review)

unread,
May 16, 2012, 4:13:38 AM5/16/12
to Sonny Rao, asharif, Chris Sosa
Nirnimesh has posted comments on this change.

Change subject: Pull in and install the pc_dep for PageCycler
......................................................................


Patch Set 2: (1 inline comment)

....................................................
File client/site_tests/desktopui_PyAutoPerfTests/desktopui_PyAutoPerfTests.py
Line 30: dep = 'pc_dep'
I'd much recommend that the dep was called 'page_cycler-dep' for clarity. unpopular acronyms hinder readability.
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-Reviewer: Nirnimesh <nirn...@chromium.org>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: asharif <ash...@chromium.org>

Nirnimesh (Code Review)

unread,
May 16, 2012, 6:31:46 AM5/16/12
to Sonny Rao, asharif, Chris Sosa
Nirnimesh has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 3: (3 inline comments)

A few minor comments.

....................................................
File client/site_tests/desktopui_PyAutoPerfTests/desktopui_PyAutoPerfTests.py
Line 28: def initialize(self, args=[]):
args is unused?

Line 32: self.job.install_pkg(dep, 'dep', dep_dir)
call setup_dep instead
I think lines 29-32 should move to a setup() function, which gets called on the host.

Line 33: try:
Add a comment about what it's doing since it's not obvious
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Sonny Rao (Code Review)

unread,
May 16, 2012, 3:31:50 PM5/16/12
to asharif, Nirnimesh, Chris Sosa
Sonny Rao has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File client/site_tests/desktopui_PyAutoPerfTests/desktopui_PyAutoPerfTests.py
Line 32: self.job.install_pkg(dep, 'dep', dep_dir)
I originally did something like this but it didn't work because the page_cycler dep didn't end up installed. After talking with Sosa I found out the setup function is called on the host during the ebuild, but it doesn't result in the dep getting installed on the target. The code added here is mostly copied from the code in the the chrome_test initialize() function which installs the chrome_test dep onto the target. We have to do the same thing here to install the page_cycler dep onto the target.
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Sonny Rao (Code Review)

unread,
May 16, 2012, 3:35:21 PM5/16/12
to asharif, Nirnimesh, Chris Sosa
Sonny Rao has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 3: (2 inline comments)

....................................................
File client/site_tests/desktopui_PyAutoPerfTests/desktopui_PyAutoPerfTests.py
Line 28: def initialize(self, args=[]):
Done

Line 33: try:
Done
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Nirnimesh (Code Review)

unread,
May 16, 2012, 3:42:21 PM5/16/12
to Sonny Rao, asharif, Chris Sosa
Nirnimesh has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 4: Looks good to me, approved

(1 inline comment)

....................................................
File client/site_tests/desktopui_PyAutoPerfTests/desktopui_PyAutoPerfTests.py
Line 30: # the next few lines install the page_cycler depdendency onto the
Capitalize 'the'
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Sonny Rao (Code Review)

unread,
May 16, 2012, 4:52:55 PM5/16/12
to asharif, Nirnimesh, Chris Sosa
Sonny Rao has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 5: Verified; Looks good to me, approved

Inheriting LGTM
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Sonny Rao (Code Review)

unread,
May 17, 2012, 8:46:57 PM5/17/12
to asharif, Nirnimesh, Chris Sosa
Sonny Rao has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 5: Ready
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>

Sonny Rao (Code Review)

unread,
May 18, 2012, 3:30:42 AM5/18/12
to asharif, Nirnimesh, Chris Sosa
Sonny Rao has posted comments on this change.

Change subject: Pull in and install the page_cycler_dep for PageCycler
......................................................................


Patch Set 6: Verified; Looks good to me, approved; Ready
Gerrit-MessageType: comment
Gerrit-Change-Id: I10af753cc9081b6ef9d012adf65c41bc0b238be9
Gerrit-PatchSet: 6
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Sonny Rao <sonn...@chromium.org>
Reply all
Reply to author
Forward
0 new messages