Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
4557002:
[autotest] Change ChromeTestBase to work with pre-installed chrome test binaries. (Closed)
|
Created:
10 years, 1 month ago by Chris Masone Modified:
9 years, 7 months ago CC:
chromium-os-reviews_chromium.org, truty+cc_chromium.org, sosa+cc_chromium.org, seano+cc_chromium.org, petkov+cc_chromium.org, ericli Visibility:
Public. |
Description[autotest] Change ChromeTestBase to work with pre-installed chrome test binaries.
Instead of pulling chrome test binaries as a dep (which takes too long on VMs),
we'll be pre-installing them on test images. Update the ChromeTestBase class
to expect this new behavior.
BUG=8544
TEST=Build a test image with chrome > -r35, run desktopui_BrowserTest
Change-Id: I013b689b4ad7fe5e47760312da573a9a2b29d669
Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=c18e883
Patch Set 1 #
Total comments: 2
Patch Set 2 : intermediate results #Patch Set 3 : work with new pre-install mechanism #
Total comments: 4
Messages
Total messages: 13 (0 generated)
There is easier ways to achieve the same goal. inside buildbot, under the prebuild autotest directory, call utils/packager.py, with some command args. something like utils/packagers.py -d chrome-test It will package the chromeos-test into a tarball, then you can simply put it under /usr/local/autotest-pkgs directory on the test image. autotest harness will automatically fetch/untar the package from that location. The autotest lab is doing similar things to pre-install every test cases and deps on the test image. http://codereview.chromium.org/4557002/diff/1/2 File client/bin/site_chrome_test.py (left): http://codereview.chromium.org/4557002/diff/1/2#oldcode13 client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) why you want to remove this? I am not sure if it will have consequence to our prebuild process?
On 2010/11/05 21:40:30, ericli wrote: > There is easier ways to achieve the same goal. > > inside buildbot, under the prebuild autotest directory, call utils/packager.py, > with some command args. what if I'm not doing this on the buildbot? > > something like utils/packagers.py -d chrome-test > > It will package the chromeos-test into a tarball, then you can simply put it > under /usr/local/autotest-pkgs directory on the test image. > > autotest harness will automatically fetch/untar the package from that location. > > The autotest lab is doing similar things to pre-install every test cases and > deps on the test image. It seems way easier to just install the binaries onto the stateful partition in the ebuild, which I've already done in http://codereview.chromium.org/4525006/show > > http://codereview.chromium.org/4557002/diff/1/2 > File client/bin/site_chrome_test.py (left): > > http://codereview.chromium.org/4557002/diff/1/2#oldcode13 > client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) > why you want to remove this? I am not sure if it will have consequence to our > prebuild process? Because I'm not expressing the dependency on the chrome test binaries as an autotest dep anymore. I'm just installing them with chrome. Easier.
On 2010/11/05 21:44:44, Chris Masone wrote: > On 2010/11/05 21:40:30, ericli wrote: > > There is easier ways to achieve the same goal. > > > > inside buildbot, under the prebuild autotest directory, call > utils/packager.py, > > with some command args. > > what if I'm not doing this on the buildbot? > > > > > something like utils/packagers.py -d chrome-test > > > > It will package the chromeos-test into a tarball, then you can simply put it > > under /usr/local/autotest-pkgs directory on the test image. > > > > autotest harness will automatically fetch/untar the package from that > location. > > > > The autotest lab is doing similar things to pre-install every test cases and > > deps on the test image. > > It seems way easier to just install the binaries onto the stateful partition in > the ebuild, which I've already done in > http://codereview.chromium.org/4525006/show > > > > > http://codereview.chromium.org/4557002/diff/1/2 > > File client/bin/site_chrome_test.py (left): > > > > http://codereview.chromium.org/4557002/diff/1/2#oldcode13 > > client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) > > why you want to remove this? I am not sure if it will have consequence to our > > prebuild process? > > Because I'm not expressing the dependency on the chrome test binaries as an > autotest dep anymore. I'm just installing them with chrome. Easier. Update: I was not aware of the added complexity introduced by dependencies from some of these tests upon things in autotest-private. So, I'm going to work on the approach that eric suggested above.
PTAL http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py File client/bin/site_chrome_test.py (left): http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py#... client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) On 2010/11/05 21:40:30, ericli wrote: > why you want to remove this? I am not sure if it will have consequence to our > prebuild process? I undid that now
I also looked http://codereview.chromium.org/4756001/, are you installing only chrome-test into /usr/local/autotest-pkgs for now? Or the entire autotest client tests onto there? The later is an idea scenario for us. On Tue, Nov 9, 2010 at 3:08 PM, <cmasone@chromium.org> wrote: > PTAL > > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py > File client/bin/site_chrome_test.py (left): > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py#... > client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) > On 2010/11/05 21:40:30, ericli wrote: >> >> why you want to remove this? I am not sure if it will have consequence > > to our >> >> prebuild process? > > I undid that now > > http://codereview.chromium.org/4557002/ > -- Eric Li 李咏竹 Google Kirkland
On 2010/11/09 23:19:44, ericli wrote: > I also looked http://codereview.chromium.org/4756001/, are you > installing only chrome-test into /usr/local/autotest-pkgs for now? > Or the entire autotest client tests onto there? The later is an idea > scenario for us. At this time, I'm only installing the chrome_test dependency. It seems like we could evaluate doing this for more stuff and make that change separately. Getting to the point where we can run all the chrome tests efficiently has so many moving parts that I'm hesitant to add more :-/ > > > > On Tue, Nov 9, 2010 at 3:08 PM, <mailto:cmasone@chromium.org> wrote: > > PTAL > > > > > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py > > File client/bin/site_chrome_test.py (left): > > > > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py#... > > client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) > > On 2010/11/05 21:40:30, ericli wrote: > >> > >> why you want to remove this? I am not sure if it will have consequence > > > > to our > >> > >> prebuild process? > > > > I undid that now > > > > http://codereview.chromium.org/4557002/ > > > > > > -- > Eric Li > 李咏竹 > Google Kirkland
Agree. I did that once, and it blows up the stateful partition and the factory guys dont like it either due to increased size. But I would be happy to discuss with you when you have time. Eric On Tue, Nov 9, 2010 at 3:40 PM, <cmasone@chromium.org> wrote: > On 2010/11/09 23:19:44, ericli wrote: >> >> I also looked http://codereview.chromium.org/4756001/, are you >> installing only chrome-test into /usr/local/autotest-pkgs for now? >> Or the entire autotest client tests onto there? The later is an idea >> scenario for us. > > At this time, I'm only installing the chrome_test dependency. It seems like > we > could evaluate doing this for more stuff and make that change separately. > Getting to the point where we can run all the chrome tests efficiently has > so > many moving parts that I'm hesitant to add more :-/ > > > > >> On Tue, Nov 9, 2010 at 3:08 PM, <mailto:cmasone@chromium.org> wrote: >> > PTAL >> > >> > >> > >> > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py >> > File client/bin/site_chrome_test.py (left): >> > >> > > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py#... >> >> > client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) >> > On 2010/11/05 21:40:30, ericli wrote: >> >> >> >> why you want to remove this? I am not sure if it will have consequence >> > >> > to our >> >> >> >> prebuild process? >> > >> > I undid that now >> > >> > http://codereview.chromium.org/4557002/ >> > > > > >> -- >> Eric Li >> 李咏竹 >> Google Kirkland > > > > http://codereview.chromium.org/4557002/ > -- Eric Li 李咏竹 Google Kirkland
So, does this look good to you folks, zel and eric? On 2010/11/10 00:51:22, ericli wrote: > Agree. > > I did that once, and it blows up the stateful partition and the > factory guys dont like it either due to increased size. But I would be > happy to discuss with you when you have time. > > Eric > > On Tue, Nov 9, 2010 at 3:40 PM, <mailto:cmasone@chromium.org> wrote: > > On 2010/11/09 23:19:44, ericli wrote: > >> > >> I also looked http://codereview.chromium.org/4756001/, are you > >> installing only chrome-test into /usr/local/autotest-pkgs for now? > >> Or the entire autotest client tests onto there? The later is an idea > >> scenario for us. > > > > At this time, I'm only installing the chrome_test dependency. It seems like > > we > > could evaluate doing this for more stuff and make that change separately. > > Getting to the point where we can run all the chrome tests efficiently has > > so > > many moving parts that I'm hesitant to add more :-/ > > > > > > > > > >> On Tue, Nov 9, 2010 at 3:08 PM, <mailto:cmasone@chromium.org> wrote: > >> > PTAL > >> > > >> > > >> > > >> > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py > >> > File client/bin/site_chrome_test.py (left): > >> > > >> > > > > > > http://codereview.chromium.org/4557002/diff/1/client/bin/site_chrome_test.py#... > >> > >> > client/bin/site_chrome_test.py:13: self.job.setup_dep(['chrome_test']) > >> > On 2010/11/05 21:40:30, ericli wrote: > >> >> > >> >> why you want to remove this? I am not sure if it will have consequence > >> > > >> > to our > >> >> > >> >> prebuild process? > >> > > >> > I undid that now > >> > > >> > http://codereview.chromium.org/4557002/ > >> > > > > > > > > >> -- > >> Eric Li > >> 李咏竹 > >> Google Kirkland > > > > > > > > http://codereview.chromium.org/4557002/ > > > > > > -- > Eric Li > 李咏竹 > Google Kirkland
Ping? Is anyone ok with this changelist?
LGTM with some nits. http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.py File client/bin/site_chrome_test.py (right): http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.... client/bin/site_chrome_test.py:23: self.job.add_repository(['/usr/local/autotest-pkgs']) get this value from globle_config.ini. from autotest_lib.client.common_lib import global_config repo = global_config.global_config.get_config_value( 'PACKAGES', 'fetch_location', type=str, default='/usr/local/autotest-pkgs') http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.... client/bin/site_chrome_test.py:68: def run_chrome_test(self, test_to_run, extra_params = ''): no space for cmd arg default value. extra_params=''
LGTM On Fri, Nov 12, 2010 at 2:16 PM, <ericli@chromium.org> wrote: > LGTM with some nits. > > > > > > http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.py > File client/bin/site_chrome_test.py (right): > > > http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.... > client/bin/site_chrome_test.py:23: > self.job.add_repository(['/usr/local/autotest-pkgs']) > get this value from globle_config.ini. > > from autotest_lib.client.common_lib import global_config > repo = global_config.global_config.get_config_value( > 'PACKAGES', 'fetch_location', > type=str, default='/usr/local/autotest-pkgs') > > > http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.... > client/bin/site_chrome_test.py:68: def run_chrome_test(self, > > test_to_run, extra_params = ''): > no space for cmd arg default value. > extra_params='' > > > http://codereview.chromium.org/4557002/ >
fixed nits and pushed http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.py File client/bin/site_chrome_test.py (right): http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.... client/bin/site_chrome_test.py:23: self.job.add_repository(['/usr/local/autotest-pkgs']) On 2010/11/12 22:16:26, ericli wrote: > get this value from globle_config.ini. > > from autotest_lib.client.common_lib import global_config > repo = global_config.global_config.get_config_value( > 'PACKAGES', 'fetch_location', > type=str, default='/usr/local/autotest-pkgs') Done. http://codereview.chromium.org/4557002/diff/8001/client/bin/site_chrome_test.... client/bin/site_chrome_test.py:68: def run_chrome_test(self, test_to_run, extra_params = ''): On 2010/11/12 22:16:26, ericli wrote: > no space for cmd arg default value. > extra_params='' Done. |
