Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(86)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 6541008: au_test_harness: fix a function call, add sudo in front of ifconfig (Closed)

Created:
9 years, 10 months ago by zbehan
Modified:
9 years, 7 months ago
Reviewers:
dgarrett, sosa
CC:
chromium-os-reviews_chromium.org, Mandeep Singh Baines, anush, sosa
Visibility:
Public.

Description

au_test_harness: fix a function call, add sudo in front of ifconfig Note: sudo before ifconfig is for running on distros which do not have /sbin or /usr/sbin in common user's paths, like gentoo. Change-Id: I85bd379ad059d6ecaa8c11f3167fae27987479dd BUG=5246 TEST=run cros_au_test_harness and see it not fail Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=c264a45

Patch Set 1 #

Patch Set 2 : 80 chars #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M bin/cros_au_test_harness.py View 1 chunk +1 line, -1 line 0 comments Download
M lib/cros_build_lib.py View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
zbehan
9 years, 10 months ago (2011-02-18 01:03:57 UTC) #1
zbehan
9 years, 10 months ago (2011-02-18 01:05:51 UTC) #2
sosa
LGTM! Thanks for the fix On Thu, Feb 17, 2011 at 5:05 PM, <zbehan@chromium.org> wrote: ...
9 years, 10 months ago (2011-02-18 04:38:01 UTC) #3
Mandeep Singh Baines
sudo seems like overkill here. This is a PATH issue and not a permission issue. ...
9 years, 10 months ago (2011-02-18 15:53:50 UTC) #4
zbehan
I couldn't find a standard either, that's why i used sudo. Technically, ifconfig will be ...
9 years, 10 months ago (2011-02-18 16:24:12 UTC) #5
Mandeep Singh Baines
Zdenek Behan (zbehan@chromium.org) wrote: > I couldn't find a standard either, http://www.pathname.com/fhs/pub/fhs-2.3.html > that's why ...
9 years, 10 months ago (2011-02-18 17:10:31 UTC) #6
zbehan
9 years, 10 months ago (2011-02-18 17:15:58 UTC) #7 
OK. I can change it, in both places where I used this.

On Fri, Feb 18, 2011 at 6:10 PM, Mandeep Singh Baines <msb@chromium.org>wrote:

> Zdenek Behan (zbehan@chromium.org) wrote:
> > I couldn't find a standard either,
>
> http://www.pathname.com/fhs/pub/fhs-2.3.html
>
> > that's why i used sudo. Technically,
> > ifconfig will
> > be half-functional for a normal user anyway. You won't be able to modify
> any
> > parameters, for example. Also, we use sudo all across anyway.
> >
>
> Still. It would be nice to keep sudo to a minimum. Here sudo is not
> required.
>
> > On Fri, Feb 18, 2011 at 4:53 PM, Mandeep Singh Baines <msb@chromium.org
> >wrote:
> >
> > > sudo seems like overkill here. This is a PATH issue and not a
> permission
> > > issue. Just use /sbin/ifconfig. That is the standard location. Was
> hoping
> > > to reference a standard but couldn't find a reference.
> > >
> > > zbehan@chromium.org (zbehan@chromium.org) wrote:
> > > > Reviewers: sosa, dgarrett,
> > > >
> > > > Description:
> > > > au_test_harness: fix a function call, add sudo in front of ifconfig
> > > >
> > > > Note: sudo before ifconfig is for running on distros which do not
> > > > have /sbin or /usr/sbin in common user's paths, like gentoo.
> > > >
> > > > Change-Id: I85bd379ad059d6ecaa8c11f3167fae27987479dd
> > > >
> > > > BUG=5246
> > > > TEST=run cros_au_test_harness and see it not fail
> > > >
> > > > Please review this at http://codereview.chromium.org/6541008/
> > > >
> > > > SVN Base: ssh://git@gitrw.chromium.org:9222/crosutils.git@master
> > > >
> > > > Affected files:
> > > >   M bin/cros_au_test_harness.py
> > > >   M lib/cros_build_lib.py
> > > >
> > > >
> > > > Index: bin/cros_au_test_harness.py
> > > > diff --git a/bin/cros_au_test_harness.py
> b/bin/cros_au_test_harness.py
> > > > index
> > >
>
994476caaca7a0d7b09f523edf9eb5b9faea4665..e398625d938142a1b70fc05b533c07c147bb9a71
> > > > 100755
> > > > --- a/bin/cros_au_test_harness.py
> > > > +++ b/bin/cros_au_test_harness.py
> > > > @@ -482,7 +482,7 @@ class RealAUTest(unittest.TestCase, AUTest):
> > > >
> > > >    def PrepareBase(self, image_path):
> > > >      """Auto-update to base image to prepare for test."""
> > > > -    _PrepareRealBase(image_path)
> > > > +    self._PrepareRealBase(image_path)
> > > >
> > > >    def _UpdateImage(self, image_path, src_image_path='',
> > > > stateful_change='old',
> > > >                     proxy_port=None, private_key_path=None):
> > > > Index: lib/cros_build_lib.py
> > > > diff --git a/lib/cros_build_lib.py b/lib/cros_build_lib.py
> > > > index
> > >
>
11fa40cac9cf94813f27c70fda281940124f1f96..f37b1ef5976f4dffc2def8b4fc5c0f5854f23def
> > > > 100644
> > > > --- a/lib/cros_build_lib.py
> > > > +++ b/lib/cros_build_lib.py
> > > > @@ -256,7 +256,7 @@ def GetIPAddress(device='eth0'):
> > > >    this method gives you a generic way to get the address so you are
> > > > reachable
> > > >    either via a VM or remote machine on the same network.
> > > >    """
> > > > -  ifconfig_output = RunCommand(['ifconfig', device],
> > > redirect_stdout=True,
> > > > +  ifconfig_output = RunCommand(['sudo', 'ifconfig', device],
> > > > redirect_stdout=True,
> > > >                                 print_cmd=False)
> > > >    match = re.search('.*inet addr:(\d+\.\d+\.\d+\.\d+).*',
> > > ifconfig_output)
> > > >    if match:
> > > >
> > > >
> > >
>

Powered by Google App Engine
This is Rietveld 408576698